Repository: metal3-io/baremetal-operator
Pull request #866 (2021-04-27 06:43:48)

Fix some typos
Signed-off-by: Zou Yu <zouy.fnst@cn.fujitsu.com>


Patch:
diff --git a/.travis.yml b/.travis.yml
index d90312f3..4e900c34 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -10,7 +10,5 @@ go:
 
 env:
   - TARGET=docker
-  - TARGET=unit-verbose
-  - TARGET=lint
-  
+
 script: make $TARGET
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types.go b/apis/metal3.io/v1alpha1/baremetalhost_types.go
index 28a1de88..abf72416 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_types.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_types.go
@@ -149,7 +149,7 @@ const (
 	// PreparationError is an error condition occurring when do
 	// cleaning steps failed.
 	PreparationError ErrorType = "preparation error"
-	// ProvisioningError is an error condition occuring when the controller
+	// ProvisioningError is an error condition occurring when the controller
 	// fails to provision or deprovision the Host.
 	ProvisioningError ErrorType = "provisioning error"
 	// PowerManagementError is an error condition occurring when the
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index 243e6eaf..75bcf635 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -403,7 +403,7 @@ func (p *ironicProvisioner) ValidateManagementAccess(data provisioner.Management
 	}
 
 	var ironicNode *nodes.Node
-	updater := updateOptsBuilder(p.debugLog)
+	var updates nodes.UpdateOpts
 
 	p.debugLog.Info("validating management access")
 
@@ -433,6 +433,8 @@ func (p *ironicProvisioner) ValidateManagementAccess(data provisioner.Management
 	driverInfo["deploy_kernel"] = deployKernelURL
 	driverInfo["deploy_ramdisk"] = deployRamdiskURL
 
+	result, err = operationComplete()
+
 	// If we have not found a node yet, we need to create one
 	if ironicNode == nil {
 		p.log.Info("registering host in ironic")
@@ -497,29 +499,72 @@ func (p *ironicProvisioner) ValidateManagementAccess(data provisioner.Management
 		// if there are differences.
 		provID = ironicNode.UUID
 
-		updater.SetTopLevelOpt("name", ironicNodeName(p.objectMeta), ironicNode.Name)
+		if ironicNode.Name != ironicNodeName(p.objectMeta) {
+			updates = append(
+				updates,
+				nodes.UpdateOperation{
+					Op:    nodes.ReplaceOp,
+					Path:  "/name",
+					Value: ironicNodeName(p.objectMeta),
+				},
+			)
+		}
 
 		// Look for the case where we previously enrolled this node
 		// and now the credentials have changed.
 		if credentialsChanged {
-			updater.SetTopLevelOpt("driver_info", driverInfo, nil)
+			updates = append(
+				updates,
+				nodes.UpdateOperation{
+					Op:    nodes.ReplaceOp,
+					Path:  "/driver_info",
+					Value: driverInfo,
+				},
+			)
+			// We don't return here because we also have to set the
+			// target provision state to manageable, which happens
+			// below.
 		}
-
-		// We don't return here because we also have to set the
-		// target provision state to manageable, which happens
-		// below.
 	}
 	if data.CurrentImage != nil {
-		p.getImageUpdateOptsForNode(ironicNode, data.CurrentImage, data.BootMode, updater)
+		updatesImage, optsErr := p.getImageUpdateOptsForNode(ironicNode, data.CurrentImage, data.BootMode)
+		if optsErr != nil {
+			result, err = transientError(errors.Wrap(optsErr, "Could not get Image options for node"))
+			return
+		}
+		if len(updatesImage) != 0 {
+			updates = append(updates, updatesImage...)
+		}
 	}
-	updater.SetTopLevelOpt("automated_clean",
-		data.AutomatedCleaningMode != metal3v1alpha1.CleaningModeDisabled,
-		ironicNode.AutomatedClean)
-
-	var success bool
-	success, result, err = p.tryUpdateNode(ironicNode, updater)
-	if !success {
-		return
+	if ironicNode.AutomatedClean == nil ||
+		(data.AutomatedCleaningMode == metal3v1alpha1.CleaningModeDisabled && *ironicNode.AutomatedClean) ||
+		(data.AutomatedCleaningMode == metal3v1alpha1.CleaningModeMetadata && !*ironicNode.AutomatedClean) {
+		p.log.Info("setting automated cleaning mode to",
+			"ID", ironicNode.UUID,
+			"mode", data.AutomatedCleaningMode)
+
+		value := data.AutomatedCleaningMode != metal3v1alpha1.CleaningModeDisabled
+		updates = append(
+			updates,
+			nodes.UpdateOperation{
+				Op:    nodes.ReplaceOp,
+				Path:  "/automated_clean",
+				Value: value,
+			},
+		)
+	}
+	if len(updates) != 0 {
+		_, err = nodes.Update(p.client, ironicNode.UUID, updates).Extract()
+		switch err.(type) {
+		case nil:
+		case gophercloud.ErrDefault409:
+			p.log.Info("could not update host driver settings, busy")
+			result, err = retryAfterDelay(provisionRequeueDelay)
+			return
+		default:
+			result, err = transientError(errors.Wrap(err, "failed to update host settings in ironic"))
+			return
+		}
 	}
 	// ironicNode, err = nodes.Get(p.client, p.status.ID).Extract()
 	// if err != nil {
@@ -531,8 +576,6 @@ func (p *ironicProvisioner) ValidateManagementAccess(data provisioner.Management
 		"target", ironicNode.TargetProvisionState,
 	)
 
-	result, err = operationComplete()
-
 	// Ensure the node is marked manageable.
 	switch nodes.ProvisionState(ironicNode.ProvisionState) {
 
@@ -582,26 +625,6 @@ func (p *ironicProvisioner) ValidateManagementAccess(data provisioner.Management
 	}
 }
 
-func (p *ironicProvisioner) tryUpdateNode(ironicNode *nodes.Node, updater *nodeUpdater) (success bool, result provisioner.Result, err error) {
-	if len(updater.Updates) == 0 {
-		success = true
-		return
-	}
-
-	p.log.Info("updating node settings in ironic")
-	_, err = nodes.Update(p.client, ironicNode.UUID, updater.Updates).Extract()
-	switch err.(type) {
-	case nil:
-		success = true
-	case gophercloud.ErrDefault409:
-		p.log.Info("could not update node settings in ironic, busy")
-		result, err = retryAfterDelay(provisionRequeueDelay)
-	default:
-		result, err = transientError(errors.Wrap(err, "failed to update host settings in ironic"))
-	}
-	return
-}
-
 func (p *ironicProvisioner) tryChangeNodeProvisionState(ironicNode *nodes.Node, opts nodes.ProvisionStateOpts) (success bool, result provisioner.Result, err error) {
 	p.log.Info("changing provisioning state",
 		"current", ironicNode.ProvisionState,
@@ -665,19 +688,29 @@ func (p *ironicProvisioner) InspectHardware(data provisioner.InspectData, force,
 				}
 				fallthrough
 			default:
-				var success bool
-				success, result, err = p.tryUpdateNode(
-					ironicNode,
-					updateOptsBuilder(p.debugLog).
-						SetPropertiesOpts(optionsData{
-							"capabilities": buildCapabilitiesValue(ironicNode, data.BootMode),
-						}, ironicNode),
-				)
-				if !success {
+				p.log.Info("updating boot mode before hardware inspection")
+				op, value := buildCapabilitiesValue(ironicNode, data.BootMode)
+				updates := nodes.UpdateOpts{
+					nodes.UpdateOperation{
+						Op:    op,
+						Path:  "/properties/capabilities",
+						Value: value,
+					},
+				}
+				_, err = nodes.Update(p.client, ironicNode.UUID, updates).Extract()
+				switch err.(type) {
+				case nil:
+				case gophercloud.ErrDefault409:
+					p.log.Info("could not update host settings in ironic, busy")
+					result, err = retryAfterDelay(provisionRequeueDelay)
+					return
+				default:
+					result, err = transientError(errors.Wrap(err, "failed to update host boot mode settings in ironic"))
 					return
 				}
 
 				p.log.Info("starting new hardware inspection")
+				var success bool
 				success, result, err = p.tryChangeNodeProvisionState(
 					ironicNode,
 					nodes.ProvisionStateOpts{Target: nodes.TargetInspect},
@@ -742,99 +775,297 @@ func (p *ironicProvisioner) UpdateHardwareState() (hwState provisioner.HardwareS
 	return
 }
 
-func (p *ironicProvisioner) setLiveIsoUpdateOptsForNode(ironicNode *nodes.Node, imageData *metal3v1alpha1.Image, updater *nodeUpdater) {
-	optValues := optionsData{
-		"boot_iso": imageData.URL,
+func (p *ironicProvisioner) setLiveIsoUpdateOptsForNode(ironicNode *nodes.Node, imageData *metal3v1alpha1.Image, updates nodes.UpdateOpts) (nodes.UpdateOpts, error) {
+	var op nodes.UpdateOp
 
-		// remove any image_source or checksum options
-		"image_source":        nil,
-		"image_os_hash_value": nil,
-		"image_os_hash_algo":  nil,
-		"image_checksum":      nil,
+	if _, ok := ironicNode.InstanceInfo["boot_iso"]; !ok {
+		op = nodes.AddOp
+		p.log.Info("adding boot_iso")
+	} else {
+		op = nodes.ReplaceOp
+		p.log.Info("updating boot_iso")
+	}
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    op,
+			Path:  "/instance_info/boot_iso",
+			Value: imageData.URL,
+		},
+	)
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    nodes.ReplaceOp,
+			Path:  "/deploy_interface",
+			Value: "ramdisk",
+		},
+	)
+	// remove any image_source or checksum options
+	removals := []string{
+		"image_source", "image_os_hash_value", "image_os_hash_algo", "image_checksum"}
+	op = nodes.RemoveOp
+	for _, item := range removals {
+		if _, ok := ironicNode.InstanceInfo[item]; ok {
+			p.log.Info("removing " + item)
+			updates = append(
+				updates,
+				nodes.UpdateOperation{
+					Op:   op,
+					Path: "/instance_info/" + item,
+				},
+			)
+		}
 	}
-	updater.
-		SetInstanceInfoOpts(optValues, ironicNode).
-		SetTopLevelOpt("deploy_interface", "ramdisk", ironicNode.DeployInterface)
+	return updates, nil
 }
 
-func (p *ironicProvisioner) setDirectDeployUpdateOptsForNode(ironicNode *nodes.Node, imageData *metal3v1alpha1.Image, updater *nodeUpdater) {
+func (p *ironicProvisioner) setDirectDeployUpdateOptsForNode(ironicNode *nodes.Node, imageData *metal3v1alpha1.Image, updates nodes.UpdateOpts) (nodes.UpdateOpts, error) {
 	checksum, checksumType, ok := imageData.GetChecksum()
 	if !ok {
 		p.log.Info("image/checksum not found for host")
-		return
+		return updates, nil
+	}
+
+	var op nodes.UpdateOp
+
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    nodes.ReplaceOp,
+			Path:  "/deploy_interface",
+			Value: "direct",
+		},
+	)
+	// Remove any boot_iso field
+	if _, ok := ironicNode.InstanceInfo["boot_iso"]; ok {
+		p.log.Info("removing boot_iso")
+		updates = append(
+			updates,
+			nodes.UpdateOperation{
+				Op:   nodes.RemoveOp,
+				Path: "/instance_info/boot_iso",
+			},
+		)
 	}
+	// image_source
+	if _, ok := ironicNode.InstanceInfo["image_source"]; !ok {
+		op = nodes.AddOp
+		p.log.Info("adding image_source")
+	} else {
+		op = nodes.ReplaceOp
+		p.log.Info("updating image_source")
+	}
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    op,
+			Path:  "/instance_info/image_source",
+			Value: imageData.URL,
+		},
+	)
+
+	// image_os_hash_algo
+	if _, ok := ironicNode.InstanceInfo["image_os_hash_algo"]; !ok {
+		op = nodes.AddOp
+		p.log.Info("adding image_os_hash_algo")
+	} else {
+		op = nodes.ReplaceOp
+		p.log.Info("updating image_os_hash_algo")
+	}
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    op,
+			Path:  "/instance_info/image_os_hash_algo",
+			Value: checksumType,
+		},
+	)
 
+	// image_os_hash_value
+	if _, ok := ironicNode.InstanceInfo["image_os_hash_value"]; !ok {
+		op = nodes.AddOp
+		p.log.Info("adding image_os_hash_value")
+	} else {
+		op = nodes.ReplaceOp
+		p.log.Info("updating image_os_hash_value")
+	}
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    op,
+			Path:  "/instance_info/image_os_hash_value",
+			Value: checksum,
+		},
+	)
+
+	// image_checksum
+	//
 	// FIXME: For older versions of ironic that do not have
 	// https://review.opendev.org/#/c/711816/ failing to include the
 	// 'image_checksum' causes ironic to refuse to provision the
 	// image, even if the other hash value parameters are given. We
 	// only want to do that for MD5, however, because those versions
 	// of ironic only support MD5 checksums.
