Repository: metal3-io/baremetal-operator
Pull request #1297 (2023-06-20 12:51:42)

🌱 Stop using Extra when building hardware inventory
The core inventory has all we need. Extra comes from a plug-in that
may not be present (e.g. in upstream IPA images).


Patch:
diff --git a/go.mod b/go.mod
index 9ab59e97..f3de30e8 100644
--- a/go.mod
+++ b/go.mod
@@ -5,7 +5,7 @@ go 1.19
 require (
 	github.com/go-logr/logr v1.2.4
 	github.com/google/safetext v0.0.0-20230106111101-7156a760e523
-	github.com/gophercloud/gophercloud v1.3.0
+	github.com/gophercloud/gophercloud v1.4.1-0.20230614092438-44d55f08cdc0
 	github.com/metal3-io/baremetal-operator/apis v0.2.0
 	github.com/metal3-io/baremetal-operator/pkg/hardwareutils v0.2.0
 	github.com/pkg/errors v0.9.1
diff --git a/go.sum b/go.sum
index d8ba3395..3e2ca6de 100644
--- a/go.sum
+++ b/go.sum
@@ -71,8 +71,8 @@ github.com/google/safetext v0.0.0-20230106111101-7156a760e523 h1:i4NsbmB9pD5+Ggp
 github.com/google/safetext v0.0.0-20230106111101-7156a760e523/go.mod h1:mJNEy0r5YPHC7ChQffpOszlGB4L1iqjXWpIEKcFpr9s=
 github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
-github.com/gophercloud/gophercloud v1.3.0 h1:RUKyCMiZoQR3VlVR5E3K7PK1AC3/qppsWYo6dtBiqs8=
-github.com/gophercloud/gophercloud v1.3.0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM=
+github.com/gophercloud/gophercloud v1.4.1-0.20230614092438-44d55f08cdc0 h1:TNfpghMRzEdmU8xPrmMsNZ4Qhx7im/d8BSh9XufBwG8=
+github.com/gophercloud/gophercloud v1.4.1-0.20230614092438-44d55f08cdc0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM=
 github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
 github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
 github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
diff --git a/pkg/provisioner/ironic/hardwaredetails/hardwaredetails.go b/pkg/provisioner/ironic/hardwaredetails/hardwaredetails.go
index 9d07744e..6c312889 100644
--- a/pkg/provisioner/ironic/hardwaredetails/hardwaredetails.go
+++ b/pkg/provisioner/ironic/hardwaredetails/hardwaredetails.go
@@ -14,10 +14,10 @@ import (
 // GetHardwareDetails converts Ironic introspection data into BareMetalHost HardwareDetails.
 func GetHardwareDetails(data *introspection.Data) *metal3api.HardwareDetails {
 	details := new(metal3api.HardwareDetails)
-	details.Firmware = getFirmwareDetails(data.Extra.Firmware)
+	details.Firmware = getFirmwareDetails(data.Inventory.SystemVendor.Firmware)
 	details.SystemVendor = getSystemVendorDetails(data.Inventory.SystemVendor)
 	details.RAMMebibytes = data.MemoryMB
-	details.NIC = getNICDetails(data.Inventory.Interfaces, data.AllInterfaces, data.Extra.Network)
+	details.NIC = getNICDetails(data.Inventory.Interfaces, data.AllInterfaces)
 	details.Storage = getStorageDetails(data.Inventory.Disks)
 	details.CPU = getCPUDetails(&data.Inventory.CPU)
 	details.Hostname = data.Inventory.Hostname
@@ -47,18 +47,8 @@ func getVLANs(intf introspection.BaseInterfaceType) (vlans []metal3api.VLAN, vla
 	return
 }
 
-func getNICSpeedGbps(intfExtradata introspection.ExtraHardwareData) (speedGbps int) {
-	if speed, ok := intfExtradata["speed"].(string); ok {
-		if strings.HasSuffix(speed, "Gbps") {
-			fmt.Sscanf(speed, "%d", &speedGbps)
-		}
-	}
-	return
-}
-
 func getNICDetails(ifdata []introspection.InterfaceType,
-	basedata map[string]introspection.BaseInterfaceType,
-	extradata introspection.ExtraHardwareDataSection) []metal3api.NIC {
+	basedata map[string]introspection.BaseInterfaceType) []metal3api.NIC {
 	var nics []metal3api.NIC
 	for _, intf := range ifdata {
 		baseIntf := basedata[intf.Name]
@@ -74,7 +64,7 @@ func getNICDetails(ifdata []introspection.InterfaceType,
 				IP:        intf.IPV4Address,
 				VLANs:     vlans,
 				VLANID:    vlanid,
-				SpeedGbps: getNICSpeedGbps(extradata[intf.Name]),
+				SpeedGbps: intf.SpeedMbps / 1000,
 				PXE:       baseIntf.PXE,
 			})
 		}
@@ -87,7 +77,7 @@ func getNICDetails(ifdata []introspection.InterfaceType,
 				IP:        intf.IPV6Address,
 				VLANs:     vlans,
 				VLANID:    vlanid,
-				SpeedGbps: getNICSpeedGbps(extradata[intf.Name]),
+				SpeedGbps: intf.SpeedMbps / 1000,
 				PXE:       baseIntf.PXE,
 			})
 		}
@@ -155,22 +145,12 @@ func getCPUDetails(cpudata *introspection.CPUType) metal3api.CPU {
 	return cpu
 }
 
-func getFirmwareDetails(firmwaredata introspection.ExtraHardwareDataSection) metal3api.Firmware {
-
-	// handle bios optionally
-	var bios metal3api.BIOS
-
-	if biosdata, ok := firmwaredata["bios"]; ok {
-		// we do not know if all fields will be supplied
-		// as this is not a structured response
-		// so we must handle each field conditionally
-		bios.Vendor, _ = biosdata["vendor"].(string)
-		bios.Version, _ = biosdata["version"].(string)
-		bios.Date, _ = biosdata["date"].(string)
-	}
-
+func getFirmwareDetails(firmwaredata introspection.SystemFirmwareType) metal3api.Firmware {
 	return metal3api.Firmware{
-		BIOS: bios,
+		BIOS: metal3api.BIOS{
+			Vendor:  firmwaredata.Vendor,
+			Version: firmwaredata.Version,
+			Date:    firmwaredata.BuildDate,
+		},
 	}
-
 }


Test patch:
diff --git a/pkg/provisioner/ironic/hardwaredetails/hardwaredetails_test.go b/pkg/provisioner/ironic/hardwaredetails/hardwaredetails_test.go
index 540febba..14ad04bf 100644
--- a/pkg/provisioner/ironic/hardwaredetails/hardwaredetails_test.go
+++ b/pkg/provisioner/ironic/hardwaredetails/hardwaredetails_test.go
@@ -158,7 +158,8 @@ func TestGetNICDetails(t *testing.T) {
 			{
 				Name:        "eth1",
 				IPV6Address: "2001:db8::1",
-				MACAddress:  "66:77:88:99:aa:bb"},
+				MACAddress:  "66:77:88:99:aa:bb",
+				SpeedMbps:   1000},
 			{
 				Name:        "eth46",
 				IPV6Address: "2001:db8::2",
@@ -180,11 +181,6 @@ func TestGetNICDetails(t *testing.T) {
 					"switch_port_untagged_vlan_id": 1,
 				},
 			},
-		},
-		introspection.ExtraHardwareDataSection{
-			"eth1": introspection.ExtraHardwareData{
-				"speed": "1Gbps",
-			},
 		})
 
 	// 5 expected because eth46 results in two items
