🐛 Fixes for HostFirmwareComponents

controllers/metal3.io/baremetalhost_controller.go

@@ -872,14 +872,19 @@ 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
 	if info.host.Name != "" {
 		if !info.host.DeletionTimestamp.IsZero() {
-			info.log.Info(fmt.Sprintf("will not attempt to create new hostFirmwareSettings in %s", info.host.Namespace))
+			info.log.Info(fmt.Sprintf("will not attempt to create new hostFirmwareSettings and hostFirmwareComponents in %s", info.host.Namespace))
 		} else {
 			if err = r.createHostFirmwareSettings(info); err != nil {
 				info.log.Info("failed creating hostfirmwaresettings")
 				return actionError{errors.Wrap(err, "failed creating hostFirmwareSettings")}
 			}
+			if err = r.createHostFirmwareComponents(info); err != nil {
+				info.log.Info("failed creating hostfirmwarecomponents")
+				return actionError{errors.Wrap(err, "failed creating hostFirmwareComponents")}
+			}
 		}
 	}
 
@@ -1125,7 +1130,20 @@ func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner,
 		prepareData.TargetFirmwareSettings = hfs.Spec.Settings.DeepCopy()
 	}
 
-	provResult, started, err := prov.Prepare(prepareData, bmhDirty || hfsDirty,
+	// The hfcDirty flag is used to push the new versions of components to Ironic as part of the clean steps.
+	// The HFC Status field will be updated in the HostFirmwareComponentsReconciler when it reads the settings from Ironic.
+	// After manual cleaning is complete the HFC Spec should match the Status.
+	hfcDirty, hfc, err := r.getHostFirmwareComponents(info)
+
+	if err != nil {
+		// wait until hostFirmwareComponents are ready
+		return actionContinue{subResourceNotReadyRetryDelay}
+	}
+	if hfcDirty {
+		prepareData.TargetFirmwareComponents = hfc.Spec.Updates
+	}
+
+	provResult, started, err := prov.Prepare(prepareData, bmhDirty || hfsDirty || hfcDirty,
 		info.host.Status.ErrorType == metal3api.PreparationError)
 
 	if err != nil {
@@ -1498,6 +1516,36 @@ func saveHostProvisioningSettings(host *metal3api.BareMetalHost, info *reconcile
 	return
 }
 
+func (r *BareMetalHostReconciler) createHostFirmwareComponents(info *reconcileInfo) error {
+	// Check if HostFirmwareComponents already exists
+	hfc := &metal3api.HostFirmwareComponents{}
+	if err := r.Get(info.ctx, info.request.NamespacedName, hfc); err != nil {
+		if k8serrors.IsNotFound(err) {
+			// A resource doesn't exist, create one
+			hfc.ObjectMeta = metav1.ObjectMeta{
+				Name:      info.host.Name,
+				Namespace: info.host.Namespace}
+
+			hfc.Spec = metal3api.HostFirmwareComponentsSpec{
+				Updates: []metal3api.FirmwareUpdate{}}
+
+			// Set bmh as owner, this makes sure the resource is deleted when bmh is deleted
+			if err = controllerutil.SetControllerReference(info.host, hfc, r.Scheme()); err != nil {
+				return errors.Wrap(err, "could not set bmh as controller")
+			}
+			if err = r.Create(info.ctx, hfc); err != nil {
+				return errors.Wrap(err, "failure creating hostFirmwareComponents resource")
+			}
+
+			info.log.Info("created new hostFirmwareComponents resource")
+		} else {
+			// Error reading the object
+			return errors.Wrap(err, "could not load hostFirmwareComponents resource")
+		}
+	}
+	return nil
+}
+
 func (r *BareMetalHostReconciler) createHostFirmwareSettings(info *reconcileInfo) error {
 	// Check if HostFirmwareSettings already exists
 	hfs := &metal3api.HostFirmwareSettings{}
@@ -1562,6 +1610,45 @@ func (r *BareMetalHostReconciler) getHostFirmwareSettings(info *reconcileInfo) (
 	return false, nil, nil
 }
 
+// Get the stored firmware settings if there are valid changes.
+
+func (r *BareMetalHostReconciler) getHostFirmwareComponents(info *reconcileInfo) (dirty bool, hfc *metal3api.HostFirmwareComponents, err error) {
+	hfc = &metal3api.HostFirmwareComponents{}
+	if err = r.Get(info.ctx, info.request.NamespacedName, hfc); err != nil {
+		if !k8serrors.IsNotFound(err) {
+			// Error reading the object
+			return false, nil, errors.Wrap(err, "could not load host firmware components")
+		}
+
+		// Could not get settings, log it but don't return error as settings may not have been available at provisioner
+		info.log.Info("could not get hostFirmwareComponents", "namespacename", info.request.NamespacedName)
+		return false, nil, nil
+	}
+
+	// Check if there are Updates in the Spec that are different than the Status
+	if meta.IsStatusConditionTrue(hfc.Status.Conditions, string(metal3api.HostFirmwareComponentsChangeDetected)) {
+		// Check if the status have been populated
+		if len(hfc.Status.Updates) == 0 {
+			return false, nil, errors.New("host firmware status updates not available")
+		}
+
+		if len(hfc.Status.Components) == 0 {
+			return false, nil, errors.New("host firmware status components not available")
+		}
+
+		if meta.IsStatusConditionTrue(hfc.Status.Conditions, string(metal3api.HostFirmwareComponentsValid)) {
+			info.log.Info("hostFirmwareComponents indicating ChangeDetected", "namespacename", info.request.NamespacedName)
+			return true, hfc, nil
+		}
+
+		info.log.Info("hostFirmwareComponents not valid", "namespacename", info.request.NamespacedName)
+		return false, nil, nil
+	}
+
+	info.log.Info("hostFirmwareComponents no updates", "namespacename", info.request.NamespacedName)
+	return false, nil, nil
+}
+
 func (r *BareMetalHostReconciler) saveHostStatus(ctx context.Context, host *metal3api.BareMetalHost) error {
 	t := metav1.Now()
 	host.Status.LastUpdated = &t

pkg/provisioner/ironic/ironic.go

@@ -1165,10 +1165,10 @@ func (p *ironicProvisioner) GetFirmwareComponents() ([]metal3api.FirmwareCompone
 	// Get the components from Ironic via Gophercloud
 	componentList, componentListErr := nodes.ListFirmware(p.ctx, p.client, ironicNode.UUID).Extract()
 
-	if componentListErr != nil {
+	if componentListErr != nil || len(componentList) == 0 {
 		bmcAccess, _ := p.bmcAccess()
-		if bmcAccess.FirmwareInterface() == "no-firmware" {
-			return nil, fmt.Errorf("node %s is using firmware interface %s: %w", ironicNode.UUID, bmcAccess.FirmwareInterface(), componentListErr)
+		if ironicNode.FirmwareInterface == "no-firmware" {
+			return nil, fmt.Errorf("driver %s does not support firmware updates", bmcAccess.Driver())
 		}
 
 		return nil, fmt.Errorf("could not get firmware components for node %s: %w", ironicNode.UUID, componentListErr)
@@ -1434,6 +1434,28 @@ func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails
 		)
 	}
 
+	// extract to generate the updates that will trigger a clean step
+	newUpdates := make(map[string]string)
+	if data.TargetFirmwareComponents != nil {
+		for _, update := range data.TargetFirmwareComponents {
+			newUpdates[update.Component] = update.URL
+		}
+	}
+
+	if len(newUpdates) != 0 {
+		p.log.Info("Applying Firmware Update clean steps", "settings", newUpdates)
+		cleanSteps = append(
+			cleanSteps,
+			nodes.CleanStep{
+				Interface: nodes.InterfaceFirmware,
+				Step:      "update",
+				Args: map[string]interface{}{
+					"settings": newUpdates,
+				},
+			},
+		)
+	}
+
 	// TODO: Add manual cleaning steps for host configuration
 
 	return

pkg/provisioner/provisioner.go

@@ -99,12 +99,13 @@ type InspectData struct {
 // values are vendor specific.
 // TargetFirmwareSettings contains values that the user has changed.
 type PrepareData struct {
-	TargetRAIDConfig       *metal3api.RAIDConfig
-	ActualRAIDConfig       *metal3api.RAIDConfig
-	RootDeviceHints        *metal3api.RootDeviceHints
-	FirmwareConfig         *metal3api.FirmwareConfig
-	TargetFirmwareSettings metal3api.DesiredSettingsMap
-	ActualFirmwareSettings metal3api.SettingsMap
+	TargetRAIDConfig         *metal3api.RAIDConfig
+	ActualRAIDConfig         *metal3api.RAIDConfig
+	RootDeviceHints          *metal3api.RootDeviceHints
+	FirmwareConfig           *metal3api.FirmwareConfig
+	TargetFirmwareSettings   metal3api.DesiredSettingsMap
+	ActualFirmwareSettings   metal3api.SettingsMap
+	TargetFirmwareComponents []metal3api.FirmwareUpdate
 }
 
 type ProvisionData struct {