From dd6d7f5e092e4810d9e45dbf1a42be7704a94b47 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 26 Feb 2024 13:49:12 +0100 Subject: [PATCH 01/22] allow the API to toggle switch ports --- cmd/metal-api/internal/metal/network.go | 124 ++++++++++++++++++ .../internal/service/switch-service.go | 104 +++++++++++++++ .../internal/service/switch-service_test.go | 111 ++++++++++++++++ cmd/metal-api/internal/service/v1/switch.go | 38 +++++- 4 files changed, 370 insertions(+), 7 deletions(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 09ecb4499..79895f7ab 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -6,6 +6,33 @@ import ( "strings" ) +// SwitchPortStatus is a type alias for a string that represents the status of a switch port. +// Valid values are defined as constants in this package. +type SwitchPortStatus string + +// SwitchPortStatus defines the possible statuses for a switch port. +// UNKNOWN indicates the status is not known. +// UP indicates the port is up and operational. +// DOWN indicates the port is down and not operational. +const ( + SwitchPortStatusUnknown SwitchPortStatus = "UNKNOWN" + SwitchPortStatusUp SwitchPortStatus = "UP" + SwitchPortStatusDown SwitchPortStatus = "DOWN" +) + +// IsConcrete returns true if the SwitchPortStatus is UP or DOWN, +// which are concrete, known statuses. It returns false if the status +// is UNKNOWN, which indicates the status is not known. +func (s SwitchPortStatus) IsConcrete() bool { + return s == SwitchPortStatusUp || s == SwitchPortStatusDown +} + +// IsValid returns true if the SwitchPortStatus is a known valid value +// (UP, DOWN, UNKNOWN). +func (s SwitchPortStatus) IsValid() bool { + return s == SwitchPortStatusUp || s == SwitchPortStatusDown || s == SwitchPortStatusUnknown +} + // A MacAddress is the type for mac adresses. When using a // custom type, we cannot use strings directly. type MacAddress string @@ -18,6 +45,103 @@ type Nic struct { Vrf string `rethinkdb:"vrf" json:"vrf"` Neighbors Nics `rethinkdb:"neighbors" json:"neighbors"` Hostname string `rethinkdb:"hostname" json:"hostname"` + State *NicState `rethinkdb:"state" json:"state"` +} + +// 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 actuale state is equal to the desired state. +type NicState struct { + Desired *SwitchPortStatus `rethinkdb:"desired" json:"desired"` + Actual SwitchPortStatus `rethinkdb:"actual" json:"actual"` +} + +// 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 differes 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) { + if ns == nil { + return NicState{ + Actual: s, + 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 { + // we already have the reported state, but the desired one is different + // so nothing changed + return NicState{ + Actual: s, + Desired: ns.Desired, + }, true + } + } + return NicState{ + Actual: s, + Desired: nil, + }, true +} + +// 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{ + Actual: SwitchPortStatusUnknown, + 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 NicState{ + Actual: ns.Actual, + Desired: &s, + }, true } // GetIdentifier returns the identifier of a nic. diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index d0a392bfb..0af0a6140 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "sort" + "strings" "time" "github.com/avast/retry-go/v4" @@ -104,6 +105,17 @@ func (r *switchResource) webService() *restful.WebService { Returns(http.StatusConflict, "Conflict", httperrors.HTTPErrorResponse{}). DefaultReturns("Error", httperrors.HTTPErrorResponse{})) + ws.Route(ws.POST("/{id}/port"). + To(admin(r.toggleSwitchPort)). + Operation("toggleSwitchPort"). + Doc("toggles the port of the switch with a nicname to the given state"). + Metadata(restfulspec.KeyOpenAPITags, tags). + Reads(v1.SwitchPortToggleRequest{}). + Returns(http.StatusOK, "OK", v1.SwitchResponse{}). + Returns(http.StatusConflict, "Conflict", httperrors.HTTPErrorResponse{}). + Returns(http.StatusNotModified, "Not modified", nil). + DefaultReturns("Error", httperrors.HTTPErrorResponse{})) + ws.Route(ws.POST("/{id}/notify"). To(editor(r.notifySwitch)). Doc("notify the metal-api about a configuration change of a switch"). @@ -244,9 +256,100 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu return } + oldSwitch, err := r.ds.FindSwitch(id) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + newSwitch := *oldSwitch + switchUpdated := false + for i, nic := range newSwitch.Nics { + state, has := requestPayload.PortStates[strings.ToLower(nic.Name)] + if has { + reported := metal.SwitchPortStatus(state) + newstate, changed := nic.State.SetState(reported) + if changed { + newSwitch.Nics[i].State = &newstate + switchUpdated = true + } + } else { + // this should NEVER happen; if the switch reports the state of an unknown port + // we log this and ignore it, but something is REALLY wrong in this case + r.log.Errorw("unknown switch port", "id", id, "nic", nic.Name) + } + } + + if switchUpdated { + if err := r.ds.UpdateSwitch(oldSwitch, &newSwitch); err != nil { + r.sendError(request, response, defaultError(err)) + return + } + } + r.send(request, response, http.StatusOK, v1.NewSwitchNotifyResponse(&newSS)) } +func (r *switchResource) toggleSwitchPort(request *restful.Request, response *restful.Response) { + var requestPayload v1.SwitchPortToggleRequest + err := request.ReadEntity(&requestPayload) + if err != nil { + r.sendError(request, response, httperrors.BadRequest(err)) + return + } + + desired := metal.SwitchPortStatus(requestPayload.Status) + + if !desired.IsConcrete() { + r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("the status %q must be concrete", requestPayload.Status))) + return + } + + id := request.PathParameter("id") + oldSwitch, err := r.ds.FindSwitch(id) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + newSwitch := *oldSwitch + nicname := strings.ToLower(requestPayload.NicName) + updated := false + found := false + for i, nic := range newSwitch.Nics { + if nic.Name == nicname { + found = true + newstate, changed := nic.State.WantState(desired) + if changed { + newSwitch.Nics[i].State = &newstate + updated = true + break + } + } + } + if !found { + r.sendError(request, response, httperrors.NotFound(fmt.Errorf("the nic %q does not exist in this switch", requestPayload.NicName))) + return + } + if !updated { + r.send(request, response, http.StatusNotModified, nil) + return + } + + if err := r.ds.UpdateSwitch(oldSwitch, &newSwitch); err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + resp, err := makeSwitchResponse(&newSwitch, r.ds) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + + r.send(request, response, http.StatusOK, resp) +} + func (r *switchResource) updateSwitch(request *restful.Request, response *restful.Response) { var requestPayload v1.SwitchUpdateRequest err := request.ReadEntity(&requestPayload) @@ -732,6 +835,7 @@ func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines) Identifier: n.Identifier, Vrf: n.Vrf, BGPFilter: filter, + Actual: v1.SwitchPortStatus(pointer.SafeDerefOrDefault(n.State, metal.NicState{Actual: metal.SwitchPortStatusUnknown}).Actual), } 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 efaa69b06..509185d07 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -3,6 +3,7 @@ package service import ( "bytes" "encoding/json" + "fmt" "net/http" "net/http/httptest" "reflect" @@ -19,6 +20,7 @@ import ( "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" 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" ) func TestRegisterSwitch(t *testing.T) { @@ -521,6 +523,9 @@ func TestMakeSwitchNics(t *testing.T) { metal.Nic{ Name: "swp1", Vrf: "vrf1", + State: &metal.NicState{ + Actual: metal.SwitchPortStatusUp, + }, }, metal.Nic{ Name: "swp2", @@ -563,6 +568,7 @@ func TestMakeSwitchNics(t *testing.T) { CIDRs: []string{"212.89.1.1/32"}, VNIs: []string{}, }, + Actual: v1.SwitchPortStatusUp, }, v1.SwitchNic{ Name: "swp2", @@ -571,6 +577,7 @@ func TestMakeSwitchNics(t *testing.T) { CIDRs: []string{}, VNIs: []string{"1", "2"}, }, + Actual: v1.SwitchPortStatusUnknown, }, }, }, @@ -1274,3 +1281,107 @@ func TestNotifyErrorSwitch(t *testing.T) { require.Equal(t, d, result.LastSyncError.Duration) require.Equal(t, e, *result.LastSyncError.Error) } + +func TestToggleSwitchWrongNic(t *testing.T) { + ds, mock := datastore.InitMockDB(t) + testdata.InitMockDBData(mock) + log := zaptest.NewLogger(t).Sugar() + + switchservice := NewSwitch(log, ds) + container := restful.NewContainer().Add(switchservice) + + updateRequest := v1.SwitchPortToggleRequest{ + NicName: "wrongname", + Status: v1.SwitchPortStatusDown, + } + js, err := json.Marshal(updateRequest) + require.NoError(t, err) + body := bytes.NewBuffer(js) + req := httptest.NewRequest("POST", "/v1/switch/"+testdata.Switch1.ID+"/port", body) + container = injectAdmin(log, container, req) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + container.ServeHTTP(w, req) + + resp := w.Result() + defer resp.Body.Close() + require.Equal(t, http.StatusNotFound, resp.StatusCode, w.Body.String()) + var result httperrors.HTTPErrorResponse + err = json.NewDecoder(resp.Body).Decode(&result) + + require.NoError(t, err) + require.Equal(t, result.Message, "the nic \"wrongname\" does not exist in this switch") +} + +func TestToggleSwitchWrongState(t *testing.T) { + ds, mock := datastore.InitMockDB(t) + testdata.InitMockDBData(mock) + log := zaptest.NewLogger(t).Sugar() + + switchservice := NewSwitch(log, ds) + container := restful.NewContainer().Add(switchservice) + + states := []v1.SwitchPortStatus{ + v1.SwitchPortStatusUnknown, + v1.SwitchPortStatus("illegal"), + } + + for _, s := range states { + + updateRequest := v1.SwitchPortToggleRequest{ + NicName: testdata.Switch1.Nics[0].Name, + Status: s, + } + js, err := json.Marshal(updateRequest) + require.NoError(t, err) + body := bytes.NewBuffer(js) + req := httptest.NewRequest("POST", "/v1/switch/"+testdata.Switch1.ID+"/port", body) + container = injectAdmin(log, container, req) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + container.ServeHTTP(w, req) + + resp := w.Result() + defer resp.Body.Close() + require.Equal(t, http.StatusBadRequest, resp.StatusCode, w.Body.String()) + var result httperrors.HTTPErrorResponse + err = json.NewDecoder(resp.Body).Decode(&result) + + require.NoError(t, err) + require.Equal(t, result.Message, fmt.Sprintf("the status %q must be concrete", s)) + } +} + +func TestToggleSwitch(t *testing.T) { + ds, mock := datastore.InitMockDB(t) + testdata.InitMockDBData(mock) + log := zaptest.NewLogger(t).Sugar() + + switchservice := NewSwitch(log, ds) + container := restful.NewContainer().Add(switchservice) + + updateRequest := v1.SwitchPortToggleRequest{ + NicName: testdata.Switch1.Nics[0].Name, + Status: v1.SwitchPortStatusDown, + } + + js, err := json.Marshal(updateRequest) + require.NoError(t, err) + body := bytes.NewBuffer(js) + req := httptest.NewRequest("POST", "/v1/switch/"+testdata.Switch1.ID+"/port", body) + container = injectAdmin(log, container, req) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + container.ServeHTTP(w, req) + + resp := w.Result() + defer resp.Body.Close() + 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, testdata.Switch1.ID, result.ID) + require.Equal(t, testdata.Switch1.Name, *result.Name) + require.Equal(t, result.Nics[0].Actual, v1.SwitchPortStatusUnknown) +} diff --git a/cmd/metal-api/internal/service/v1/switch.go b/cmd/metal-api/internal/service/v1/switch.go index 7fad81462..cfd05b523 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -8,6 +8,20 @@ import ( "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" ) +// SwitchPortStatus is a type alias for a string that represents the status of a switch port. +// Valid values are defined as constants in this package. +type SwitchPortStatus string + +// SwitchPortStatus defines the possible statuses for a switch port. +// UNKNOWN indicates the status is not known. +// UP indicates the port is up and operational. +// DOWN indicates the port is down and not operational. +const ( + SwitchPortStatusUnknown SwitchPortStatus = "UNKNOWN" + SwitchPortStatusUp SwitchPortStatus = "UP" + SwitchPortStatusDown SwitchPortStatus = "DOWN" +) + type SwitchBase struct { RackID string `json:"rack_id" modelDescription:"A switch that can register at the api." description:"the id of the rack in which this switch is located"` Mode string `json:"mode" description:"the mode the switch currently has" optional:"true"` @@ -26,11 +40,12 @@ type SwitchOS struct { type SwitchNics []SwitchNic type SwitchNic struct { - MacAddress string `json:"mac" description:"the mac address of this network interface"` - Name string `json:"name" description:"the name of this network interface"` - Identifier string `json:"identifier" description:"the identifier of this network interface"` - 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"` + MacAddress string `json:"mac" description:"the mac address of this network interface"` + Name string `json:"name" description:"the name of this network interface"` + Identifier string `json:"identifier" description:"the identifier of this network interface"` + 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"` } type BGPFilter struct { @@ -70,9 +85,18 @@ type SwitchUpdateRequest struct { SwitchBase } +type SwitchPortToggleRequest struct { + NicName string `json:"nic" description:"the nic of the switch you want to change"` + Status SwitchPortStatus `json:"status" description:"sets the port status"` +} + +// SwitchNotifyRequest represents the notification sent from the switch +// to the metal-api after a sync operation. It contains the duration of +// the sync, any error that occurred, and the updated switch port states. type SwitchNotifyRequest struct { - Duration time.Duration `json:"sync_duration" description:"the duration of the switch synchronization"` - Error *string `json:"error"` + Duration time.Duration `json:"sync_duration" description:"the duration of the switch synchronization"` + Error *string `json:"error"` + PortStates map[string]SwitchPortStatus `json:"port_states" description:"the current switch port states"` } type SwitchNotifyResponse struct { From bae5b4b9e02cdb3dff8dfcf8e0dd01b5a0f5f6e1 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Thu, 29 Feb 2024 09:18:28 +0100 Subject: [PATCH 02/22] add path parameter --- .../internal/service/switch-service.go | 1 + spec/metal-api.json | 84 +++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 0af0a6140..50e205cde 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -108,6 +108,7 @@ func (r *switchResource) webService() *restful.WebService { ws.Route(ws.POST("/{id}/port"). To(admin(r.toggleSwitchPort)). Operation("toggleSwitchPort"). + Param(ws.PathParameter("id", "identifier of the switch").DataType("string")). Doc("toggles the port of the switch with a nicname to the given state"). Metadata(restfulspec.KeyOpenAPITags, tags). Reads(v1.SwitchPortToggleRequest{}). diff --git a/spec/metal-api.json b/spec/metal-api.json index c5ef503fa..84f645f0c 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4862,6 +4862,10 @@ }, "v1.SwitchNic": { "properties": { + "actual": { + "description": "the current state of the nic", + "type": "string" + }, "filter": { "$ref": "#/definitions/v1.BGPFilter", "description": "configures the bgp filter applied at the switch port" @@ -4884,6 +4888,7 @@ } }, "required": [ + "actual", "identifier", "mac", "name" @@ -4894,6 +4899,13 @@ "error": { "type": "string" }, + "port_states": { + "additionalProperties": { + "type": "string" + }, + "description": "the current switch port states", + "type": "object" + }, "sync_duration": { "description": "the duration of the switch synchronization", "format": "int64", @@ -4902,6 +4914,7 @@ }, "required": [ "error", + "port_states", "sync_duration" ] }, @@ -4948,6 +4961,22 @@ } } }, + "v1.SwitchPortToggleRequest": { + "properties": { + "nic": { + "description": "the nic of the switch you want to change", + "type": "string" + }, + "status": { + "description": "sets the port status", + "type": "string" + } + }, + "required": [ + "nic", + "status" + ] + }, "v1.SwitchRegisterRequest": { "properties": { "console_command": { @@ -9438,6 +9467,61 @@ ] } }, + "/v1/switch/{id}/port": { + "post": { + "consumes": [ + "application/json" + ], + "operationId": "toggleSwitchPort", + "parameters": [ + { + "description": "identifier of the switch", + "in": "path", + "name": "id", + "required": true, + "type": "string" + }, + { + "in": "body", + "name": "body", + "required": true, + "schema": { + "$ref": "#/definitions/v1.SwitchPortToggleRequest" + } + } + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/v1.SwitchResponse" + } + }, + "304": { + "description": "Not modified" + }, + "409": { + "description": "Conflict", + "schema": { + "$ref": "#/definitions/httperrors.HTTPErrorResponse" + } + }, + "default": { + "description": "Error", + "schema": { + "$ref": "#/definitions/httperrors.HTTPErrorResponse" + } + } + }, + "summary": "toggles the port of the switch with a nicname to the given state", + "tags": [ + "switch" + ] + } + }, "/v1/tenant": { "get": { "consumes": [ From b5591434eec87a03f44da6928cd0ad4e68259648 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Thu, 29 Feb 2024 09:32:44 +0100 Subject: [PATCH 03/22] repair test to make linter happy --- cmd/metal-api/internal/service/switch-service_test.go | 4 ++-- 1 file changed, 2 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 509185d07..d8b7acb1f 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -1310,7 +1310,7 @@ func TestToggleSwitchWrongNic(t *testing.T) { err = json.NewDecoder(resp.Body).Decode(&result) require.NoError(t, err) - require.Equal(t, result.Message, "the nic \"wrongname\" does not exist in this switch") + require.Equal(t, "the nic \"wrongname\" does not exist in this switch", result.Message) } func TestToggleSwitchWrongState(t *testing.T) { @@ -1383,5 +1383,5 @@ 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, result.Nics[0].Actual, v1.SwitchPortStatusUnknown) + require.Equal(t, v1.SwitchPortStatusUnknown, result.Nics[0].Actual) } From ef31bab0dccec06b45034ffcbaec5f444ebac48c Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Thu, 29 Feb 2024 10:07:01 +0100 Subject: [PATCH 04/22] see linter errors on local machine before pushing to gh --- .golangci.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.golangci.yaml b/.golangci.yaml index 9449c1ce5..a4a01a8fc 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -3,4 +3,8 @@ run: deadline: 10m linters: disable: - - musttag \ No newline at end of file + - musttag + enable: + - testifylint + - unused +fast: true From aa588f01c793dc1fc7815fef8d673c91c6117bd3 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 18 Mar 2024 15:14:38 +0100 Subject: [PATCH 05/22] not needed any more --- Dockerfile.dev | 4 ---- Makefile | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) delete mode 100644 Dockerfile.dev diff --git a/Dockerfile.dev b/Dockerfile.dev deleted file mode 100644 index d62669243..000000000 --- a/Dockerfile.dev +++ /dev/null @@ -1,4 +0,0 @@ -FROM alpine:3.19 -RUN apk add ca-certificates -COPY bin/metal-api /metal-api -ENTRYPOINT [ "/metal-api" ] \ No newline at end of file diff --git a/Makefile b/Makefile index 808974e26..247d16e40 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ protoc: .PHONY: mini-lab-push mini-lab-push: make - docker build -f Dockerfile.dev -t metalstack/metal-api:latest . + docker build -f Dockerfile -t metalstack/metal-api:latest . kind --name metal-control-plane load docker-image metalstack/metal-api:latest kubectl --kubeconfig=$(MINI_LAB_KUBECONFIG) patch deployments.apps -n metal-control-plane metal-api --patch='{"spec":{"template":{"spec":{"containers":[{"name": "metal-api","imagePullPolicy":"IfNotPresent","image":"metalstack/metal-api:latest"}]}}}}' kubectl --kubeconfig=$(MINI_LAB_KUBECONFIG) delete pod -n metal-control-plane -l app=metal-api From 20f329d3e43b239aeaa569c40471ec47d875e3da Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 18 Mar 2024 15:15:55 +0100 Subject: [PATCH 06/22] send desired state to switch if it is set --- .gitignore | 1 + .../internal/service/switch-service.go | 22 +++++++++++-------- .../internal/service/switch-service_test.go | 2 +- spec/metal-api.json | 3 --- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index 564f528a0..482cf0eac 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ vendor generate coverage.out __debug_bin +.mirrord diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 50e205cde..3ca45ebb4 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -114,7 +114,6 @@ func (r *switchResource) webService() *restful.WebService { Reads(v1.SwitchPortToggleRequest{}). Returns(http.StatusOK, "OK", v1.SwitchResponse{}). Returns(http.StatusConflict, "Conflict", httperrors.HTTPErrorResponse{}). - Returns(http.StatusNotModified, "Not modified", nil). DefaultReturns("Error", httperrors.HTTPErrorResponse{})) ws.Route(ws.POST("/{id}/notify"). @@ -332,14 +331,12 @@ func (r *switchResource) toggleSwitchPort(request *restful.Request, response *re r.sendError(request, response, httperrors.NotFound(fmt.Errorf("the nic %q does not exist in this switch", requestPayload.NicName))) return } - if !updated { - r.send(request, response, http.StatusNotModified, nil) - return - } - if err := r.ds.UpdateSwitch(oldSwitch, &newSwitch); err != nil { - r.sendError(request, response, defaultError(err)) - return + if updated { + if err := r.ds.UpdateSwitch(oldSwitch, &newSwitch); err != nil { + r.sendError(request, response, defaultError(err)) + return + } } resp, err := makeSwitchResponse(&newSwitch, r.ds) @@ -836,7 +833,14 @@ func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines) Identifier: n.Identifier, Vrf: n.Vrf, BGPFilter: filter, - Actual: v1.SwitchPortStatus(pointer.SafeDerefOrDefault(n.State, metal.NicState{Actual: metal.SwitchPortStatusUnknown}).Actual), + Actual: v1.SwitchPortStatusUnknown, + } + if n.State != nil { + if n.State.Desired != nil { + nic.Actual = v1.SwitchPortStatus(*n.State.Desired) + } else { + nic.Actual = v1.SwitchPortStatus(n.State.Actual) + } } 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 d8b7acb1f..39713281a 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -1383,5 +1383,5 @@ 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.SwitchPortStatusUnknown, result.Nics[0].Actual) + require.Equal(t, v1.SwitchPortStatusDown, result.Nics[0].Actual) } diff --git a/spec/metal-api.json b/spec/metal-api.json index 84f645f0c..3ad891562 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -9500,9 +9500,6 @@ "$ref": "#/definitions/v1.SwitchResponse" } }, - "304": { - "description": "Not modified" - }, "409": { "description": "Conflict", "schema": { From e3e8d5d3ae238c394f11b0c19fd6f1398d9615f3 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Tue, 19 Mar 2024 09:47:33 +0100 Subject: [PATCH 07/22] add enum types for port state --- cmd/metal-api/internal/service/v1/switch.go | 2 +- spec/metal-api.json | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/v1/switch.go b/cmd/metal-api/internal/service/v1/switch.go index cfd05b523..3bbc4a6d3 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -45,7 +45,7 @@ type SwitchNic struct { Identifier string `json:"identifier" description:"the identifier of this network interface"` 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"` + Actual SwitchPortStatus `json:"actual" description:"the current state of the nic" enum:"UP|DOWN|UNKNOWN"` } type BGPFilter struct { diff --git a/spec/metal-api.json b/spec/metal-api.json index 3ad891562..5dcb467c4 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4864,6 +4864,11 @@ "properties": { "actual": { "description": "the current state of the nic", + "enum": [ + "DOWN", + "UNKNOWN", + "UP" + ], "type": "string" }, "filter": { From 2d0093aac3038c98a300ca9387faa6d9d5aadebe Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Tue, 19 Mar 2024 12:08:32 +0100 Subject: [PATCH 08/22] set the enum values in the endpoint --- 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 3bbc4a6d3..92d912e4f 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -87,7 +87,7 @@ type SwitchUpdateRequest struct { type SwitchPortToggleRequest struct { NicName string `json:"nic" description:"the nic of the switch you want to change"` - Status SwitchPortStatus `json:"status" description:"sets the port status"` + Status SwitchPortStatus `json:"status" description:"sets the port status" enum:"UP|DOWN"` } // SwitchNotifyRequest represents the notification sent from the switch From 0a629c9e2bbfbe3d1bf8ed3bc812a4765ddd75eb Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Tue, 2 Apr 2024 09:58:54 +0200 Subject: [PATCH 09/22] merge master --- spec/metal-api.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/metal-api.json b/spec/metal-api.json index 76bde6d9a..149dd0bf2 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4981,6 +4981,10 @@ }, "status": { "description": "sets the port status", + "enum": [ + "DOWN", + "UP" + ], "type": "string" } }, From d5e6e9f68f2f06a37fbd922a22ef402a090f2b53 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Thu, 4 Apr 2024 10:53:01 +0200 Subject: [PATCH 10/22] comment code --- cmd/metal-api/internal/service/switch-service.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index fda64e699..6884fe51f 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -290,6 +290,8 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu r.send(request, response, http.StatusOK, v1.NewSwitchNotifyResponse(&newSS)) } +// toggleSwitchPort handles a request to toggle the state of a port on a switch. It reads the request body, validates the requested status is concrete, finds the switch, updates its NIC state if needed, and returns the updated switch on success. +// toggleSwitchPort handles a request to toggle the state of a port on a switch. It reads the request body to get the switch ID, NIC name and desired state. It finds the switch, updates the state of the matching NIC if needed, and returns the updated switch on success. func (r *switchResource) toggleSwitchPort(request *restful.Request, response *restful.Response) { var requestPayload v1.SwitchPortToggleRequest err := request.ReadEntity(&requestPayload) @@ -313,11 +315,18 @@ func (r *switchResource) toggleSwitchPort(request *restful.Request, response *re } newSwitch := *oldSwitch - nicname := strings.ToLower(requestPayload.NicName) updated := false found := false + + // Updates the state of a NIC on the switch if the requested state change is valid + // + // Loops through each NIC on the switch and checks if the name matches the + // requested NIC. If a match is found, it tries to update the NIC's state to + // the requested state. If the state change is valid, it sets the new state on + // the NIC and marks that an update was made. for i, nic := range newSwitch.Nics { - if nic.Name == nicname { + // compre nic-names case-insensitive + if strings.EqualFold(nic.Name, requestPayload.NicName) { found = true newstate, changed := nic.State.WantState(desired) if changed { From b118ddc0f9b46feaf42c211222475f3086b72d44 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Thu, 4 Apr 2024 11:04:00 +0200 Subject: [PATCH 11/22] fix typo --- 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 6884fe51f..84398619e 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -325,7 +325,7 @@ func (r *switchResource) toggleSwitchPort(request *restful.Request, response *re // the requested state. If the state change is valid, it sets the new state on // the NIC and marks that an update was made. for i, nic := range newSwitch.Nics { - // compre nic-names case-insensitive + // compare nic-names case-insensitive if strings.EqualFold(nic.Name, requestPayload.NicName) { found = true newstate, changed := nic.State.WantState(desired) From 5276d552c552c9759d2731157eacda190d75ebdd Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Thu, 4 Apr 2024 11:09:56 +0200 Subject: [PATCH 12/22] fix typo --- cmd/metal-api/internal/metal/network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 2bb70763e..5e4db3c5c 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -51,7 +51,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 actuale state is equal to the desired state. +// state will be removed when the actual state is equal to the desired state. type NicState struct { Desired *SwitchPortStatus `rethinkdb:"desired" json:"desired"` Actual SwitchPortStatus `rethinkdb:"actual" json:"actual"` From d3d32eda95f5d72ed18eca3368355b00c11ae502 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 8 Apr 2024 13:14:33 +0200 Subject: [PATCH 13/22] add check for bugs and unused --- .golangci.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.golangci.yaml b/.golangci.yaml index a4a01a8fc..a9dd7d49f 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -4,7 +4,11 @@ run: linters: disable: - musttag + - protogetter enable: - testifylint - unused + presets: + - bugs + - unused fast: true From cb1fac489dc3eb01c7a5f308958823b6c23fdc96 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Wed, 17 Apr 2024 13:03:07 +0200 Subject: [PATCH 14/22] set connection nic state to the REAL state not the desired state --- cmd/metal-api/internal/service/switch-service.go | 15 +++++++++++++++ .../internal/service/switch-service_test.go | 1 + 2 files changed, 16 insertions(+) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 84398619e..07fa8c31b 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -864,13 +864,28 @@ func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines) func makeSwitchCons(s *metal.Switch) []v1.SwitchConnection { 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 + if n != nil { + state = n.State.Actual + } nic := v1.SwitchNic{ MacAddress: string(mc.Nic.MacAddress), Name: mc.Nic.Name, Identifier: mc.Nic.Identifier, Vrf: mc.Nic.Vrf, + Actual: v1.SwitchPortStatus(state), } con := v1.SwitchConnection{ Nic: nic, diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index 0b5e3c730..32325beff 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -1384,4 +1384,5 @@ func TestToggleSwitch(t *testing.T) { 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.SwitchPortStatusUnknown, result.Connections[0].Nic.Actual) } From e2f3552bc291f193d6a1707a354f97d3abf91142 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 22 Apr 2024 11:16:07 +0200 Subject: [PATCH 15/22] nic state could be nil, so check state too --- 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 07fa8c31b..107f85ef6 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -877,7 +877,7 @@ func makeSwitchCons(s *metal.Switch) []v1.SwitchConnection { // connection map. n := nicMap[mc.Nic.Name] state := metal.SwitchPortStatusUnknown - if n != nil { + if n != nil && n.State != nil { state = n.State.Actual } nic := v1.SwitchNic{ From 5b62d9957a03eaa8a0e023394ceb5f28a266dd90 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 22 Apr 2024 13:01:50 +0200 Subject: [PATCH 16/22] make sure we do not crash with old versions of metal-core --- .../internal/service/switch-service.go | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 107f85ef6..91cfd55f5 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -264,19 +264,23 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu newSwitch := *oldSwitch switchUpdated := false - for i, nic := range newSwitch.Nics { - state, has := requestPayload.PortStates[strings.ToLower(nic.Name)] - if has { - reported := metal.SwitchPortStatus(state) - newstate, changed := nic.State.SetState(reported) - if changed { - newSwitch.Nics[i].State = &newstate - switchUpdated = true + + // old versions of metal-core do not send this field, so make sure we do not crash here + if requestPayload.PortStates != nil { + for i, nic := range newSwitch.Nics { + state, has := requestPayload.PortStates[strings.ToLower(nic.Name)] + if has { + reported := metal.SwitchPortStatus(state) + newstate, changed := nic.State.SetState(reported) + if changed { + newSwitch.Nics[i].State = &newstate + switchUpdated = true + } + } else { + // this should NEVER happen; if the switch reports the state of an unknown port + // we log this and ignore it, but something is REALLY wrong in this case + r.log.Error("unknown switch port", "id", id, "nic", nic.Name) } - } else { - // this should NEVER happen; if the switch reports the state of an unknown port - // we log this and ignore it, but something is REALLY wrong in this case - r.log.Error("unknown switch port", "id", id, "nic", nic.Name) } } From 3a3c6d6249ff7cd3ff42ac0516b5ad612cf81b23 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 22 Apr 2024 13:02:16 +0200 Subject: [PATCH 17/22] resolve review conversation --- 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 91cfd55f5..50445e649 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -336,8 +336,8 @@ func (r *switchResource) toggleSwitchPort(request *restful.Request, response *re if changed { newSwitch.Nics[i].State = &newstate updated = true - break } + break } } if !found { From 4dfae64cf7ce952be1c97e26b8a53a144c1c86a8 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 22 Apr 2024 13:07:51 +0200 Subject: [PATCH 18/22] FIX: wrong comment --- cmd/metal-api/internal/service/switch-service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 50445e649..2614535a5 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -277,8 +277,8 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu switchUpdated = true } } else { - // this should NEVER happen; if the switch reports the state of an unknown port - // we log this and ignore it, but something is REALLY wrong in this case + // This should NEVER happen; the switch does not know the given NIC. + // We log this and ignore it, but something is REALLY wrong in this case r.log.Error("unknown switch port", "id", id, "nic", nic.Name) } } From 2345b8b8e2d17cd6c6fa84e8bc2a401b6dbc7331 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 22 Apr 2024 13:11:13 +0200 Subject: [PATCH 19/22] document the different nic-states in the different fields of the response --- cmd/metal-api/internal/service/v1/switch.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/service/v1/switch.go b/cmd/metal-api/internal/service/v1/switch.go index 92d912e4f..99eaf221d 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -108,9 +108,9 @@ type SwitchNotifyResponse struct { type SwitchResponse struct { Common SwitchBase - Nics SwitchNics `json:"nics" description:"the list of network interfaces on the switch"` + Nics SwitchNics `json:"nics" description:"the list of network interfaces on the switch with the desired nic states"` Partition PartitionResponse `json:"partition" description:"the partition in which this switch is located"` - Connections []SwitchConnection `json:"connections" description:"a connection between a switch port and a machine"` + Connections []SwitchConnection `json:"connections" description:"a connection between a switch port and a machine with the real nic states"` LastSync *SwitchSync `json:"last_sync" description:"last successful synchronization to the switch" optional:"true"` LastSyncError *SwitchSync `json:"last_sync_error" description:"last synchronization to the switch that was erroneous" optional:"true"` Timestamps From 2af9749bf8b777f4954d53589629cc4cd03e8c64 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Tue, 23 Apr 2024 16:13:36 +0200 Subject: [PATCH 20/22] resolve review comments --- cmd/metal-api/internal/metal/network.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 5e4db3c5c..d1488d501 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -4,6 +4,8 @@ import ( "fmt" "net" "strings" + + "github.com/samber/lo" ) // SwitchPortStatus is a type alias for a string that represents the status of a switch port. @@ -103,8 +105,8 @@ func (ns *NicState) SetState(s SwitchPortStatus) (NicState, bool) { Desired: nil, }, true } else { - // we already have the reported state, but the desired one is different - // so nothing changed + // 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, @@ -138,10 +140,12 @@ func (ns *NicState) WantState(s SwitchPortStatus) (NicState, bool) { } 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, - }, true + }, lo.FromPtr(ns.Desired) != s } // GetIdentifier returns the identifier of a nic. From 8cd2f0dbf444fce48d39aa28eae12210048e41d8 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Tue, 23 Apr 2024 16:13:48 +0200 Subject: [PATCH 21/22] add tests for state methods --- cmd/metal-api/internal/metal/network_test.go | 216 +++++++++++++++++++ 1 file changed, 216 insertions(+) diff --git a/cmd/metal-api/internal/metal/network_test.go b/cmd/metal-api/internal/metal/network_test.go index fa9c96950..2ef535bef 100644 --- a/cmd/metal-api/internal/metal/network_test.go +++ b/cmd/metal-api/internal/metal/network_test.go @@ -119,3 +119,219 @@ func TestPrefix_Equals(t *testing.T) { }) } } + +func TestNicState_WantState(t *testing.T) { + up := SwitchPortStatusUp + down := SwitchPortStatusDown + unknown := SwitchPortStatusUnknown + + tests := []struct { + name string + nic *NicState + arg SwitchPortStatus + want NicState + changed bool + }{ + { + name: "up to desired down", + nic: &NicState{ + Desired: nil, + Actual: down, + }, + arg: up, + want: NicState{ + Desired: &up, + Actual: down, + }, + changed: true, + }, + { + name: "up to up with empty desired", + nic: &NicState{ + Desired: nil, + Actual: up, + }, + arg: up, + want: NicState{ + Desired: nil, + Actual: up, + }, + changed: false, + }, + { + name: "up to up with other desired", + nic: &NicState{ + Desired: &down, + Actual: up, + }, + arg: up, + want: NicState{ + Desired: nil, + Actual: up, + }, + changed: true, + }, + { + name: "nil to up", + nic: nil, + arg: up, + want: NicState{ + Desired: &up, + Actual: unknown, + }, + changed: true, + }, + { + name: "different actual with same desired", + nic: &NicState{ + Desired: &down, + Actual: up, + }, + arg: down, + want: NicState{ + Desired: &down, + Actual: up, + }, + changed: false, + }, + { + name: "different actual with other desired", + nic: &NicState{ + Desired: &up, + Actual: up, + }, + arg: down, + want: NicState{ + Desired: &down, + Actual: up, + }, + changed: true, + }, + { + name: "different actual with empty desired", + nic: &NicState{ + Desired: nil, + Actual: up, + }, + arg: down, + want: NicState{ + Desired: &down, + Actual: up, + }, + changed: true, + }, + } + 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) + } + if got1 != tt.changed { + t.Errorf("NicState.WantState() got1 = %v, want %v", got1, tt.changed) + } + }) + } +} + +func TestNicState_SetState(t *testing.T) { + up := SwitchPortStatusUp + down := SwitchPortStatusDown + unknown := SwitchPortStatusUnknown + + tests := []struct { + name string + nic *NicState + arg SwitchPortStatus + want NicState + changed bool + }{ + { + name: "different actual with empty desired", + nic: &NicState{ + Desired: nil, + Actual: up, + }, + arg: down, + want: NicState{ + Desired: nil, + Actual: down, + }, + changed: true, + }, + { + name: "different actual with same state in desired", + nic: &NicState{ + Desired: &down, + Actual: up, + }, + arg: down, + want: NicState{ + Desired: nil, + Actual: down, + }, + changed: true, + }, + { + name: "different actual with other state in desired", + nic: &NicState{ + Desired: &unknown, + Actual: up, + }, + arg: down, + want: NicState{ + Desired: &unknown, + Actual: down, + }, + changed: true, + }, + { + name: "nil nic", + nic: nil, + arg: down, + want: NicState{ + Desired: nil, + Actual: down, + }, + changed: true, + }, + { + name: "same state with same desired", + nic: &NicState{ + Desired: &down, + Actual: down, + }, + arg: down, + want: NicState{ + Desired: nil, + Actual: down, + }, + changed: true, + }, + { + name: "same state with other desired", + nic: &NicState{ + Desired: &up, + Actual: down, + }, + arg: down, + want: NicState{ + Desired: &up, + Actual: down, + }, + changed: 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) + } + if got1 != tt.changed { + t.Errorf("NicState.SetState() got1 = %v, want %v", got1, tt.changed) + } + }) + } +} From 109a56a3cfa1431c10022f263abefe7233ddc974 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Thu, 25 Apr 2024 13:15:34 +0200 Subject: [PATCH 22/22] the NIC names of sonic are case sensitive --- cmd/metal-api/internal/service/switch-service.go | 2 +- spec/metal-api.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 2614535a5..d021d66f0 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -268,7 +268,7 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu // old versions of metal-core do not send this field, so make sure we do not crash here if requestPayload.PortStates != nil { for i, nic := range newSwitch.Nics { - state, has := requestPayload.PortStates[strings.ToLower(nic.Name)] + state, has := requestPayload.PortStates[nic.Name] if has { reported := metal.SwitchPortStatus(state) newstate, changed := nic.State.SetState(reported) diff --git a/spec/metal-api.json b/spec/metal-api.json index 1f0a02c07..cde2ba203 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -5057,7 +5057,7 @@ "type": "string" }, "connections": { - "description": "a connection between a switch port and a machine", + "description": "a connection between a switch port and a machine with the real nic states", "items": { "$ref": "#/definitions/v1.SwitchConnection" }, @@ -5106,7 +5106,7 @@ "type": "string" }, "nics": { - "description": "the list of network interfaces on the switch", + "description": "the list of network interfaces on the switch with the desired nic states", "items": { "$ref": "#/definitions/v1.SwitchNic" },