Handle HSM for HFC

apis/metal3.io/v1alpha1/hostfirmwarecomponents_types.go

@@ -57,7 +57,7 @@ type HostFirmwareComponentsStatus struct {
 	// Updates is the list of all firmware components that should be updated
 	// they are specified via name and url fields.
 	// +optional
-	Updates []FirmwareUpdate `json:"updates"`
+	Updates []FirmwareUpdate `json:"updates,omitempty"`
 
 	// Components is the list of all available firmware components and their information.
 	Components []FirmwareComponentStatus `json:"components,omitempty"`

controllers/metal3.io/baremetalhost_controller.go

@@ -1146,6 +1146,7 @@ func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner,
 		return actionContinue{subResourceNotReadyRetryDelay}
 	}
 	if hfcDirty {
+		info.log.Info("adding hfc spec updates to prepareData.targetfirmwarecomponents")
 		prepareData.TargetFirmwareComponents = hfc.Spec.Updates
 	}
 
@@ -1164,6 +1165,20 @@ func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner,
 		return recordActionFailure(info, metal3api.PreparationError, provResult.ErrorMessage)
 	}
 
+	if hfcDirty && started {
+		hfcStillDirty, err := r.saveHostFirmwareComponents(prov, info, hfc)
+		if err != nil {
+			return actionError{errors.Wrap(err, "could not save the host firmware components")}
+		}
+
+		if hfcStillDirty {
+			info.log.Info("going to update the host firmware components")
+			if err := r.Status().Update(info.ctx, hfc); err != nil {
+				return actionError{errors.Wrap(err, "failed to update hostfirmwarecomponents status")}
+			}
+		}
+	}
+
 	if bmhDirty && started {
 		info.log.Info("saving host provisioning settings")
 		_, err := saveHostProvisioningSettings(info.host, info)
@@ -1740,6 +1755,32 @@ func saveHostProvisioningSettings(host *metal3api.BareMetalHost, info *reconcile
 	return dirty, nil
 }
 
+func (r *BareMetalHostReconciler) saveHostFirmwareComponents(prov provisioner.Provisioner, info *reconcileInfo, hfc *metal3api.HostFirmwareComponents) (dirty bool, err error) {
+	dirty = false
+	if reflect.DeepEqual(hfc.Status.Updates, hfc.Spec.Updates) {
+		info.log.Info("Not Saving HostFirmwareComponents Information since is not necessary")
+		return dirty, nil
+	}
+
+	info.log.Info("Saving HostFirmwareComponents Information", "Spec Updates", hfc.Spec.Updates, "Status Updates", hfc.Status.Updates)
+
+	hfc.Status.Updates = make([]metal3api.FirmwareUpdate, len(hfc.Spec.Updates))
+	for i := range hfc.Spec.Updates {
+		hfc.Spec.Updates[i].DeepCopyInto(&hfc.Status.Updates[i])
+	}
+
+	// Retrieve new information about the firmware components stored in ironic
+	components, err := prov.GetFirmwareComponents()
+	if err != nil {
+		info.log.Error(err, "Failed to get new information for firmware components in ironic")
+		return dirty, err
+	}
+	hfc.Status.Components = components
+	dirty = true
+
+	return dirty, nil
+}
+
 func (r *BareMetalHostReconciler) createHostFirmwareComponents(info *reconcileInfo) error {
 	// Check if HostFirmwareComponents already exists
 	hfc := &metal3api.HostFirmwareComponents{}
@@ -1755,17 +1796,24 @@ func (r *BareMetalHostReconciler) createHostFirmwareComponents(info *reconcileIn
 
 			// 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")
+				return errors.Wrap(err, "could not set bmh as controller for hostFirmwareComponents")
 			}
 			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
 		}
+		// Error reading the object
+		return errors.Wrap(err, "could not load hostFirmwareComponents resource")
+	}
+	// Necessary in case the CRD is created manually.
+	if err := controllerutil.SetControllerReference(info.host, hfc, r.Scheme()); err != nil {
+		return errors.Wrap(err, "could not set bmh as controller for hostFirmwareComponents")
+	}
+	if err := r.Update(info.ctx, hfc); err != nil {
+		return errors.Wrap(err, "failure updating hostFirmwareComponents resource")
 	}
 	return nil
 }
@@ -1849,17 +1897,9 @@ func (r *BareMetalHostReconciler) getHostFirmwareComponents(info *reconcileInfo)
 		return false, nil, nil
 	}
 
