diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go index c264980acd..46e0c880d7 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/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go index 2e70b08d57..d96104a207 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/demo/demo.go b/pkg/provisioner/demo/demo.go index d4e8acd453..4f436cb547 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 c8e9ab3ecc..51fa446e39 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 4446e2cdda..18f08cedec 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 0000000000..5560117129 --- /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/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 cac8a18982..ae0252c85d 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 \"\": 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) diff --git a/pkg/provisioner/provisioner.go b/pkg/provisioner/provisioner.go index 80b9bf1639..c19f970c25 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