From 3df3908ace8bad1efeb680dc605d23841e7a9f99 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Fri, 26 Jun 2026 13:35:36 +0200 Subject: [PATCH 01/14] Add admin status to nic port --- cmd/metal-api/internal/service/switch-service.go | 5 ++--- cmd/metal-api/internal/service/switch-service_test.go | 3 ++- cmd/metal-api/internal/service/v1/switch.go | 1 + spec/metal-api.json | 9 +++++++++ 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index c37551ae6..b5365196e 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -1196,10 +1196,9 @@ func (r *switchResource) makeSwitchNics(s *metal.Switch, nws metal.NetworkMap, i BGPPortState: n.BGPPortState, } if n.State != nil { + nic.Actual = v1.SwitchPortStatus(n.State.Actual) if n.State.Desired != nil { - nic.Actual = v1.SwitchPortStatus(*n.State.Desired) - } else { - nic.Actual = v1.SwitchPortStatus(n.State.Actual) + nic.AdminStatus = new(v1.SwitchPortStatus(*n.State.Desired)) } } nics = append(nics, nic) diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index b963d58c0..2880a071b 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -23,6 +23,7 @@ import ( v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1" "github.com/metal-stack/metal-api/cmd/metal-api/internal/testdata" "github.com/metal-stack/metal-lib/httperrors" + "github.com/metal-stack/metal-lib/pkg/pointer" ) func TestRegisterSwitch(t *testing.T) { @@ -1748,7 +1749,7 @@ func TestToggleSwitch(t *testing.T) { require.NoError(t, err) require.Equal(t, testdata.Switch1.ID, result.ID) require.Equal(t, testdata.Switch1.Name, *result.Name) - require.Equal(t, v1.SwitchPortStatusDown, result.Nics[0].Actual) + require.Equal(t, v1.SwitchPortStatusDown, pointer.SafeDeref(result.Nics[0].AdminStatus)) require.Equal(t, v1.SwitchPortStatusUnknown, result.Connections[0].Nic.Actual) } diff --git a/cmd/metal-api/internal/service/v1/switch.go b/cmd/metal-api/internal/service/v1/switch.go index 8a969c937..2e35f0f30 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -46,6 +46,7 @@ type SwitchNic struct { Vrf string `json:"vrf" description:"the vrf this network interface is part of" optional:"true"` BGPFilter *BGPFilter `json:"filter" description:"configures the bgp filter applied at the switch port" optional:"true"` Actual SwitchPortStatus `json:"actual" description:"the current state of the nic" enum:"UP|DOWN|UNKNOWN"` + AdminStatus *SwitchPortStatus `json:"admin_status" description:"the desired state of the nic" enum:"UP|DOWN"` BGPPortState *metal.SwitchBGPPortState `json:"bgp_port_state" description:"the current bgp port state" optional:"true"` } diff --git a/spec/metal-api.json b/spec/metal-api.json index a908ed4f9..83f53bb48 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -5426,6 +5426,14 @@ ], "type": "string" }, + "admin_status": { + "description": "the desired state of the nic", + "enum": [ + "DOWN", + "UP" + ], + "type": "string" + }, "bgp_port_state": { "$ref": "#/definitions/metal.SwitchBGPPortState", "description": "the current bgp port state" @@ -5453,6 +5461,7 @@ }, "required": [ "actual", + "admin_status", "identifier", "mac", "name" From 140c659bfcb3474c189589261412c280b6b1423c Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 30 Jun 2026 14:43:24 +0200 Subject: [PATCH 02/14] keep desired state after successful apply --- cmd/metal-api/internal/metal/network.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 8241aa464..e71d4ae6e 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -144,18 +144,6 @@ func (ns *NicState) WantState(s SwitchPortStatus) (NicState, bool) { Desired: &s, }, true } - if ns.Actual == s { - // we want a state we already have - if ns.Desired != nil { - return NicState{ - Actual: s, - Desired: nil, - }, true - } - return *ns, false - } - // return a new state with the desired state set and a bool indicating a state change - // only if the desired state is different from the current one return NicState{ Actual: ns.Actual, Desired: &s, From 33f6b599539d242ec4ba46e8dade2b201420f6c9 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 30 Jun 2026 15:23:45 +0200 Subject: [PATCH 03/14] change WantState impl and fix test --- cmd/metal-api/internal/metal/network.go | 16 ++-- cmd/metal-api/internal/metal/network_test.go | 83 +++++++++----------- 2 files changed, 45 insertions(+), 54 deletions(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index e71d4ae6e..5d8639327 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -7,8 +7,6 @@ import ( "slices" "strconv" "strings" - - "github.com/samber/lo" ) // SwitchPortStatus is a type alias for a string that represents the status of a switch port. @@ -135,8 +133,6 @@ func (ns *NicState) SetState(s SwitchPortStatus) (NicState, bool) { // WantState sets the desired state for the NIC. It returns a new NicState // struct with the desired state set and a bool indicating if the state changed. -// If the current state already matches the desired state, it returns a state -// with a cleared desired field. func (ns *NicState) WantState(s SwitchPortStatus) (NicState, bool) { if ns == nil { return NicState{ @@ -144,10 +140,18 @@ func (ns *NicState) WantState(s SwitchPortStatus) (NicState, bool) { Desired: &s, }, true } + if ns.Desired == nil { + changed := s != ns.Actual + return NicState{ + Desired: &s, + Actual: ns.Actual, + }, changed + } + changed := s != *ns.Desired return NicState{ - Actual: ns.Actual, Desired: &s, - }, lo.FromPtr(ns.Desired) != s + Actual: ns.Actual, + }, changed } // GetIdentifier returns the identifier of a nic. diff --git a/cmd/metal-api/internal/metal/network_test.go b/cmd/metal-api/internal/metal/network_test.go index ca88aaf6a..8c57a6c11 100644 --- a/cmd/metal-api/internal/metal/network_test.go +++ b/cmd/metal-api/internal/metal/network_test.go @@ -136,103 +136,90 @@ func TestNicState_WantState(t *testing.T) { changed bool }{ { - name: "up to desired down", - nic: &NicState{ - Desired: nil, - Actual: down, - }, - arg: up, + name: "current is nil", + nic: nil, + arg: up, want: NicState{ Desired: &up, - Actual: down, + Actual: unknown, }, changed: true, }, { - name: "up to up with empty desired", + name: "current desired is nil, new desired matches current actual", nic: &NicState{ Desired: nil, - Actual: up, + Actual: down, }, - arg: up, + arg: down, want: NicState{ - Desired: nil, - Actual: up, + Desired: &down, + Actual: down, }, changed: false, }, { - name: "up to up with other desired", + name: "current desired is nil, new desired differs from current actual", nic: &NicState{ - Desired: &down, - Actual: up, - }, - arg: up, - want: NicState{ Desired: nil, - Actual: up, + Actual: down, }, - changed: true, - }, - { - name: "nil to up", - nic: nil, - arg: up, + arg: up, want: NicState{ Desired: &up, - Actual: unknown, + Actual: down, }, changed: true, }, { - name: "different actual with same desired", + name: "new desired differs from current desired and actual", nic: &NicState{ Desired: &down, - Actual: up, + Actual: down, }, - arg: down, + arg: up, want: NicState{ - Desired: &down, - Actual: up, + Desired: &up, + Actual: down, }, - changed: false, + changed: true, }, { - name: "different actual with other desired", + name: "new desired differs from current desired", nic: &NicState{ - Desired: &up, + Desired: &down, Actual: up, }, - arg: down, + arg: up, want: NicState{ - Desired: &down, + Desired: &up, Actual: up, }, changed: true, }, { - name: "different actual with empty desired", + name: "new desired matches current desired but differs from current actual", nic: &NicState{ - Desired: nil, - Actual: up, + Desired: &up, + Actual: down, }, - arg: down, + arg: up, want: NicState{ - Desired: &down, - Actual: up, + Desired: &up, + Actual: down, }, - changed: true, + changed: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1 := tt.nic.WantState(tt.arg) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("NicState.WantState() got = %+v, want %+v", got, tt.want) + got, changed := tt.nic.WantState(tt.arg) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("NicState.WantState() diff = %s", diff) } - if got1 != tt.changed { - t.Errorf("NicState.WantState() got1 = %v, want %v", got1, tt.changed) + if changed != tt.changed { + t.Errorf("NicState.WantState() changed = %v, want %v", changed, tt.changed) } }) } From 3ddd71a8061b4d0fb9e3c4c8f7cb1dfde0ab99d4 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 30 Jun 2026 16:21:57 +0200 Subject: [PATCH 04/14] more debug logging during port toggle --- cmd/metal-api/internal/service/switch-service.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index b5365196e..53ecc63f6 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -374,6 +374,7 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu // toggleSwitchPort handles a request to toggle the state of a port on a switch. It reads the request body, finds the switch, updates its NIC state if needed, and returns the updated switch on success. // If the given port is not found or the given status is not concrete, a 400 error is returned. Another requirement is that there must be a machine connected to the port. func (r *switchResource) toggleSwitchPort(request *restful.Request, response *restful.Response) { + r.log.Debug("toggle switch port", "request", request) var requestPayload v1.SwitchPortToggleRequest err := request.ReadEntity(&requestPayload) if err != nil { @@ -439,6 +440,7 @@ func (r *switchResource) toggleSwitchPort(request *restful.Request, response *re } if updated { + r.log.Debug("toggle switch port update switch", "new nics", newSwitch.Nics) if err := r.ds.UpdateSwitch(oldSwitch, &newSwitch); err != nil { r.sendError(request, response, defaultError(err)) return From e5b0ff185266643b144fc92195dd25ac9d6868ea Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Tue, 30 Jun 2026 16:56:02 +0200 Subject: [PATCH 05/14] fix log --- cmd/metal-api/internal/service/switch-service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 53ecc63f6..58e94b56a 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -374,13 +374,13 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu // toggleSwitchPort handles a request to toggle the state of a port on a switch. It reads the request body, finds the switch, updates its NIC state if needed, and returns the updated switch on success. // If the given port is not found or the given status is not concrete, a 400 error is returned. Another requirement is that there must be a machine connected to the port. func (r *switchResource) toggleSwitchPort(request *restful.Request, response *restful.Response) { - r.log.Debug("toggle switch port", "request", request) var requestPayload v1.SwitchPortToggleRequest err := request.ReadEntity(&requestPayload) if err != nil { r.sendError(request, response, httperrors.BadRequest(err)) return } + r.log.Debug("toggle switch port", "request", requestPayload) desired := metal.SwitchPortStatus(requestPayload.Status) From 439dd8a8a435e9d73e3a6b8d2a8aca48e74849d6 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Wed, 1 Jul 2026 10:52:56 +0200 Subject: [PATCH 06/14] do not clear desired state --- cmd/metal-api/internal/metal/network.go | 56 ++-------- cmd/metal-api/internal/metal/network_test.go | 107 +++++++++++-------- 2 files changed, 66 insertions(+), 97 deletions(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 5d8639327..7bd2d46dc 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -73,62 +73,18 @@ type SwitchBGPPortState struct { // SetState updates the NicState with the given SwitchPortStatus. It returns // a new NicState and a bool indicating if the state was changed. -// -// If the given status matches the current Actual state, it checks if Desired -// is set and matches too. If so, Desired is set to nil since the desired -// state has been reached. -// -// If the given status differs from the current Actual state, Desired is left -// unchanged if it differs from the new state so the desired state is still tracked. -// The Actual state is updated to the given status. -// -// This allows tracking both the desired and actual states, while clearing -// Desired once the desired state is achieved. -func (ns *NicState) SetState(s SwitchPortStatus) (NicState, bool) { +func (ns *NicState) SetState(status SwitchPortStatus) (NicState, bool) { if ns == nil { return NicState{ - Actual: s, + Actual: status, Desired: nil, }, true } - if ns.Actual == s { - if ns.Desired != nil { - if *ns.Desired == s { - // we now have the desired state, so set the desired state to nil - return NicState{ - Actual: s, - Desired: nil, - }, true - } else { - // we already have the reported state, but the desired one is different - // so nothing changed - return *ns, false - } - } - // nothing changed - return *ns, false - } - // we got another state as we had before - if ns.Desired != nil { - if *ns.Desired == s { - // we now have the desired state, so set the desired state to nil - return NicState{ - Actual: s, - Desired: nil, - }, true - } else { - // a new state was reported, but the desired one is different - // so we have to update the state but keep the desired state - return NicState{ - Actual: s, - Desired: ns.Desired, - }, true - } - } + changed := ns.Actual != status return NicState{ - Actual: s, - Desired: nil, - }, true + Actual: status, + Desired: ns.Desired, + }, changed } // WantState sets the desired state for the NIC. It returns a new NicState diff --git a/cmd/metal-api/internal/metal/network_test.go b/cmd/metal-api/internal/metal/network_test.go index 8c57a6c11..85556d587 100644 --- a/cmd/metal-api/internal/metal/network_test.go +++ b/cmd/metal-api/internal/metal/network_test.go @@ -231,96 +231,109 @@ func TestNicState_SetState(t *testing.T) { unknown := SwitchPortStatusUnknown tests := []struct { - name string - nic *NicState - arg SwitchPortStatus - want NicState - changed bool + name string + ns *NicState + status SwitchPortStatus + want NicState + wantChanged bool }{ { - name: "different actual with empty desired", - nic: &NicState{ + name: "state is nil", + ns: nil, + status: down, + want: NicState{ + Desired: nil, + Actual: down, + }, + wantChanged: true, + }, + { + name: "desired is nil and actual unchanged", + ns: &NicState{ Desired: nil, Actual: up, }, - arg: down, + status: up, want: NicState{ Desired: nil, - Actual: down, + Actual: up, }, - changed: true, + wantChanged: false, }, { - name: "different actual with same state in desired", - nic: &NicState{ - Desired: &down, + name: "desired is nil and actual changes", + ns: &NicState{ + Desired: nil, Actual: up, }, - arg: down, + status: unknown, want: NicState{ Desired: nil, - Actual: down, + Actual: unknown, }, - changed: true, + wantChanged: true, }, { - name: "different actual with other state in desired", - nic: &NicState{ - Desired: &unknown, - Actual: up, + name: "desired is set, new state changed and does not match desired", + ns: &NicState{ + Desired: &up, + Actual: unknown, }, - arg: down, + status: down, want: NicState{ - Desired: &unknown, + Desired: &up, Actual: down, }, - changed: true, + wantChanged: true, }, { - name: "nil nic", - nic: nil, - arg: down, + name: "desired is set, new state unchanged and does not match desired", + ns: &NicState{ + Desired: &up, + Actual: unknown, + }, + status: unknown, want: NicState{ - Desired: nil, - Actual: down, + Desired: &up, + Actual: unknown, }, - changed: true, + wantChanged: false, }, { - name: "same state with same desired", - nic: &NicState{ - Desired: &down, + name: "desired is set, new state changed and matches desired", + ns: &NicState{ + Desired: &up, Actual: down, }, - arg: down, + status: up, want: NicState{ - Desired: nil, - Actual: down, + Desired: &up, + Actual: up, }, - changed: true, + wantChanged: true, }, { - name: "same state with other desired", - nic: &NicState{ + name: "desired is set, new state unchanged and matches desired", + ns: &NicState{ Desired: &up, - Actual: down, + Actual: up, }, - arg: down, + status: up, want: NicState{ Desired: &up, - Actual: down, + Actual: up, }, - changed: false, + wantChanged: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1 := tt.nic.SetState(tt.arg) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("NicState.SetState() got = %+v, want %+v", got, tt.want) + got, changed := tt.ns.SetState(tt.status) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("NicState.SetState() diff = %s", diff) } - if got1 != tt.changed { - t.Errorf("NicState.SetState() got1 = %v, want %v", got1, tt.changed) + if changed != tt.wantChanged { + t.Errorf("NicState.SetState() got1 = %v, want %v", changed, tt.wantChanged) } }) } From 69462473ed527a37d4e1030e6991e13071690b37 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Wed, 1 Jul 2026 11:49:09 +0200 Subject: [PATCH 07/14] add admin status to machine connections as well --- .../internal/service/switch-service.go | 54 ++++++++++--------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 58e94b56a..76f066f6a 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -1025,7 +1025,10 @@ func (r *switchResource) makeSwitchResponse(s *metal.Switch) (*v1.SwitchResponse if err != nil { return nil, err } - cons := r.makeSwitchCons(s) + cons, err := r.makeSwitchCons(s) + if err != nil { + return nil, err + } return v1.NewSwitchResponse(s, ss, p, nics, cons), nil } @@ -1213,38 +1216,36 @@ func (r *switchResource) makeSwitchNics(s *metal.Switch, nws metal.NetworkMap, i return nics, nil } -func (r *switchResource) makeSwitchCons(s *metal.Switch) []v1.SwitchConnection { +func (r *switchResource) makeSwitchCons(s *metal.Switch) ([]v1.SwitchConnection, error) { cons := []v1.SwitchConnection{} nicMap := s.Nics.ByName() for _, metalConnections := range s.MachineConnections { for _, mc := range metalConnections { - // The connection state is set to the state of the NIC in the database. - // This state is not necessarily the actual state of the port on the switch. - // When the port is toggled, the connection state in the DB is updated after - // the real switch port changed state. - // So if a client queries the current switch state, it will see the desired - // state in the global NIC state, but the actual state of the port in the - // connection map. - n := nicMap[mc.Nic.Name] - state := metal.SwitchPortStatusUnknown - var bps *metal.SwitchBGPPortState - if n != nil && n.State != nil { - state = n.State.Actual - } - if n != nil && n.BGPPortState != nil { - bps = n.BGPPortState + n, ok := nicMap[mc.Nic.Name] + if !ok || n == nil { + return nil, fmt.Errorf("nic %s is connected to machine %s but could not be found on the switch %s", mc.Nic.Name, mc.MachineID, s.ID) } nic := v1.SwitchNic{ - MacAddress: string(mc.Nic.MacAddress), - Name: mc.Nic.Name, - Identifier: mc.Nic.Identifier, - Vrf: mc.Nic.Vrf, - Actual: v1.SwitchPortStatus(state), - BGPPortState: bps, + MacAddress: string(mc.Nic.MacAddress), + Name: mc.Nic.Name, + Identifier: mc.Nic.Identifier, + Vrf: mc.Nic.Vrf, + Actual: v1.SwitchPortStatusUnknown, + } + + if n.BGPPortState != nil { + nic.BGPPortState = n.BGPPortState } + if n.State != nil { + nic.Actual = v1.SwitchPortStatus(n.State.Actual) + if n.State.Desired != nil { + nic.AdminStatus = new(v1.SwitchPortStatus(*n.State.Desired)) + } + } + con := v1.SwitchConnection{ Nic: nic, MachineID: mc.MachineID, @@ -1257,7 +1258,7 @@ func (r *switchResource) makeSwitchCons(s *metal.Switch) []v1.SwitchConnection { return cons[i].MachineID < cons[j].MachineID }) - return cons + return cons, nil } func (r *switchResource) findSwitchReferencedEntities(s *metal.Switch) (*metal.Partition, metal.NetworkMap, metal.IPsMap, metal.Machines, *metal.SwitchStatus, error) { @@ -1321,7 +1322,10 @@ func (r *switchResource) makeSwitchResponseList(ss metal.Switches) ([]*v1.Switch if err != nil { return nil, err } - cons := r.makeSwitchCons(&sw) + cons, err := r.makeSwitchCons(&sw) + if err != nil { + return nil, err + } ss, err := r.ds.GetSwitchStatus(sw.ID) if err != nil && !metal.IsNotFound(err) { return nil, err From 892ce079e0505d720efe5faf8d21f23dfa874a36 Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Wed, 1 Jul 2026 12:19:22 +0200 Subject: [PATCH 08/14] fix test --- cmd/metal-api/internal/testdata/testdata.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/cmd/metal-api/internal/testdata/testdata.go b/cmd/metal-api/internal/testdata/testdata.go index e6974d651..9f3c43e24 100644 --- a/cmd/metal-api/internal/testdata/testdata.go +++ b/cmd/metal-api/internal/testdata/testdata.go @@ -486,7 +486,7 @@ var ( Name: "IPAM Network", Description: "description IPAM", }, - Prefixes: prefixesIPAM, + Prefixes: prefixesIPAM, } // IPs @@ -595,12 +595,6 @@ var ( }, MachineID: "1", }, - metal.Connection{ - Nic: metal.Nic{ - MacAddress: metal.MacAddress("11:11:11:11:11:22"), - }, - MachineID: "1", - }, }, }, } From f4b560992ff3bc9275bee8e6dc7e56dfcbe6c2dc Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Thu, 2 Jul 2026 16:56:09 +0200 Subject: [PATCH 09/14] allow switch port toggle when no machine is connected --- cmd/metal-api/internal/service/switch-service.go | 16 ---------------- .../internal/service/switch-service_test.go | 8 +++++--- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 76f066f6a..9cc2e1e34 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -423,22 +423,6 @@ func (r *switchResource) toggleSwitchPort(request *restful.Request, response *re return } - // now check if there is something connected at the given nic. - machineConnection := false - - for _, mcs := range newSwitch.MachineConnections { - for _, mc := range mcs { - if strings.EqualFold(mc.Nic.Name, requestPayload.NicName) { - machineConnection = true - break - } - } - } - if !machineConnection { - r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("switch %q does not have a connected machine at port %q", id, requestPayload.NicName))) - return - } - if updated { r.log.Debug("toggle switch port update switch", "new nics", newSwitch.Nics) if err := r.ds.UpdateSwitch(oldSwitch, &newSwitch); err != nil { diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index 2880a071b..29343f166 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -1777,12 +1777,14 @@ func TestToggleSwitchNicWithoutMachine(t *testing.T) { resp := w.Result() defer resp.Body.Close() - require.Equal(t, http.StatusBadRequest, resp.StatusCode, w.Body.String()) - var result httperrors.HTTPErrorResponse + require.Equal(t, http.StatusOK, resp.StatusCode, w.Body.String()) + var result v1.SwitchResponse err = json.NewDecoder(resp.Body).Decode(&result) require.NoError(t, err) - require.Equal(t, result.Message, fmt.Sprintf("switch %q does not have a connected machine at port %q", testdata.Switch1.ID, testdata.Switch1.Nics[1].Name)) + require.Equal(t, testdata.Switch1.ID, result.ID) + require.Equal(t, testdata.Switch1.Name, *result.Name) + require.Equal(t, v1.SwitchPortStatusDown, pointer.SafeDeref(result.Nics[1].AdminStatus)) } func Test_adjustMachineNics(t *testing.T) { From 49d60272197870354b8c96bac08d6afcb87d605c Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Fri, 3 Jul 2026 13:12:19 +0200 Subject: [PATCH 10/14] state is changed when desired changes --- cmd/metal-api/internal/metal/network.go | 3 +-- cmd/metal-api/internal/metal/network_test.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 7bd2d46dc..ea1519eb9 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -97,11 +97,10 @@ func (ns *NicState) WantState(s SwitchPortStatus) (NicState, bool) { }, true } if ns.Desired == nil { - changed := s != ns.Actual return NicState{ Desired: &s, Actual: ns.Actual, - }, changed + }, true } changed := s != *ns.Desired return NicState{ diff --git a/cmd/metal-api/internal/metal/network_test.go b/cmd/metal-api/internal/metal/network_test.go index 85556d587..3406a0977 100644 --- a/cmd/metal-api/internal/metal/network_test.go +++ b/cmd/metal-api/internal/metal/network_test.go @@ -156,7 +156,7 @@ func TestNicState_WantState(t *testing.T) { Desired: &down, Actual: down, }, - changed: false, + changed: true, }, { name: "current desired is nil, new desired differs from current actual", From 6d325c3f4a586092f7d19157faefee87d62ddd74 Mon Sep 17 00:00:00 2001 From: Markus Wennrich Date: Wed, 15 Jul 2026 08:40:31 +0200 Subject: [PATCH 11/14] fix test --- .../internal/service/switch-service_test.go | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index 31fbf8d00..02ee0e625 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -148,11 +148,22 @@ func TestRegisterExistingSwitchWithRoomChange(t *testing.T) { RackID: "1", RoomID: oldRoomID, OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, + Nics: metal.Nics{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:01", + }, + { + Name: "swp2", + MacAddress: "aa:aa:aa:aa:aa:02", + }, + }, MachineConnections: metal.ConnectionMap{ "machine-1": metal.Connections{ { Nic: metal.Nic{ - Name: "swp1", + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:01", }, MachineID: "machine-1", }, @@ -160,7 +171,8 @@ func TestRegisterExistingSwitchWithRoomChange(t *testing.T) { "machine-2": metal.Connections{ { Nic: metal.Nic{ - Name: "swp2", + Name: "swp2", + MacAddress: "aa:aa:aa:aa:aa:02", }, MachineID: "machine-2", }, @@ -198,6 +210,16 @@ func TestRegisterExistingSwitchWithRoomChange(t *testing.T) { ID: switchID, }, }, + Nics: v1.SwitchNics{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:01", + }, + { + Name: "swp2", + MacAddress: "aa:aa:aa:aa:aa:02", + }, + }, PartitionID: "1", SwitchBase: v1.SwitchBase{ RackID: "1", From fbd16d79db80a325e872e64d658fe61017614ac8 Mon Sep 17 00:00:00 2001 From: Ilja Rotar <77339620+iljarotar@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:11:16 +0200 Subject: [PATCH 12/14] adminstatus optional Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cmd/metal-api/internal/service/v1/switch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/v1/switch.go b/cmd/metal-api/internal/service/v1/switch.go index b41a70da3..5e76c51e4 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -47,7 +47,7 @@ type SwitchNic struct { Vrf string `json:"vrf" description:"the vrf this network interface is part of" optional:"true"` BGPFilter *BGPFilter `json:"filter" description:"configures the bgp filter applied at the switch port" optional:"true"` Actual SwitchPortStatus `json:"actual" description:"the current state of the nic" enum:"UP|DOWN|UNKNOWN"` - AdminStatus *SwitchPortStatus `json:"admin_status" description:"the desired state of the nic" enum:"UP|DOWN"` + AdminStatus *SwitchPortStatus `json:"admin_status" description:"the desired state of the nic" enum:"UP|DOWN" optional:"true"` BGPPortState *metal.SwitchBGPPortState `json:"bgp_port_state" description:"the current bgp port state" optional:"true"` } From 2738b0148748d219645122f78240912db0ebd3fb Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Wed, 15 Jul 2026 11:14:30 +0200 Subject: [PATCH 13/14] review findings --- cmd/metal-api/internal/metal/network.go | 3 +-- cmd/metal-api/internal/service/switch-service.go | 3 +-- spec/metal-api.json | 1 - 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index ea1519eb9..5aee1c403 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -54,8 +54,7 @@ type Nic struct { // NicState represents the desired and actual state of a network interface // controller (NIC). The Desired field indicates the intended state of the -// NIC, while Actual indicates its current operational state. The Desired -// state will be removed when the actual state is equal to the desired state. +// NIC, while Actual indicates its current operational state. type NicState struct { Desired *SwitchPortStatus `rethinkdb:"desired" json:"desired"` Actual SwitchPortStatus `rethinkdb:"actual" json:"actual"` diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 35e76557c..c8f2b0ba1 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -373,7 +373,7 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu } // toggleSwitchPort handles a request to toggle the state of a port on a switch. It reads the request body, finds the switch, updates its NIC state if needed, and returns the updated switch on success. -// If the given port is not found or the given status is not concrete, a 400 error is returned. Another requirement is that there must be a machine connected to the port. +// If the given port is not found or the given status is not concrete, a 400 error is returned. func (r *switchResource) toggleSwitchPort(request *restful.Request, response *restful.Response) { var requestPayload v1.SwitchPortToggleRequest err := request.ReadEntity(&requestPayload) @@ -425,7 +425,6 @@ func (r *switchResource) toggleSwitchPort(request *restful.Request, response *re } if updated { - r.log.Debug("toggle switch port update switch", "new nics", newSwitch.Nics) if err := r.ds.UpdateSwitch(oldSwitch, &newSwitch); err != nil { r.sendError(request, response, defaultError(err)) return diff --git a/spec/metal-api.json b/spec/metal-api.json index 887ad7a87..7dfde82d0 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -5503,7 +5503,6 @@ }, "required": [ "actual", - "admin_status", "identifier", "mac", "name" From b68730f7ed8cf9fd7b59125ba9ccf46b04307c9d Mon Sep 17 00:00:00 2001 From: Ilja Rotar Date: Wed, 15 Jul 2026 11:22:00 +0200 Subject: [PATCH 14/14] linter --- .../internal/service/machine-service_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/machine-service_integration_test.go b/cmd/metal-api/internal/service/machine-service_integration_test.go index 51cf4a1f9..1bc2fda94 100644 --- a/cmd/metal-api/internal/service/machine-service_integration_test.go +++ b/cmd/metal-api/internal/service/machine-service_integration_test.go @@ -300,7 +300,7 @@ func BenchmarkMachineList(b *testing.B) { b.ResetTimer() - for range b.N { + for b.Loop() { var machines []v1.MachineResponse code := webRequestGet(b, machineService, &testUserDirectory.admin, nil, "/v1/machine", &machines)