Handle HSM for HFC

controllers/metal3.io/baremetalhost_controller.go

@@ -1144,6 +1144,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
 	}
 
@@ -1181,6 +1182,11 @@ func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner,
 		return result
 	}
 
+	if hfc != nil {
+		info.log.Info("saving hostfirmwarecomponents updates into status")
+		r.saveHostFirmwareComponents(hfc, info)
+	}
+
 	return actionComplete{}
 }
 
@@ -1738,6 +1744,17 @@ func saveHostProvisioningSettings(host *metal3api.BareMetalHost, info *reconcile
 	return
 }
 
+func (r *BareMetalHostReconciler) saveHostFirmwareComponents(hfc *metal3api.HostFirmwareComponents, info *reconcileInfo) error {
+	if !reflect.DeepEqual(hfc.Status.Updates, hfc.Spec.Updates) {
+		info.log.Info("Updates for HostFirmwareComponents have changed")
+		hfc.Status.Updates = hfc.Spec.Updates
+		t := metav1.Now()
+		hfc.Status.LastUpdated = &t
+		r.Status().Update(info.ctx, hfc)
+	}
+	return nil
+}
+
 func (r *BareMetalHostReconciler) createHostFirmwareComponents(info *reconcileInfo) error {
 	// Check if HostFirmwareComponents already exists
 	hfc := &metal3api.HostFirmwareComponents{}
@@ -1847,13 +1864,11 @@ 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")
-		}
-
+		info.log.Info("debug - hfcDirty - change detected")
 		if len(hfc.Status.Components) == 0 {
 			return false, nil, errors.New("host firmware status components not available")
 		}

controllers/metal3.io/host_state_machine.go

@@ -481,6 +481,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

@@ -183,8 +183,6 @@ func (r *HostFirmwareComponentsReconciler) Reconcile(ctx context.Context, req ct
 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
 
@@ -198,6 +196,7 @@ func (r *HostFirmwareComponentsReconciler) updateHostFirmware(info *rhfcInfo, co
 	generation := info.hfc.GetGeneration()
 
 	if updatesMismatch {
+		info.log.Info("updatemismatch is true")
 		if setUpdatesCondition(generation, &newStatus, info, metal3api.HostFirmwareComponentsChangeDetected, metav1.ConditionTrue, reason, "") {
 			dirty = true
 		}
@@ -213,6 +212,7 @@ func (r *HostFirmwareComponentsReconciler) updateHostFirmware(info *rhfcInfo, co
 			dirty = true
 		}
 	} else {
+		info.log.Info("updatesmismatch is false")
 		if setUpdatesCondition(generation, &newStatus, info, metal3api.HostFirmwareComponentsValid, metav1.ConditionTrue, reason, "") {
 			dirty = true
 		}
@@ -223,9 +223,8 @@ func (r *HostFirmwareComponentsReconciler) updateHostFirmware(info *rhfcInfo, co
 
 	// 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)
@@ -296,8 +295,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

@@ -4,9 +4,13 @@ import (
 	"context"
 	"testing"
 
-	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	"github.com/stretchr/testify/assert"
 
+	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"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -27,20 +31,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.
@@ -169,12 +168,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",
@@ -403,7 +397,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",
@@ -418,6 +411,8 @@ 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"),
@@ -430,26 +425,13 @@ func TestStoreHostFirmwareComponents(t *testing.T) {
 			err = r.updateHostFirmware(info, components)
 			assert.NoError(t, err)
 
+			// r.saveHostFirmwareComponents(hfc, infoBMH)
 			// Check that resources get created or updated
 			key := client.ObjectKey{
 				Namespace: hfc.ObjectMeta.Namespace, Name: hfc.ObjectMeta.Name}
 			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)
 		})
 	}
 }

pkg/provisioner/fixture/fixture.go

@@ -63,6 +63,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
@@ -81,6 +85,8 @@ type Fixture struct {
 	customDeploy *metal3api.CustomDeploy
 
 	HostFirmwareSettings HostFirmwareSettingsMock
+
+	HostFirmwareComponents HostFirmwareComponentsMock
 }
 
 // NewProvisioner returns a new Fixture Provisioner.
@@ -358,7 +364,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) {

pkg/provisioner/ironic/ironic.go

@@ -1426,10 +1426,18 @@ func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails
 	}
 
 	// extract to generate the updates that will trigger a clean step
-	newUpdates := make(map[string]string)
+	// the format we send to ironic is:
+	// [{"component":"...", "url":"..."}, {"component":"...","url":".."}]
+	var newUpdates []map[string]string
+	p.log.Info("Evaluating TargetFirmwareComponents in data")
 	if data.TargetFirmwareComponents != nil {
 		for _, update := range data.TargetFirmwareComponents {
-			newUpdates[update.Component] = update.URL
+			newComponentUpdate := map[string]string{
+				"component": update.Component,
+				"url":       update.URL,
+			}
+			newUpdates = append(newUpdates, newComponentUpdate)
+			p.log.Info("Added TargetFirmwareComponents data to newUpdates")
 		}
 	}