From 7c8d310f817a510f0bfe9408b44b9ee62bac1181 Mon Sep 17 00:00:00 2001 From: Ulrich Schreiner Date: Mon, 26 Feb 2024 13:49:12 +0100 Subject: [PATCH] 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..1ee04d436 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() || !desired.IsValid() { + r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("the status %q must be valid and 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..0871f49f9 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 valid and 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 {