Merge pull request #2755 from dtantsur/no-inspector

⚠️ Completely remove support for ironic-inspector

Author: metal3-io-bot

pkg/provisioner/ironic/hardwaredetails/hardwaredetails.go

@@ -9,13 +9,12 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/gophercloud/gophercloud/v2/openstack/baremetal/inventory"
 	"github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes"
-	"github.com/gophercloud/gophercloud/v2/openstack/baremetalintrospection/v1/introspection"
 	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 )
 
 // GetHardwareDetails converts Ironic introspection data into BareMetalHost HardwareDetails.
 func GetHardwareDetails(data *nodes.InventoryData, logger logr.Logger) *metal3api.HardwareDetails {
-	ironicData, inspectorData, err := data.PluginData.GuessFormat()
+	ironicData, err := data.PluginData.AsStandardData()
 	if err != nil {
 		logger.Error(err, "cannot get plugin data from inventory, some fields will not be available")
 	}
@@ -24,7 +23,7 @@ func GetHardwareDetails(data *nodes.InventoryData, logger logr.Logger) *metal3ap
 	details.Firmware = getFirmwareDetails(data.Inventory.SystemVendor.Firmware)
 	details.SystemVendor = getSystemVendorDetails(data.Inventory.SystemVendor)
 	details.RAMMebibytes = data.Inventory.Memory.PhysicalMb
-	details.NIC = getNICDetails(data.Inventory.Interfaces, ironicData, inspectorData)
+	details.NIC = getNICDetails(data.Inventory.Interfaces, ironicData)
 	details.Storage = getStorageDetails(data.Inventory.Disks)
 	details.CPU = getCPUDetails(&data.Inventory.CPU)
 	details.Hostname = data.Inventory.Hostname
@@ -54,20 +53,11 @@ func getVLANs(lldp map[string]interface{}) (vlans []metal3api.VLAN, vlanid metal
 	return
 }
 
-func getNICDetails(ifdata []inventory.InterfaceType,
-	ironicData *inventory.StandardPluginData,
-	inspectorData *introspection.Data) []metal3api.NIC {
+func getNICDetails(ifdata []inventory.InterfaceType, ironicData inventory.StandardPluginData) []metal3api.NIC {
 	var nics []metal3api.NIC
 	for _, intf := range ifdata {
-		var lldp map[string]interface{}
-		var pxeEnabled bool
-		if ironicData != nil {
-			pxeEnabled = ironicData.AllInterfaces[intf.Name].PXEEnabled
-			lldp = ironicData.ParsedLLDP[intf.Name]
-		} else {
-			pxeEnabled = inspectorData.AllInterfaces[intf.Name].PXE
-			lldp = inspectorData.AllInterfaces[intf.Name].LLDPProcessed
-		}
+		pxeEnabled := ironicData.AllInterfaces[intf.Name].PXEEnabled
+		lldp := ironicData.ParsedLLDP[intf.Name]
 
 		vlans, vlanid := getVLANs(lldp)
 		// We still store one nic even if both ips are unset

pkg/provisioner/ironic/hardwaredetails/hardwaredetails_test.go

@@ -5,7 +5,6 @@ import (
 	"testing"
 
 	"github.com/gophercloud/gophercloud/v2/openstack/baremetal/inventory"
-	"github.com/gophercloud/gophercloud/v2/openstack/baremetalintrospection/v1/introspection"
 	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	"github.com/stretchr/testify/assert"
 )
@@ -129,7 +128,7 @@ func TestGetVLANsMalformed(t *testing.T) {
 	}
 }
 
-func TestGetNICDetailsInspector(t *testing.T) {
+func TestGetNICDetails(t *testing.T) {
 	ironicData := inventory.StandardPluginData{
 		AllInterfaces: map[string]inventory.ProcessedInterfaceType{
 			"eth0": {
@@ -147,21 +146,6 @@ func TestGetNICDetailsInspector(t *testing.T) {
 			},
 		},
 	}
-	inspectorData := introspection.Data{
-		AllInterfaces: map[string]introspection.BaseInterfaceType{
-			"eth0": {
-				PXE: true,
-				LLDPProcessed: map[string]interface{}{
-					"switch_port_vlans": []map[string]interface{}{
-						{
-							"id": 1,
-						},
-					},
-					"switch_port_untagged_vlan_id": 1,
-				},
-			},
-		},
-	}
 	interfaces := []inventory.InterfaceType{
 		{
 			Name:        "eth0",
@@ -182,69 +166,49 @@ func TestGetNICDetailsInspector(t *testing.T) {
 			MACAddress: "00:11:22:33:44:77"},
 	}
 
-	cases := []struct {
-		name          string
-		ironicData    *inventory.StandardPluginData
-		inspectorData *introspection.Data
-	}{
-		{
-			name:          "with-ironic",
-			ironicData:    &ironicData,
-			inspectorData: nil,
-		},
-		{
-			name:          "with-inspector",
-			ironicData:    nil,
-			inspectorData: &inspectorData,
-		},
-	}
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			nics := getNICDetails(interfaces, tc.ironicData, tc.inspectorData)
+	nics := getNICDetails(interfaces, ironicData)
 
-			// 5 expected because eth46 results in two items
-			assert.Len(t, nics, 5)
-			if (!reflect.DeepEqual(nics[0], metal3api.NIC{
-				Name: "eth0",
-				MAC:  "00:11:22:33:44:55",
-				IP:   "192.0.2.1",
-				PXE:  true,
-				VLANs: []metal3api.VLAN{
-					{ID: 1},
-				},
-				VLANID: 1,
-			})) {
-				t.Errorf("Unexpected NIC data")
-			}
-			if (!reflect.DeepEqual(nics[1], metal3api.NIC{
-				Name:      "eth1",
-				MAC:       "66:77:88:99:aa:bb",
-				IP:        "2001:db8::1",
-				SpeedGbps: 1,
-			})) {
-				t.Errorf("Unexpected NIC data")
-			}
-			if (!reflect.DeepEqual(nics[2], metal3api.NIC{
-				Name: "eth46",
-				MAC:  "00:11:22:33:44:66",
-				IP:   "192.0.2.2",
-			})) {
-				t.Errorf("Unexpected NIC data")
-			}
-			if (!reflect.DeepEqual(nics[3], metal3api.NIC{
-				Name: "eth46",
-				MAC:  "00:11:22:33:44:66",
-				IP:   "2001:db8::2",
-			})) {
-				t.Errorf("Unexpected NIC data")
-			}
-			if (!reflect.DeepEqual(nics[4], metal3api.NIC{
-				Name: "ethNoIp",
-				MAC:  "00:11:22:33:44:77",
-			})) {
-				t.Errorf("Unexpected NIC data")
-			}
-		})
+	// 5 expected because eth46 results in two items
+	assert.Len(t, nics, 5)
+	if (!reflect.DeepEqual(nics[0], metal3api.NIC{
+		Name: "eth0",
+		MAC:  "00:11:22:33:44:55",
+		IP:   "192.0.2.1",
+		PXE:  true,
+		VLANs: []metal3api.VLAN{
+			{ID: 1},
+		},
+		VLANID: 1,
+	})) {
+		t.Errorf("Unexpected NIC data")
+	}
+	if (!reflect.DeepEqual(nics[1], metal3api.NIC{
+		Name:      "eth1",
+		MAC:       "66:77:88:99:aa:bb",
+		IP:        "2001:db8::1",
+		SpeedGbps: 1,
+	})) {
+		t.Errorf("Unexpected NIC data")
+	}
+	if (!reflect.DeepEqual(nics[2], metal3api.NIC{
+		Name: "eth46",
+		MAC:  "00:11:22:33:44:66",
+		IP:   "192.0.2.2",
+	})) {
+		t.Errorf("Unexpected NIC data")
+	}
+	if (!reflect.DeepEqual(nics[3], metal3api.NIC{
+		Name: "eth46",
+		MAC:  "00:11:22:33:44:66",
+		IP:   "2001:db8::2",
+	})) {
+		t.Errorf("Unexpected NIC data")
+	}
+	if (!reflect.DeepEqual(nics[4], metal3api.NIC{
+		Name: "ethNoIp",
+		MAC:  "00:11:22:33:44:77",
+	})) {
+		t.Errorf("Unexpected NIC data")
 	}
 }
 

pkg/provisioner/ironic/inspecthardware.go

@@ -3,32 +3,16 @@ package ironic
 import (
 	"fmt"
 	"net/http"
-	"slices"
 	"strings"
 
 	"github.com/gophercloud/gophercloud/v2"
-	"github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/drivers"
 	"github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes"
 	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
-	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/clients"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/hardwaredetails"
 )
 
-func (p *ironicProvisioner) getInspectInterface(bmcAccess bmc.AccessDetails) (string, error) {
-	driver, err := drivers.GetDriverDetails(p.ctx, p.client, bmcAccess.Driver()).Extract()
-	if err != nil {
-		return "", fmt.Errorf("cannot load information about driver %s: %w", bmcAccess.Driver(), err)
-	}
-
-	if slices.Contains(driver.EnabledInspectInterfaces, "agent") {
-		return "agent", nil
-	}
-
-	return "inspector", nil // backward compatibility
-}
-
 func (p *ironicProvisioner) abortInspection(ironicNode *nodes.Node) (result provisioner.Result, started bool, details *metal3api.HardwareDetails, err error) {
 	// Set started to let the controller know about the change
 	p.log.Info("aborting inspection to force reboot of preprovisioning image")

pkg/provisioner/ironic/register.go

@@ -15,6 +15,10 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
+const (
+	defaultInspectInterface = "agent"
+)
+
 func bmcAddressMatches(ironicNode *nodes.Node, driverInfo map[string]interface{}) bool {
 	newAddress := make(map[string]interface{})
 	ironicAddress := make(map[string]interface{})
@@ -234,19 +238,14 @@ func (p *ironicProvisioner) Register(data provisioner.ManagementAccessData, cred
 }
 
 func (p *ironicProvisioner) enrollNode(data provisioner.ManagementAccessData, bmcAccess bmc.AccessDetails, driverInfo map[string]interface{}) (ironicNode *nodes.Node, retry bool, err error) {
-	inspectInterface, err := p.getInspectInterface(bmcAccess)
-	if err != nil {
-		return nil, true, err
-	}
-
 	nodeCreateOpts := nodes.CreateOpts{
 		Driver:              bmcAccess.Driver(),
 		BIOSInterface:       bmcAccess.BIOSInterface(),
 		BootInterface:       bmcAccess.BootInterface(),
 		Name:                ironicNodeName(p.objectMeta),
 		DriverInfo:          driverInfo,
 		DeployInterface:     p.deployInterface(data),
-		InspectInterface:    inspectInterface,
+		InspectInterface:    defaultInspectInterface,
 		ManagementInterface: bmcAccess.ManagementInterface(),
 		PowerInterface:      bmcAccess.PowerInterface(),
 		RAIDInterface:       bmcAccess.RAIDInterface(),

pkg/provisioner/ironic/register_test.go

@@ -108,44 +108,6 @@ func TestRegisterCreateNode(t *testing.T) {
 	assert.Equal(t, "agent", createdNode.InspectInterface)
 }
 
-func TestRegisterCreateNodeOldInspection(t *testing.T) {
-	// Create a host without a bootMACAddress and with a BMC that
-	// does not require one.
-	host := makeHost()
-	host.Spec.BootMACAddress = ""
-	host.Spec.Image = nil
-	host.Status.Provisioning.ID = "" // so we don't lookup by uuid
-
-	var createdNode *nodes.Node
-
-	createCallback := func(node nodes.Node) {
-		createdNode = &node
-	}
-
-	ironic := testserver.NewIronic(t).CreateNodes(createCallback).NoNode(host.Namespace + nameSeparator + host.Name).NoNode(host.Name)
-	ironic.AddDefaultResponse("/v1/nodes/node-0", "PATCH", http.StatusOK, "{}")
-	ironic.AddDefaultResponse("/v1/drivers/test", "GET", 200, `
-	    {"enabled_inspect_interfaces": ["inspector", "no-inspect"]}
-	`)
-	ironic.Start()
-	defer ironic.Stop()
-
-	auth := clients.AuthConfig{Type: clients.NoAuth}
-	prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nullEventPublisher, ironic.Endpoint(), auth)
-	if err != nil {
-		t.Fatalf("could not create provisioner: %s", err)
-	}
-
-	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
-	if err != nil {
-		t.Fatalf("error from Register: %s", err)
-	}
-	assert.Empty(t, result.ErrorMessage)
-	assert.NotEmpty(t, createdNode.UUID)
-	assert.Equal(t, createdNode.UUID, provID)
-	assert.Equal(t, "inspector", createdNode.InspectInterface)
-}
-
 func TestRegisterExistingNode(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// does not require one.

pkg/provisioner/ironic/testserver/ironic.go

@@ -106,7 +106,7 @@ func (m *IronicMock) WithDrivers() *IronicMock {
 	m.ResponseWithCode("/v1/drivers/test", `
 	{
 	    "enabled_deploy_interfaces": ["direct", "ramdisk", "custom-agent"],
-	    "enabled_inspect_interfaces": ["agent", "inspector", "no-inspect"]
+	    "enabled_inspect_interfaces": ["agent", "no-inspect"]
 	}
 	`, http.StatusOK)
 	return m