From 55f2d19a66e0e6f81a292dcde5ef810e7abbdabd Mon Sep 17 00:00:00 2001 From: Markus Wennrich Date: Fri, 15 Nov 2024 13:29:56 +0100 Subject: [PATCH 1/2] feat: add support for reporting of switch BGP states by metal-core --- cmd/metal-api/internal/metal/network.go | 25 ++++-- .../internal/service/switch-service.go | 59 ++++++++++--- cmd/metal-api/internal/service/v1/switch.go | 30 +++++-- spec/metal-api.json | 85 +++++++++++++++++++ 4 files changed, 172 insertions(+), 27 deletions(-) diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 117e0772..e2b2b6e0 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -41,13 +41,14 @@ type MacAddress string // Nic information. type Nic struct { - MacAddress MacAddress `rethinkdb:"macAddress" json:"macAddress"` - Name string `rethinkdb:"name" json:"name"` - Identifier string `rethinkdb:"identifier" json:"identifier"` - Vrf string `rethinkdb:"vrf" json:"vrf"` - Neighbors Nics `rethinkdb:"neighbors" json:"neighbors"` - Hostname string `rethinkdb:"hostname" json:"hostname"` - State *NicState `rethinkdb:"state" json:"state"` + MacAddress MacAddress `rethinkdb:"macAddress" json:"macAddress"` + Name string `rethinkdb:"name" json:"name"` + Identifier string `rethinkdb:"identifier" json:"identifier"` + Vrf string `rethinkdb:"vrf" json:"vrf"` + Neighbors Nics `rethinkdb:"neighbors" json:"neighbors"` + Hostname string `rethinkdb:"hostname" json:"hostname"` + State *NicState `rethinkdb:"state" json:"state"` + BGPPortState *SwitchBGPPortState `rethinkdb:"bgpPortState" json:"bgpPortState"` } // NicState represents the desired and actual state of a network interface @@ -59,6 +60,16 @@ type NicState struct { Actual SwitchPortStatus `rethinkdb:"actual" json:"actual"` } +type SwitchBGPPortState struct { + Neighbor string + PeerGroup string + VrfName string + BgpState string + BgpTimerUpEstablished int64 + SentPrefixCounter int64 + AcceptedPrefixCounter int64 +} + // SetState updates the NicState with the given SwitchPortStatus. It returns // a new NicState and a bool indicating if the state was changed. // diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 88a9d433..743e080a 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -6,6 +6,7 @@ import ( "log/slog" "net/http" "net/netip" + "reflect" "sort" "strconv" "strings" @@ -325,6 +326,35 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu } } + if requestPayload.BGPPortStates != nil { + r.log.Debug("bgp port states", "id", id, "states", requestPayload.BGPPortStates) + for i, nic := range newSwitch.Nics { + bpsnew, ok := requestPayload.BGPPortStates[nic.Name] + if !ok && nic.BGPPortState == nil { + continue + } + if nic.BGPPortState == nil { + newSwitch.Nics[i].BGPPortState = nil + switchUpdated = true + } + if reflect.DeepEqual(nic.BGPPortState, &bpsnew) { + continue + } + + r.log.Debug("bgp port state", "id", id, "nic", nic.Name, "state", bpsnew) + newSwitch.Nics[i].BGPPortState = &metal.SwitchBGPPortState{ + Neighbor: bpsnew.Neighbor, + PeerGroup: bpsnew.PeerGroup, + VrfName: bpsnew.VrfName, + BgpState: bpsnew.BgpState, + BgpTimerUpEstablished: bpsnew.BgpTimerUpEstablished, + SentPrefixCounter: bpsnew.SentPrefixCounter, + AcceptedPrefixCounter: bpsnew.AcceptedPrefixCounter, + } + switchUpdated = true + } + } + if switchUpdated { if err := r.ds.UpdateSwitch(oldSwitch, &newSwitch); err != nil { r.sendError(request, response, defaultError(err)) @@ -1151,12 +1181,13 @@ func (r *switchResource) makeSwitchNics(s *metal.Switch, nws metal.NetworkMap, i filter = &f } nic := v1.SwitchNic{ - MacAddress: string(n.MacAddress), - Name: n.Name, - Identifier: n.Identifier, - Vrf: n.Vrf, - BGPFilter: filter, - Actual: v1.SwitchPortStatusUnknown, + MacAddress: string(n.MacAddress), + Name: n.Name, + Identifier: n.Identifier, + Vrf: n.Vrf, + BGPFilter: filter, + Actual: v1.SwitchPortStatusUnknown, + BGPPortState: n.BGPPortState, } if n.State != nil { if n.State.Desired != nil { @@ -1191,15 +1222,21 @@ func (r *switchResource) makeSwitchCons(s *metal.Switch) []v1.SwitchConnection { // 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 + } + nic := v1.SwitchNic{ - MacAddress: string(mc.Nic.MacAddress), - Name: mc.Nic.Name, - Identifier: mc.Nic.Identifier, - Vrf: mc.Nic.Vrf, - Actual: v1.SwitchPortStatus(state), + MacAddress: string(mc.Nic.MacAddress), + Name: mc.Nic.Name, + Identifier: mc.Nic.Identifier, + Vrf: mc.Nic.Vrf, + Actual: v1.SwitchPortStatus(state), + BGPPortState: bps, } con := v1.SwitchConnection{ Nic: nic, diff --git a/cmd/metal-api/internal/service/v1/switch.go b/cmd/metal-api/internal/service/v1/switch.go index 514ecbdc..8a969c93 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -40,12 +40,13 @@ 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"` - Actual SwitchPortStatus `json:"actual" description:"the current state of the nic" enum:"UP|DOWN|UNKNOWN"` + 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" enum:"UP|DOWN|UNKNOWN"` + BGPPortState *metal.SwitchBGPPortState `json:"bgp_port_state" description:"the current bgp port state" optional:"true"` } type BGPFilter struct { @@ -100,9 +101,20 @@ type SwitchMigrateRequest struct { // 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"` - PortStates map[string]SwitchPortStatus `json:"port_states" description:"the current switch port states"` + 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"` + BGPPortStates map[string]SwitchBGPPortState `json:"bgp_port_states" description:"the current bgp port states" optional:"true"` +} + +type SwitchBGPPortState struct { + Neighbor string + PeerGroup string + VrfName string + BgpState string + BgpTimerUpEstablished int64 + SentPrefixCounter int64 + AcceptedPrefixCounter int64 } type SwitchNotifyResponse struct { diff --git a/spec/metal-api.json b/spec/metal-api.json index 627f942c..02b48536 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -348,6 +348,43 @@ "type": "HTTPErrorResponse" } }, + "metal.SwitchBGPPortState": { + "properties": { + "AcceptedPrefixCounter": { + "format": "int64", + "type": "integer" + }, + "BgpState": { + "type": "string" + }, + "BgpTimerUpEstablished": { + "format": "int64", + "type": "integer" + }, + "Neighbor": { + "type": "string" + }, + "PeerGroup": { + "type": "string" + }, + "SentPrefixCounter": { + "format": "int64", + "type": "integer" + }, + "VrfName": { + "type": "string" + } + }, + "required": [ + "AcceptedPrefixCounter", + "BgpState", + "BgpTimerUpEstablished", + "Neighbor", + "PeerGroup", + "SentPrefixCounter", + "VrfName" + ] + }, "rest.HealthResponse": { "properties": { "message": { @@ -5135,6 +5172,43 @@ "id" ] }, + "v1.SwitchBGPPortState": { + "properties": { + "AcceptedPrefixCounter": { + "format": "int64", + "type": "integer" + }, + "BgpState": { + "type": "string" + }, + "BgpTimerUpEstablished": { + "format": "int64", + "type": "integer" + }, + "Neighbor": { + "type": "string" + }, + "PeerGroup": { + "type": "string" + }, + "SentPrefixCounter": { + "format": "int64", + "type": "integer" + }, + "VrfName": { + "type": "string" + } + }, + "required": [ + "AcceptedPrefixCounter", + "BgpState", + "BgpTimerUpEstablished", + "Neighbor", + "PeerGroup", + "SentPrefixCounter", + "VrfName" + ] + }, "v1.SwitchBase": { "description": "A switch that can register at the api.", "properties": { @@ -5231,6 +5305,10 @@ ], "type": "string" }, + "bgp_port_state": { + "$ref": "#/definitions/metal.SwitchBGPPortState", + "description": "the current bgp port state" + }, "filter": { "$ref": "#/definitions/v1.BGPFilter", "description": "configures the bgp filter applied at the switch port" @@ -5261,6 +5339,13 @@ }, "v1.SwitchNotifyRequest": { "properties": { + "bgp_port_states": { + "additionalProperties": { + "$ref": "#/definitions/v1.SwitchBGPPortState" + }, + "description": "the current bgp port states", + "type": "object" + }, "error": { "type": "string" }, From da1e14cb0b793edd248360f8d667e83029c08eb0 Mon Sep 17 00:00:00 2001 From: Markus Wennrich Date: Tue, 26 Nov 2024 08:32:10 +0100 Subject: [PATCH 2/2] review finding: replace deepequal --- .../internal/service/switch-service.go | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 743e080a..97e3f6c7 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -6,7 +6,6 @@ import ( "log/slog" "net/http" "net/netip" - "reflect" "sort" "strconv" "strings" @@ -327,21 +326,29 @@ func (r *switchResource) notifySwitch(request *restful.Request, response *restfu } if requestPayload.BGPPortStates != nil { - r.log.Debug("bgp port states", "id", id, "states", requestPayload.BGPPortStates) + r.log.Debug("bgp port states received", "id", id, "states", requestPayload.BGPPortStates) for i, nic := range newSwitch.Nics { - bpsnew, ok := requestPayload.BGPPortStates[nic.Name] - if !ok && nic.BGPPortState == nil { + bpsnew, has := requestPayload.BGPPortStates[nic.Name] + if !has && nic.BGPPortState == nil { continue } - if nic.BGPPortState == nil { + if !has { newSwitch.Nics[i].BGPPortState = nil switchUpdated = true + continue } - if reflect.DeepEqual(nic.BGPPortState, &bpsnew) { + if nic.BGPPortState != nil && + nic.BGPPortState.Neighbor == bpsnew.Neighbor && + nic.BGPPortState.PeerGroup == bpsnew.PeerGroup && + nic.BGPPortState.VrfName == bpsnew.VrfName && + nic.BGPPortState.BgpState == bpsnew.BgpState && + nic.BGPPortState.BgpTimerUpEstablished == bpsnew.BgpTimerUpEstablished && + nic.BGPPortState.SentPrefixCounter == bpsnew.SentPrefixCounter && + nic.BGPPortState.AcceptedPrefixCounter == bpsnew.AcceptedPrefixCounter { continue } - r.log.Debug("bgp port state", "id", id, "nic", nic.Name, "state", bpsnew) + r.log.Debug("bgp port state updated", "id", id, "nic", nic.Name, "state", bpsnew) newSwitch.Nics[i].BGPPortState = &metal.SwitchBGPPortState{ Neighbor: bpsnew.Neighbor, PeerGroup: bpsnew.PeerGroup,