@@ -233,74 +229,16 @@ func TestGetNICDetails(t *testing.T) {
 	}
 }
 
-func TestGetNICSpeedGbps(t *testing.T) {
-	s1 := getNICSpeedGbps(introspection.ExtraHardwareData{
-		"speed": "25Gbps",
-	})
-	if s1 != 25 {
-		t.Errorf("Expected speed 25, got %d", s1)
-	}
-
-	s2 := getNICSpeedGbps(introspection.ExtraHardwareData{
-		"speed": "100Mbps",
-	})
-	if s2 != 0 {
-		t.Errorf("Expected speed 0, got %d", s2)
-	}
-
-	s3 := getNICSpeedGbps(introspection.ExtraHardwareData{
-		"speed": 10,
-	})
-	if s3 != 0 {
-		t.Errorf("Expected speed 0, got %d", s3)
-	}
-
-	s4 := getNICSpeedGbps(introspection.ExtraHardwareData{})
-	if s4 != 0 {
-		t.Errorf("Expected speed 0, got %d", s4)
-	}
-}
-
 func TestGetFirmwareDetails(t *testing.T) {
 	// Test full (known) firmware payload
-	firmware := getFirmwareDetails(introspection.ExtraHardwareDataSection{
-		"bios": {
-			"vendor":  "foobar",
-			"version": "1.2.3",
-			"date":    "2019-07-10",
-		},
+	firmware := getFirmwareDetails(introspection.SystemFirmwareType{
+		Vendor:    "foobar",
+		Version:   "1.2.3",
+		BuildDate: "2019-07-10",
 	})
 
 	if firmware.BIOS.Vendor != "foobar" {
 		t.Errorf("Expected firmware BIOS vendor to be foobar, but got: %s", firmware)
 	}
 
-	// Ensure we can handle partial firmware/bios data
-	firmware = getFirmwareDetails(introspection.ExtraHardwareDataSection{
-		"bios": {
-			"vendor":  "foobar",
-			"version": "1.2.3",
-		},
-	})
-
-	if firmware.BIOS.Date != "" {
-		t.Errorf("Expected firmware BIOS date to be empty but got: %s", firmware)
-	}
-
-	// Ensure we can handle unexpected types
-	firmware = getFirmwareDetails(introspection.ExtraHardwareDataSection{
-		"bios": {
-			"vendor":  3,
-			"version": []int{2, 1},
-			"date":    map[string]string{"year": "2019", "month": "07", "day": "10"},
-		},
-	})
-
-	// Finally, ensure we can handle completely empty firmware data
-	firmware = getFirmwareDetails(introspection.ExtraHardwareDataSection{})
-
-	if (firmware != metal3api.Firmware{}) {
-		t.Errorf("Expected firmware data to be empty but got: %s", firmware)
-	}
-
 }