-	var legacyChecksum *string
 	if checksumType == string(metal3v1alpha1.MD5) {
-		legacyChecksum = &checksum
+		if _, ok := ironicNode.InstanceInfo["image_checksum"]; !ok {
+			op = nodes.AddOp
+			p.log.Info("adding image_checksum")
+		} else {
+			op = nodes.ReplaceOp
+			p.log.Info("updating image_checksum")
+		}
+		updates = append(
+			updates,
+			nodes.UpdateOperation{
+				Op:    op,
+				Path:  "/instance_info/image_checksum",
+				Value: checksum,
+			},
+		)
 	}
 
-	optValues := optionsData{
-		// Remove any boot_iso field
-		"boot_iso": nil,
-
-		"image_source":        imageData.URL,
-		"image_os_hash_algo":  checksumType,
-		"image_os_hash_value": checksum,
-		"image_checksum":      legacyChecksum,
-		"image_disk_format":   imageData.DiskFormat,
+	if imageData.DiskFormat != nil {
+		updates = append(updates, nodes.UpdateOperation{
+			Op:    nodes.AddOp,
+			Path:  "/instance_info/image_disk_format",
+			Value: *imageData.DiskFormat,
+		})
 	}
-	updater.
-		SetInstanceInfoOpts(optValues, ironicNode).
-		SetTopLevelOpt("deploy_interface", "direct", ironicNode.DeployInterface)
+
+	return updates, nil
 }
 
