Repository: metal3-io/baremetal-operator
Pull request #2041 (2024-11-05 01:50:19)

✨ Add reference to HostUpdatePolicy in Servicing with HFS Support
**What this PR does / why we need it**:

This PR enables BMO to run Ironic servicing operations (such as applying firmware settings changes - or in the future firmware updates to already provisioned nodes). Servicing is an opt-in feature and is controlled by creation of a HostUpdatePolicy for a node with attributes indicating the desire to make changes to firmware configuration onReboot.

This is a partial implementation of metal3-io/metal3-docs@e260d42304a5822816ebc1752ab6c0818b9ee589  (please note only firmware settings changes are currently supported, firmware update support will be added next).


metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please assign [elfosardo](https://github.com/elfosardo) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["elfosardo"]} -->
iurygregory: /assign @zaneb 
metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [zaneb](https://github.com/zaneb). For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["zaneb"]} -->

Patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types.go b/apis/metal3.io/v1alpha1/baremetalhost_types.go
index 73ad1921..4d6cc737 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_types.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_types.go
@@ -147,6 +147,10 @@ const (
 	// OperationalStatusDetached is the status value when the host is
 	// marked unmanaged via the detached annotation.
 	OperationalStatusDetached OperationalStatus = "detached"
+
+	// OperationalStatusServicing is the status value when the host is
+	// undergoing servicing (e.g. checking firmware settings).
+	OperationalStatusServicing OperationalStatus = "servicing"
 )
 
 // OperationalStatusAllowed represents the allowed values of OperationalStatus.
@@ -179,6 +183,9 @@ const (
 	// DetachError is an error condition occurring when the
 	// controller is unable to detatch the host from the provisioner.
 	DetachError ErrorType = "detach error"
+	// ServicingError is an error condition occurring when
+	// service steps failed.
+	ServicingError ErrorType = "servicing error"
 )
 
 // ErrorTypeAllowed represents the allowed values of ErrorType.
@@ -800,12 +807,12 @@ type BareMetalHostStatus struct {
 	// after modifying this file
 
 	// OperationalStatus holds the status of the host
-	// +kubebuilder:validation:Enum="";OK;discovered;error;delayed;detached
+	// +kubebuilder:validation:Enum="";OK;discovered;error;delayed;detached;servicing
 	OperationalStatus OperationalStatus `json:"operationalStatus"`
 
 	// ErrorType indicates the type of failure encountered when the
 	// OperationalStatus is OperationalStatusError
-	// +kubebuilder:validation:Enum=provisioned registration error;registration error;inspection error;preparation error;provisioning error;power management error
+	// +kubebuilder:validation:Enum=provisioned registration error;registration error;inspection error;preparation error;provisioning error;power management error;servicing error
 	ErrorType ErrorType `json:"errorType,omitempty"`
 
 	// LastUpdated identifies when this status was last observed.
diff --git a/config/base/crds/bases/metal3.io_baremetalhosts.yaml b/config/base/crds/bases/metal3.io_baremetalhosts.yaml
index 9ef80edc..636f1bcf 100644
--- a/config/base/crds/bases/metal3.io_baremetalhosts.yaml
+++ b/config/base/crds/bases/metal3.io_baremetalhosts.yaml
@@ -625,6 +625,7 @@ spec:
                 - preparation error
                 - provisioning error
                 - power management error
+                - servicing error
                 type: string
               goodCredentials:
                 description: The last credentials we were able to validate as working.
@@ -897,6 +898,7 @@ spec:
                 - error
                 - delayed
                 - detached
+                - servicing
                 type: string
               poweredOn:
                 description: |-
diff --git a/config/render/capm3.yaml b/config/render/capm3.yaml
index 1c4793fa..85758df0 100644
--- a/config/render/capm3.yaml
+++ b/config/render/capm3.yaml
@@ -625,6 +625,7 @@ spec:
                 - preparation error
                 - provisioning error
                 - power management error
+                - servicing error
                 type: string
               goodCredentials:
                 description: The last credentials we were able to validate as working.
@@ -897,6 +898,7 @@ spec:
                 - error
                 - delayed
                 - detached
+                - servicing
                 type: string
               poweredOn:
                 description: |-
diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go
index 538bc191..f71d4b8d 100644
--- a/controllers/metal3.io/baremetalhost_controller.go
+++ b/controllers/metal3.io/baremetalhost_controller.go
@@ -325,6 +325,8 @@ func recordActionFailure(info *reconcileInfo, errorType metal3api.ErrorType, err
 		metal3api.InspectionError:              "InspectionError",
 		metal3api.ProvisioningError:            "ProvisioningError",
 		metal3api.PowerManagementError:         "PowerManagementError",
+		metal3api.PreparationError:             "PreparationError",
+		metal3api.ServicingError:               "ServicingError",
 	}[errorType]
 
 	counter := actionFailureCounters.WithLabelValues(eventType)
@@ -464,9 +466,9 @@ func hasInspectAnnotation(host *metal3api.BareMetalHost) bool {
 	return false
 }
 
-// clearError removes any existing error message.
-func clearError(host *metal3api.BareMetalHost) (dirty bool) {
-	dirty = host.SetOperationalStatus(metal3api.OperationalStatusOK)
+// clearErrorWithStatus removes any existing error message and sets operational status.
+func clearErrorWithStatus(host *metal3api.BareMetalHost, status metal3api.OperationalStatus) (dirty bool) {
+	dirty = host.SetOperationalStatus(status)
 	var emptyErrType metal3api.ErrorType
 	if host.Status.ErrorType != emptyErrType {
 		host.Status.ErrorType = emptyErrType
@@ -479,6 +481,11 @@ func clearError(host *metal3api.BareMetalHost) (dirty bool) {
 	return dirty
 }
 
+// clearError removes any existing error message.
+func clearError(host *metal3api.BareMetalHost) (dirty bool) {
+	return clearErrorWithStatus(host, metal3api.OperationalStatusOK)
+}
+
 // setErrorMessage updates the ErrorMessage in the host Status struct
 // and increases the ErrorCount.
 func setErrorMessage(host *metal3api.BareMetalHost, errType metal3api.ErrorType, message string) {
@@ -886,7 +893,6 @@ func (r *BareMetalHostReconciler) registerHost(prov provisioner.Provisioner, inf
 
 	// Create the hostFirmwareSettings resource with same host name/namespace if it doesn't exist
 	// Create the hostFirmwareComponents resource with same host name/namespace if it doesn't exist
-	// Set owner reference on hostUpdatePolicy resource if not set
 	if info.host.Name != "" {
 		if !info.host.DeletionTimestamp.IsZero() {
 			info.log.Info(fmt.Sprintf("will not attempt to create new hostFirmwareSettings and hostFirmwareComponents in %s", info.host.Namespace))
@@ -899,7 +905,7 @@ func (r *BareMetalHostReconciler) registerHost(prov provisioner.Provisioner, inf
 				info.log.Info("failed creating hostfirmwarecomponents")
 				return actionError{errors.Wrap(err, "failed creating hostFirmwareComponents")}
 			}
-			if err = r.hostUpdatePolicySetOwnerReference(info); err != nil {
+			if _, err = r.acquireHostUpdatePolicy(info); err != nil {
 				info.log.Info("failed setting owner reference on hostupdatepolicy")
 				return actionError{errors.Wrap(err, "failed setting owner reference on hostUpdatePolicy")}
 			}
@@ -1359,6 +1365,90 @@ func (r *BareMetalHostReconciler) actionDeprovisioning(prov provisioner.Provisio
 	return actionComplete{}
 }
 
+func (r *BareMetalHostReconciler) doServiceIfNeeded(prov provisioner.Provisioner, info *reconcileInfo, hup *metal3api.HostUpdatePolicy) (result actionResult) {
+	servicingData := provisioner.ServicingData{}
+
+	// (NOTE)janders: since Servicing is an opt-in feature that requires HostUpdatePolicy to be created and set to onReboot
+	// set below booleans to false by default and change to true based on policy settings
+
+	var fwDirty bool
+	var hfsDirty bool
+	var liveFirmwareSettingsAllowed bool
+
+	if hup != nil {
+		liveFirmwareSettingsAllowed = (hup.Spec.FirmwareSettings == metal3api.HostUpdatePolicyOnReboot)
+	}
+
+	if liveFirmwareSettingsAllowed {
+		// handling pre-HFS FirmwareSettings here
+		if !reflect.DeepEqual(info.host.Status.Provisioning.Firmware, info.host.Spec.Firmware) {
+			servicingData.FirmwareConfig = info.host.Spec.Firmware
+			fwDirty = true
+		}
+		// handling HFS based FirmwareSettings here
+		var hfs *metal3api.HostFirmwareSettings
+		var err error
+		hfsDirty, hfs, err = r.getHostFirmwareSettings(info)
+		if err != nil {
+			return actionError{fmt.Errorf("could not determine updated settings: %w", err)}
+		}
+		if hfsDirty {
+			servicingData.ActualFirmwareSettings = hfs.Status.Settings
+			servicingData.TargetFirmwareSettings = hfs.Spec.Settings
+		}
+	}
+
+	hasChanges := fwDirty || hfsDirty
+
+	// Even if settings are clean, we need to check the result of the current servicing.
+	if !hasChanges && info.host.Status.OperationalStatus != metal3api.OperationalStatusServicing && info.host.Status.ErrorType != metal3api.ServicingError {
+		// If nothing is going on, return control to the power management.
+		return nil
+	}
+
+	// FIXME(janders/dtantsur): this implementation may lead to a scenario where if we never actually
+	// succeed before leaving this state (e.g. by deprovisioning) we lose the signal that the
+	// update didn't actually happen. This is deemed an acceptable risk for the moment since it is only
+	// going to impact a small subset of Firmware Settings implementations.
+	if info.host.Status.OperationalStatus != metal3api.OperationalStatusServicing {
+		info.host.Status.OperationalStatus = metal3api.OperationalStatusServicing
+		return actionUpdate{}
+	}
+
+	provResult, started, err := prov.Service(servicingData, hasChanges,
+		info.host.Status.ErrorType == metal3api.ServicingError)
+	if err != nil {
+		return actionError{fmt.Errorf("error servicing host: %w", err)}
+	}
+	if provResult.ErrorMessage != "" {
+		info.host.Status.Provisioning.Firmware = nil
+		result = recordActionFailure(info, metal3api.ServicingError, provResult.ErrorMessage)
+		return result
+	}
+
+	dirty := clearErrorWithStatus(info.host, metal3api.OperationalStatusServicing)
+
+	if started && fwDirty {
+		info.host.Status.Provisioning.Firmware = info.host.Spec.Firmware.DeepCopy()
+		dirty = true
+	}
+
+	resultAction := actionContinue{delay: provResult.RequeueAfter}
+	if dirty {
+		return actionUpdate{resultAction}
+	} else if provResult.Dirty {
+		return resultAction
+	}
+
+	// Servicing is finished at this point, clean up operational status
+	if clearErrorWithStatus(info.host, metal3api.OperationalStatusOK) {
+		// FIXME(janders/dtantsur): this can be racy. We should consider
+		// using a generation number to decide if we start servicing or not.
+		return actionUpdate{actionContinue{delay: subResourceNotReadyRetryDelay}}
+	}
+	return nil
+}
+
 // Check the current power status against the desired power status.
 func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
 	var provResult provisioner.Result
@@ -1372,12 +1462,20 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 	if hwState.PoweredOn != nil && *hwState.PoweredOn != info.host.Status.PoweredOn {
 		info.log.Info("updating power status", "discovered", *hwState.PoweredOn)
 		info.host.Status.PoweredOn = *hwState.PoweredOn
-		clearError(info.host)
+		if info.host.Status.OperationalStatus == metal3api.OperationalStatusError && info.host.Status.ErrorType == metal3api.PowerManagementError {
+			clearError(info.host)
+		}
 		return actionUpdate{}
 	}
 
 	desiredPowerOnState := info.host.Spec.Online
 
+	provState := info.host.Status.Provisioning.State
+	// Normal reboots only work in provisioned states, changing online is also possible for available hosts.
+	isProvisioned := provState == metal3api.StateProvisioned || provState == metal3api.StateExternallyProvisioned
+	// FIXME(janders/dtantsur) it would be preferrable to pass in state as an argument
+	// however this falls outside the scope of this specific change.
+
 	if !info.host.Status.PoweredOn {
 		if _, suffixlessAnnotationExists := info.host.Annotations[metal3api.RebootAnnotationPrefix]; suffixlessAnnotationExists {
 			delete(info.host.Annotations, metal3api.RebootAnnotationPrefix)
@@ -1390,9 +1488,19 @@ func (r *BareMetalHostReconciler) manageHostPower(prov provisioner.Provisioner,
 		}
 	}
 
-	provState := info.host.Status.Provisioning.State
-	// Normal reboots only work in provisioned states, changing online is also possible for available hosts.
-	isProvisioned := provState == metal3api.StateProvisioned || provState == metal3api.StateExternallyProvisioned
+	servicingAllowed := isProvisioned && !info.host.Status.PoweredOn && desiredPowerOnState
+	if servicingAllowed || info.host.Status.OperationalStatus == metal3api.OperationalStatusServicing || info.host.Status.ErrorType == metal3api.ServicingError {
+		hup, err := r.acquireHostUpdatePolicy(info)
+		if err != nil {
+			info.log.Info("failed setting owner reference on hostupdatepolicy")
+			return actionError{errors.Wrap(err, "failed setting owner reference on hostUpdatePolicy")}
+		}
+
+		result := r.doServiceIfNeeded(prov, info, hup)
+		if result != nil {
+			return result
+		}
+	}
 
 	desiredReboot, desiredRebootMode := hasRebootAnnotation(info, !isProvisioned)
 
@@ -1866,7 +1974,7 @@ func (r *BareMetalHostReconciler) createHostFirmwareSettings(info *reconcileInfo
 	return nil
 }
 
-func (r *BareMetalHostReconciler) hostUpdatePolicySetOwnerReference(info *reconcileInfo) error {
+func (r *BareMetalHostReconciler) acquireHostUpdatePolicy(info *reconcileInfo) (policy *metal3api.HostUpdatePolicy, err error) {
 	// NOTE(janders) the goal here is to ensure that the controller reads the hup resource and adds OwnerReference to it
 	hup := &metal3api.HostUpdatePolicy{}
 	if err := r.Get(info.ctx, info.request.NamespacedName, hup); err != nil {
@@ -1876,23 +1984,23 @@ func (r *BareMetalHostReconciler) hostUpdatePolicySetOwnerReference(info *reconc
 			// garbage collected. For additional cleanup logic use
 			// finalizers.  Return and don't requeue
 
-			return nil
+			return nil, nil
 		}
 		// Error reading the object
-		return fmt.Errorf("could not load hostUpdatePolicy resource due to %w", err)
+		return nil, fmt.Errorf("could not load hostUpdatePolicy resource due to %w", err)
 	}
 	if !ownerReferenceExists(info.host, hup) {
 		if err := controllerutil.SetOwnerReference(info.host, hup, r.Scheme()); err != nil {
-			return fmt.Errorf("could not set bmh as owner for hostUpdatePolicy due to %w", err)
+			return hup, fmt.Errorf("could not set bmh as owner for hostUpdatePolicy due to %w", err)
 		}
 		if err := r.Update(info.ctx, hup); err != nil {
-			return fmt.Errorf("failure updating hostUpdatePolicy resource due to %w", err)
+			return hup, fmt.Errorf("failure updating hostUpdatePolicy resource due to %w", err)
 		}
 
-		return nil
+		return hup, nil
 	}
 
-	return nil
+	return hup, nil
 }
 
 // Get the stored firmware settings if there are valid changes.
diff --git a/pkg/provisioner/demo/demo.go b/pkg/provisioner/demo/demo.go
index bbae60aa..d4e8acd4 100644
--- a/pkg/provisioner/demo/demo.go
+++ b/pkg/provisioner/demo/demo.go
@@ -211,6 +211,28 @@ func (p *demoProvisioner) Prepare(_ provisioner.PrepareData, unprepared bool, _
 	return
 }
 
+func (p *demoProvisioner) Service(_ provisioner.ServicingData, unprepared bool, _ bool) (result provisioner.Result, started bool, err error) {
+	hostName := p.objectMeta.Name
+
+	switch hostName {
+	case PreparingErrorHost:
+		p.log.Info("servicing error host")
+		result.ErrorMessage = "servicing failed"
+
+	case PreparingHost:
+		p.log.Info("servicing host")
+		started = unprepared
+		result.Dirty = true
+		result.RequeueAfter = time.Second * 5
+
+	default:
+		p.log.Info("finished servicing")
+		started = true
+	}
+
+	return
+}
+
 // Adopt notifies the provisioner that the state machine believes the host
 // to be currently provisioned, and that it should be managed as such.
 func (p *demoProvisioner) Adopt(_ provisioner.AdoptData, _ bool) (result provisioner.Result, err error) {
diff --git a/pkg/provisioner/fixture/fixture.go b/pkg/provisioner/fixture/fixture.go
index f6df8d93..85806863 100644
--- a/pkg/provisioner/fixture/fixture.go
+++ b/pkg/provisioner/fixture/fixture.go
@@ -218,11 +218,21 @@ func (p *fixtureProvisioner) UpdateHardwareState() (hwState provisioner.Hardware
 
 // Prepare remove existing configuration and set new configuration.
 func (p *fixtureProvisioner) Prepare(_ provisioner.PrepareData, unprepared bool, _ bool) (result provisioner.Result, started bool, err error) {
-	p.log.Info("preparing host")
+	p.log.Info("preparing host", "unprepared", unprepared)
 	started = unprepared
 	return
 }
 
+// Service remove existing configuration and set new configuration.
+func (p *fixtureProvisioner) Service(_ provisioner.ServicingData, unprepared bool, _ bool) (result provisioner.Result, started bool, err error) {
+	p.log.Info("servicing host", "unprepared", unprepared)
+	started = unprepared
+	if started {
+		result.Dirty = true
+	}
+	return
+}
+
 // Adopt notifies the provisioner that the state machine believes the host
 // to be currently provisioned, and that it should be managed as such.
 func (p *fixtureProvisioner) Adopt(_ provisioner.AdoptData, _ bool) (result provisioner.Result, err error) {
diff --git a/pkg/provisioner/ironic/clients/features.go b/pkg/provisioner/ironic/clients/features.go
index 10c4b7be..6998d5f9 100644
--- a/pkg/provisioner/ironic/clients/features.go
+++ b/pkg/provisioner/ironic/clients/features.go
@@ -50,6 +50,10 @@ func (af AvailableFeatures) HasFirmwareUpdates() bool {
 	return af.MaxVersion >= 86
 }
 
+func (af AvailableFeatures) HasServicing() bool {
+	return af.MaxVersion >= 87
+}
+
 func (af AvailableFeatures) HasDataImage() bool {
 	return af.MaxVersion >= 89
 }
@@ -59,6 +63,10 @@ func (af AvailableFeatures) ChooseMicroversion() string {
 		return "1.89"
 	}
 
+	if af.HasServicing() {
+		return "1.87"
+	}
+
 	if af.HasFirmwareUpdates() {
 		return "1.86"
 	}
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index 7b5fbc1b..9d3bf66e 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -1179,31 +1179,12 @@ func (p *ironicProvisioner) ironicHasSameImage(ironicNode *nodes.Node, image met
 	return sameImage
 }
 
-func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails, data provisioner.PrepareData) (cleanSteps []nodes.CleanStep, err error) {
-	// Build raid clean steps
-	raidCleanSteps, err := BuildRAIDCleanSteps(bmcAccess.RAIDInterface(), data.TargetRAIDConfig, data.ActualRAIDConfig)
-	if err != nil {
-		return nil, err
-	}
-	cleanSteps = append(cleanSteps, raidCleanSteps...)
-
-	// Get the subset (currently 3) of vendor specific BIOS settings converted from common names
-	var firmwareConfig *bmc.FirmwareConfig
-	if data.FirmwareConfig != nil {
-		bmcConfig := bmc.FirmwareConfig(*data.FirmwareConfig)
-		firmwareConfig = &bmcConfig
-	}
-	fwConfigSettings, err := bmcAccess.BuildBIOSSettings(firmwareConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	var newSettings []map[string]interface{}
-	if data.ActualFirmwareSettings != nil {
+func (p *ironicProvisioner) getNewFirmwareSettings(actualFirmwareSettings metal3api.SettingsMap, targetFirmwareSettings metal3api.DesiredSettingsMap, fwConfigSettings []map[string]string) (newSettings []map[string]interface{}) {
+	if actualFirmwareSettings != nil {
 		// If we have the current settings from Ironic, update the settings to contain:
 		// 1. settings converted by BMC drivers that are different than current settings
 		for _, fwConfigSetting := range fwConfigSettings {
-			if val, exists := data.ActualFirmwareSettings[fwConfigSetting["name"]]; exists {
+			if val, exists := actualFirmwareSettings[fwConfigSetting["name"]]; exists {
 				if fwConfigSetting["value"] != val {
 					newSettings = buildFirmwareSettings(newSettings, fwConfigSetting["name"], intstr.FromString(fwConfigSetting["value"]))
 				}
@@ -1213,17 +1194,15 @@ func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails
 		}
 
 		// 2. target settings that are different than current settings
-		if data.TargetFirmwareSettings != nil {
-			for k, v := range data.TargetFirmwareSettings {
-				if data.ActualFirmwareSettings[k] != v.String() {
-					// Skip changing this setting if it was defined in the vendor specific settings
-					for _, fwConfigSetting := range fwConfigSettings {
-						if fwConfigSetting["name"] == k {
-							continue
-						}
+		for k, v := range targetFirmwareSettings {
+			if actualFirmwareSettings[k] != v.String() {
+				// Skip changing this setting if it was defined in the vendor specific settings
+				for _, fwConfigSetting := range fwConfigSettings {
+					if fwConfigSetting["name"] == k {
+						continue
 					}
-					newSettings = buildFirmwareSettings(newSettings, k, v)
 				}
+				newSettings = buildFirmwareSettings(newSettings, k, v)
 			}
 		}
 	} else {
@@ -1233,6 +1212,29 @@ func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails
 		}
 	}
 
+	return newSettings
+}
+
+func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails, data provisioner.PrepareData) (cleanSteps []nodes.CleanStep, err error) {
+	// Build raid clean steps
+	raidCleanSteps, err := BuildRAIDCleanSteps(bmcAccess.RAIDInterface(), data.TargetRAIDConfig, data.ActualRAIDConfig)
+	if err != nil {
+		return nil, err
+	}
+	cleanSteps = append(cleanSteps, raidCleanSteps...)
+
+	// Get the subset (currently 3) of vendor specific BIOS settings converted from common names
+	var firmwareConfig *bmc.FirmwareConfig
+	if data.FirmwareConfig != nil {
+		bmcConfig := bmc.FirmwareConfig(*data.FirmwareConfig)
+		firmwareConfig = &bmcConfig
+	}
+	fwConfigSettings, err := bmcAccess.BuildBIOSSettings(firmwareConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	newSettings := p.getNewFirmwareSettings(data.ActualFirmwareSettings, data.TargetFirmwareSettings, fwConfigSettings)
 	if len(newSettings) != 0 {
 		p.log.Info("Applying BIOS config clean steps", "settings", newSettings)
 		cleanSteps = append(
diff --git a/pkg/provisioner/ironic/servicing.go b/pkg/provisioner/ironic/servicing.go
new file mode 100644
index 00000000..c6698475
--- /dev/null
+++ b/pkg/provisioner/ironic/servicing.go
@@ -0,0 +1,124 @@
+package ironic
+
+import (
+	"fmt"
+
+	"github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes"
+	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
+	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
+)
+
+func (p *ironicProvisioner) buildServiceSteps(bmcAccess bmc.AccessDetails, data provisioner.ServicingData) (serviceSteps []nodes.ServiceStep, err error) {
+	// Get the subset (currently 3) of vendor specific BIOS settings converted from common names
+	var firmwareConfig *bmc.FirmwareConfig
+	if data.FirmwareConfig != nil {
+		bmcConfig := bmc.FirmwareConfig(*data.FirmwareConfig)
+		firmwareConfig = &bmcConfig
+	}
+	fwConfigSettings, err := bmcAccess.BuildBIOSSettings(firmwareConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	newSettings := p.getNewFirmwareSettings(data.ActualFirmwareSettings, data.TargetFirmwareSettings, fwConfigSettings)
+	if len(newSettings) != 0 {
+		p.log.Info("Applying BIOS config clean steps", "settings", newSettings)
+		serviceSteps = append(
+			serviceSteps,
+			nodes.ServiceStep{
+				Interface: nodes.InterfaceBIOS,
+				Step:      "apply_configuration",
+				Args: map[string]interface{}{
+					"settings": newSettings,
+				},
+			},
+		)
+	}
+
+	// TODO: Add service steps for firmware updates
+
+	return
+}
+
+func (p *ironicProvisioner) startServicing(bmcAccess bmc.AccessDetails, ironicNode *nodes.Node, data provisioner.ServicingData) (success bool, result provisioner.Result, err error) {
+	// Build service steps
+	serviceSteps, err := p.buildServiceSteps(bmcAccess, data)
+	if err != nil {
+		result, err = operationFailed(err.Error())
+		return
+	}
+
+	// Start servicing
+	if len(serviceSteps) != 0 {
+		p.log.Info("remove existing configuration and set new configuration", "serviceSteps", serviceSteps)
+		return p.tryChangeNodeProvisionState(
+			ironicNode,
+			nodes.ProvisionStateOpts{
+				Target:       nodes.TargetService,
+				ServiceSteps: serviceSteps,
+			},
+		)
+	}
+	result, err = operationComplete()
+	return
+}
+
+func (p *ironicProvisioner) Service(data provisioner.ServicingData, unprepared, restartOnFailure bool) (result provisioner.Result, started bool, err error) {
+	if !p.availableFeatures.HasServicing() {
+		result, err = operationFailed(fmt.Sprintf("servicing not supported: requires API version 1.87, available is 1.%d", p.availableFeatures.MaxVersion))
+		return result, started, err
+	}
+
+	bmcAccess, err := p.bmcAccess()
+	if err != nil {
+		result, err = transientError(err)
+		return result, started, err
+	}
+
+	ironicNode, err := p.getNode()
+	if err != nil {
+		result, err = transientError(err)
+		return result, started, err
+	}
+
+	switch nodes.ProvisionState(ironicNode.ProvisionState) {
+	case nodes.ServiceFail:
+		// When servicing failed, we need to clean host provisioning settings.
+		// If restartOnFailure is false, it means the settings aren't cleared.
+		if !restartOnFailure {
+			result, err = operationFailed(ironicNode.LastError)
+			return result, started, err
+		}
+
+		if ironicNode.Maintenance {
+			p.log.Info("clearing maintenance flag after a servicing failure")
+			result, err = p.setMaintenanceFlag(ironicNode, false, "")
+			return result, started, err
+		}
+
+		p.log.Info("restarting servicing because of a previous failure")
+		unprepared = true
+		fallthrough
+	case nodes.Active:
+		if unprepared {
+			started, result, err = p.startServicing(bmcAccess, ironicNode, data)
+			if started || result.Dirty || result.ErrorMessage != "" || err != nil {
+				return result, started, err
+			}
+			// nothing to do
+			started = true
+		}
+		// Servicing finished
+		p.log.Info("servicing finished on the host")
+		result, err = operationComplete()
+	case nodes.Servicing, nodes.ServiceWait:
+		p.log.Info("waiting for host to become active",
+			"state", ironicNode.ProvisionState,
+			"serviceStep", ironicNode.ServiceStep)
+		result, err = operationContinuing(provisionRequeueDelay)
+
+	default:
+		result, err = transientError(fmt.Errorf("have unexpected ironic node state %s", ironicNode.ProvisionState))
+	}
+	return result, started, err
+}
diff --git a/pkg/provisioner/provisioner.go b/pkg/provisioner/provisioner.go
index f9564ada..c18d3506 100644
--- a/pkg/provisioner/provisioner.go
+++ b/pkg/provisioner/provisioner.go
@@ -107,6 +107,13 @@ type PrepareData struct {
 	TargetFirmwareComponents []metal3api.FirmwareUpdate
 }
 
+type ServicingData struct {
+	FirmwareConfig         *metal3api.FirmwareConfig
+	TargetFirmwareSettings metal3api.DesiredSettingsMap
+	ActualFirmwareSettings metal3api.SettingsMap
+	// TargetFirmwareComponents []metal3api.FirmwareUpdate
+}
+
 type ProvisionData struct {
 	Image           metal3api.Image
 	HostConfig      HostConfigData
@@ -154,6 +161,9 @@ type Provisioner interface {
 	// Prepare remove existing configuration and set new configuration
 	Prepare(data PrepareData, unprepared bool, restartOnFailure bool) (result Result, started bool, err error)
 
+	// Servicing updates configuration for a provisioned host.
+	Service(data ServicingData, unprepared, restartOnFailure bool) (result Result, started bool, err error)
+
 	// Provision writes the image from the host spec to the host. It
 	// may be called multiple times, and should return true for its
 	// dirty flag until the provisioning operation is completed.


Test patch:
diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go
index 25ac0df1..4d683afc 100644
--- a/controllers/metal3.io/baremetalhost_controller_test.go
+++ b/controllers/metal3.io/baremetalhost_controller_test.go
@@ -22,6 +22,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -796,6 +797,122 @@ func TestRebootWithSuffixedAnnotation(t *testing.T) {
 	)
 }
 
+// TestRebootWithServicing tests full reboot cycle with suffixless
+// annotation and servicing.
+func TestRebootWithServicing(t *testing.T) {
+	host := newDefaultHost(t)
+	host.Annotations = make(map[string]string)
+	host.Annotations[metal3api.RebootAnnotationPrefix] = ""
+	host.Status.PoweredOn = true
+	host.Status.Provisioning.State = metal3api.StateProvisioned
+	host.Spec.Online = true
+	host.Spec.Image = &metal3api.Image{URL: "foo", Checksum: "123"}
+	host.Spec.Image.URL = "foo"
+	host.Spec.Firmware = &metal3api.FirmwareConfig{
+		VirtualizationEnabled: ptr.To(true),
+	}
+	host.Status.Provisioning.Image.URL = "foo"
+
+	// HUP creation
+	hup := &metal3api.HostUpdatePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      host.Name,
+			Namespace: namespace,
+		},
+		Spec: metal3api.HostUpdatePolicySpec{
+			FirmwareSettings: metal3api.HostUpdatePolicyOnReboot,
+			FirmwareUpdates:  metal3api.HostUpdatePolicyOnReboot,
+		},
+	}
+
+	r := newTestReconciler(host, hup)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			return host.Status.OperationalStatus == metal3api.OperationalStatusOK && !host.Status.PoweredOn
+		},
+	)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			_, exists := host.Annotations[metal3api.RebootAnnotationPrefix]
+			return host.Status.OperationalStatus == metal3api.OperationalStatusOK && !exists
+		},
+	)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			return host.Status.OperationalStatus == metal3api.OperationalStatusServicing && !host.Status.PoweredOn
+		},
+	)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			return host.Status.OperationalStatus == metal3api.OperationalStatusOK && !host.Status.PoweredOn
+		},
+	)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			return host.Status.PoweredOn
+		},
+	)
+
+	// make sure we don't go into another reboot
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			return host.Status.PoweredOn
+		},
+	)
+}
+
+// TestRebootWithoutServicing ensures Servicing is not triggered if HostUpdatePolicy doesn't exist.
+func TestRebootWithoutServicing(t *testing.T) {
+	host := newDefaultHost(t)
+	host.Annotations = make(map[string]string)
+	host.Annotations[metal3api.RebootAnnotationPrefix] = ""
+	host.Status.PoweredOn = true
+	host.Status.Provisioning.State = metal3api.StateProvisioned
+	host.Spec.Online = true
+	host.Spec.Image = &metal3api.Image{URL: "foo", Checksum: "123"}
+	host.Spec.Image.URL = "foo"
+	host.Status.Provisioning.Image.URL = "foo"
+	host.Spec.Firmware = &metal3api.FirmwareConfig{
+		VirtualizationEnabled: ptr.To(true),
+	}
+
+	r := newTestReconciler(host)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			return !host.Status.PoweredOn
+		},
+	)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			if _, exists := host.Annotations[metal3api.RebootAnnotationPrefix]; exists {
+				return false
+			}
+
+			return true
+		},
+	)
+
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			return host.Status.PoweredOn
+		},
+	)
+
+	// make sure we don't go into another reboot
+	tryReconcile(t, r, host,
+		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
+			return host.Status.PoweredOn
+		},
+	)
+}
+
 func getHostSecret(t *testing.T, r *BareMetalHostReconciler, host *metal3api.BareMetalHost) (secret *corev1.Secret) {
 	t.Helper()
 	secret = &corev1.Secret{}
diff --git a/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go
index 6fd35754..2e70b08d 100644
--- a/controllers/metal3.io/host_state_machine_test.go
+++ b/controllers/metal3.io/host_state_machine_test.go
@@ -1350,6 +1350,10 @@ func (m *mockProvisioner) Prepare(_ provisioner.PrepareData, _ bool, _ bool) (re
 	return m.getNextResultByMethod("Prepare"), m.nextResults["Prepare"].Dirty, err
 }
 
+func (m *mockProvisioner) Service(_ provisioner.ServicingData, _ bool, _ bool) (result provisioner.Result, started bool, err error) {
+	return m.getNextResultByMethod("Service"), m.nextResults["Service"].Dirty, err
+}
+
 func (m *mockProvisioner) Adopt(_ provisioner.AdoptData, _ bool) (result provisioner.Result, err error) {
 	return m.getNextResultByMethod("Adopt"), err
 }
diff --git a/pkg/provisioner/ironic/servicing_test.go b/pkg/provisioner/ironic/servicing_test.go
new file mode 100644
index 00000000..a878afce
--- /dev/null
+++ b/pkg/provisioner/ironic/servicing_test.go
@@ -0,0 +1,196 @@
+package ironic
+
+import (
+	"net/url"
+	"testing"
+	"time"
+
+	"github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes"
+	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/util/intstr"
+
+	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
+	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
+	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
+	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/clients"
+	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/testserver"
+)
+
+type BIOSTestBMC struct{}
+
+func (r *BIOSTestBMC) Type() string                                          { return "bios-test" }
+func (r *BIOSTestBMC) NeedsMAC() bool                                        { return false }
+func (r *BIOSTestBMC) Driver() string                                        { return "bios-test" }
+func (r *BIOSTestBMC) DisableCertificateVerification() bool                  { return false }
+func (r *BIOSTestBMC) DriverInfo(bmc.Credentials) (i map[string]interface{}) { return }
+func (r *BIOSTestBMC) SupportsISOPreprovisioningImage() bool                 { return false }
+func (r *BIOSTestBMC) BIOSInterface() string                                 { return "" }
+func (r *BIOSTestBMC) BootInterface() string                                 { return "" }
+func (r *BIOSTestBMC) FirmwareInterface() string                             { return "" }
+func (r *BIOSTestBMC) ManagementInterface() string                           { return "" }
+func (r *BIOSTestBMC) PowerInterface() string                                { return "" }
+func (r *BIOSTestBMC) RAIDInterface() string                                 { return "" }
+func (r *BIOSTestBMC) VendorInterface() string                               { return "" }
+func (r *BIOSTestBMC) SupportsSecureBoot() bool                              { return false }
+func (r *BIOSTestBMC) RequiresProvisioningNetwork() bool                     { return true }
+func (r *BIOSTestBMC) BuildBIOSSettings(_ *bmc.FirmwareConfig) ([]map[string]string, error) {
+	return nil, nil
+}
+
+func TestService(t *testing.T) {
+	bmc.RegisterFactory("bios-test", func(u *url.URL, dcv bool) (bmc.AccessDetails, error) {
+		return &BIOSTestBMC{}, nil
+	}, []string{})
+
+	nodeUUID := "33ce8659-7400-4c68-9535-d10766f07a58"
+	cases := []struct {
+		name                 string
+		ironic               *testserver.IronicMock
+		unprepared           bool
+		skipConfig           bool
+		expectedStarted      bool
+		expectedDirty        bool
+		expectedError        bool
+		expectedRequestAfter int
+	}{
+		{
+			name: "active, no new steps",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.Active),
+				UUID:           nodeUUID,
+			}),
+			skipConfig:           true,
+			unprepared:           true,
+			expectedStarted:      true,
+			expectedRequestAfter: 0,
+			expectedDirty:        false,
+		},
+		{
+			name: "active with steps",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.Active),
+				UUID:           nodeUUID,
+			}),
+			unprepared:           true,
+			expectedStarted:      true,
+			expectedRequestAfter: 10,
+			expectedDirty:        true,
+		},
+		{
+			name: "serviceFail state(cleaned provision settings)",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.ServiceFail),
+				UUID:           nodeUUID,
+			}),
+			expectedStarted:      false,
+			expectedRequestAfter: 0,
+			expectedDirty:        false,
+		},
+		{
+			name: "serviceFail state(retry)",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.ServiceFail),
+				UUID:           nodeUUID,
+			}),
+			unprepared:           true,
+			expectedStarted:      true,
+			expectedRequestAfter: 10,
+			expectedDirty:        true,
+		},
+		{
+			name: "serviceFail state(retry with maintenance)",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.ServiceFail),
+				UUID:           nodeUUID,
+				Maintenance:    true,
+			}).NodeMaintenance(nodes.Node{
+				UUID: nodeUUID,
+			}, false),
+			unprepared:      true,
+			expectedStarted: false,
+			expectedDirty:   true,
+		},
+		{
+			name: "servicing state",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.Servicing),
+				UUID:           nodeUUID,
+			}),
+			expectedStarted:      false,
+			expectedRequestAfter: 10,
+			expectedDirty:        true,
+		},
+		{
+			name: "serviceWait state",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.ServiceWait),
+				UUID:           nodeUUID,
+			}),
+			expectedStarted:      false,
+			expectedRequestAfter: 10,
+			expectedDirty:        true,
+		},
+		{
+			name: "active state(servicing finished)",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.Active),
+				UUID:           nodeUUID,
+			}),
+			expectedStarted:      false,
+			expectedRequestAfter: 0,
+			expectedDirty:        false,
+		},
+		{
+			name: "unexpected state",
+			ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{
+				ProvisionState: string(nodes.Cleaning),
+				UUID:           nodeUUID,
+			}),
+			expectedStarted:      false,
+			expectedRequestAfter: 0,
+			expectedDirty:        false,
+			expectedError:        true,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.ironic != nil {
+				tc.ironic.Start()
+				defer tc.ironic.Stop()
+			}
+
+			host := makeHost()
+			host.Status.Provisioning.ID = nodeUUID
+			prepData := provisioner.ServicingData{}
+			if !tc.skipConfig {
+				host.Spec.BMC.Address = "raid-test://test.bmc/"
+				prepData.ActualFirmwareSettings = metal3api.SettingsMap{
+					"Answer": "unknown",
+				}
+				prepData.TargetFirmwareSettings = metal3api.DesiredSettingsMap{
+					"Answer": intstr.FromInt(42),
+				}
+			}
+
+			publisher := func(reason, message string) {}
+			auth := clients.AuthConfig{Type: clients.NoAuth}
+			prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, publisher, tc.ironic.Endpoint(), auth)
+			if err != nil {
+				t.Fatalf("could not create provisioner: %s", err)
+			}
+			prov.availableFeatures = clients.AvailableFeatures{MaxVersion: 87}
+
+			result, started, err := prov.Service(prepData, tc.unprepared, tc.unprepared)
+
+			assert.Equal(t, tc.expectedStarted, started)
+			assert.Equal(t, tc.expectedDirty, result.Dirty)
+			assert.Equal(t, time.Second*time.Duration(tc.expectedRequestAfter), result.RequeueAfter)
+			if !tc.expectedError {
+				assert.NoError(t, err)
+			} else {
+				assert.Error(t, err)
+			}
+		})
+	}
+}
diff --git a/pkg/provisioner/ironic/testserver/ironic.go b/pkg/provisioner/ironic/testserver/ironic.go
index a85351bd..e56182b0 100644
--- a/pkg/provisioner/ironic/testserver/ironic.go
+++ b/pkg/provisioner/ironic/testserver/ironic.go
@@ -42,7 +42,7 @@ const versionedRootResult = `
     "links": [ { "href": "/v1/", "rel": "self" } ],
     "status": "CURRENT",
     "min_version": "1.1",
-    "version": "1.86"
+    "version": "1.87"
   }
 }`