Repository: metal3-io/baremetal-operator
Pull request #2154 (2024-12-24 17:17:42)

🌱 Refactor and rename ValidateManagementAccess
Validating access is not its primary responsibility: it's main job is to
make sure the Ironic node exists and is correctly configured. For a lack
of a perfect name, I'm calling it Register here.

The renamed method is moved to a new file and split into more manageable
pieces. In the future, we can even unit-test them separately.

Signed-off-by: Dmitry Tantsur <dtantsur@protonmail.com>


Patch:
diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go
index c264980a..46e0c880 100644
--- a/controllers/metal3.io/baremetalhost_controller.go
+++ b/controllers/metal3.io/baremetalhost_controller.go
@@ -831,7 +831,7 @@ func (r *BareMetalHostReconciler) registerHost(prov provisioner.Provisioner, inf
 		return recordActionFailure(info, metal3api.RegistrationError, "failed to read preprovisioningNetworkData")
 	}
 
-	provResult, provID, err := prov.ValidateManagementAccess(
+	provResult, provID, err := prov.Register(
 		provisioner.ManagementAccessData{
 			BootMode:                   info.host.Status.Provisioning.BootMode,
 			AutomatedCleaningMode:      info.host.Spec.AutomatedCleaningMode,
diff --git a/pkg/provisioner/demo/demo.go b/pkg/provisioner/demo/demo.go
index d4e8acd4..4f436cb5 100644
--- a/pkg/provisioner/demo/demo.go
+++ b/pkg/provisioner/demo/demo.go
@@ -80,9 +80,9 @@ func (p *demoProvisioner) HasCapacity() (result bool, err error) {
 	return true, nil
 }
 
-// ValidateManagementAccess tests the connection information for the
+// Register tests the connection information for the
 // host to verify that the location and credentials work.
-func (p *demoProvisioner) ValidateManagementAccess(_ provisioner.ManagementAccessData, _, _ bool) (result provisioner.Result, provID string, err error) {
+func (p *demoProvisioner) Register(_ provisioner.ManagementAccessData, _, _ bool) (result provisioner.Result, provID string, err error) {
 	p.log.Info("testing management access")
 
 	hostName := p.objectMeta.Name
diff --git a/pkg/provisioner/fixture/fixture.go b/pkg/provisioner/fixture/fixture.go
index c8e9ab3e..51fa446e 100644
--- a/pkg/provisioner/fixture/fixture.go
+++ b/pkg/provisioner/fixture/fixture.go
@@ -111,9 +111,9 @@ func (p *fixtureProvisioner) HasCapacity() (result bool, err error) {
 	return true, nil
 }
 
-// ValidateManagementAccess tests the connection information for the
+// Register tests the connection information for the
 // host to verify that the location and credentials work.
-func (p *fixtureProvisioner) ValidateManagementAccess(_ provisioner.ManagementAccessData, _, _ bool) (result provisioner.Result, provID string, err error) {
+func (p *fixtureProvisioner) Register(_ provisioner.ManagementAccessData, _, _ bool) (result provisioner.Result, provID string, err error) {
 	p.log.Info("testing management access")
 
 	if p.state.validateError != "" {
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index 4446e2cd..18f08ced 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -4,8 +4,6 @@ import (
 	"context"
 	"fmt"
 	"net"
-	"reflect"
-	"regexp"
 	"strings"
 	"time"
 
@@ -305,261 +303,6 @@ func (p *ironicProvisioner) createPXEEnabledNodePort(uuid, macAddress string) er
 	return nil
 }
 
-// ValidateManagementAccess registers the host with the provisioning
-// system and tests the connection information for the host to verify
-// that the location and credentials work.
-//
-// FIXME(dhellmann): We should rename this method to describe what it
-// actually does.
-func (p *ironicProvisioner) ValidateManagementAccess(data provisioner.ManagementAccessData, credentialsChanged, restartOnFailure bool) (result provisioner.Result, provID string, err error) {
-	bmcAccess, err := p.bmcAccess()
-	if err != nil {
-		result, err = operationFailed(err.Error())
-		return result, "", err
-	}
-
-	var ironicNode *nodes.Node
-	updater := clients.UpdateOptsBuilder(p.log)
-
-	p.debugLog.Info("validating management access")
-
-	ironicNode, err = p.findExistingHost(p.bootMACAddress)
-	if err != nil {
-		switch err.(type) {
-		case macAddressConflictError:
-			result, err = operationFailed(err.Error())
-		default:
-			result, err = transientError(errors.Wrap(err, "failed to find existing host"))
-		}
-		return result, "", err
-	}
-
-	// Some BMC types require a MAC address, so ensure we have one
-	// when we need it. If not, place the host in an error state.
-	if bmcAccess.NeedsMAC() && p.bootMACAddress == "" {
-		msg := fmt.Sprintf("BMC driver %s requires a BootMACAddress value", bmcAccess.Type())
-		p.log.Info(msg)
-		result, err = operationFailed(msg)
-		return result, "", err
-	}
-
-	driverInfo := bmcAccess.DriverInfo(p.bmcCreds)
-	driverInfo = setExternalURL(p, driverInfo)
-
-	// If we have not found a node yet, we need to create one
-	if ironicNode == nil {
-		p.log.Info("registering host in ironic")
-
-		if data.BootMode == metal3api.UEFISecureBoot && !bmcAccess.SupportsSecureBoot() {
-			msg := fmt.Sprintf("BMC driver %s does not support secure boot", bmcAccess.Type())
-			p.log.Info(msg)
-			result, err = operationFailed(msg)
-			return result, "", err
-		}
-
-		inspectInterface, driverErr := p.getInspectInterface(bmcAccess)
-		if driverErr != nil {
-			result, err = transientError(driverErr)
-			return result, "", 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,
-			ManagementInterface: bmcAccess.ManagementInterface(),
-			PowerInterface:      bmcAccess.PowerInterface(),
-			RAIDInterface:       bmcAccess.RAIDInterface(),
-			VendorInterface:     bmcAccess.VendorInterface(),
-			Properties: map[string]interface{}{
-				"capabilities": buildCapabilitiesValue(nil, data.BootMode),
-			},
-		}
-
-		if p.availableFeatures.HasFirmwareUpdates() {
-			nodeCreateOpts.FirmwareInterface = bmcAccess.FirmwareInterface()
-		}
-
-		ironicNode, err = nodes.Create(p.ctx, p.client, nodeCreateOpts).Extract()
-		if err == nil {
-			p.publisher("Registered", "Registered new host")
-		} else if gophercloud.ResponseCodeIs(err, 409) {
-			p.log.Info("could not register host in ironic, busy")
-			result, err = retryAfterDelay(provisionRequeueDelay)
-			return result, "", err
-		} else {
-			result, err = transientError(errors.Wrap(err, "failed to register host in ironic"))
-			return result, "", err
-		}
-
-		// Store the ID so other methods can assume it is set and so
-		// we can find the node again later.
-		provID = ironicNode.UUID
-
-		// If we know the MAC, create a port. Otherwise we will have
-		// to do this after we run the introspection step.
-		if p.bootMACAddress != "" {
-			err = p.createPXEEnabledNodePort(ironicNode.UUID, p.bootMACAddress)
-			if err != nil {
-				result, err = transientError(err)
-				return result, provID, err
-			}
-		}
-	} else {
-		// FIXME(dhellmann): At this point we have found an existing
-		// node in ironic by looking it up. We need to check its
-		// settings against what we have in the host, and change them
-		// if there are differences.
-		provID = ironicNode.UUID
-
-		updater.SetTopLevelOpt("name", ironicNodeName(p.objectMeta), ironicNode.Name)
-
-		var bmcAddressChanged bool
-		newAddress := make(map[string]interface{})
-		ironicAddress := make(map[string]interface{})
-		reg := regexp.MustCompile("_address$")
-		for key, value := range driverInfo {
-			if reg.MatchString(key) {
-				newAddress[key] = value
-				break
-			}
-		}
-		for key, value := range ironicNode.DriverInfo {
-			if reg.MatchString(key) {
-				ironicAddress[key] = value
-				break
-			}
-		}
-		if !reflect.DeepEqual(newAddress, ironicAddress) {
-			bmcAddressChanged = true
-		}
-
-		// When node exists but has no assigned port to it by Ironic and actuall address (MAC) is present
-		// in host config and is not allocated to different node lets try to create port for this node.
-		if p.bootMACAddress != "" {
-			var nodeHasAssignedPort, addressIsAllocatedToPort bool
-
-			nodeHasAssignedPort, err = p.nodeHasAssignedPort(ironicNode)
-			if err != nil {
-				result, err = transientError(err)
-				return result, provID, err
-			}
-
-			if !nodeHasAssignedPort {
-				addressIsAllocatedToPort, err = p.isAddressAllocatedToPort(p.bootMACAddress)
-				if err != nil {
-					result, err = transientError(err)
-					return result, provID, err
-				}
-
-				if !addressIsAllocatedToPort {
-					err = p.createPXEEnabledNodePort(ironicNode.UUID, p.bootMACAddress)
-					if err != nil {
-						result, err = transientError(err)
-						return result, provID, err
-					}
-				}
-			}
-		}
-
-		// The actual password is not returned from ironic, so we want to
-		// update the whole DriverInfo only if the credentials or BMC address
-		// has changed, otherwise we will be writing on every call to this
-		// function.
-		if credentialsChanged || bmcAddressChanged {
-			p.log.Info("Updating driver info because the credentials and/or the BMC address changed")
-			updater.SetTopLevelOpt("driver_info", driverInfo, ironicNode.DriverInfo)
-		}
-
-		// We don't return here because we also have to set the
-		// target provision state to manageable, which happens
-		// below.
-	}
-
-	// If no PreprovisioningImage builder is enabled we set the Node network_data
-	// this enables Ironic to inject the network_data into the ramdisk image
-	if !p.config.havePreprovImgBuilder {
-		networkDataRaw := data.PreprovisioningNetworkData
-		if networkDataRaw != "" {
-			var networkData map[string]interface{}
-			if yamlErr := yaml.Unmarshal([]byte(networkDataRaw), &networkData); yamlErr != nil {
-				p.log.Info("failed to unmarshal networkData from PreprovisioningNetworkData")
-				result, err = transientError(fmt.Errorf("invalid preprovisioningNetworkData: %w", yamlErr))
-				return result, provID, err
-			}
-			numUpdates := len(updater.Updates)
-			updater.SetTopLevelOpt("network_data", networkData, ironicNode.NetworkData)
-			if len(updater.Updates) != numUpdates {
-				p.log.Info("adding preprovisioning network_data for node", "node", ironicNode.UUID)
-			}
-		}
-	}
-
-	ironicNode, success, result, err := p.tryUpdateNode(ironicNode, updater)
-	if !success {
-		return result, provID, err
-	}
-
-	p.log.Info("current provision state",
-		"lastError", ironicNode.LastError,
-		"current", ironicNode.ProvisionState,
-		"target", ironicNode.TargetProvisionState,
-	)
-
-	// Ensure the node is marked manageable.
-	switch nodes.ProvisionState(ironicNode.ProvisionState) {
-	case nodes.Enroll:
-
-		// If ironic is reporting an error, stop working on the node.
-		if ironicNode.LastError != "" && !(credentialsChanged || restartOnFailure) {
-			result, err = operationFailed(ironicNode.LastError)
-			return result, provID, err
-		}
-
-		if ironicNode.TargetProvisionState == string(nodes.TargetManage) {
-			// We have already tried to manage the node and did not
-			// get an error, so do nothing and keep trying.
-			result, err = operationContinuing(provisionRequeueDelay)
-			return result, provID, err
-		}
-
-		result, err = p.changeNodeProvisionState(
-			ironicNode,
-			nodes.ProvisionStateOpts{Target: nodes.TargetManage},
-		)
-		return result, provID, err
-
-	case nodes.Verifying:
-		// If we're still waiting for the state to change in Ironic,
-		// return true to indicate that we're dirty and need to be
-		// reconciled again.
-		result, err = operationContinuing(provisionRequeueDelay)
-		return result, provID, err
-
-	case nodes.CleanWait,
-		nodes.Cleaning,
-		nodes.DeployWait,
-		nodes.Deploying,
-		nodes.Inspecting:
-		// Do not try to update the node if it's in a transient state other than InspectWait - will fail anyway.
-		result, err = operationComplete()
-		return result, provID, err
-
-	case nodes.Active:
-		// The host is already running, maybe it's a controlplane host?
-		p.debugLog.Info("have active host", "image_source", ironicNode.InstanceInfo["image_source"])
-		fallthrough
-
-	default:
-		result, err = p.configureImages(data, ironicNode, bmcAccess)
-		return result, provID, err
-	}
-}
-
 func (p *ironicProvisioner) configureImages(data provisioner.ManagementAccessData, ironicNode *nodes.Node, bmcAccess bmc.AccessDetails) (result provisioner.Result, err error) {
 	updater := clients.UpdateOptsBuilder(p.log)
 
diff --git a/pkg/provisioner/ironic/register.go b/pkg/provisioner/ironic/register.go
new file mode 100644
index 00000000..55601171
--- /dev/null
+++ b/pkg/provisioner/ironic/register.go
@@ -0,0 +1,287 @@
+package ironic
+
+import (
+	"fmt"
+	"reflect"
+	"regexp"
+
+	"github.com/gophercloud/gophercloud/v2"
+	"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/pkg/errors"
+	"sigs.k8s.io/yaml"
+)
+
+func bmcAddressMatches(ironicNode *nodes.Node, driverInfo map[string]interface{}) bool {
+	newAddress := make(map[string]interface{})
+	ironicAddress := make(map[string]interface{})
+	reg := regexp.MustCompile("_address$")
+	for key, value := range driverInfo {
+		if reg.MatchString(key) {
+			newAddress[key] = value
+			break
+		}
+	}
+	for key, value := range ironicNode.DriverInfo {
+		if reg.MatchString(key) {
+			ironicAddress[key] = value
+			break
+		}
+	}
+	return reflect.DeepEqual(newAddress, ironicAddress)
+}
+
+// Register registers the host in the internal database if it does not
+// exist, updates the existing host if needed, and tests the connection
+// information for the host to verify that the credentials work.
+// The credentialsChanged argument tells the provisioner whether the
+// current set of credentials it has are different from the credentials
+// it has previously been using, without implying that either set of
+// credentials is correct.
+func (p *ironicProvisioner) Register(data provisioner.ManagementAccessData, credentialsChanged, restartOnFailure bool) (result provisioner.Result, provID string, err error) {
+	bmcAccess, err := p.bmcAccess()
+	if err != nil {
+		result, err = operationFailed(err.Error())
+		return result, "", err
+	}
+
+	if data.BootMode == metal3api.UEFISecureBoot && !bmcAccess.SupportsSecureBoot() {
+		msg := fmt.Sprintf("BMC driver %s does not support secure boot", bmcAccess.Type())
+		p.log.Info(msg)
+		result, err = operationFailed(msg)
+		return result, "", err
+	}
+
+	var ironicNode *nodes.Node
+	updater := clients.UpdateOptsBuilder(p.log)
+
+	p.debugLog.Info("validating management access")
+
+	ironicNode, err = p.findExistingHost(p.bootMACAddress)
+	if err != nil {
+		switch err.(type) {
+		case macAddressConflictError:
+			result, err = operationFailed(err.Error())
+		default:
+			result, err = transientError(errors.Wrap(err, "failed to find existing host"))
+		}
+		return result, "", err
+	}
+
+	// Some BMC types require a MAC address, so ensure we have one
+	// when we need it. If not, place the host in an error state.
+	if bmcAccess.NeedsMAC() && p.bootMACAddress == "" {
+		msg := fmt.Sprintf("BMC driver %s requires a BootMACAddress value", bmcAccess.Type())
+		p.log.Info(msg)
+		result, err = operationFailed(msg)
+		return result, "", err
+	}
+
+	driverInfo := bmcAccess.DriverInfo(p.bmcCreds)
+	driverInfo = setExternalURL(p, driverInfo)
+
+	// If we have not found a node yet, we need to create one
+	if ironicNode == nil {
+		p.log.Info("registering host in ironic")
+		var retry bool
+		ironicNode, retry, err = p.enrollNode(data, bmcAccess, driverInfo)
+		if err != nil {
+			result, err = transientError(err)
+			return result, "", err
+		}
+		if retry {
+			result, err = retryAfterDelay(provisionRequeueDelay)
+			return result, "", err
+		}
+		// Store the ID so other methods can assume it is set and so
+		// we can find the node again later.
+		provID = ironicNode.UUID
+	} else {
+		// FIXME(dhellmann): At this point we have found an existing
+		// node in ironic by looking it up. We need to check its
+		// settings against what we have in the host, and change them
+		// if there are differences.
+		provID = ironicNode.UUID
+
+		updater.SetTopLevelOpt("name", ironicNodeName(p.objectMeta), ironicNode.Name)
+
+		// When node exists but has no assigned port to it by Ironic and actuall address (MAC) is present
+		// in host config and is not allocated to different node lets try to create port for this node.
+		if p.bootMACAddress != "" {
+			err = p.ensurePort(ironicNode)
+			if err != nil {
+				result, err = transientError(err)
+				return result, provID, err
+			}
+		}
+
+		bmcAddressChanged := !bmcAddressMatches(ironicNode, driverInfo)
+
+		// The actual password is not returned from ironic, so we want to
+		// update the whole DriverInfo only if the credentials or BMC address
+		// has changed, otherwise we will be writing on every call to this
+		// function.
+		if credentialsChanged || bmcAddressChanged {
+			p.log.Info("Updating driver info because the credentials and/or the BMC address changed")
+			updater.SetTopLevelOpt("driver_info", driverInfo, ironicNode.DriverInfo)
+		}
+
+		// We don't return here because we also have to set the
+		// target provision state to manageable, which happens
+		// below.
+	}
+
+	// If no PreprovisioningImage builder is enabled we set the Node network_data
+	// this enables Ironic to inject the network_data into the ramdisk image
+	if !p.config.havePreprovImgBuilder {
+		networkDataRaw := data.PreprovisioningNetworkData
+		if networkDataRaw != "" {
+			var networkData map[string]interface{}
+			if yamlErr := yaml.Unmarshal([]byte(networkDataRaw), &networkData); yamlErr != nil {
+				p.log.Info("failed to unmarshal networkData from PreprovisioningNetworkData")
+				result, err = transientError(fmt.Errorf("invalid preprovisioningNetworkData: %w", yamlErr))
+				return result, provID, err
+			}
+			numUpdates := len(updater.Updates)
+			updater.SetTopLevelOpt("network_data", networkData, ironicNode.NetworkData)
+			if len(updater.Updates) != numUpdates {
+				p.log.Info("adding preprovisioning network_data for node", "node", ironicNode.UUID)
+			}
+		}
+	}
+
+	ironicNode, success, result, err := p.tryUpdateNode(ironicNode, updater)
+	if !success {
+		return result, provID, err
+	}
+
+	p.log.Info("current provision state",
+		"lastError", ironicNode.LastError,
+		"current", ironicNode.ProvisionState,
+		"target", ironicNode.TargetProvisionState,
+	)
+
+	// Ensure the node is marked manageable.
+	switch nodes.ProvisionState(ironicNode.ProvisionState) {
+	case nodes.Enroll:
+
+		// If ironic is reporting an error, stop working on the node.
+		if ironicNode.LastError != "" && !(credentialsChanged || restartOnFailure) {
+			result, err = operationFailed(ironicNode.LastError)
+			return result, provID, err
+		}
+
+		if ironicNode.TargetProvisionState == string(nodes.TargetManage) {
+			// We have already tried to manage the node and did not
+			// get an error, so do nothing and keep trying.
+			result, err = operationContinuing(provisionRequeueDelay)
+			return result, provID, err
+		}
+
+		result, err = p.changeNodeProvisionState(
+			ironicNode,
+			nodes.ProvisionStateOpts{Target: nodes.TargetManage},
+		)
+		return result, provID, err
+
+	case nodes.Verifying:
+		// If we're still waiting for the state to change in Ironic,
+		// return true to indicate that we're dirty and need to be
+		// reconciled again.
+		result, err = operationContinuing(provisionRequeueDelay)
+		return result, provID, err
+
+	case nodes.CleanWait,
+		nodes.Cleaning,
+		nodes.DeployWait,
+		nodes.Deploying,
+		nodes.Inspecting:
+		// Do not try to update the node if it's in a transient state other than InspectWait - will fail anyway.
+		result, err = operationComplete()
+		return result, provID, err
+
+	case nodes.Active:
+		// The host is already running, maybe it's a controlplane host?
+		p.debugLog.Info("have active host", "image_source", ironicNode.InstanceInfo["image_source"])
+		fallthrough
+
+	default:
+		result, err = p.configureImages(data, ironicNode, bmcAccess)
+		return result, provID, err
+	}
+}
+
+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,
+		ManagementInterface: bmcAccess.ManagementInterface(),
+		PowerInterface:      bmcAccess.PowerInterface(),
+		RAIDInterface:       bmcAccess.RAIDInterface(),
+		VendorInterface:     bmcAccess.VendorInterface(),
+		Properties: map[string]interface{}{
+			"capabilities": buildCapabilitiesValue(nil, data.BootMode),
+		},
+	}
+
+	if p.availableFeatures.HasFirmwareUpdates() {
+		nodeCreateOpts.FirmwareInterface = bmcAccess.FirmwareInterface()
+	}
+
+	ironicNode, err = nodes.Create(p.ctx, p.client, nodeCreateOpts).Extract()
+	if err == nil {
+		p.publisher("Registered", "Registered new host")
+	} else if gophercloud.ResponseCodeIs(err, 409) {
+		p.log.Info("could not register host in ironic, busy")
+		return nil, true, nil
+	} else {
+		return nil, true, fmt.Errorf("failed to register host in ironic: %s", err)
+	}
+
+	// If we know the MAC, create a port. Otherwise we will have
+	// to do this after we run the introspection step.
+	if p.bootMACAddress != "" {
+		err = p.createPXEEnabledNodePort(ironicNode.UUID, p.bootMACAddress)
+		if err != nil {
+			return nil, true, err
+		}
+	}
+
+	return ironicNode, false, nil
+}
+
+func (p *ironicProvisioner) ensurePort(ironicNode *nodes.Node) error {
+	nodeHasAssignedPort, err := p.nodeHasAssignedPort(ironicNode)
+	if err != nil {
+		return err
+	}
+
+	if !nodeHasAssignedPort {
+		addressIsAllocatedToPort, err := p.isAddressAllocatedToPort(p.bootMACAddress)
+		if err != nil {
+			return err
+		}
+
+		if !addressIsAllocatedToPort {
+			err = p.createPXEEnabledNodePort(ironicNode.UUID, p.bootMACAddress)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
diff --git a/pkg/provisioner/provisioner.go b/pkg/provisioner/provisioner.go
index 80b9bf16..c19f970c 100644
--- a/pkg/provisioner/provisioner.go
+++ b/pkg/provisioner/provisioner.go
@@ -129,13 +129,14 @@ type HTTPHeaders []map[string]string
 // Provisioner holds the state information for talking to the
 // provisioning backend.
 type Provisioner interface {
-	// ValidateManagementAccess tests the connection information for
-	// the host to verify that the location and credentials work. The
-	// boolean argument tells the provisioner whether the current set
-	// of credentials it has are different from the credentials it has
-	// previously been using, without implying that either set of
+	// Register registers the host in the internal database if it does not
+	// exist, updates the existing host if needed, and tests the connection
+	// information for the host to verify that the credentials work.
+	// The credentialsChanged argument tells the provisioner whether the
+	// current set of credentials it has are different from the credentials
+	// it has previously been using, without implying that either set of
 	// credentials is correct.
-	ValidateManagementAccess(data ManagementAccessData, credentialsChanged, restartOnFailure bool) (result Result, provID string, err error)
+	Register(data ManagementAccessData, credentialsChanged, restartOnFailure bool) (result Result, provID string, err error)
 
 	// PreprovisioningImageFormats returns a list of acceptable formats for a
 	// pre-provisioning image to be built by a PreprovisioningImage object. The


Test patch:
diff --git a/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go
index 2e70b08d..d96104a2 100644
--- a/controllers/metal3.io/host_state_machine_test.go
+++ b/controllers/metal3.io/host_state_machine_test.go
@@ -1329,7 +1329,7 @@ func (m *mockProvisioner) calledNoError(methodName string) bool {
 	return m.callsNoError[methodName]
 }
 
-func (m *mockProvisioner) ValidateManagementAccess(_ provisioner.ManagementAccessData, _, _ bool) (result provisioner.Result, provID string, err error) {
+func (m *mockProvisioner) Register(_ provisioner.ManagementAccessData, _, _ bool) (result provisioner.Result, provID string, err error) {
 	return m.getNextResultByMethod("ValidateManagementAccess"), "", err
 }
 
diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/register_test.go
similarity index 89%
rename from pkg/provisioner/ironic/validatemanagementaccess_test.go
rename to pkg/provisioner/ironic/register_test.go
index cac8a189..ae0252c8 100644
--- a/pkg/provisioner/ironic/validatemanagementaccess_test.go
+++ b/pkg/provisioner/ironic/register_test.go
@@ -15,7 +15,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestValidateManagementAccessNoMAC(t *testing.T) {
+func TestRegisterNoMAC(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// requires one.
 	host := makeHost()
@@ -33,14 +33,14 @@ func TestValidateManagementAccessNoMAC(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, _, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Contains(t, result.ErrorMessage, "requires a BootMACAddress")
 }
 
-func TestValidateManagementAccessMACOptional(t *testing.T) {
+func TestRegisterMACOptional(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// does not require one.
 	host := makeHost()
@@ -64,14 +64,14 @@ func TestValidateManagementAccessMACOptional(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, _, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 }
 
-func TestValidateManagementAccessCreateNodeNoImage(t *testing.T) {
+func TestRegisterCreateNodeNoImage(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// does not require one.
 	host := makeHost()
@@ -96,9 +96,9 @@ func TestValidateManagementAccessCreateNodeNoImage(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.NotEqual(t, "", createdNode.UUID)
@@ -107,7 +107,7 @@ func TestValidateManagementAccessCreateNodeNoImage(t *testing.T) {
 	assert.Equal(t, "agent", createdNode.InspectInterface)
 }
 
-func TestValidateManagementAccessCreateNodeOldInspection(t *testing.T) {
+func TestRegisterCreateNodeOldInspection(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// does not require one.
 	host := makeHost()
@@ -135,9 +135,9 @@ func TestValidateManagementAccessCreateNodeOldInspection(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.NotEqual(t, "", createdNode.UUID)
@@ -145,7 +145,7 @@ func TestValidateManagementAccessCreateNodeOldInspection(t *testing.T) {
 	assert.Equal(t, "inspector", createdNode.InspectInterface)
 }
 
-func TestValidateManagementAccessCreateWithImage(t *testing.T) {
+func TestRegisterCreateWithImage(t *testing.T) {
 	// Create a host with Image specified in the Spec
 	host := makeHost()
 	host.Status.Provisioning.ID = "" // so we don't lookup by uuid
@@ -170,9 +170,9 @@ func TestValidateManagementAccessCreateWithImage(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{CurrentImage: host.Spec.Image.DeepCopy()}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{CurrentImage: host.Spec.Image.DeepCopy()}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.Equal(t, createdNode.UUID, provID)
@@ -184,7 +184,7 @@ func TestValidateManagementAccessCreateWithImage(t *testing.T) {
 	assert.Contains(t, updates, host.Spec.Image.Checksum)
 }
 
-func TestValidateManagementAccessCreateWithLiveIso(t *testing.T) {
+func TestRegisterCreateWithLiveIso(t *testing.T) {
 	// Create a host with Image specified in the Spec
 	host := makeHostLiveIso()
 	host.Status.Provisioning.ID = "" // so we don't lookup by uuid
@@ -206,9 +206,9 @@ func TestValidateManagementAccessCreateWithLiveIso(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{CurrentImage: host.Spec.Image.DeepCopy()}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{CurrentImage: host.Spec.Image.DeepCopy()}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.Equal(t, createdNode.UUID, provID)
@@ -218,7 +218,7 @@ func TestValidateManagementAccessCreateWithLiveIso(t *testing.T) {
 	assert.Contains(t, updates, host.Spec.Image.URL)
 }
 
-func TestValidateManagementAccessExistingNode(t *testing.T) {
+func TestRegisterExistingNode(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// does not require one.
 	host := makeHost()
@@ -245,15 +245,15 @@ func TestValidateManagementAccessExistingNode(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.Equal(t, "uuid", provID)
 }
 
-func TestValidateManagementAccessExistingNodeNameUpdate(t *testing.T) {
+func TestRegisterExistingNodeNameUpdate(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// does not require one.
 	host := makeHost()
@@ -280,15 +280,15 @@ func TestValidateManagementAccessExistingNodeNameUpdate(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, _, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.Equal(t, false, result.Dirty)
 }
 
-func TestValidateManagementAccessExistingNodeContinue(t *testing.T) {
+func TestRegisterExistingNodeContinue(t *testing.T) {
 	statuses := []nodes.ProvisionState{
 		nodes.Manageable,
 		nodes.Available,
@@ -353,9 +353,9 @@ func TestValidateManagementAccessExistingNodeContinue(t *testing.T) {
 				t.Fatalf("could not create provisioner: %s", err)
 			}
 
-			result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+			result, _, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 			if err != nil {
-				t.Fatalf("error from ValidateManagementAccess: %s", err)
+				t.Fatalf("error from Register: %s", err)
 			}
 			assert.Equal(t, "", result.ErrorMessage)
 			assert.Equal(t, false, result.Dirty)
@@ -364,7 +364,7 @@ func TestValidateManagementAccessExistingNodeContinue(t *testing.T) {
 	}
 }
 
-func TestValidateManagementAccessExistingSteadyStateNoUpdate(t *testing.T) {
+func TestRegisterExistingSteadyStateNoUpdate(t *testing.T) {
 	liveFormat := "live-iso"
 	imageTypes := []struct {
 		DeployInterface string
@@ -533,9 +533,9 @@ func TestValidateManagementAccessExistingSteadyStateNoUpdate(t *testing.T) {
 			}
 
 			data := provisioner.ManagementAccessData{CurrentImage: imageType.Image, HasCustomDeploy: imageType.HasCustomDeploy}
-			result, _, err := prov.ValidateManagementAccess(data, false, false)
+			result, _, err := prov.Register(data, false, false)
 			if err != nil {
-				t.Fatalf("error from ValidateManagementAccess: %s", err)
+				t.Fatalf("error from Register: %s", err)
 			}
 			assert.Equal(t, "", result.ErrorMessage)
 			assert.Equal(t, false, result.Dirty)
@@ -544,7 +544,7 @@ func TestValidateManagementAccessExistingSteadyStateNoUpdate(t *testing.T) {
 	}
 }
 
-func TestValidateManagementAccessExistingNodeWaiting(t *testing.T) {
+func TestRegisterExistingNodeWaiting(t *testing.T) {
 	statuses := []nodes.ProvisionState{
 		nodes.Enroll,
 		nodes.Verifying,
@@ -589,9 +589,9 @@ func TestValidateManagementAccessExistingNodeWaiting(t *testing.T) {
 				t.Fatalf("could not create provisioner: %s", err)
 			}
 
-			result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+			result, _, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 			if err != nil {
-				t.Fatalf("error from ValidateManagementAccess: %s", err)
+				t.Fatalf("error from Register: %s", err)
 			}
 			assert.Equal(t, "", result.ErrorMessage)
 
@@ -613,7 +613,7 @@ func TestValidateManagementAccessExistingNodeWaiting(t *testing.T) {
 	}
 }
 
-func TestValidateManagementAccessNewCredentials(t *testing.T) {
+func TestRegisterNewCredentials(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// does not require one.
 	host := makeHost()
@@ -645,9 +645,9 @@ func TestValidateManagementAccessNewCredentials(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, true, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, true, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.Equal(t, "uuid", provID)
@@ -658,7 +658,7 @@ func TestValidateManagementAccessNewCredentials(t *testing.T) {
 	assert.Equal(t, "test.bmc", newValues["test_address"])
 }
 
-func TestValidateManagementAccessLinkExistingIronicNodeByMAC(t *testing.T) {
+func TestRegisterLinkExistingIronicNodeByMAC(t *testing.T) {
 	// Create an Ironic node, and then create a host with a matching MAC
 	// Test to see if the node was found, and if the link is made
 
@@ -692,17 +692,17 @@ func TestValidateManagementAccessLinkExistingIronicNodeByMAC(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.NotEqual(t, "", provID)
 }
 
-func TestValidateManagementAccessExistingPortWithWrongUUID(t *testing.T) {
+func TestRegisterExistingPortWithWrongUUID(t *testing.T) {
 	// Create a node, and a port.  The port has a node uuid that doesn't match the node.
-	// ValidateManagementAccess should return an error.
+	// Register should return an error.
 
 	existingNode := nodes.Node{
 		UUID: "33ce8659-7400-4c68-9535-d10766f07a58",
@@ -734,17 +734,17 @@ func TestValidateManagementAccessExistingPortWithWrongUUID(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	_, _, err = prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	_, _, err = prov.Register(provisioner.ManagementAccessData{}, false, false)
 	expected := "failed to find existing host: port 11:11:11:11:11:11 exists but linked node random-wrong-id doesn't:"
 	assert.ErrorContains(t, err, expected)
 }
 
-func TestValidateManagementAccessExistingPortButHasName(t *testing.T) {
+func TestRegisterExistingPortButHasName(t *testing.T) {
 	// Create a node, and a port.
 	// The port is linked to the node.
 	// The port address matches the BMH BootMACAddress.
 	// The node has a name, and the name doesn't match the BMH.
-	// ValidateManagementAccess should return an error.
+	// Register should return an error.
 
 	existingNode := nodes.Node{
 		UUID: "33ce8659-7400-4c68-9535-d10766f07a58",
@@ -776,12 +776,12 @@ func TestValidateManagementAccessExistingPortButHasName(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	res, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	res, _, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	assert.Nil(t, err)
 	assert.Equal(t, res.ErrorMessage, "MAC address 11:11:11:11:11:11 conflicts with existing node wrong-name")
 }
 
-func TestValidateManagementAccessAddTwoHostsWithSameMAC(t *testing.T) {
+func TestRegisterAddTwoHostsWithSameMAC(t *testing.T) {
 	existingNode := nodes.Node{
 		UUID: "33ce8659-7400-4c68-9535-d10766f07a58",
 		Name: "myns" + nameSeparator + "myhost",
@@ -814,15 +814,15 @@ func TestValidateManagementAccessAddTwoHostsWithSameMAC(t *testing.T) {
 	}
 
 	// MAC address value is different than the port that actually exists
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.NotEqual(t, "", provID)
 }
 
-func TestValidateManagementAccessUnsupportedSecureBoot(t *testing.T) {
+func TestRegisterUnsupportedSecureBoot(t *testing.T) {
 	// Create a host without a bootMACAddress and with a BMC that
 	// requires one.
 	host := makeHost()
@@ -838,14 +838,14 @@ func TestValidateManagementAccessUnsupportedSecureBoot(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{BootMode: metal3api.UEFISecureBoot}, false, false)
+	result, _, err := prov.Register(provisioner.ManagementAccessData{BootMode: metal3api.UEFISecureBoot}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Contains(t, result.ErrorMessage, "does not support secure boot")
 }
 
-func TestValidateManagementAccessNoBMCDetails(t *testing.T) {
+func TestRegisterNoBMCDetails(t *testing.T) {
 	ironic := testserver.NewIronic(t)
 	ironic.Start()
 	defer ironic.Stop()
@@ -859,14 +859,14 @@ func TestValidateManagementAccessNoBMCDetails(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, _, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "failed to parse BMC address information: missing BMC address", result.ErrorMessage)
 }
 
-func TestValidateManagementAccessMalformedBMCAddress(t *testing.T) {
+func TestRegisterMalformedBMCAddress(t *testing.T) {
 	ironic := testserver.NewIronic(t)
 	ironic.Start()
 	defer ironic.Stop()
@@ -882,14 +882,14 @@ func TestValidateManagementAccessMalformedBMCAddress(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, _, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "failed to parse BMC address information: failed to parse BMC address information: parse \"<ipmi://192.168.122.1:6233>\": first path segment in URL cannot contain colon", result.ErrorMessage)
 }
 
-func TestValidateManagementAccessUpdateBMCAddressIP(t *testing.T) {
+func TestRegisterUpdateBMCAddressIP(t *testing.T) {
 	host := makeHost()
 	host.Spec.BMC.Address = "ipmi://192.168.122.10:623"
 	host.Status.Provisioning.ID = "uuid"
@@ -930,9 +930,9 @@ func TestValidateManagementAccessUpdateBMCAddressIP(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.Equal(t, "uuid", provID)
@@ -943,7 +943,7 @@ func TestValidateManagementAccessUpdateBMCAddressIP(t *testing.T) {
 	assert.Equal(t, "192.168.122.10", newValues["ipmi_address"])
 }
 
-func TestValidateManagementAccessUpdateBMCAddressProtocol(t *testing.T) {
+func TestRegisterUpdateBMCAddressProtocol(t *testing.T) {
 	host := makeHost()
 	host.Spec.BMC.Address = "redfish://192.168.122.1:623"
 	host.Spec.BootMACAddress = "11:11:11:11:11:11"
@@ -991,9 +991,9 @@ func TestValidateManagementAccessUpdateBMCAddressProtocol(t *testing.T) {
 		t.Fatalf("could not create provisioner: %s", err)
 	}
 
-	result, provID, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
+	result, provID, err := prov.Register(provisioner.ManagementAccessData{}, false, false)
 	if err != nil {
-		t.Fatalf("error from ValidateManagementAccess: %s", err)
+		t.Fatalf("error from Register: %s", err)
 	}
 	assert.Equal(t, "", result.ErrorMessage)
 	assert.Equal(t, "uuid", provID)