-func (p *ironicProvisioner) getImageUpdateOptsForNode(ironicNode *nodes.Node, imageData *metal3v1alpha1.Image, bootMode metal3v1alpha1.BootMode, updater *nodeUpdater) {
+func (p *ironicProvisioner) getImageUpdateOptsForNode(ironicNode *nodes.Node, imageData *metal3v1alpha1.Image, bootMode metal3v1alpha1.BootMode) (updates nodes.UpdateOpts, err error) {
 	// instance_uuid
-	updater.SetTopLevelOpt("instance_uuid", string(p.objectMeta.UID), ironicNode.InstanceUUID)
+	p.log.Info("setting instance_uuid")
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    nodes.ReplaceOp,
+			Path:  "/instance_uuid",
+			Value: string(p.objectMeta.UID),
+		},
+	)
 
 	// Secure boot is a normal capability that goes into instance_info (we
 	// also put it to properties for consistency, although it's not
 	// strictly required in our case).
 
-	// Instance info capabilities were invented later and
-	// use a normal JSON mapping instead of a custom
-	// string value.
-	capabilitiesII := map[string]string{}
 	if bootMode == metal3v1alpha1.UEFISecureBoot {
-		capabilitiesII["secure_boot"] = "true"
+		updates = append(updates, nodes.UpdateOperation{
+			Op:   nodes.AddOp,
+			Path: "/instance_info/capabilities",
+			// Instance info capabilities were invented later and
+			// use a normal JSON mapping instead of a custom
+			// string value.
+			Value: map[string]string{
+				"secure_boot": "true",
+			},
+		})
+	} else {
+		updates = append(updates, nodes.UpdateOperation{
+			Op:    nodes.AddOp,
+			Path:  "/instance_info/capabilities",
+			Value: map[string]string{},
+		})
 	}
 
-	updater.SetInstanceInfoOpts(optionsData{"capabilities": capabilitiesII}, ironicNode)
-
+	// Set live-iso format options
 	if imageData.DiskFormat != nil && *imageData.DiskFormat == "live-iso" {
-		// Set live-iso format options
-		p.setLiveIsoUpdateOptsForNode(ironicNode, imageData, updater)
-	} else {
-		// Set deploy_interface direct options when not booting a live-iso
-		p.setDirectDeployUpdateOptsForNode(ironicNode, imageData, updater)
+		return p.setLiveIsoUpdateOptsForNode(ironicNode, imageData, updates)
 	}
-}
-
-func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node, data provisioner.ProvisionData) *nodeUpdater {
-	updater := updateOptsBuilder(p.debugLog)
 
-	p.getImageUpdateOptsForNode(ironicNode, &data.Image, data.BootMode, updater)
+	// Set deploy_interface direct options when not booting a live-iso
+	return p.setDirectDeployUpdateOptsForNode(ironicNode, imageData, updates)
+}
 
-	opts := optionsData{
-		"root_device": devicehints.MakeHintMap(data.RootDeviceHints),
+func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node, data provisioner.ProvisionData) (updates nodes.UpdateOpts, err error) {
+	imageOpts, err := p.getImageUpdateOptsForNode(ironicNode, &data.Image, data.BootMode)
+	if err != nil {
+		return updates, errors.Wrap(err, "Could not get Image options for node")
+	}
+	updates = append(updates, imageOpts...)
+
+	// root_device
+	//
+	// FIXME(dhellmann): We need to specify the root device to receive
+	// the image. That should come from some combination of inspecting
+	// the host to see what is available and the hardware profile to
+	// give us instructions.
+	var op nodes.UpdateOp
+	if _, ok := ironicNode.Properties["root_device"]; !ok {
+		op = nodes.AddOp
+		p.log.Info("adding root_device")
+	} else {
+		op = nodes.ReplaceOp
+		p.log.Info("updating root_device")
+	}
+
+	// hints
+	//
+	// If the user has provided explicit root device hints, they take
+	// precedence. Otherwise use the values from the hardware profile.
+	hints := devicehints.MakeHintMap(data.RootDeviceHints)
+	p.log.Info("using root device", "hints", hints)
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    op,
+			Path:  "/properties/root_device",
+			Value: hints,
+		},
+	)
 
-		// FIXME(dhellmann): This should come from inspecting the host.
-		"cpu_arch": data.HardwareProfile.CPUArch,
+	// cpu_arch
+	//
+	// FIXME(dhellmann): This should come from inspecting the
+	// host.
+	if _, ok := ironicNode.Properties["cpu_arch"]; !ok {
+		op = nodes.AddOp
+		p.log.Info("adding cpu_arch")
+	} else {
+		op = nodes.ReplaceOp
+		p.log.Info("updating cpu_arch")
+	}
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    op,
+			Path:  "/properties/cpu_arch",
+			Value: data.HardwareProfile.CPUArch,
+		},
+	)
 
-		"local_gb": data.HardwareProfile.LocalGB,
+	// local_gb
+	if _, ok := ironicNode.Properties["local_gb"]; !ok {
+		op = nodes.AddOp
+		p.log.Info("adding local_gb")
+	} else {
+		op = nodes.ReplaceOp
+		p.log.Info("updating local_gb")
+	}
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    op,
+			Path:  "/properties/local_gb",
+			Value: data.HardwareProfile.LocalGB,
+		},
+	)
 
-		"capabilities": buildCapabilitiesValue(ironicNode, data.BootMode),
-	}
-	updater.SetPropertiesOpts(opts, ironicNode)
+	// boot_mode
+	op, value := buildCapabilitiesValue(ironicNode, data.BootMode)
+	updates = append(
+		updates,
+		nodes.UpdateOperation{
+			Op:    op,
+			Path:  "/properties/capabilities",
+			Value: value,
+		},
+	)
 
-	return updater
+	return updates, nil
 }
 
 // We can't just replace the capabilities because we need to keep the
@@ -843,19 +1074,23 @@ func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node, data pr
 // look at the existing value and modify it. This function
 // encapsulates the logic for building the value and knowing which
 // update operation to use with the results.
