diff --git a/cmd/metal-api/internal/datastore/switch.go b/cmd/metal-api/internal/datastore/switch.go index d5982c19c..8e0fc9dae 100644 --- a/cmd/metal-api/internal/datastore/switch.go +++ b/cmd/metal-api/internal/datastore/switch.go @@ -177,9 +177,21 @@ func (rs *RethinkStore) ConnectMachineWithSwitches(m *metal.Machine) error { if err != nil { return err } + // e.g. "swp0s0" -> "Ethernet0" + dictionary, err := s1.MapPortNames(s2.OS.Vendor) + if err != nil { + return fmt.Errorf("could not create port mapping %w", err) + } + for _, con := range s1.MachineConnections[m.ID] { - if con2, has := byNicName[con.Nic.Name]; has { - if con.Nic.Name != con2.Nic.Name { + // get the corresponding interface name for s2 + name, ok := dictionary[con.Nic.Name] + if !ok { + return fmt.Errorf("could not translate port name %s to equivalent port name of switch os %s", con.Nic.Name, s1.OS.Vendor) + } + // check if s2 contains nic of name corresponding to con.Nic.Name + if con2, has := byNicName[name]; has { + if name != con2.Nic.Name { return connectionMapError } } else { diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 9f2477d71..48f501657 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -302,9 +302,12 @@ func (nics Nics) FilterByHostname(hostname string) (res Nics) { return res } +// NicMap maps nic names to the corresponding nics +type NicMap map[string]*Nic + // ByName creates a map (nic names --> nic) from a nic list. -func (nics Nics) ByName() map[string]*Nic { - res := make(map[string]*Nic) +func (nics Nics) ByName() NicMap { + res := make(NicMap) for i, n := range nics { res[n.Name] = &nics[i] @@ -314,8 +317,8 @@ func (nics Nics) ByName() map[string]*Nic { } // ByIdentifier creates a map (nic identifier --> nic) from a nic list. -func (nics Nics) ByIdentifier() map[string]*Nic { - res := make(map[string]*Nic) +func (nics Nics) ByIdentifier() NicMap { + res := make(NicMap) for i, n := range nics { res[n.GetIdentifier()] = &nics[i] diff --git a/cmd/metal-api/internal/metal/network_test.go b/cmd/metal-api/internal/metal/network_test.go index a7dcfddfe..e935bd74f 100644 --- a/cmd/metal-api/internal/metal/network_test.go +++ b/cmd/metal-api/internal/metal/network_test.go @@ -23,7 +23,7 @@ func TestNics_ByIdentifier(t *testing.T) { nicArray[i].Neighbors = append(nicArray[0:i], nicArray[i+1:countOfNics]...) } - map1 := map[string]*Nic{} + map1 := NicMap{} for i, n := range nicArray { map1[string(n.MacAddress)] = &nicArray[i] } @@ -31,7 +31,7 @@ func TestNics_ByIdentifier(t *testing.T) { tests := []struct { name string nics Nics - want map[string]*Nic + want NicMap }{ { name: "TestNics_ByIdentifier Test 1", diff --git a/cmd/metal-api/internal/metal/switch.go b/cmd/metal-api/internal/metal/switch.go index e3adf8a8c..2cb281546 100644 --- a/cmd/metal-api/internal/metal/switch.go +++ b/cmd/metal-api/internal/metal/switch.go @@ -2,6 +2,8 @@ package metal import ( "fmt" + "strconv" + "strings" "time" ) @@ -24,11 +26,20 @@ type Switch struct { type Switches []Switch type SwitchOS struct { - Vendor string `rethinkdb:"vendor" json:"vendor"` - Version string `rethinkdb:"version" json:"version"` - MetalCoreVersion string `rethinkdb:"metal_core_version" json:"metal_core_version"` + Vendor SwitchOSVendor `rethinkdb:"vendor" json:"vendor"` + Version string `rethinkdb:"version" json:"version"` + MetalCoreVersion string `rethinkdb:"metal_core_version" json:"metal_core_version"` } +// SwitchOSVendor is an enum denoting the name of a switch OS +type SwitchOSVendor string + +// The enums for switch OS vendors +const ( + SwitchOSVendorSonic SwitchOSVendor = "SONiC" + SwitchOSVendorCumulus SwitchOSVendor = "Cumulus" +) + // Connection between switch port and machine. type Connection struct { Nic Nic `rethinkdb:"nic" json:"nic"` @@ -144,3 +155,196 @@ func (s *Switch) SetVrfOfMachine(m *Machine, vrf string) { } s.Nics = nics } + +// TranslateNicMap creates a NicMap where the keys are translated to the naming convention of the target OS +// example mapping from cumulus to sonic for one single port: +// +// map[string]Nic { +// "swp0s1": Nic{ +// Name: "Ethernet1", +// MacAddress: "" +// } +// } +func (s *Switch) TranslateNicMap(targetOS SwitchOSVendor) (NicMap, error) { + nicMap := s.Nics.ByName() + translatedNicMap := make(NicMap) + + if s.OS.Vendor == targetOS { + return nicMap, nil + } + + ports := make([]string, 0) + for name := range nicMap { + ports = append(ports, name) + } + + lines, err := getLinesFromPortNames(ports, s.OS.Vendor) + if err != nil { + return nil, err + } + + for _, p := range ports { + targetPort, err := mapPortName(p, s.OS.Vendor, targetOS, lines) + if err != nil { + return nil, err + } + + nic, ok := nicMap[p] + if !ok { + return nil, fmt.Errorf("an unknown error occured during port name translation") + } + translatedNicMap[targetPort] = nic + } + + return translatedNicMap, nil +} + +// MapPortNames creates a dictionary that maps the naming convention of this switch's OS to that of the target OS +func (s *Switch) MapPortNames(targetOS SwitchOSVendor) (map[string]string, error) { + nics := s.Nics.ByName() + portNamesMap := make(map[string]string, len(s.Nics)) + + ports := make([]string, 0) + for name := range nics { + ports = append(ports, name) + } + + lines, err := getLinesFromPortNames(ports, s.OS.Vendor) + if err != nil { + return nil, err + } + + for _, p := range ports { + targetPort, err := mapPortName(p, s.OS.Vendor, targetOS, lines) + if err != nil { + return nil, err + } + portNamesMap[p] = targetPort + } + + return portNamesMap, nil +} + +func mapPortName(port string, sourceOS, targetOS SwitchOSVendor, allLines []int) (string, error) { + line, err := portNameToLine(port, sourceOS) + if err != nil { + return "", fmt.Errorf("unable to get line number from port name, %w", err) + } + + if targetOS == SwitchOSVendorCumulus { + return cumulusPortByLineNumber(line, allLines), nil + } + if targetOS == SwitchOSVendorSonic { + return sonicPortByLineNumber(line), nil + } + + return "", fmt.Errorf("unknown target switch os %s", targetOS) +} + +func getLinesFromPortNames(ports []string, os SwitchOSVendor) ([]int, error) { + lines := make([]int, 0) + for _, p := range ports { + l, err := portNameToLine(p, os) + if err != nil { + return nil, fmt.Errorf("unable to get line number from port name, %w", err) + } + + lines = append(lines, l) + } + + return lines, nil +} + +func portNameToLine(port string, os SwitchOSVendor) (int, error) { + if os == SwitchOSVendorSonic { + return sonicPortNameToLine(port) + } + if os == SwitchOSVendorCumulus { + return cumulusPortNameToLine(port) + } + return 0, fmt.Errorf("unknow switch os %s", os) +} + +func sonicPortNameToLine(port string) (int, error) { + // to prevent accidentally parsing a substring to a negative number + if strings.Contains(port, "-") { + return 0, fmt.Errorf("invalid token '-' in port name %s", port) + } + + prefix, lineString, found := strings.Cut(port, "Ethernet") + if !found { + return 0, fmt.Errorf("invalid port name %s, expected to find prefix 'Ethernet'", port) + } + + if prefix != "" { + return 0, fmt.Errorf("invalid port name %s, port name is expected to start with 'Ethernet'", port) + } + + line, err := strconv.Atoi(lineString) + if err != nil { + return 0, fmt.Errorf("unable to convert port name to line number: %w", err) + } + + return line, nil +} + +func cumulusPortNameToLine(port string) (int, error) { + // to prevent accidentally parsing a substring to a negative number + if strings.Contains(port, "-") { + return 0, fmt.Errorf("invalid token '-' in port name %s", port) + } + + prefix, suffix, found := strings.Cut(port, "swp") + if !found { + return 0, fmt.Errorf("invalid port name %s, expected to find prefix 'swp'", port) + } + + if prefix != "" { + return 0, fmt.Errorf("invalid port name %s, port name is expected to start with 'swp'", port) + } + + var line int + + countString, indexString, found := strings.Cut(suffix, "s") + if !found { + count, err := strconv.Atoi(suffix) + if err != nil { + return 0, fmt.Errorf("unable to convert port name to line number: %w", err) + } + line = count * 4 + } else { + count, err := strconv.Atoi(countString) + if err != nil { + return 0, fmt.Errorf("unable to convert port name to line number: %w", err) + } + + index, err := strconv.Atoi(indexString) + if err != nil { + return 0, fmt.Errorf("unable to convert port name to line number: %w", err) + } + line = count*4 + index + } + + return line, nil +} + +func sonicPortByLineNumber(line int) string { + return fmt.Sprintf("Ethernet%d", line) +} + +func cumulusPortByLineNumber(line int, allLines []int) string { + if line%4 > 0 { + return fmt.Sprintf("swp%ds%d", line/4, line%4) + } + + for _, l := range allLines { + if l == line { + continue + } + if l/4 == line/4 { + return fmt.Sprintf("swp%ds%d", line/4, line%4) + } + } + + return fmt.Sprintf("swp%d", line/4) +} diff --git a/cmd/metal-api/internal/metal/switch_test.go b/cmd/metal-api/internal/metal/switch_test.go index 9ef480925..9c66da467 100644 --- a/cmd/metal-api/internal/metal/switch_test.go +++ b/cmd/metal-api/internal/metal/switch_test.go @@ -1,8 +1,12 @@ package metal import ( + "fmt" "reflect" + "strconv" "testing" + + "github.com/google/go-cmp/cmp" ) var ( @@ -277,3 +281,487 @@ func TestSwitch_ConnectMachine2(t *testing.T) { }) } } + +func TestSwitch_TranslateNicNames(t *testing.T) { + tests := []struct { + name string + sw *Switch + targetOS SwitchOSVendor + want NicMap + wantErr bool + }{ + { + name: "both twins have the same os", + sw: &Switch{ + Nics: []Nic{ + {Name: "swp0s0"}, + {Name: "swp0s1"}, + {Name: "swp0s2"}, + {Name: "swp0s3"}, + }, + OS: &SwitchOS{Vendor: SwitchOSVendorCumulus}, + }, + targetOS: SwitchOSVendorCumulus, + want: map[string]*Nic{ + "swp0s0": {Name: "swp0s0"}, + "swp0s1": {Name: "swp0s1"}, + "swp0s2": {Name: "swp0s2"}, + "swp0s3": {Name: "swp0s3"}, + }, + wantErr: false, + }, + { + name: "cumulus to sonic", + sw: &Switch{ + Nics: []Nic{ + {Name: "Ethernet1"}, + {Name: "Ethernet2"}, + {Name: "Ethernet3"}, + {Name: "Ethernet4"}, + }, + OS: &SwitchOS{Vendor: SwitchOSVendorSonic}, + }, + targetOS: SwitchOSVendorCumulus, + want: map[string]*Nic{ + "swp0s1": {Name: "Ethernet1"}, + "swp0s2": {Name: "Ethernet2"}, + "swp0s3": {Name: "Ethernet3"}, + "swp1": {Name: "Ethernet4"}, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.sw.TranslateNicMap(tt.targetOS) + if (err != nil) != tt.wantErr { + t.Errorf("translateNicNames() error = %v, wantErr %v", err, tt.wantErr) + return + } + if cmp.Diff(got, tt.want) != "" { + t.Errorf("translateNicNames() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestSwitch_MapPortNames(t *testing.T) { + tests := []struct { + name string + sw *Switch + targetOS SwitchOSVendor + want map[string]string + wantErr bool + }{ + { + name: "same os", + sw: &Switch{ + Nics: []Nic{ + {Name: "swp0s0"}, + {Name: "swp0s1"}, + {Name: "swp0s2"}, + {Name: "swp0s3"}, + }, + OS: &SwitchOS{Vendor: SwitchOSVendorCumulus}, + }, + targetOS: SwitchOSVendorCumulus, + want: map[string]string{ + "swp0s0": "swp0s0", + "swp0s1": "swp0s1", + "swp0s2": "swp0s2", + "swp0s3": "swp0s3", + }, + wantErr: false, + }, + { + name: "cumulus to sonic", + sw: &Switch{ + Nics: []Nic{ + {Name: "swp0s0"}, + {Name: "swp1s0"}, + {Name: "swp1s1"}, + {Name: "swp1s2"}, + }, + OS: &SwitchOS{Vendor: SwitchOSVendorCumulus}, + }, + targetOS: SwitchOSVendorSonic, + want: map[string]string{ + "swp0s0": "Ethernet0", + "swp1s0": "Ethernet4", + "swp1s1": "Ethernet5", + "swp1s2": "Ethernet6", + }, + wantErr: false, + }, + { + name: "sonic to cumulus", + sw: &Switch{ + Nics: []Nic{ + {Name: "Ethernet0"}, + {Name: "Ethernet4"}, + {Name: "Ethernet8"}, + {Name: "Ethernet9"}, + }, + OS: &SwitchOS{Vendor: SwitchOSVendorSonic}, + }, + targetOS: SwitchOSVendorCumulus, + want: map[string]string{ + "Ethernet0": "swp0", + "Ethernet4": "swp1", + "Ethernet8": "swp2s0", + "Ethernet9": "swp2s1", + }, + wantErr: false, + }, + { + name: "sonic names in cumulus switch", + sw: &Switch{ + Nics: []Nic{ + {Name: "Ethernet0"}, + {Name: "Ethernet4"}, + {Name: "Ethernet8"}, + {Name: "Ethernet9"}, + }, + OS: &SwitchOS{Vendor: SwitchOSVendorCumulus}, + }, + targetOS: SwitchOSVendorSonic, + want: nil, + wantErr: true, + }, + { + name: "cumulus names in sonic switch", + sw: &Switch{ + Nics: []Nic{ + {Name: "swp0s0"}, + {Name: "swp0s1"}, + {Name: "swp0s2"}, + {Name: "swp0s3"}, + }, + OS: &SwitchOS{Vendor: SwitchOSVendorSonic}, + }, + targetOS: SwitchOSVendorCumulus, + want: nil, + wantErr: true, + }, + { + name: "invalid name", + sw: &Switch{ + Nics: []Nic{ + {Name: "swp0s"}, + }, + OS: &SwitchOS{Vendor: SwitchOSVendorSonic}, + }, + targetOS: SwitchOSVendorCumulus, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.sw.MapPortNames(tt.targetOS) + if (err != nil) != tt.wantErr { + t.Errorf("Switch.MapPortNames() error = %v, wantErr %v", err, tt.wantErr) + return + } + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Errorf("%v", diff) + } + }) + } +} + +func Test_mapPortName(t *testing.T) { + tests := []struct { + name string + port string + sourceOS SwitchOSVendor + targetOS SwitchOSVendor + allLines []int + want string + wantErr error + }{ + { + name: "invalid target os", + port: "Ethernet0", + sourceOS: SwitchOSVendorSonic, + targetOS: "cumulus", + allLines: []int{0, 1}, + want: "", + wantErr: fmt.Errorf("unknown target switch os cumulus"), + }, + { + name: "sonic to cumulus", + port: "Ethernet11", + sourceOS: SwitchOSVendorSonic, + targetOS: SwitchOSVendorCumulus, + allLines: []int{11}, + want: "swp2s3", + wantErr: nil, + }, + { + name: "cumulus to sonic", + port: "swp4s0", + sourceOS: SwitchOSVendorCumulus, + targetOS: SwitchOSVendorSonic, + allLines: []int{0, 4, 16, 17}, + want: "Ethernet16", + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := mapPortName(tt.port, tt.sourceOS, tt.targetOS, tt.allLines) + if !errorsAreEqual(err, tt.wantErr) { + t.Errorf("MapPortName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("MapPortName() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_getLinesFromPortNames(t *testing.T) { + tests := []struct { + name string + ports []string + os SwitchOSVendor + want []int + wantErr bool + }{ + { + name: "invalid switch os", + ports: []string{"swp0", "swp1s2"}, + os: "cumulus", + want: nil, + wantErr: true, + }, + { + name: "mismatch between port names and os cumulus", + ports: []string{"Ethernet0", "Ethernet1"}, + os: SwitchOSVendorCumulus, + want: nil, + wantErr: true, + }, + { + name: "mismatch between port names and os sonic", + ports: []string{"swp0s0", "swp0s1"}, + os: SwitchOSVendorSonic, + want: nil, + wantErr: true, + }, + { + name: "sonic conversion successful", + ports: []string{"Ethernet0", "Ethernet2"}, + os: SwitchOSVendorSonic, + want: []int{0, 2}, + wantErr: false, + }, + { + name: "cumulus conversion successful", + ports: []string{"swp0", "swp1s3"}, + os: SwitchOSVendorCumulus, + want: []int{0, 7}, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getLinesFromPortNames(tt.ports, tt.os) + if (err != nil) != tt.wantErr { + t.Errorf("GetLinesFromPortNames() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetLinesFromPortNames() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_sonicPortNameToLine(t *testing.T) { + _, parseIntError := strconv.Atoi("_1") + + tests := []struct { + name string + port string + want int + wantErr error + }{ + { + name: "invalid token", + port: "Ethernet-0", + want: 0, + wantErr: fmt.Errorf("invalid token '-' in port name Ethernet-0"), + }, + { + name: "missing prefix 'Ethernet'", + port: "swp0s0", + want: 0, + wantErr: fmt.Errorf("invalid port name swp0s0, expected to find prefix 'Ethernet'"), + }, + { + name: "invalid prefix before 'Ethernet'", + port: "port_Ethernet0", + want: 0, + wantErr: fmt.Errorf("invalid port name port_Ethernet0, port name is expected to start with 'Ethernet'"), + }, + { + name: "cannot convert line number", + port: "Ethernet_1", + want: 0, + wantErr: fmt.Errorf("unable to convert port name to line number: %w", parseIntError), + }, + { + name: "conversion successful", + port: "Ethernet25", + want: 25, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := sonicPortNameToLine(tt.port) + if !errorsAreEqual(err, tt.wantErr) { + t.Errorf("sonicPortNameToLine() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("sonicPortNameToLine() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_cumulusPortNameToLine(t *testing.T) { + _, parseIntError1 := strconv.Atoi("0t0") + _, parseIntError2 := strconv.Atoi("_0") + + tests := []struct { + name string + port string + want int + wantErr error + }{ + { + name: "invalid token", + port: "swp-0s1", + want: 0, + wantErr: fmt.Errorf("invalid token '-' in port name swp-0s1"), + }, + { + name: "missing prefix 'swp'", + port: "Ethernet0", + want: 0, + wantErr: fmt.Errorf("invalid port name Ethernet0, expected to find prefix 'swp'"), + }, + { + name: "invalid prefix before 'swp'", + port: "port_swp0s0", + want: 0, + wantErr: fmt.Errorf("invalid port name port_swp0s0, port name is expected to start with 'swp'"), + }, + { + name: "wrong delimiter", + port: "swp0t0", + want: 0, + wantErr: fmt.Errorf("unable to convert port name to line number: %w", parseIntError1), + }, + { + name: "cannot convert first number", + port: "swp_0s0", + want: 0, + wantErr: fmt.Errorf("unable to convert port name to line number: %w", parseIntError2), + }, + { + name: "cannot convert second number", + port: "swp0s_0", + want: 0, + wantErr: fmt.Errorf("unable to convert port name to line number: %w", parseIntError2), + }, + { + name: "convert line without breakout", + port: "swp4", + want: 16, + wantErr: nil, + }, + { + name: "convert line with breakout", + port: "swp3s3", + want: 15, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := cumulusPortNameToLine(tt.port) + if !errorsAreEqual(err, tt.wantErr) { + t.Errorf("cumulusPortNameToLine() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("cumulusPortNameToLine() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_cumulusPortByLineNumber(t *testing.T) { + tests := []struct { + name string + line int + allLines []int + want string + }{ + { + name: "only one line", + line: 4, + allLines: []int{4}, + want: "swp1", + }, + { + name: "line number 0 without breakout", + line: 0, + allLines: []int{0, 4}, + want: "swp0", + }, + { + name: "higher line number without breakout", + line: 4, + allLines: []int{0, 1, 2, 3, 4, 8}, + want: "swp1", + }, + { + name: "line number divisible by 4 with breakout", + line: 4, + allLines: []int{4, 5, 6, 7}, + want: "swp1s0", + }, + { + name: "line number not divisible by 4", + line: 13, + allLines: []int{13}, + want: "swp3s1", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := cumulusPortByLineNumber(tt.line, tt.allLines); got != tt.want { + t.Errorf("cumulusPortByLineNumber() = %v, want %v", got, tt.want) + } + }) + } +} + +func errorsAreEqual(err1, err2 error) bool { + if err1 == nil && err2 == nil { + return true + } + + if err1 != nil && err2 == nil || err1 == nil && err2 != nil { + return false + } + + return err1.Error() == err2.Error() +} diff --git a/cmd/metal-api/internal/service/integration_test.go b/cmd/metal-api/internal/service/integration_test.go index d462d0664..1f7cd7427 100644 --- a/cmd/metal-api/internal/service/integration_test.go +++ b/cmd/metal-api/internal/service/integration_test.go @@ -221,6 +221,7 @@ func createTestEnvironment(t *testing.T, log *slog.Logger, ds *datastore.Rethink }, SwitchBase: v1.SwitchBase{ RackID: "test-rack", + OS: &v1.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, }, Nics: v1.SwitchNics{ { @@ -249,6 +250,7 @@ func createTestEnvironment(t *testing.T, log *slog.Logger, ds *datastore.Rethink }, SwitchBase: v1.SwitchBase{ RackID: "test-rack", + OS: &v1.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, }, Nics: v1.SwitchNics{ { diff --git a/cmd/metal-api/internal/service/machine-service_allocation_test.go b/cmd/metal-api/internal/service/machine-service_allocation_test.go index ffde006e5..c5c61f659 100644 --- a/cmd/metal-api/internal/service/machine-service_allocation_test.go +++ b/cmd/metal-api/internal/service/machine-service_allocation_test.go @@ -286,7 +286,7 @@ func createMachineRegisterRequest(i int) *grpcv1.BootServiceRegisterRequest { Mac: fmt.Sprintf("aa:ba:%d", i), Neighbors: []*grpcv1.MachineNic{ { - Name: fmt.Sprintf("swp-%d", i), + Name: fmt.Sprintf("swp%d", i), Mac: fmt.Sprintf("%s:%d", swp1MacPrefix, i), }, }, @@ -296,7 +296,7 @@ func createMachineRegisterRequest(i int) *grpcv1.BootServiceRegisterRequest { Mac: fmt.Sprintf("aa:bb:%d", i), Neighbors: []*grpcv1.MachineNic{ { - Name: fmt.Sprintf("swp-%d", i), + Name: fmt.Sprintf("swp%d", i), Mac: fmt.Sprintf("%s:%d", swp2MacPrefix, i), }, }, @@ -411,19 +411,19 @@ func createTestdata(machineCount int, rs *datastore.RethinkStore, ipamer ipam.IP sw2nics := metal.Nics{} for j := range machineCount { sw1nic := metal.Nic{ - Name: fmt.Sprintf("swp-%d", j), + Name: fmt.Sprintf("swp%d", j), MacAddress: metal.MacAddress(fmt.Sprintf("%s:%d", swp1MacPrefix, j)), } sw2nic := metal.Nic{ - Name: fmt.Sprintf("swp-%d", j), + Name: fmt.Sprintf("swp%d", j), MacAddress: metal.MacAddress(fmt.Sprintf("%s:%d", swp2MacPrefix, j)), } sw1nics = append(sw1nics, sw1nic) sw2nics = append(sw2nics, sw2nic) } - err = rs.CreateSwitch(&metal.Switch{Base: metal.Base{ID: "sw1"}, PartitionID: "p1", Nics: sw1nics, MachineConnections: metal.ConnectionMap{}}) + err = rs.CreateSwitch(&metal.Switch{Base: metal.Base{ID: "sw1"}, OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, PartitionID: "p1", Nics: sw1nics, MachineConnections: metal.ConnectionMap{}}) require.NoError(t, err) - err = rs.CreateSwitch(&metal.Switch{Base: metal.Base{ID: "sw2"}, PartitionID: "p1", Nics: sw2nics, MachineConnections: metal.ConnectionMap{}}) + err = rs.CreateSwitch(&metal.Switch{Base: metal.Base{ID: "sw2"}, OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, PartitionID: "p1", Nics: sw2nics, MachineConnections: metal.ConnectionMap{}}) require.NoError(t, err) err = rs.CreateFilesystemLayout(&metal.FilesystemLayout{Base: metal.Base{ID: "fsl1"}, Constraints: metal.FilesystemLayoutConstraints{Sizes: []string{"s1"}, Images: map[string]string{"i": "*"}}}) require.NoError(t, err) diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 8fea0f965..81e3f1c36 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -585,6 +585,15 @@ func (r *switchResource) replaceSwitch(old, new *metal.Switch) error { return err } + nicMap, err := s.TranslateNicMap(old.OS.Vendor) + if err != nil { + return err + } + err = r.adjustMachineConnections(old.MachineConnections, nicMap) + if err != nil { + return err + } + return r.ds.UpdateSwitch(old, s) } @@ -640,13 +649,14 @@ func adoptFromTwin(old, twin, new *metal.Switch) (*metal.Switch, error) { s.Mode = metal.SwitchOperational return &s, nil } + changeOS := old.OS.Vendor != new.OS.Vendor - newNics, err := adoptNics(twin, new) + newNics, err := adoptNics(twin, new, changeOS) if err != nil { return nil, fmt.Errorf("could not adopt nic configuration from twin, err: %w", err) } - newMachineConnections, err := adoptMachineConnections(twin, new) + newMachineConnections, err := adoptMachineConnections(twin, new, changeOS) if err != nil { return nil, err } @@ -660,11 +670,19 @@ func adoptFromTwin(old, twin, new *metal.Switch) (*metal.Switch, error) { // adoptNics checks whether new switch has all nics of its twin switch // copies vrf configuration and returns the new nics for the replacement switch -func adoptNics(twin, newSwitch *metal.Switch) (metal.Nics, error) { +func adoptNics(twin, newSwitch *metal.Switch, changeOS bool) (metal.Nics, error) { newNics := metal.Nics{} newNicMap := newSwitch.Nics.ByName() + if changeOS { + nicMap, err := newSwitch.TranslateNicMap(twin.OS.Vendor) + if err != nil { + return nil, err + } + newNicMap = nicMap + } missingNics := []string{} twinNicsByName := twin.Nics.ByName() + for name := range twinNicsByName { if _, ok := newNicMap[name]; !ok { missingNics = append(missingNics, name) @@ -675,15 +693,12 @@ func adoptNics(twin, newSwitch *metal.Switch) (metal.Nics, error) { } for name, nic := range newNicMap { + newNic := *nic // check for configuration at twin if twinNic, ok := twinNicsByName[name]; ok { - newNic := *nic newNic.Vrf = twinNic.Vrf - newNics = append(newNics, newNic) - } else { - // leave unchanged - newNics = append(newNics, *nic) } + newNics = append(newNics, newNic) } sort.SliceStable(newNics, func(i, j int) bool { @@ -693,8 +708,15 @@ func adoptNics(twin, newSwitch *metal.Switch) (metal.Nics, error) { } // adoptMachineConnections copies machine connections from twin and maps mac addresses based on the nic name -func adoptMachineConnections(twin, newSwitch *metal.Switch) (metal.ConnectionMap, error) { +func adoptMachineConnections(twin, newSwitch *metal.Switch, changeOS bool) (metal.ConnectionMap, error) { newNicMap := newSwitch.Nics.ByName() + if changeOS { + nicMap, err := newSwitch.TranslateNicMap(twin.OS.Vendor) + if err != nil { + return nil, err + } + newNicMap = nicMap + } newConnectionMap := metal.ConnectionMap{} missingNics := []string{} @@ -703,6 +725,7 @@ func adoptMachineConnections(twin, newSwitch *metal.Switch) (metal.ConnectionMap for _, con := range cons { if n, ok := newNicMap[con.Nic.Name]; ok { newCon := con + newCon.Nic.Name = n.Name newCon.Nic.Identifier = n.Identifier newCon.Nic.MacAddress = n.MacAddress newConnections = append(newConnections, newCon) @@ -721,6 +744,60 @@ func adoptMachineConnections(twin, newSwitch *metal.Switch) (metal.ConnectionMap return newConnectionMap, nil } +// adjustMachineConnections updates the neighbor entries for all machines connected to the switch +func (r *switchResource) adjustMachineConnections(oldConnections metal.ConnectionMap, nicMap metal.NicMap) error { + for mid, cons := range oldConnections { + m, err := r.ds.FindMachineByID(mid) + if err != nil { + return err + } + newNics, err := adjustMachineNics(m.Hardware.Nics, cons, nicMap) + if err != nil { + return err + } + newMachine := *m + newMachine.Hardware.Nics = newNics + err = r.ds.UpdateMachine(m, &newMachine) + if err != nil { + return err + } + } + return nil +} + +func adjustMachineNics(nics metal.Nics, connections metal.Connections, nicMap metal.NicMap) (metal.Nics, error) { + newNics := make(metal.Nics, 0) + + for _, nic := range nics { + newNic := nic + newNeighbors := make([]metal.Nic, 0) + for _, neigh := range nic.Neighbors { + if nicInConnections(neigh.Name, neigh.MacAddress, connections) { + n, ok := nicMap[neigh.Name] + if !ok { + return nil, fmt.Errorf("unable to find corresponding new neighbor nic") + } + newNeighbors = append(newNeighbors, *n) + } else { + newNeighbors = append(newNeighbors, neigh) + } + } + newNic.Neighbors = newNeighbors + newNics = append(newNics, newNic) + } + + return newNics, nil +} + +func nicInConnections(name string, mac metal.MacAddress, connections metal.Connections) bool { + for _, con := range connections { + if con.Nic.Name == name && con.Nic.MacAddress == mac { + return true + } + } + return false +} + func updateSwitchNics(oldNics, newNics map[string]*metal.Nic, currentConnections metal.ConnectionMap) (metal.Nics, error) { // To start off we just prevent basic things that can go wrong nicsThatGetLost := metal.Nics{} diff --git a/cmd/metal-api/internal/service/switch-service_integration_test.go b/cmd/metal-api/internal/service/switch-service_integration_test.go new file mode 100644 index 000000000..9289a73af --- /dev/null +++ b/cmd/metal-api/internal/service/switch-service_integration_test.go @@ -0,0 +1,506 @@ +//go:build integration +// +build integration + +package service + +import ( + "context" + "fmt" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + + "github.com/emicklei/go-restful/v3" + mdmv1 "github.com/metal-stack/masterdata-api/api/v1" + mdmv1mock "github.com/metal-stack/masterdata-api/api/v1/mocks" + mdm "github.com/metal-stack/masterdata-api/pkg/client" + "github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore" + "github.com/metal-stack/metal-api/cmd/metal-api/internal/ipam" + "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/test" + "github.com/metal-stack/metal-lib/bus" + "github.com/metal-stack/security" + testifymock "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go" +) + +func TestSwitchReplacementIntegration(t *testing.T) { + ts := createTestService(t) + defer ts.terminate() + + testPartitionID := "test-partition" + testRackID := "test-rack" + ts.createPartition(testPartitionID, testPartitionID, "Test Partition") + + cumulus1 := metal.Switch{ + Base: metal.Base{ + ID: "test-switch01", + Name: "", + }, + Nics: []metal.Nic{ + { + MacAddress: "aa:aa:aa:aa:aa:aa", + Name: "swp1", + }, + }, + PartitionID: testPartitionID, + RackID: testRackID, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, + } + ts.registerSwitch(cumulus1, true) + + cumulus2 := metal.Switch{ + Base: metal.Base{ + ID: "test-switch02", + Name: "", + }, + Nics: []metal.Nic{ + { + MacAddress: "bb:bb:bb:bb:bb:bb", + Name: "swp1", + }, + }, + PartitionID: testPartitionID, + RackID: testRackID, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, + } + ts.registerSwitch(cumulus2, true) + + m := &metal.Machine{ + Base: metal.Base{ + ID: "test-machine", + }, + PartitionID: testPartitionID, + RackID: testRackID, + Hardware: metal.MachineHardware{ + Nics: []metal.Nic{ + { + Name: "eth0", + MacAddress: "11:11:11:11:11:11", + Neighbors: []metal.Nic{ + { + Name: cumulus1.Nics[0].Name, + MacAddress: cumulus1.Nics[0].MacAddress, + }, + }, + }, + { + Name: "eth1", + MacAddress: "22:22:22:22:22:22", + Neighbors: []metal.Nic{ + { + Name: cumulus2.Nics[0].Name, + MacAddress: cumulus2.Nics[0].MacAddress, + }, + }, + }, + }, + }, + } + ts.createMachine(m) + + sonic1 := metal.Switch{ + Base: metal.Base{ + ID: cumulus1.ID, + }, + Nics: []metal.Nic{ + { + MacAddress: "cc:cc:cc:cc:cc:cc", + Name: "Ethernet4", + }, + }, + PartitionID: testPartitionID, + RackID: testRackID, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorSonic}, + } + wantConnections := metal.ConnectionMap{ + m.ID: metal.Connections{ + { + Nic: metal.Nic{ + Name: sonic1.Nics[0].Name, + MacAddress: sonic1.Nics[0].MacAddress, + }, + MachineID: m.ID, + }, + }, + } + ts.replaceSwitch(sonic1, wantConnections) + + wantMachineNics := metal.Nics{ + { + Name: m.Hardware.Nics[0].Name, + MacAddress: m.Hardware.Nics[0].MacAddress, + Neighbors: []metal.Nic{ + { + Name: sonic1.Nics[0].Name, + MacAddress: sonic1.Nics[0].MacAddress, + }, + }, + }, + { + Name: m.Hardware.Nics[1].Name, + MacAddress: m.Hardware.Nics[1].MacAddress, + Neighbors: []metal.Nic{ + { + Name: cumulus2.Nics[0].Name, + MacAddress: cumulus2.Nics[0].MacAddress, + }, + }, + }, + } + ts.checkMachineNics(m.ID, wantMachineNics) + + sonic2 := metal.Switch{ + Base: metal.Base{ + ID: cumulus2.ID, + }, + Nics: []metal.Nic{ + { + MacAddress: "dd:dd:dd:dd:dd:dd", + Name: "Ethernet4", + }, + }, + PartitionID: testPartitionID, + RackID: testRackID, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorSonic}, + } + wantConnections = metal.ConnectionMap{ + m.ID: metal.Connections{ + { + Nic: metal.Nic{ + Name: sonic2.Nics[0].Name, + MacAddress: sonic2.Nics[0].MacAddress, + }, + MachineID: m.ID, + }, + }, + } + ts.replaceSwitch(sonic2, wantConnections) + + wantMachineNics = metal.Nics{ + { + Name: m.Hardware.Nics[0].Name, + MacAddress: m.Hardware.Nics[0].MacAddress, + Neighbors: []metal.Nic{ + { + Name: sonic1.Nics[0].Name, + MacAddress: sonic1.Nics[0].MacAddress, + }, + }, + }, + { + Name: m.Hardware.Nics[1].Name, + MacAddress: m.Hardware.Nics[1].MacAddress, + Neighbors: []metal.Nic{ + { + Name: sonic2.Nics[0].Name, + MacAddress: sonic2.Nics[0].MacAddress, + }, + }, + }, + } + ts.checkMachineNics(m.ID, wantMachineNics) +} + +type testService struct { + partitionService *restful.WebService + switchService *restful.WebService + machineService *restful.WebService + ds *datastore.RethinkStore + rethinkContainer testcontainers.Container + ctx context.Context + t *testing.T +} + +func (ts *testService) terminate() { + _ = ts.rethinkContainer.Terminate(ts.ctx) +} + +func createTestService(t *testing.T) testService { + ipamer := ipam.InitTestIpam(t) + rethinkContainer, c, err := test.StartRethink(t) + require.NoError(t, err) + + log := slog.Default() + ds := datastore.New(log, c.IP+":"+c.Port, c.DB, c.User, c.Password) + ds.VRFPoolRangeMax = 1000 + ds.ASNPoolRangeMax = 1000 + + err = ds.Connect() + require.NoError(t, err) + err = ds.Initialize() + require.NoError(t, err) + + psc := &mdmv1mock.ProjectServiceClient{} + psc.On("Get", testifymock.Anything, &mdmv1.ProjectGetRequest{Id: "test-project-1"}).Return(&mdmv1.ProjectResponse{Project: &mdmv1.Project{ + Meta: &mdmv1.Meta{ + Id: "test-project-1", + }, + }}, nil) + psc.On("Find", testifymock.Anything, &mdmv1.ProjectFindRequest{}).Return(&mdmv1.ProjectListResponse{Projects: []*mdmv1.Project{ + {Meta: &mdmv1.Meta{Id: "test-project-1"}}, + }}, nil) + mdc := mdm.NewMock(psc, nil, nil, nil) + + hma := security.NewHMACAuth(testUserDirectory.admin.Name, []byte{1, 2, 3}, security.WithUser(testUserDirectory.admin)) + usergetter := security.NewCreds(security.WithHMAC(hma)) + machineService, err := NewMachine(log, ds, &emptyPublisher{}, bus.DirectEndpoints(), ipamer, mdc, nil, usergetter, 0, nil, metal.DisabledIPMISuperUser()) + require.NoError(t, err) + switchService := NewSwitch(log, ds) + require.NoError(t, err) + partitionService := NewPartition(log, ds, &emptyPublisher{}) + require.NoError(t, err) + + ts := testService{ + partitionService: partitionService, + switchService: switchService, + machineService: machineService, + ds: ds, + rethinkContainer: rethinkContainer, + ctx: context.TODO(), + t: t, + } + return ts +} + +func (ts *testService) partitionCreate(t *testing.T, icr v1.PartitionCreateRequest, response interface{}) int { + return webRequestPut(t, ts.partitionService, &testUserDirectory.admin, icr, "/v1/partition/", response) +} + +func (ts *testService) switchRegister(t *testing.T, srr v1.SwitchRegisterRequest, response interface{}) int { + return webRequestPost(t, ts.switchService, &testUserDirectory.admin, srr, "/v1/switch/register", response) +} + +func (ts *testService) switchGet(t *testing.T, swid string, response interface{}) int { + return webRequestGet(t, ts.switchService, &testUserDirectory.admin, emptyBody{}, "/v1/switch/"+swid, response) +} + +func (ts *testService) switchUpdate(t *testing.T, sur v1.SwitchUpdateRequest, response interface{}) int { + return webRequestPost(t, ts.switchService, &testUserDirectory.admin, sur, "/v1/switch/", response) +} + +func (ts *testService) machineGet(t *testing.T, mid string, response interface{}) int { + return webRequestGet(t, ts.machineService, &testUserDirectory.admin, emptyBody{}, "/v1/machine/"+mid, response) +} + +func (ts *testService) createPartition(id, name, description string) { + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "I am a downloadable content") + })) + defer s.Close() + + downloadableFile := s.URL + partitionName := name + partitionDesc := description + partition := v1.PartitionCreateRequest{ + Common: v1.Common{ + Identifiable: v1.Identifiable{ + ID: id, + }, + Describable: v1.Describable{ + Name: &partitionName, + Description: &partitionDesc, + }, + }, + PartitionBootConfiguration: v1.PartitionBootConfiguration{ + ImageURL: &downloadableFile, + KernelURL: &downloadableFile, + }, + } + var createdPartition v1.PartitionResponse + status := ts.partitionCreate(ts.t, partition, &createdPartition) + require.Equal(ts.t, http.StatusCreated, status) + require.NotNil(ts.t, createdPartition) + require.Equal(ts.t, partition.Name, createdPartition.Name) + require.NotEmpty(ts.t, createdPartition.ID) + +} + +func (ts *testService) registerSwitch(sw metal.Switch, isNewId bool) { + nics := make([]v1.SwitchNic, 0) + for _, nic := range sw.Nics { + nic := v1.SwitchNic{ + MacAddress: string(nic.MacAddress), + Name: nic.Name, + } + nics = append(nics, nic) + } + + srr := v1.SwitchRegisterRequest{ + Common: v1.Common{ + Identifiable: v1.Identifiable{ + ID: sw.ID, + }, + }, + Nics: nics, + PartitionID: sw.PartitionID, + SwitchBase: v1.SwitchBase{ + RackID: sw.RackID, + OS: &v1.SwitchOS{ + Vendor: sw.OS.Vendor, + }, + }, + } + + wantStatus := http.StatusOK + if isNewId { + wantStatus = http.StatusCreated + } + var res v1.SwitchResponse + status := ts.switchRegister(ts.t, srr, &res) + require.Equal(ts.t, wantStatus, status) + ts.checkSwitchResponse(sw, &res) +} + +func (ts *testService) createMachine(m *metal.Machine) { + err := ts.ds.CreateMachine(m) + require.NoError(ts.t, err) + err = ts.ds.ConnectMachineWithSwitches(m) + require.NoError(ts.t, err) + + err = ts.ds.CreateProvisioningEventContainer(&metal.ProvisioningEventContainer{ + Base: metal.Base{ID: m.ID}, + Liveliness: metal.MachineLivelinessAlive, + }) + require.NoError(ts.t, err) +} + +func (ts *testService) replaceSwitch(newSwitch metal.Switch, wantConnections metal.ConnectionMap) { + ts.setReplaceMode(newSwitch.ID) + ts.registerSwitch(newSwitch, false) + wantSwitch := newSwitch + wantSwitch.Mode = metal.SwitchOperational + wantSwitch.MachineConnections = wantConnections + + var res v1.SwitchResponse + status := ts.switchGet(ts.t, wantSwitch.ID, &res) + require.Equal(ts.t, http.StatusOK, status) + ts.checkSwitchResponse(wantSwitch, &res) +} + +func (ts *testService) setReplaceMode(id string) { + var res v1.SwitchResponse + + sur := v1.SwitchUpdateRequest{ + Common: v1.Common{ + Identifiable: v1.Identifiable{ + ID: id, + }, + }, + SwitchBase: v1.SwitchBase{ + Mode: string(metal.SwitchReplace), + }, + } + + status := ts.switchUpdate(ts.t, sur, &res) + require.Equal(ts.t, http.StatusOK, status) + require.Equal(ts.t, string(metal.SwitchReplace), res.SwitchBase.Mode) +} + +func (ts *testService) checkSwitchResponse(sw metal.Switch, res *v1.SwitchResponse) { + require.NotNil(ts.t, res) + require.Equal(ts.t, sw.Mode, metal.SwitchMode(res.Mode)) + + require.Len(ts.t, res.Nics, len(sw.Nics)) + for _, nic := range sw.Nics { + n := findNicByNameInSwitchNics(nic.Name, res.Nics) + ts.checkCorrectNic(n, nic) + } + + require.Len(ts.t, res.Connections, len(sw.MachineConnections)) + connectionsByNicName, err := sw.MachineConnections.ByNicName() + require.NoError(ts.t, err) + for nicName, con := range connectionsByNicName { + c, found := findMachineConnection(nicName, res.Connections) + require.True(ts.t, found) + require.Equal(ts.t, con.MachineID, c.MachineID) + require.Equal(ts.t, con.Nic.Name, c.Nic.Name) + require.Equal(ts.t, con.Nic.MacAddress, c.Nic.MacAddress) + } +} + +func (ts *testService) checkMachineNics(mid string, wantNics metal.Nics) { + var m v1.MachineResponse + status := ts.machineGet(ts.t, mid, &m) + require.Equal(ts.t, http.StatusOK, status) + require.NotNil(ts.t, m) + + for _, wantNic := range wantNics { + nic := findNicByNameInMachineNics(wantNic.Name, m.Hardware.Nics) + ts.checkCorrectNic(nic, wantNic) + for _, wantNeigh := range wantNic.Neighbors { + neigh := findNicByName(wantNeigh.Name, nic.Neighbors) + ts.checkCorrectNic(neigh, wantNeigh) + } + } +} + +func (ts *testService) checkCorrectNic(nic *metal.Nic, wantNic metal.Nic) { + require.NotNil(ts.t, wantNic) + require.Equal(ts.t, wantNic.Name, nic.Name) + require.Equal(ts.t, wantNic.MacAddress, nic.MacAddress) +} + +func findNicByName(name string, nics metal.Nics) *metal.Nic { + for _, nic := range nics { + n := nic + if nic.Name == name { + return &n + } + } + return nil +} + +func findNicByNameInSwitchNics(name string, nics v1.SwitchNics) *metal.Nic { + for _, nic := range nics { + if nic.Name == name { + n := &metal.Nic{ + Name: nic.Name, + MacAddress: metal.MacAddress(nic.MacAddress), + } + return n + } + } + return nil +} + +func findNicByNameInMachineNics(name string, nics v1.MachineNics) *metal.Nic { + for _, nic := range nics { + if nic.Name == name { + neighbors := make(metal.Nics, 0) + for _, neigh := range nic.Neighbors { + n := metal.Nic{ + Name: neigh.Name, + MacAddress: metal.MacAddress(neigh.MacAddress), + } + neighbors = append(neighbors, n) + } + n := &metal.Nic{ + Name: nic.Name, + MacAddress: metal.MacAddress(nic.MacAddress), + Neighbors: neighbors, + } + return n + } + } + return nil +} + +func findMachineConnection(nicName string, connections []v1.SwitchConnection) (*metal.Connection, bool) { + for _, con := range connections { + if con.Nic.Name == nicName { + c := &metal.Connection{ + Nic: metal.Nic{ + Name: con.Nic.Name, + MacAddress: metal.MacAddress(con.Nic.MacAddress), + }, + MachineID: con.MachineID, + } + return c, true + } + } + return nil, false +} diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index 8d003d12b..17e3e880a 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -196,6 +196,7 @@ func TestConnectMachineWithSwitches(t *testing.T) { s1 := metal.Switch{ Base: metal.Base{ID: "1"}, PartitionID: partitionID, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, MachineConnections: metal.ConnectionMap{}, Nics: metal.Nics{ s1swp1, @@ -208,6 +209,7 @@ func TestConnectMachineWithSwitches(t *testing.T) { s2 := metal.Switch{ Base: metal.Base{ID: "2"}, PartitionID: partitionID, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, MachineConnections: metal.ConnectionMap{}, Nics: metal.Nics{ s2swp1, @@ -627,9 +629,15 @@ func Test_adoptFromTwin(t *testing.T) { name: "adopt machine connections and nic configuration from twin", args: args{ old: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, Mode: metal.SwitchReplace, }, twin: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, Nics: metal.Nics{ metal.Nic{ Name: "swp1s0", @@ -665,6 +673,9 @@ func Test_adoptFromTwin(t *testing.T) { }, }, newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, Nics: metal.Nics{ metal.Nic{ Name: "swp1s0", @@ -687,6 +698,9 @@ func Test_adoptFromTwin(t *testing.T) { }, want: &metal.Switch{ Mode: metal.SwitchOperational, + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, Nics: metal.Nics{ metal.Nic{ Name: "swp1s0", @@ -784,21 +798,146 @@ func Test_adoptFromTwin(t *testing.T) { RackID: "1", }, twin: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, PartitionID: "1", RackID: "1", }, newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, PartitionID: "1", RackID: "1", }, }, want: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, PartitionID: "1", RackID: "1", Mode: metal.SwitchOperational, }, wantErr: false, }, + { + name: "adopt machine connections and nic configuration from twin with different switch os", + args: args{ + old: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, + Mode: metal.SwitchReplace, + }, + twin: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, + Nics: metal.Nics{ + metal.Nic{ + Name: "swp1s0", + MacAddress: "aa:aa:aa:aa:aa:a1", + Vrf: "1", + }, + metal.Nic{ + Name: "swp1s1", + MacAddress: "aa:aa:aa:aa:aa:a2", + }, + metal.Nic{ + Name: "swp1s2", + MacAddress: "aa:aa:aa:aa:aa:a3", + }, + }, + MachineConnections: metal.ConnectionMap{ + "m1": metal.Connections{ + metal.Connection{ + Nic: metal.Nic{ + Name: "swp1s0", + MacAddress: "aa:aa:aa:aa:aa:a1", + }, + }, + }, + "fw1": metal.Connections{ + metal.Connection{ + Nic: metal.Nic{ + Name: "swp1s1", + MacAddress: "aa:aa:aa:aa:aa:a2", + }, + }, + }, + }, + }, + newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorSonic, + }, + Nics: metal.Nics{ + metal.Nic{ + Name: "Ethernet4", + MacAddress: "bb:bb:bb:bb:bb:b1", + }, + metal.Nic{ + Name: "Ethernet5", + MacAddress: "bb:bb:bb:bb:bb:b2", + }, + metal.Nic{ + Name: "Ethernet6", + MacAddress: "bb:bb:bb:bb:bb:b3", + }, + metal.Nic{ + Name: "Ethernet7", + MacAddress: "bb:bb:bb:bb:bb:b4", + }, + }, + }, + }, + want: &metal.Switch{ + Mode: metal.SwitchOperational, + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorSonic, + }, + Nics: metal.Nics{ + metal.Nic{ + Name: "Ethernet4", + MacAddress: "bb:bb:bb:bb:bb:b1", + Vrf: "1", + }, + metal.Nic{ + Name: "Ethernet5", + MacAddress: "bb:bb:bb:bb:bb:b2", + }, + metal.Nic{ + Name: "Ethernet6", + MacAddress: "bb:bb:bb:bb:bb:b3", + }, + metal.Nic{ + Name: "Ethernet7", + MacAddress: "bb:bb:bb:bb:bb:b4", + }, + }, + MachineConnections: metal.ConnectionMap{ + "m1": metal.Connections{ + metal.Connection{ + Nic: metal.Nic{ + Name: "Ethernet4", + MacAddress: "bb:bb:bb:bb:bb:b1", + }, + }, + }, + "fw1": metal.Connections{ + metal.Connection{ + Nic: metal.Nic{ + Name: "Ethernet5", + MacAddress: "bb:bb:bb:bb:bb:b2", + }, + }, + }, + }, + }, + wantErr: false, + }, } for i := range tests { @@ -820,6 +959,7 @@ func Test_adoptNicsFromTwin(t *testing.T) { type args struct { twin *metal.Switch newSwitch *metal.Switch + changeOS bool } tests := []struct { name string @@ -831,6 +971,9 @@ func Test_adoptNicsFromTwin(t *testing.T) { name: "adopt vrf configuration, leaf underlay ports untouched, newSwitch might have additional ports", args: args{ twin: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, Nics: metal.Nics{ metal.Nic{ Name: "swp1s0", @@ -845,6 +988,9 @@ func Test_adoptNicsFromTwin(t *testing.T) { }, }, newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, Nics: metal.Nics{ metal.Nic{ Name: "swp1s0", @@ -883,6 +1029,9 @@ func Test_adoptNicsFromTwin(t *testing.T) { name: "new switch misses nic", args: args{ twin: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, Nics: metal.Nics{ metal.Nic{ Name: "swp1s0", @@ -892,6 +1041,9 @@ func Test_adoptNicsFromTwin(t *testing.T) { }, }, newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, Nics: metal.Nics{ metal.Nic{ Name: "swp1s1", @@ -902,18 +1054,81 @@ func Test_adoptNicsFromTwin(t *testing.T) { }, wantErr: true, }, + { + name: "switch os from cumulus to sonic", + args: args{ + changeOS: true, + twin: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorCumulus, + }, + Nics: metal.Nics{ + metal.Nic{ + Name: "swp1s0", + MacAddress: "aa:aa:aa:aa:aa:a1", + Vrf: "vrf1", + }, + metal.Nic{ + Name: "swp1s1", + MacAddress: "aa:aa:aa:aa:aa:a2", + Vrf: "", + }, + metal.Nic{ + Name: "swp99", + MacAddress: "aa:aa:aa:aa:aa:a3", + }, + }, + }, + newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{ + Vendor: metal.SwitchOSVendorSonic, + }, + Nics: metal.Nics{ + metal.Nic{ + Name: "Ethernet4", + MacAddress: "bb:bb:bb:bb:bb:b2", + }, + metal.Nic{ + Name: "Ethernet5", + MacAddress: "bb:bb:bb:bb:bb:b3", + }, + metal.Nic{ + Name: "Ethernet396", + MacAddress: "bb:bb:bb:bb:bb:b4", + }, + }, + }, + }, + want: metal.Nics{ + metal.Nic{ + Name: "Ethernet396", + MacAddress: "bb:bb:bb:bb:bb:b4", + }, + metal.Nic{ + Name: "Ethernet4", + MacAddress: "bb:bb:bb:bb:bb:b2", + Vrf: "vrf1", + }, + metal.Nic{ + Name: "Ethernet5", + MacAddress: "bb:bb:bb:bb:bb:b3", + Vrf: "", + }, + }, + wantErr: false, + }, } for i := range tests { tt := tests[i] t.Run(tt.name, func(t *testing.T) { - got, err := adoptNics(tt.args.twin, tt.args.newSwitch) + got, err := adoptNics(tt.args.twin, tt.args.newSwitch, tt.args.changeOS) if (err != nil) != tt.wantErr { t.Errorf("adoptNics() error = %v, wantErr %v", err, tt.wantErr) return } - if !reflect.DeepEqual(got.ByIdentifier(), tt.want.ByIdentifier()) { - t.Errorf("adoptNics() = %v, want %v", got, tt.want) + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Errorf("diff %v", diff) } }) } @@ -923,6 +1138,7 @@ func Test_adoptMachineConnections(t *testing.T) { type args struct { twin *metal.Switch newSwitch *metal.Switch + changeOS bool } tests := []struct { name string @@ -934,6 +1150,7 @@ func Test_adoptMachineConnections(t *testing.T) { name: "adopt machine connections from twin", args: args{ twin: &metal.Switch{ + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, MachineConnections: metal.ConnectionMap{ "m1": metal.Connections{ metal.Connection{ @@ -954,6 +1171,7 @@ func Test_adoptMachineConnections(t *testing.T) { }, }, newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, Nics: metal.Nics{ metal.Nic{ Name: "swp1s0", @@ -990,6 +1208,7 @@ func Test_adoptMachineConnections(t *testing.T) { name: "new switch misses nic for existing machine connection at twin", args: args{ twin: &metal.Switch{ + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, MachineConnections: metal.ConnectionMap{ "m1": metal.Connections{ metal.Connection{ @@ -1002,6 +1221,7 @@ func Test_adoptMachineConnections(t *testing.T) { }, }, newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, Nics: metal.Nics{ metal.Nic{ Name: "swp1s1", @@ -1012,12 +1232,71 @@ func Test_adoptMachineConnections(t *testing.T) { }, wantErr: true, }, + { + name: "adopt from twin with different switch os", + args: args{ + changeOS: true, + twin: &metal.Switch{ + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, + MachineConnections: metal.ConnectionMap{ + "m1": metal.Connections{ + metal.Connection{ + Nic: metal.Nic{ + Name: "swp1s0", + MacAddress: "aa:aa:aa:aa:aa:a1", + }, + }, + }, + "m2": metal.Connections{ + metal.Connection{ + Nic: metal.Nic{ + Name: "swp1s1", + MacAddress: "aa:aa:aa:aa:aa:a2", + }, + }, + }, + }, + }, + newSwitch: &metal.Switch{ + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorSonic}, + Nics: metal.Nics{ + metal.Nic{ + Name: "Ethernet4", + MacAddress: "bb:bb:bb:bb:bb:b1", + }, + metal.Nic{ + Name: "Ethernet5", + MacAddress: "bb:bb:bb:bb:bb:b2", + }, + }, + }, + }, + want: metal.ConnectionMap{ + "m1": metal.Connections{ + metal.Connection{ + Nic: metal.Nic{ + Name: "Ethernet4", + MacAddress: "bb:bb:bb:bb:bb:b1", + }, + }, + }, + "m2": metal.Connections{ + metal.Connection{ + Nic: metal.Nic{ + Name: "Ethernet5", + MacAddress: "bb:bb:bb:bb:bb:b2", + }, + }, + }, + }, + wantErr: false, + }, } for i := range tests { tt := tests[i] t.Run(tt.name, func(t *testing.T) { - got, err := adoptMachineConnections(tt.args.twin, tt.args.newSwitch) + got, err := adoptMachineConnections(tt.args.twin, tt.args.newSwitch, tt.args.changeOS) if (err != nil) != tt.wantErr { t.Errorf("adoptMachineConnections() error = %v, wantErr %v", err, tt.wantErr) return @@ -1437,6 +1716,188 @@ func TestToggleSwitchNicWithoutMachine(t *testing.T) { 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)) } +func Test_adjustNics(t *testing.T) { + tests := []struct { + name string + nics metal.Nics + connections metal.Connections + nicMap metal.NicMap + want metal.Nics + wantErr bool + }{ + { + name: "nothing to adjust", + nics: []metal.Nic{ + { + Name: "eth0", + MacAddress: "11:11:11:11:11:11", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:aa", + }, + }, + }, + { + Name: "eth1", + MacAddress: "11:11:11:11:11:22", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:bb", + }, + }, + }, + }, + connections: []metal.Connection{ + { + Nic: metal.Nic{ + Name: "swp1", + MacAddress: "cc:cc:cc:cc:cc:cc", + }, + }, + }, + want: []metal.Nic{ + { + Name: "eth0", + MacAddress: "11:11:11:11:11:11", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:aa", + }, + }, + }, + { + Name: "eth1", + MacAddress: "11:11:11:11:11:22", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:bb", + }, + }, + }, + }, + wantErr: false, + }, + { + name: "unrealistic error case", + nics: []metal.Nic{ + { + Name: "eth0", + MacAddress: "11:11:11:11:11:11", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:aa", + }, + }, + }, + { + Name: "eth1", + MacAddress: "11:11:11:11:11:22", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:bb", + }, + }, + }, + }, + connections: []metal.Connection{ + { + Nic: metal.Nic{ + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:aa", + }, + }, + }, + nicMap: map[string]*metal.Nic{ + "swp0": { + Name: "Ethernet0", + MacAddress: "dd:dd:dd:dd:dd:dd", + }, + }, + want: nil, + wantErr: true, + }, + { + name: "adjust nics from cumulus to sonic", + nics: []metal.Nic{ + { + Name: "eth0", + MacAddress: "11:11:11:11:11:11", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:aa", + }, + }, + }, + { + Name: "eth1", + MacAddress: "11:11:11:11:11:22", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:bb", + }, + }, + }, + }, + connections: []metal.Connection{ + { + Nic: metal.Nic{ + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:aa", + }, + }, + }, + nicMap: map[string]*metal.Nic{ + "swp1": { + Name: "Ethernet4", + MacAddress: "dd:dd:dd:dd:dd:dd", + }, + }, + want: []metal.Nic{ + { + Name: "eth0", + MacAddress: "11:11:11:11:11:11", + Neighbors: []metal.Nic{ + { + Name: "Ethernet4", + MacAddress: "dd:dd:dd:dd:dd:dd", + }, + }, + }, + { + Name: "eth1", + MacAddress: "11:11:11:11:11:22", + Neighbors: []metal.Nic{ + { + Name: "swp1", + MacAddress: "aa:aa:aa:aa:aa:bb", + }, + }, + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := adjustMachineNics(tt.nics, tt.connections, tt.nicMap) + if (err != nil) != tt.wantErr { + t.Errorf("adjustMachineNics() error = %v, wantErr %v", err, tt.wantErr) + } + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Errorf("adjustMachineNics() diff = %v", diff) + } + }) + } +} + func Test_SwitchDelete(t *testing.T) { tests := []struct { name string diff --git a/cmd/metal-api/internal/service/v1/switch.go b/cmd/metal-api/internal/service/v1/switch.go index 85c214c0d..e8e6f4729 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -32,9 +32,10 @@ type SwitchBase struct { } type SwitchOS struct { - Vendor string `json:"vendor" description:"the operating system vendor the switch currently has" optional:"true"` - Version string `json:"version" description:"the operating system version the switch currently has" optional:"true"` - MetalCoreVersion string `json:"metal_core_version" description:"the version of metal-core running" optional:"true"` + // TODO: do we need a distribution e.g. edgecore vs broadcom? + Vendor metal.SwitchOSVendor `json:"vendor" description:"the operating system vendor the switch currently has" optional:"true"` + Version string `json:"version" description:"the operating system version the switch currently has" optional:"true"` + MetalCoreVersion string `json:"metal_core_version" description:"the version of metal-core running" optional:"true"` } type SwitchNics []SwitchNic diff --git a/cmd/metal-api/internal/testdata/testdata.go b/cmd/metal-api/internal/testdata/testdata.go index 273f5bfb0..bdbcf431d 100644 --- a/cmd/metal-api/internal/testdata/testdata.go +++ b/cmd/metal-api/internal/testdata/testdata.go @@ -538,6 +538,7 @@ var ( Base: metal.Base{ ID: "switch1", }, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, PartitionID: "1", RackID: "1", Nics: []metal.Nic{ @@ -566,6 +567,7 @@ var ( Base: metal.Base{ ID: "switch2", }, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, PartitionID: "1", RackID: "1", Nics: []metal.Nic{ @@ -577,6 +579,7 @@ var ( Base: metal.Base{ ID: "switch3", }, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, PartitionID: "1", RackID: "3", MachineConnections: metal.ConnectionMap{}, @@ -585,6 +588,7 @@ var ( Base: metal.Base{ ID: "switch1", }, + OS: &metal.SwitchOS{Vendor: metal.SwitchOSVendorCumulus}, PartitionID: "1", RackID: "1", Nics: []metal.Nic{ @@ -607,7 +611,7 @@ var ( // Nics Nic1 = metal.Nic{ MacAddress: metal.MacAddress("11:11:11:11:11:11"), - Name: "eth0", + Name: "swp0", Neighbors: []metal.Nic{ { MacAddress: "21:11:11:11:11:11",