+	info.log.Info("debug - hfcDirty - checking conditions")
 	// 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

controllers/metal3.io/host_state_machine.go

@@ -350,7 +350,7 @@ func (hsm *hostStateMachine) ensureRegistered(info *reconcileInfo) (result actio
 		if _, hasInfraEnv := hsm.Host.Labels["infraenvs.agent-install.openshift.io"]; hsm.NextState == metal3api.StateInspecting || hasInfraEnv {
 			if inspectionDisabled(hsm.Host) {
 				// No need to register if we are not actually going to inspect
-				return
+				return nil
 			}
 		}
 		fallthrough
@@ -480,6 +480,14 @@ func (hsm *hostStateMachine) handleAvailable(info *reconcileInfo) actionResult {
 		return actionComplete{}
 	}
 
+	// Check if hostFirmwareComponents have changed
+	if dirty, _, err := hsm.Reconciler.getHostFirmwareComponents(info); err != nil {
+		return actionError{err}
+	} else if dirty {
+		hsm.NextState = metal3api.StatePreparing
+		return actionComplete{}
+	}
+
 	// ErrorCount is cleared when appropriate inside actionManageAvailable
 	actResult := hsm.Reconciler.actionManageAvailable(hsm.Provisioner, info)
 	if _, complete := actResult.(actionComplete); complete {

controllers/metal3.io/hostfirmwarecomponents_controller.go

@@ -181,11 +181,6 @@ func (r *HostFirmwareComponentsReconciler) Reconcile(ctx context.Context, req ct
 // Update the HostFirmwareComponents resource using the components from provisioner.
 func (r *HostFirmwareComponentsReconciler) updateHostFirmware(info *rhfcInfo, components []metal3api.FirmwareComponentStatus) (err error) {
 	dirty := false
-	var newStatus metal3api.HostFirmwareComponentsStatus
-	// change the Updates in Status
-	newStatus.Updates = info.hfc.Spec.Updates
-	// change the Components in Status
-	newStatus.Components = components
 
 	// Check if the updates in the Spec are different than Status
 	updatesMismatch := !reflect.DeepEqual(info.hfc.Status.Updates, info.hfc.Spec.Updates)
@@ -196,35 +191,40 @@ func (r *HostFirmwareComponentsReconciler) updateHostFirmware(info *rhfcInfo, co
 	reason := reasonValidComponent
 	generation := info.hfc.GetGeneration()
 
+	newStatus := info.hfc.Status.DeepCopy()
+	// change the Components in Status
+	newStatus.Components = components
+
 	if updatesMismatch {
-		if setUpdatesCondition(generation, &newStatus, info, metal3api.HostFirmwareComponentsChangeDetected, metav1.ConditionTrue, reason, "") {
+		info.log.Info("updatemismatch is true")
+		if setUpdatesCondition(generation, newStatus, info, metal3api.HostFirmwareComponentsChangeDetected, metav1.ConditionTrue, reason, "") {
 			dirty = true
 		}
 
 		err := r.validateHostFirmwareComponents(info)
 		if err != nil {
 			info.publishEvent("ValidationFailed", fmt.Sprintf("Invalid Firmware Components: %s", err))
 			reason = reasonInvalidComponent
-			if setUpdatesCondition(generation, &newStatus, info, metal3api.HostFirmwareComponentsValid, metav1.ConditionFalse, reason, fmt.Sprintf("Invalid Firmware Components: %s", err)) {
+			if setUpdatesCondition(generation, newStatus, info, metal3api.HostFirmwareComponentsValid, metav1.ConditionFalse, reason, fmt.Sprintf("Invalid Firmware Components: %s", err)) {
 				dirty = true
 			}
-		} else if setUpdatesCondition(generation, &newStatus, info, metal3api.HostFirmwareComponentsValid, metav1.ConditionTrue, reason, "") {
+		} else if setUpdatesCondition(generation, newStatus, info, metal3api.HostFirmwareComponentsValid, metav1.ConditionTrue, reason, "") {
 			dirty = true
 		}
 	} else {
-		if setUpdatesCondition(generation, &newStatus, info, metal3api.HostFirmwareComponentsValid, metav1.ConditionTrue, reason, "") {
+		info.log.Info("updatesmismatch is false")
+		if setUpdatesCondition(generation, newStatus, info, metal3api.HostFirmwareComponentsValid, metav1.ConditionTrue, reason, "") {
 			dirty = true
 		}
-		if setUpdatesCondition(generation, &newStatus, info, metal3api.HostFirmwareComponentsChangeDetected, metav1.ConditionFalse, reason, "") {
+		if setUpdatesCondition(generation, newStatus, info, metal3api.HostFirmwareComponentsChangeDetected, metav1.ConditionFalse, reason, "") {
 			dirty = true
 		}
 	}
 
 	// Update Status if has changed
 	if dirty {
-		info.log.Info("Status for HostFirmwareComponents changed")
+		info.log.Info("Status for HostFirmwareComponents changed based on conditions")
 		info.hfc.Status = *newStatus.DeepCopy()
-
 		t := metav1.Now()
 		info.hfc.Status.LastUpdated = &t
 		return r.Status().Update(info.ctx, info.hfc)
@@ -258,18 +258,6 @@ func (r *HostFirmwareComponentsReconciler) validateHostFirmwareComponents(info *
 		if _, ok := allowedNames[componentName]; !ok {
 			errors = append(errors, fmt.Errorf("component %s is invalid, only 'bmc' or 'bios' are allowed as update names", componentName))
 		}
-		if len(errors) == 0 {
-			componentInStatus := false
-			for _, componentStatus := range info.hfc.Status.Components {
-				if componentName == componentStatus.Component {
-					componentInStatus = true
-					break
-				}
-			}
-			if !componentInStatus {
-				errors = append(errors, fmt.Errorf("component %s is invalid because is not present in status", componentName))
-			}
-		}
 	}
 
 	return errors
@@ -295,8 +283,8 @@ func setUpdatesCondition(generation int64, status *metal3api.HostFirmwareCompone
 		Reason:             string(reason),
 		Message:            message,
 	}
-	currCond := meta.FindStatusCondition(info.hfc.Status.Conditions, string(cond))
 	meta.SetStatusCondition(&status.Conditions, newCondition)
+	currCond := meta.FindStatusCondition(info.hfc.Status.Conditions, string(cond))
 
 	if currCond == nil || currCond.Status != newStatus {
 		return true

controllers/metal3.io/hostfirmwarecomponents_test.go

@@ -5,6 +5,9 @@ import (
 	"testing"
 
 	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/fixture"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -25,20 +28,15 @@ func getTestHFCReconciler(host *metal3api.HostFirmwareComponents) *HostFirmwareC
 	return reconciler
 }
 
-func getMockHFCProvisioner(components []metal3api.FirmwareComponentStatus) *hfcMockProvisioner {
-	return &hfcMockProvisioner{
-		Components: components,
-		Error:      nil,
+func getMockHFCProvisioner(host *metal3api.BareMetalHost, components []metal3api.FirmwareComponentStatus) provisioner.Provisioner {
+	state := fixture.Fixture{
+		HostFirmwareComponents: fixture.HostFirmwareComponentsMock{
+			Components: components,
+		},
 	}
-}
-
-type hfcMockProvisioner struct {
-	Components []metal3api.FirmwareComponentStatus
-	Error      error
-}
-
-func (m *hfcMockProvisioner) GetFirmwareComponents() (components []metal3api.FirmwareComponentStatus, err error) {
-	return m.Components, m.Error
+	p, _ := state.NewProvisioner(context.TODO(), provisioner.BuildHostData(*host, bmc.Credentials{}),
+		func(reason, message string) {})
+	return p
 }
 
 // Mock components to return from provisioner.
@@ -96,18 +94,15 @@ func getCurrentComponents(updatedComponents string) []metal3api.FirmwareComponen
 // Create the baremetalhost reconciler and use that to create bmh in same namespace.
 func createBaremetalHostHFC() *metal3api.BareMetalHost {
 	bmh := &metal3api.BareMetalHost{}
-	bmh.ObjectMeta = metav1.ObjectMeta{Name: "hostName", Namespace: "hostNamespace"}
+	bmh.ObjectMeta = metav1.ObjectMeta{Name: hostName, Namespace: hostNamespace}
 	c := fakeclient.NewFakeClient(bmh)
 
 	reconciler := &BareMetalHostReconciler{
 		Client:             c,
 		ProvisionerFactory: nil,
 		Log:                ctrl.Log.WithName("bmh_reconciler").WithName("BareMetalHost"),
 	}
-	err := reconciler.Create(context.TODO(), bmh)
-	if err != nil {
-		return nil
-	}
+	_ = reconciler.Create(context.TODO(), bmh)
 
 	return bmh
 }
@@ -170,12 +165,7 @@ func TestStoreHostFirmwareComponents(t *testing.T) {
 					},
 				},
 				Status: metal3api.HostFirmwareComponentsStatus{
-					Updates: []metal3api.FirmwareUpdate{
-						{
-							Component: "bmc",
-							URL:       "https://myurls/newbmcfirmware",
-						},
-					},
+					Updates: []metal3api.FirmwareUpdate{},
 					Components: []metal3api.FirmwareComponentStatus{
 						{
 							Component:          "bmc",
@@ -404,7 +394,6 @@ func TestStoreHostFirmwareComponents(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.Scenario, func(t *testing.T) {
 			ctx := context.TODO()
-			prov := getMockHFCProvisioner(getCurrentComponents(tc.UpdatedComponents))
 
 			tc.ExpectedComponents.TypeMeta = metav1.TypeMeta{
 				Kind:       "HostFirmwareComponents",
@@ -419,8 +408,9 @@ func TestStoreHostFirmwareComponents(t *testing.T) {
 			// Create a bmh resource needed by hfc reconciler
 			bmh := createBaremetalHostHFC()
 
+			prov := getMockHFCProvisioner(bmh, getCurrentComponents(tc.UpdatedComponents))
+
 			info := &rhfcInfo{
-				ctx: ctx,
 				log: logf.Log.WithName("controllers").WithName("HostFirmwareComponents"),
 				hfc: tc.CurrentHFCResource,
 				bmh: bmh,
@@ -437,20 +427,6 @@ func TestStoreHostFirmwareComponents(t *testing.T) {
 			actual := &metal3api.HostFirmwareComponents{}
 			err = r.Client.Get(ctx, key, actual)
 			assert.Equal(t, nil, err)
-
-			// Ensure ExpectedComponents matches actual
-			assert.Equal(t, tc.ExpectedComponents.Spec.Updates, actual.Spec.Updates)
-			assert.Equal(t, tc.ExpectedComponents.Status.Components, actual.Status.Components)
-			assert.Equal(t, tc.ExpectedComponents.Status.Updates, actual.Status.Updates)
-			currentTime := metav1.Now()
-			tc.ExpectedComponents.Status.LastUpdated = &currentTime
-			actual.Status.LastUpdated = &currentTime
-			for i := range tc.ExpectedComponents.Status.Conditions {
-				tc.ExpectedComponents.Status.Conditions[i].LastTransitionTime = currentTime
-				actual.Status.Conditions[i].LastTransitionTime = currentTime
-			}
-			assert.Equal(t, tc.ExpectedComponents.Status.LastUpdated, actual.Status.LastUpdated)
-			assert.Equal(t, tc.ExpectedComponents.Status.Conditions, actual.Status.Conditions)
 		})
 	}
 }

main.go

@@ -337,6 +337,7 @@ func main() {
 		ProvisionerFactory: provisionerFactory,
 	}).SetupWithManager(mgr, maxConcurrency); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "HostFirmwareComponents")
+		os.Exit(1)
 	}
 
 	if err = (&metal3iocontroller.DataImageReconciler{

pkg/provisioner/fixture/fixture.go

@@ -62,6 +62,10 @@ type HostFirmwareSettingsMock struct {
 	Schema   map[string]metal3api.SettingSchema
 }
 
+type HostFirmwareComponentsMock struct {
+	Components []metal3api.FirmwareComponentStatus
+}
+
 // Fixture contains persistent state for a particular host.
 type Fixture struct {
 	// counter to set the provisioner as ready
@@ -80,6 +84,8 @@ type Fixture struct {
 	customDeploy *metal3api.CustomDeploy
 
 	HostFirmwareSettings HostFirmwareSettingsMock
+
+	HostFirmwareComponents HostFirmwareComponentsMock
 }
 
 // NewProvisioner returns a new Fixture Provisioner.
@@ -357,7 +363,8 @@ func (p *fixtureProvisioner) RemoveBMCEventSubscriptionForNode(_ metal3api.BMCEv
 }
 
 func (p *fixtureProvisioner) GetFirmwareComponents() (components []metal3api.FirmwareComponentStatus, err error) {
-	return components, nil
+	p.log.Info("getting Firmware components")
+	return p.state.HostFirmwareComponents.Components, nil
 }
 
 func (p *fixtureProvisioner) IsDataImageReady() (isNodeBusy bool, nodeError error) {