Merge pull request #2044 from iurygregory/hfc+hup

✨  Allow FirmwareUpdates via Servicing with HostUpdatePolicy

controllers/metal3.io/baremetalhost_controller.go

@@ -1373,10 +1373,13 @@ func (r *BareMetalHostReconciler) doServiceIfNeeded(prov provisioner.Provisioner
 
 	var fwDirty bool
 	var hfsDirty bool
-	var liveFirmwareSettingsAllowed bool
+	var hfcDirty bool
+	var hfc *metal3api.HostFirmwareComponents
+	var liveFirmwareSettingsAllowed, liveFirmwareUpdatesAllowed bool
 
 	if hup != nil {
 		liveFirmwareSettingsAllowed = (hup.Spec.FirmwareSettings == metal3api.HostUpdatePolicyOnReboot)
+		liveFirmwareUpdatesAllowed = (hup.Spec.FirmwareUpdates == metal3api.HostUpdatePolicyOnReboot)
 	}
 
 	if liveFirmwareSettingsAllowed {
@@ -1398,7 +1401,18 @@ func (r *BareMetalHostReconciler) doServiceIfNeeded(prov provisioner.Provisioner
 		}
 	}
 
-	hasChanges := fwDirty || hfsDirty
+	if liveFirmwareUpdatesAllowed {
+		var err error
+		hfcDirty, hfc, err = r.getHostFirmwareComponents(info)
+		if err != nil {
+			return actionError{fmt.Errorf("could not determine firmware components: %w", err)}
+		}
+		if hfcDirty {
+			servicingData.TargetFirmwareComponents = hfc.Spec.Updates
+		}
+	}
+
+	hasChanges := fwDirty || hfsDirty || hfcDirty
 
 	// 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 {
@@ -1422,6 +1436,12 @@ func (r *BareMetalHostReconciler) doServiceIfNeeded(prov provisioner.Provisioner
 	}
 	if provResult.ErrorMessage != "" {
 		info.host.Status.Provisioning.Firmware = nil
+		if hfcDirty && hfc.Status.Updates != nil {
+			hfc.Status.Updates = nil
+			if err := r.Status().Update(info.ctx, hfc); err != nil {
+				return actionError{errors.Wrap(err, "failed to update hostfirmwarecomponents status")}
+			}
+		}
 		result = recordActionFailure(info, metal3api.ServicingError, provResult.ErrorMessage)
 		return result
 	}
@@ -1433,6 +1453,19 @@ func (r *BareMetalHostReconciler) doServiceIfNeeded(prov provisioner.Provisioner
 		dirty = true
 	}
 
+	if hfcDirty && started {
+		hfcDirty, err = r.saveHostFirmwareComponents(prov, info, hfc)
+		if err != nil {
+			return actionError{errors.Wrap(err, "could not save the host firmware components")}
+		}
+
+		if hfcDirty {
+			if err := r.Status().Update(info.ctx, hfc); err != nil {
+				return actionError{errors.Wrap(err, "failed to update hostfirmwarecomponents status")}
+			}
+		}
+	}
+
 	resultAction := actionContinue{delay: provResult.RequeueAfter}
 	if dirty {
 		return actionUpdate{resultAction}

pkg/provisioner/ironic/ironic.go

@@ -1215,6 +1215,18 @@ func (p *ironicProvisioner) getNewFirmwareSettings(actualFirmwareSettings metal3
 	return newSettings
 }
 
+// getFirmwareComponentsUpdates extract the updates in a format that ironic accepts  [{"component":"...", "url":"..."}, {"component":"...","url":".."}].
+func (p *ironicProvisioner) getFirmwareComponentsUpdates(targetFirmwareComponents []metal3api.FirmwareUpdate) (newUpdates []map[string]string) {
+	for _, update := range targetFirmwareComponents {
+		newComponentUpdate := map[string]string{
+			"component": update.Component,
+			"url":       update.URL,
+		}
+		newUpdates = append(newUpdates, newComponentUpdate)
+	}
+	return newUpdates
+}
+
 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)
@@ -1249,19 +1261,7 @@ func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails
 		)
 	}
 
-	// extract to generate the updates that will trigger a clean step
-	// the format we send to ironic is:
-	// [{"component":"...", "url":"..."}, {"component":"...","url":".."}]
-	var newUpdates []map[string]string
-	if data.TargetFirmwareComponents != nil {
-		for _, update := range data.TargetFirmwareComponents {
-			newComponentUpdate := map[string]string{
-				"component": update.Component,
-				"url":       update.URL,
-			}
-			newUpdates = append(newUpdates, newComponentUpdate)
-		}
-	}
+	newUpdates := p.getFirmwareComponentsUpdates(data.TargetFirmwareComponents)
 
 	if len(newUpdates) != 0 {
 		p.log.Info("Applying Firmware Update clean steps", "settings", newUpdates)

pkg/provisioner/ironic/servicing.go

@@ -35,9 +35,22 @@ func (p *ironicProvisioner) buildServiceSteps(bmcAccess bmc.AccessDetails, data
 		)
 	}
 
-	// TODO: Add service steps for firmware updates
+	newUpdates := p.getFirmwareComponentsUpdates(data.TargetFirmwareComponents)
+	if len(newUpdates) != 0 {
+		p.log.Info("Applying Firmware Update clean steps", "settings", newUpdates)
+		serviceSteps = append(
+			serviceSteps,
+			nodes.ServiceStep{
+				Interface: nodes.InterfaceFirmware,
+				Step:      "update",
+				Args: map[string]interface{}{
+					"settings": newUpdates,
+				},
+			},
+		)
+	}
 
-	return
+	return serviceSteps, nil
 }
 
 func (p *ironicProvisioner) startServicing(bmcAccess bmc.AccessDetails, ironicNode *nodes.Node, data provisioner.ServicingData) (success bool, result provisioner.Result, err error) {

pkg/provisioner/provisioner.go

@@ -108,10 +108,10 @@ type PrepareData struct {
 }
 
 type ServicingData struct {
-	FirmwareConfig         *metal3api.FirmwareConfig
-	TargetFirmwareSettings metal3api.DesiredSettingsMap
-	ActualFirmwareSettings metal3api.SettingsMap
-	// TargetFirmwareComponents []metal3api.FirmwareUpdate
+	FirmwareConfig           *metal3api.FirmwareConfig
+	TargetFirmwareSettings   metal3api.DesiredSettingsMap
+	ActualFirmwareSettings   metal3api.SettingsMap
+	TargetFirmwareComponents []metal3api.FirmwareUpdate
 }
 
 type ProvisionData struct {