-func buildCapabilitiesValue(ironicNode *nodes.Node, bootMode metal3v1alpha1.BootMode) string {
+func buildCapabilitiesValue(ironicNode *nodes.Node, bootMode metal3v1alpha1.BootMode) (op nodes.UpdateOp, value string) {
 
 	capabilities, ok := ironicNode.Properties["capabilities"]
 	if !ok {
 		// There is no existing capabilities value
-		return bootModeCapabilities[bootMode]
+		return nodes.AddOp, bootModeCapabilities[bootMode]
 	}
 	existingCapabilities := capabilities.(string)
 
+	// The capabilities value is set, so we will want to replace it.
+	op = nodes.ReplaceOp
+
 	if existingCapabilities == "" {
 		// The existing value is empty so we can replace the whole
 		// thing.
-		return bootModeCapabilities[bootMode]
+		value = bootModeCapabilities[bootMode]
+		return
 	}
 
 	var filteredCapabilities []string
@@ -866,17 +1101,26 @@ func buildCapabilitiesValue(ironicNode *nodes.Node, bootMode metal3v1alpha1.Boot
 	}
 	filteredCapabilities = append(filteredCapabilities, bootModeCapabilities[bootMode])
 
-	return strings.Join(filteredCapabilities, ",")
+	value = strings.Join(filteredCapabilities, ",")
+	return
 }
 
 func (p *ironicProvisioner) setUpForProvisioning(ironicNode *nodes.Node, data provisioner.ProvisionData) (result provisioner.Result, err error) {
 
 	p.log.Info("starting provisioning", "node properties", ironicNode.Properties)
 
-	success, result, err := p.tryUpdateNode(ironicNode,
-		p.getUpdateOptsForNode(ironicNode, data))
-	if !success {
-		return
+	updates, err := p.getUpdateOptsForNode(ironicNode, data)
+	if err != nil {
+		return transientError(errors.Wrap(err, "failed to update opts for node"))
+	}
+	_, err = nodes.Update(p.client, ironicNode.UUID, updates).Extract()
+	switch err.(type) {
+	case nil:
+	case gophercloud.ErrDefault409:
+		p.log.Info("could not update host settings in ironic, busy")
+		return retryAfterDelay(provisionRequeueDelay)
+	default:
+		return transientError(errors.Wrap(err, "failed to update host settings in ironic"))
 	}
 
 	p.log.Info("validating host settings")
@@ -1244,13 +1488,24 @@ func (p *ironicProvisioner) Provision(data provisioner.ProvisionData) (result pr
 }
 
 func (p *ironicProvisioner) setMaintenanceFlag(ironicNode *nodes.Node, value bool) (result provisioner.Result, err error) {
-	success, result, err := p.tryUpdateNode(ironicNode,
-		updateOptsBuilder(p.log).SetTopLevelOpt("maintenance", value, nil))
-	if err != nil {
-		err = fmt.Errorf("failed to set host maintenance flag to %v (%w)", value, err)
-	}
-	if !success {
-		return
+	_, err = nodes.Update(
+		p.client,
+		ironicNode.UUID,
+		nodes.UpdateOpts{
+			nodes.UpdateOperation{
+				Op:    nodes.ReplaceOp,
+				Path:  "/maintenance",
+				Value: value,
+			},
+		},
+	).Extract()
+	switch err.(type) {
+	case nil:
+	case gophercloud.ErrDefault409:
+		p.log.Info("could not set host maintenance flag, busy")
+		return retryAfterDelay(provisionRequeueDelay)
+	default:
+		return transientError(errors.Wrap(err, "failed to set host maintenance flag"))
 	}
 	return operationContinuing(0)
 }
diff --git a/pkg/provisioner/ironic/update_opts.go b/pkg/provisioner/ironic/update_opts.go
deleted file mode 100644
index 4cc5f4cc..00000000
--- a/pkg/provisioner/ironic/update_opts.go
+++ /dev/null
@@ -1,162 +0,0 @@
-package ironic
-
-import (
-	"fmt"
-	"reflect"
-
-	"github.com/go-logr/logr"
-
-	"github.com/gophercloud/gophercloud/openstack/baremetal/v1/nodes"
-)
-
-type optionsData map[string]interface{}
-
-func optionValueEqual(current, value interface{}) bool {
-	if reflect.DeepEqual(current, value) {
-		return true
-	}
-	switch curVal := current.(type) {
-	case []interface{}:
-		// newType could reasonably be either []interface{} or e.g. []string,
-		// so we must use reflection.
-		newType := reflect.TypeOf(value)
-		switch newType.Kind() {
-		case reflect.Slice, reflect.Array:
-		default:
-			return false
-		}
-		newList := reflect.ValueOf(value)
-		if newList.Len() != len(curVal) {
-			return false
-		}
-		for i, v := range curVal {
-			if !optionValueEqual(newList.Index(i).Interface(), v) {
-				return false
-			}
-		}
-		return true
-	case map[string]interface{}:
-		// newType could reasonably be either map[string]interface{} or
-		// e.g. map[string]string, so we must use reflection.
-		newType := reflect.TypeOf(value)
-		if newType.Kind() != reflect.Map ||
-			newType.Key().Kind() != reflect.String {
-			return false
-		}
-		newMap := reflect.ValueOf(value)
-		if newMap.Len() != len(curVal) {
-			return false
-		}
-		for k, v := range curVal {
-			newV := newMap.MapIndex(reflect.ValueOf(k))
-			if !(newV.IsValid() && optionValueEqual(newV.Interface(), v)) {
-				return false
-			}
-		}
-		return true
-	}
-	return false
-}
-
-func deref(v interface{}) interface{} {
-	if v == nil {
-		return nil
-	}
-	if reflect.TypeOf(v).Kind() != reflect.Ptr {
-		return v
-	}
-	ptrVal := reflect.ValueOf(v)
-	if ptrVal.IsNil() {
-		return nil
-	}
-	return ptrVal.Elem().Interface()
-}
-
-func getUpdateOperation(name string, currentData map[string]interface{}, desiredValue interface{}, path string, log logr.Logger) *nodes.UpdateOperation {
-	current, present := currentData[name]
-
-	desiredValue = deref(desiredValue)
-	if desiredValue != nil {
-		if !(present && optionValueEqual(deref(current), desiredValue)) {
-			if log != nil {
-				if present {
-					log.Info("updating option data",
-						"value", desiredValue, "old_value", current)
-				} else {
-					log.Info("adding option data",
-						"value", desiredValue)
-				}
-			}
-			return &nodes.UpdateOperation{
-				Op:    nodes.AddOp, // Add also does replace
-				Path:  path,
-				Value: desiredValue,
-			}
-		}
-	} else {
-		if present {
-			if log != nil {
-				log.Info("removing option data")
-			}
-			return &nodes.UpdateOperation{
-				Op:   nodes.RemoveOp,
-				Path: path,
-			}
-		}
-	}
-	return nil
-}
-
-type nodeUpdater struct {
-	Updates nodes.UpdateOpts
-	log     logr.Logger
-}
-
-func updateOptsBuilder(logger logr.Logger) *nodeUpdater {
-	return &nodeUpdater{
-		log: logger,
-	}
-}
-
-func (nu *nodeUpdater) logger(basepath, option string) logr.Logger {
-	if nu.log == nil {
-		return nil
-	}
-	log := nu.log.WithValues("option", option)
-	if basepath != "" {
-		log = log.WithValues("section", basepath[1:])
-	}
-	return log
-}
-
-func (nu *nodeUpdater) path(basepath, option string) string {
-	return fmt.Sprintf("%s/%s", basepath, option)
-}
-
-func (nu *nodeUpdater) setSectionUpdateOpts(currentData map[string]interface{}, settings optionsData, basepath string) {
-	for name, desiredValue := range settings {
-		updateOp := getUpdateOperation(name, currentData, desiredValue,
-			nu.path(basepath, name), nu.logger(basepath, name))
-		if updateOp != nil {
-			nu.Updates = append(nu.Updates, *updateOp)
-		}
-	}
-}
-
-func (nu *nodeUpdater) SetTopLevelOpt(name string, desiredValue, currentValue interface{}) *nodeUpdater {
-	currentData := map[string]interface{}{name: currentValue}
-	desiredData := optionsData{name: desiredValue}
-
-	nu.setSectionUpdateOpts(currentData, desiredData, "")
-	return nu
-}
-
-func (nu *nodeUpdater) SetPropertiesOpts(settings optionsData, node *nodes.Node) *nodeUpdater {
-	nu.setSectionUpdateOpts(node.Properties, settings, "/properties")
-	return nu
-}
-
-func (nu *nodeUpdater) SetInstanceInfoOpts(settings optionsData, node *nodes.Node) *nodeUpdater {
-	nu.setSectionUpdateOpts(node.InstanceInfo, settings, "/instance_info")
-	return nu
-}
diff --git a/tools/run_local_ironic.sh b/tools/run_local_ironic.sh
index 1749e49b..61a81259 100755
--- a/tools/run_local_ironic.sh
+++ b/tools/run_local_ironic.sh
@@ -8,7 +8,6 @@ IRONIC_IMAGE=${IRONIC_IMAGE:-"quay.io/metal3-io/ironic:master"}
 IRONIC_INSPECTOR_IMAGE=${IRONIC_INSPECTOR_IMAGE:-"quay.io/metal3-io/ironic-inspector"}
 IRONIC_KEEPALIVED_IMAGE=${IRONIC_KEEPALIVED_IMAGE:-"quay.io/metal3-io/keepalived"}
 IPA_DOWNLOADER_IMAGE=${IPA_DOWNLOADER_IMAGE:-"quay.io/metal3-io/ironic-ipa-downloader:master"}
-IPA_BASEURI=${IPA_BASEURI:-}
 IRONIC_DATA_DIR=${IRONIC_DATA_DIR:-"/opt/metal3-dev-env/ironic"}
 CONTAINER_RUNTIME="${CONTAINER_RUNTIME:-podman}"
 HTTP_PORT="6180"
@@ -77,7 +76,6 @@ IRONIC_FAST_TRACK=${IRONIC_FAST_TRACK}
 IRONIC_KERNEL_PARAMS=${IRONIC_KERNEL_PARAMS}
 INSPECTOR_REVERSE_PROXY_SETUP=${INSPECTOR_REVERSE_PROXY_SETUP}
 IRONIC_INSPECTOR_VLAN_INTERFACES=${IRONIC_INSPECTOR_VLAN_INTERFACES}
-IPA_BASEURI=${IPA_BASEURI}
 EOF
 
 sudo "${CONTAINER_RUNTIME}" pull "$IRONIC_IMAGE"


Test patch:
diff --git a/pkg/provisioner/ironic/capabilities_test.go b/pkg/provisioner/ironic/capabilities_test.go
index 79df9727..2a772a61 100644
--- a/pkg/provisioner/ironic/capabilities_test.go
+++ b/pkg/provisioner/ironic/capabilities_test.go
@@ -15,12 +15,14 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 		Node          nodes.Node
 		Mode          metal3v1alpha1.BootMode
 		ExpectedValue string
+		ExpectedOp    nodes.UpdateOp
 	}{
 		{
 			Scenario:      "unset",
 			Node:          nodes.Node{},
 			Mode:          metal3v1alpha1.UEFI,
 			ExpectedValue: "boot_mode:uefi",
+			ExpectedOp:    nodes.AddOp,
 		},
 		{
 			Scenario: "empty",
@@ -31,6 +33,7 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.UEFI,
 			ExpectedValue: "boot_mode:uefi",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 		{
 			Scenario: "not-there",
@@ -41,6 +44,7 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.UEFI,
 			ExpectedValue: "cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,boot_mode:uefi",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 		{
 			Scenario: "add-secure-boot",
@@ -51,6 +55,7 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.UEFISecureBoot,
 			ExpectedValue: "cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,boot_mode:uefi,secure_boot:true",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 		{
 			Scenario: "uefi-to-uefi",
@@ -61,6 +66,7 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.UEFI,
 			ExpectedValue: "cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,boot_mode:uefi",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 		{
 			Scenario: "bios-to-bios",
@@ -71,6 +77,7 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.Legacy,
 			ExpectedValue: "cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,boot_mode:bios",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 		{
 			Scenario: "bios-to-uefi",
@@ -81,6 +88,7 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.UEFI,
 			ExpectedValue: "cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,boot_mode:uefi",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 		{
 			Scenario: "uefi-to-bios",
@@ -91,6 +99,7 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.Legacy,
 			ExpectedValue: "cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,boot_mode:bios",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 		{
 			Scenario: "uefi-to-secure",
@@ -101,6 +110,7 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.UEFISecureBoot,
 			ExpectedValue: "cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,boot_mode:uefi,secure_boot:true",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 		{
 			Scenario: "secure-to-uefi",
@@ -111,12 +121,14 @@ func TestBuildCapabilitiesValue(t *testing.T) {
 			},
 			Mode:          metal3v1alpha1.UEFI,
 			ExpectedValue: "cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,boot_mode:uefi",
+			ExpectedOp:    nodes.ReplaceOp,
 		},
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.Scenario, func(t *testing.T) {
-			actualVal := buildCapabilitiesValue(&tc.Node, tc.Mode)
+			actualOp, actualVal := buildCapabilitiesValue(&tc.Node, tc.Mode)
+			assert.Equal(t, tc.ExpectedOp, actualOp)
 			assert.Equal(t, tc.ExpectedValue, actualVal)
 		})
 	}
diff --git a/pkg/provisioner/ironic/delete_test.go b/pkg/provisioner/ironic/delete_test.go
index 87e3e201..0b212efa 100644
--- a/pkg/provisioner/ironic/delete_test.go
+++ b/pkg/provisioner/ironic/delete_test.go
@@ -149,7 +149,7 @@ func deleteTest(t *testing.T, detach bool) {
 			expectedDirty:        true,
 			expectedRequestAfter: 0,
 			expectedUpdate: &nodes.UpdateOperation{
-				Op:    nodes.AddOp,
+				Op:    "replace",
 				Path:  "/maintenance",
 				Value: true,
 			},
diff --git a/pkg/provisioner/ironic/testserver/server.go b/pkg/provisioner/ironic/testserver/server.go
index 4e35f1ee..db09b498 100644
--- a/pkg/provisioner/ironic/testserver/server.go
+++ b/pkg/provisioner/ironic/testserver/server.go
@@ -201,7 +201,7 @@ func (m *MockServer) GetLastRequestFor(pattern string, method string) (string, b
 
 // AddDefaultResponse adds a default response for the specified pattern/method.
 // It is possible to use variables in the pattern using curly braces, ie `/v1/nodes/{id}/power`
-// Pattern variables can be reused in the payload, so that they will be substitued with the actual value when sending the response
+// Pattern variables can be reused in the payload, so that they will be substituted with the actual value when sending the response
 // If httpMethod is empty, the response will be applied for any method
 func (m *MockServer) AddDefaultResponse(patternWithVars string, httpMethod string, code int, payload string) *MockServer {
 
diff --git a/pkg/provisioner/ironic/updatehardwarestate_test.go b/pkg/provisioner/ironic/updatehardwarestate_test.go
index 8aac4457..a54b92a9 100644
--- a/pkg/provisioner/ironic/updatehardwarestate_test.go
+++ b/pkg/provisioner/ironic/updatehardwarestate_test.go
@@ -29,7 +29,7 @@ func TestUpdateHardwareState(t *testing.T) {
 		expectedError   string
 	}{
 		{
-			name: "unkown-power-state",
+			name: "unknown-power-state",
 			ironic: testserver.NewIronic(t).Ready().Node(nodes.Node{
 				UUID: nodeUUID,
 			}),
diff --git a/pkg/provisioner/ironic/updateopts_test.go b/pkg/provisioner/ironic/updateopts_test.go
index d5def408..57c119bd 100644
--- a/pkg/provisioner/ironic/updateopts_test.go
+++ b/pkg/provisioner/ironic/updateopts_test.go
@@ -17,437 +17,6 @@ import (
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/clients"
 )
 
-func TestOptionValueEqual(t *testing.T) {
-	cases := []struct {
-		Name   string
-		Before interface{}
-		After  interface{}
-		Equal  bool
-	}{
-		{
-			Name:   "nil interface",
-			Before: nil,
-			After:  "foo",
-			Equal:  false,
-		},
-		{
-			Name:   "equal string",
-			Before: "foo",
-			After:  "foo",
-			Equal:  true,
-		},
-		{
-			Name:   "unequal string",
-			Before: "foo",
-			After:  "bar",
-			Equal:  false,
-		},
-		{
-			Name:   "equal true",
-			Before: true,
-			After:  true,
-			Equal:  true,
-		},
-		{
-			Name:   "equal false",
-			Before: false,
-			After:  false,
-			Equal:  true,
-		},
-		{
-			Name:   "unequal true",
-			Before: true,
-			After:  false,
-			Equal:  false,
-		},
-		{
-			Name:   "unequal false",
-			Before: false,
-			After:  true,
-			Equal:  false,
-		},
-		{
-			Name:   "equal int",
-			Before: 42,
-			After:  42,
-			Equal:  true,
-		},
-		{
-			Name:   "unequal int",
-			Before: 27,
-			After:  42,
-			Equal:  false,
-		},
-		{
-			Name:   "string int",
-			Before: "42",
-			After:  42,
-			Equal:  false,
-		},
-		{
-			Name:   "int string",
-			Before: 42,
-			After:  "42",
-			Equal:  false,
-		},
-		{
-			Name:   "bool int",
-			Before: false,
-			After:  0,
-			Equal:  false,
-		},
-		{
-			Name:   "int bool",
-			Before: 1,
-			After:  true,
-			Equal:  false,
-		},
-		{
-			Name:   "string map",
-			Before: "foo",
-			After:  map[string]string{"foo": "foo"},
-			Equal:  false,
-		},
-		{
-			Name:   "map string",
-			Before: map[string]interface{}{"foo": "foo"},
-			After:  "foo",
-			Equal:  false,
-		},
-		{
-			Name:   "string list",
-			Before: "foo",
-			After:  []string{"foo"},
-			Equal:  false,
-		},
-		{
-			Name:   "list string",
-			Before: []string{"foo"},
-			After:  "foo",
-			Equal:  false,
-		},
-		{
-			Name:   "map list",
-			Before: map[string]interface{}{"foo": "foo"},
-			After:  []string{"foo"},
-			Equal:  false,
-		},
-		{
-			Name:   "list map",
-			Before: []string{"foo"},
-			After:  map[string]string{"foo": "foo"},
-			Equal:  false,
-		},
-		{
-			Name:   "equal map string-typed",
-			Before: map[string]interface{}{"foo": "bar"},
-			After:  map[string]string{"foo": "bar"},
-			Equal:  true,
-		},
-		{
-			Name:   "unequal map string-typed",
-			Before: map[string]interface{}{"foo": "bar"},
-			After:  map[string]string{"foo": "baz"},
-			Equal:  false,
-		},
-		{
-			Name:   "equal map int-typed",
-			Before: map[string]interface{}{"foo": 42},
-			After:  map[string]int{"foo": 42},
-			Equal:  true,
-		},
-		{
-			Name:   "unequal map int-typed",
-			Before: map[string]interface{}{"foo": "bar"},
-			After:  map[string]int{"foo": 42},
-			Equal:  false,
-		},
-		{
-			Name:   "equal map",
-			Before: map[string]interface{}{"foo": "bar", "42": 42},
-			After:  map[string]interface{}{"foo": "bar", "42": 42},
-			Equal:  true,
-		},
-		{
-			Name:   "unequal map",
-			Before: map[string]interface{}{"foo": "bar", "42": 42},
-			After:  map[string]interface{}{"foo": "bar", "42": 27},
-			Equal:  false,
-		},
-		{
-			Name:   "equal map empty string",
-			Before: map[string]interface{}{"foo": ""},
-			After:  map[string]interface{}{"foo": ""},
-			Equal:  true,
-		},
-		{
-			Name:   "unequal map replace empty string",
-			Before: map[string]interface{}{"foo": ""},
-			After:  map[string]interface{}{"foo": "bar"},
-			Equal:  false,
-		},
-		{
-			Name:   "unequal map replace with empty string",
-			Before: map[string]interface{}{"foo": "bar"},
-			After:  map[string]interface{}{"foo": ""},
-			Equal:  false,
-		},
-		{
-			Name:   "shorter map",
-			Before: map[string]interface{}{"foo": "bar", "42": 42},
-			After:  map[string]interface{}{"foo": "bar"},
-			Equal:  false,
-		},
-		{
-			Name:   "longer map",
-			Before: map[string]interface{}{"foo": "bar"},
-			After:  map[string]interface{}{"foo": "bar", "42": 42},
-			Equal:  false,
-		},
-		{
-			Name:   "different map",
-			Before: map[string]interface{}{"foo": "bar"},
-			After:  map[string]interface{}{"baz": "bar"},
-			Equal:  false,
-		},
-		{
-			Name:   "equal list string-typed",
-			Before: []interface{}{"foo", "bar"},
-			After:  []string{"foo", "bar"},
-			Equal:  true,
-		},
-		{
-			Name:   "unequal list string-typed",
-			Before: []interface{}{"foo", "bar"},
-			After:  []string{"foo", "baz"},
-			Equal:  false,
-		},
-		{
-			Name:   "equal list",
-			Before: []interface{}{"foo", 42},
-			After:  []interface{}{"foo", 42},
-			Equal:  true,
-		},
-		{
-			Name:   "unequal list",
-			Before: []interface{}{"foo", 42},
-			After:  []interface{}{"foo", 27},
-			Equal:  false,
-		},
-		{
-			Name:   "shorter list",
-			Before: []interface{}{"foo", 42},
-			After:  []interface{}{"foo"},
-			Equal:  false,
-		},
-		{
-			Name:   "longer list",
-			Before: []interface{}{"foo"},
-			After:  []interface{}{"foo", 42},
-			Equal:  false,
-		},
-	}
-
-	for _, c := range cases {
-		t.Run(c.Name, func(t *testing.T) {
-			var ne = "="
-			if c.Equal {
-				ne = "!"
-			}
-			assert.Equal(t, c.Equal, optionValueEqual(c.Before, c.After),
-				fmt.Sprintf("%v %s= %v", c.Before, ne, c.After))
-		})
-	}
-}
-
-func TestGetUpdateOperation(t *testing.T) {
-	var nilp *string
-	barExist := "bar"
-	bar := "bar"
-	baz := "baz"
-	existingData := map[string]interface{}{
-		"foo":  "bar",
-		"foop": &barExist,
-		"nil":  nilp,
-	}
-	cases := []struct {
-		Name          string
-		Field         string
-		NewValue      interface{}
-		ExpectedOp    nodes.UpdateOp
-		ExpectedValue string
-	}{
-		{
-			Name:       "add value",
-			Field:      "baz",
-			NewValue:   "quux",
-			ExpectedOp: nodes.AddOp,
-		},
-		{
-			Name:          "add value pointer",
-			Field:         "baz",
-			NewValue:      &bar,
-			ExpectedValue: bar,
-			ExpectedOp:    nodes.AddOp,
-		},
-		{
-			Name:          "add pointer value pointer",
-			Field:         "nil",
-			NewValue:      &bar,
-			ExpectedValue: bar,
-			ExpectedOp:    nodes.AddOp,
-		},
-		{
-			Name:     "keep value",
-			Field:    "foo",
-			NewValue: "bar",
-		},
-		{
-			Name:     "keep pointer value",
-			Field:    "foop",
-			NewValue: "bar",
-		},
-		{
-			Name:     "keep value pointer",
-			Field:    "foo",
-			NewValue: &bar,
-		},
-		{
-			Name:     "keep pointer value pointer",
-			Field:    "foop",
-			NewValue: &bar,
-		},
-		{
-			Name:       "change value",
-			Field:      "foo",
-			NewValue:   "baz",
-			ExpectedOp: nodes.AddOp,
-		},
-		{
-			Name:       "change pointer value",
-			Field:      "foop",
-			NewValue:   "baz",
-			ExpectedOp: nodes.AddOp,
-		},
-		{
-			Name:          "change value pointer",
-			Field:         "foo",
-			NewValue:      &baz,
-			ExpectedValue: baz,
-			ExpectedOp:    nodes.AddOp,
-		},
-		{
-			Name:          "change pointer value pointer",
-			Field:         "foop",
-			NewValue:      &baz,
-			ExpectedValue: baz,
-			ExpectedOp:    nodes.AddOp,
-		},
-		{
-			Name:       "delete value",
-			Field:      "foo",
-			NewValue:   nil,
-			ExpectedOp: nodes.RemoveOp,
-		},
-		{
-			Name:       "delete value pointer",
-			Field:      "foo",
-			NewValue:   nilp,
-			ExpectedOp: nodes.RemoveOp,
-		},
-		{
-			Name:     "nonexistent value",
-			Field:    "bar",
-			NewValue: nil,
-		},
-	}
-
-	for _, c := range cases {
-		t.Run(c.Name, func(t *testing.T) {
-			path := fmt.Sprintf("test/%s", c.Field)
-			updateOp := getUpdateOperation(
-				c.Field, existingData,
-				c.NewValue,
-				path, nil)
-
-			switch c.ExpectedOp {
-			case nodes.AddOp, nodes.ReplaceOp:
-				assert.NotNil(t, updateOp)
-				ev := c.ExpectedValue
-				if ev == "" {
-					ev = c.NewValue.(string)
-				}
-				assert.Equal(t, c.ExpectedOp, updateOp.Op)
-				assert.Equal(t, ev, updateOp.Value)
-				assert.Equal(t, path, updateOp.Path)
-			case nodes.RemoveOp:
-				assert.NotNil(t, updateOp)
-				assert.Equal(t, c.ExpectedOp, updateOp.Op)
-				assert.Equal(t, path, updateOp.Path)
-			default:
-				assert.Nil(t, updateOp)
-			}
-		})
-	}
-}
-
-func TestTopLevelUpdateOpt(t *testing.T) {
-	u := updateOptsBuilder(log)
-	u.SetTopLevelOpt("foo", "baz", "bar")
-	ops := u.Updates
-	assert.Len(t, ops, 1)
-	op := ops[0].(nodes.UpdateOperation)
-	assert.Equal(t, nodes.AddOp, op.Op)
-	assert.Equal(t, "baz", op.Value)
-	assert.Equal(t, "/foo", op.Path)
-
-	u = updateOptsBuilder(log)
-	u.SetTopLevelOpt("foo", "bar", "bar")
-	assert.Len(t, u.Updates, 0)
-}
-
-func TestPropertiesUpdateOpts(t *testing.T) {
-	newValues := optionsData{
-		"foo": "bar",
-		"baz": "quux",
-	}
-	node := nodes.Node{
-		Properties: map[string]interface{}{
-			"foo": "bar",
-		},
-	}
-
-	u := updateOptsBuilder(log)
-	u.SetPropertiesOpts(newValues, &node)
-	ops := u.Updates
-	assert.Len(t, ops, 1)
-	op := ops[0].(nodes.UpdateOperation)
-	assert.Equal(t, nodes.AddOp, op.Op)
-	assert.Equal(t, "quux", op.Value)
-	assert.Equal(t, "/properties/baz", op.Path)
-}
-
-func TestInstanceInfoUpdateOpts(t *testing.T) {
-	newValues := optionsData{
-		"foo": "bar",
-		"baz": "quux",
-	}
-	node := nodes.Node{
-		InstanceInfo: map[string]interface{}{
-			"foo": "bar",
-		},
-	}
-
-	u := updateOptsBuilder(log)
-	u.SetInstanceInfoOpts(newValues, &node)
-	ops := u.Updates
-	assert.Len(t, ops, 1)
-	op := ops[0].(nodes.UpdateOperation)
-	assert.Equal(t, nodes.AddOp, op.Op)
-	assert.Equal(t, "quux", op.Value)
-	assert.Equal(t, "/instance_info/baz", op.Path)
-}
-
 func TestGetUpdateOptsForNodeWithRootHints(t *testing.T) {
 
 	eventPublisher := func(reason, message string) {}
@@ -467,7 +36,10 @@ func TestGetUpdateOptsForNodeWithRootHints(t *testing.T) {
 		BootMode:        metal3v1alpha1.DefaultBootMode,
 		RootDeviceHints: host.Status.Provisioning.RootDeviceHints,
 	}
-	patches := prov.getUpdateOptsForNode(ironicNode, provData).Updates
+	patches, err := prov.getUpdateOptsForNode(ironicNode, provData)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Logf("patches: %v", patches)
 
@@ -563,7 +135,10 @@ func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
 		BootMode:        metal3v1alpha1.DefaultBootMode,
 		HardwareProfile: hwProf,
 	}
-	patches := prov.getUpdateOptsForNode(ironicNode, provData).Updates
+	patches, err := prov.getUpdateOptsForNode(ironicNode, provData)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Logf("patches: %v", patches)
 
@@ -670,7 +245,10 @@ func TestGetUpdateOptsForNodeDell(t *testing.T) {
 		BootMode:        metal3v1alpha1.DefaultBootMode,
 		HardwareProfile: hwProf,
 	}
-	patches := prov.getUpdateOptsForNode(ironicNode, provData).Updates
+	patches, err := prov.getUpdateOptsForNode(ironicNode, provData)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Logf("patches: %v", patches)
 
@@ -742,7 +320,10 @@ func TestGetUpdateOptsForNodeLiveIso(t *testing.T) {
 		Image:    *host.Spec.Image,
 		BootMode: metal3v1alpha1.DefaultBootMode,
 	}
-	patches := prov.getUpdateOptsForNode(ironicNode, provData).Updates
+	patches, err := prov.getUpdateOptsForNode(ironicNode, provData)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Logf("patches: %v", patches)
 
@@ -764,7 +345,7 @@ func TestGetUpdateOptsForNodeLiveIso(t *testing.T) {
 		{
 			Path:  "/deploy_interface",
 			Value: "ramdisk",
-			Op:    nodes.AddOp,
+			Op:    nodes.ReplaceOp,
 		},
 	}
 
@@ -811,7 +392,10 @@ func TestGetUpdateOptsForNodeImageToLiveIso(t *testing.T) {
 		Image:    *host.Spec.Image,
 		BootMode: metal3v1alpha1.DefaultBootMode,
 	}
-	patches := prov.getUpdateOptsForNode(ironicNode, provData).Updates
+	patches, err := prov.getUpdateOptsForNode(ironicNode, provData)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Logf("patches: %v", patches)
 
@@ -829,7 +413,7 @@ func TestGetUpdateOptsForNodeImageToLiveIso(t *testing.T) {
 		{
 			Path:  "/deploy_interface",
 			Value: "ramdisk",
-			Op:    nodes.AddOp,
+			Op:    nodes.ReplaceOp,
 		},
 		{
 			Path: "/instance_info/image_source",
@@ -890,7 +474,10 @@ func TestGetUpdateOptsForNodeLiveIsoToImage(t *testing.T) {
 		Image:    *host.Spec.Image,
 		BootMode: metal3v1alpha1.DefaultBootMode,
 	}
-	patches := prov.getUpdateOptsForNode(ironicNode, provData).Updates
+	patches, err := prov.getUpdateOptsForNode(ironicNode, provData)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Logf("patches: %v", patches)
 
@@ -907,7 +494,7 @@ func TestGetUpdateOptsForNodeLiveIsoToImage(t *testing.T) {
 		{
 			Path:  "/deploy_interface",
 			Value: "direct",
-			Op:    nodes.AddOp,
+			Op:    nodes.ReplaceOp,
 		},
 		{
 			Path:  "/instance_info/image_source",
@@ -992,7 +579,10 @@ func TestGetUpdateOptsForNodeSecureBoot(t *testing.T) {
 		BootMode:        metal3v1alpha1.UEFISecureBoot,
 		HardwareProfile: hwProf,
 	}
-	patches := prov.getUpdateOptsForNode(ironicNode, provData).Updates
+	patches, err := prov.getUpdateOptsForNode(ironicNode, provData)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	t.Logf("patches: %v", patches)
 
diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/validatemanagementaccess_test.go
index b037f376..b9242d73 100644
--- a/pkg/provisioner/ironic/validatemanagementaccess_test.go
+++ b/pkg/provisioner/ironic/validatemanagementaccess_test.go
@@ -287,7 +287,6 @@ func TestValidateManagementAccessExistingNodeContinue(t *testing.T) {
 		nodes.Rescuing,
 		nodes.UnrescueFail,
 	}
-	clean := true
 
 	for _, status := range statuses {
 		t.Run(string(status), func(t *testing.T) {
@@ -305,7 +304,6 @@ func TestValidateManagementAccessExistingNodeContinue(t *testing.T) {
 				Name:           host.Namespace + nameSeparator + host.Name,
 				UUID:           "uuid", // to match status in host
 				ProvisionState: string(status),
-				AutomatedClean: &clean,
 			}).NodeUpdate(nodes.Node{
 				UUID: "uuid",
 			})
@@ -326,93 +324,6 @@ func TestValidateManagementAccessExistingNodeContinue(t *testing.T) {
 			}
 			assert.Equal(t, "", result.ErrorMessage)
 			assert.Equal(t, false, result.Dirty)
-			assert.Len(t, ironic.GetLastNodeUpdateRequestFor("uuid"), 0)
-		})
-	}
-}
-
-func TestValidateManagementAccessExistingSteadyStateNoUpdate(t *testing.T) {
-	liveFormat := "live-iso"
-	imageTypes := []struct {
-		DeployInterface string
-		Image           *metal3v1alpha1.Image
-		InstanceInfo    map[string]interface{}
-	}{
-		{
-			DeployInterface: "",
-			InstanceInfo: map[string]interface{}{
-				"capabilities": map[string]interface{}{},
-			},
-		},
-		{
-			DeployInterface: "direct",
-			Image: &metal3v1alpha1.Image{
-				URL:      "theimage",
-				Checksum: "thechecksum",
-			},
-			InstanceInfo: map[string]interface{}{
-				"image_source":        "theimage",
-				"image_os_hash_algo":  "md5",
-				"image_os_hash_value": "thechecksum",
-				"image_checksum":      "thechecksum",
-				"capabilities":        map[string]interface{}{},
-			},
-		},
-		{
-			DeployInterface: "ramdisk",
-			Image: &metal3v1alpha1.Image{
-				URL:        "theimage",
-				DiskFormat: &liveFormat,
-			},
-			InstanceInfo: map[string]interface{}{
-				"boot_iso":     "theimage",
-				"capabilities": map[string]interface{}{},
-			},
-		},
-	}
-	clean := true
-
-	for _, imageType := range imageTypes {
-		t.Run(imageType.DeployInterface, func(t *testing.T) {
-			// Create a host without a bootMACAddress and with a BMC that
-			// does not require one.
-			host := makeHost()
-			host.Spec.BootMACAddress = ""
-			host.Status.Provisioning.ID = "" // so we don't lookup by uuid
-
-			createCallback := func(node nodes.Node) {
-				t.Fatal("create callback should not be invoked for existing node")
-			}
-
-			ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).Node(nodes.Node{
-				Name:            host.Namespace + nameSeparator + host.Name,
-				UUID:            "uuid", // to match status in host
-				ProvisionState:  string(nodes.Manageable),
-				AutomatedClean:  &clean,
-				InstanceUUID:    string(host.UID),
-				DeployInterface: imageType.DeployInterface,
-				InstanceInfo:    imageType.InstanceInfo,
-			}).NodeUpdate(nodes.Node{
-				UUID: "uuid",
-			})
-			ironic.Start()
-			defer ironic.Stop()
-
-			auth := clients.AuthConfig{Type: clients.NoAuth}
-			prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nullEventPublisher,
-				ironic.Endpoint(), auth, testserver.NewInspector(t).Endpoint(), auth,
-			)
-			if err != nil {
-				t.Fatalf("could not create provisioner: %s", err)
-			}
-
-			result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{CurrentImage: imageType.Image}, false, false)
-			if err != nil {
-				t.Fatalf("error from ValidateManagementAccess: %s", err)
-			}
-			assert.Equal(t, "", result.ErrorMessage)
-			assert.Equal(t, false, result.Dirty)
-			assert.Len(t, ironic.GetLastNodeUpdateRequestFor("uuid"), 0)
 		})
 	}
 }
@@ -460,11 +371,6 @@ func TestValidateManagementAccessExistingNodeWaiting(t *testing.T) {
 			}
 			assert.Equal(t, "", result.ErrorMessage)
 			assert.Equal(t, true, result.Dirty)
-
-			updates := ironic.GetLastNodeUpdateRequestFor("uuid")
-			assert.Len(t, updates, 1)
-			assert.Equal(t, "/automated_clean", updates[0].Path)
-			assert.Equal(t, true, updates[0].Value)
 		})
 	}
 }
@@ -509,7 +415,6 @@ func TestValidateManagementAccessNewCredentials(t *testing.T) {
 	assert.Equal(t, "uuid", provID)
 
 	updates := ironic.GetLastNodeUpdateRequestFor("uuid")
-	assert.Equal(t, "/driver_info", updates[0].Path)
 	newValues := updates[0].Value.(map[string]interface{})
 	assert.Equal(t, "test.bmc", newValues["test_address"])
 }