Change hfs controller to be level-triggered when handling bmh updates

apis/metal3.io/v1alpha1/firmwareschema_types.go

@@ -18,6 +18,7 @@ package v1alpha1
 
 import (
 	"fmt"
+	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -71,6 +72,10 @@ func (schema *SettingSchema) Validate(name string, value intstr.IntOrString) err
 		return SchemaSettingError{name: name, message: "it is ReadOnly"}
 	}
 
+	if strings.Contains(name, "Password") {
+		return SchemaSettingError{name: name, message: "Password fields can't be set"}
+	}
+
 	// Check if valid based on type
 	switch schema.AttributeType {
 	case "Enumeration":

apis/metal3.io/v1alpha1/hostfirmwaresettings_types.go

@@ -37,19 +37,16 @@ type SettingsConditionType string
 
 const (
 	// Indicates that the settings in the Spec are different than Status
-	UpdateRequested SettingsConditionType = "UpdateRequested"
+	FirmwareSettingsChangeDetected SettingsConditionType = "ChangeDetected"
 
 	// Indicates if the settings are valid and can be configured on the host
-	SettingsValid SettingsConditionType = "Valid"
+	FirmwareSettingsValid SettingsConditionType = "Valid"
 )
 
 // HostFirmwareSettingsSpec defines the desired state of HostFirmwareSettings
 type HostFirmwareSettingsSpec struct {
 
 	// Settings are the desired firmware settings stored as name/value pairs.
-	// This will be populated with the actual firmware settings and only
-	// contain the settings that can be modified (i.e. not ReadOnly), to
-	// facilitate making changes.
 	// +patchStrategy=merge
 	Settings DesiredSettingsMap `json:"settings" required:"true"`
 }
@@ -61,9 +58,13 @@ type HostFirmwareSettingsStatus struct {
 	// Namespace as the settings but it can be overwritten in the Spec
 	FirmwareSchema *SchemaReference `json:"schema,omitempty"`
 
-	// Settings are the actual firmware settings stored as name/value pairs
+	// Settings are the firmware settings stored as name/value pairs
 	Settings SettingsMap `json:"settings" required:"true"`
 
+	// Time that the status was last updated
+	// +optional
+	LastUpdated *metav1.Time `json:"lastUpdated,omitempty"`
+
 	// Track whether settings stored in the spec are valid based on the schema
 	// +patchMergeKey=type
 	// +patchStrategy=merge

config/crd/bases/metal3.io_hostfirmwaresettings.yaml

@@ -46,9 +46,7 @@ spec:
                   - type: string
                   x-kubernetes-int-or-string: true
                 description: Settings are the desired firmware settings stored as
-                  name/value pairs. This will be populated with the actual firmware
-                  settings and only contain the settings that can be modified (i.e.
-                  not ReadOnly), to facilitate making changes.
+                  name/value pairs.
                 type: object
             required:
             - settings
@@ -131,6 +129,10 @@ spec:
                 x-kubernetes-list-map-keys:
                 - type
                 x-kubernetes-list-type: map
+              lastUpdated:
+                description: Time that the status was last updated
+                format: date-time
+                type: string
               schema:
                 description: FirmwareSchema is a reference to the Schema used to describe
                   each FirmwareSetting. By default, this will be a Schema in the same
@@ -150,7 +152,7 @@ spec:
               settings:
                 additionalProperties:
                   type: string
-                description: Settings are the actual firmware settings stored as name/value
+                description: Settings are the firmware settings stored as name/value
                   pairs
                 type: object
             required:

config/render/capm3.yaml

@@ -1243,9 +1243,7 @@ spec:
                   - type: string
                   x-kubernetes-int-or-string: true
                 description: Settings are the desired firmware settings stored as
-                  name/value pairs. This will be populated with the actual firmware
-                  settings and only contain the settings that can be modified (i.e.
-                  not ReadOnly), to facilitate making changes.
+                  name/value pairs.
                 type: object
             required:
             - settings
@@ -1328,6 +1326,10 @@ spec:
                 x-kubernetes-list-map-keys:
                 - type
                 x-kubernetes-list-type: map
+              lastUpdated:
+                description: Time that the status was last updated
+                format: date-time
+                type: string
               schema:
                 description: FirmwareSchema is a reference to the Schema used to describe
                   each FirmwareSetting. By default, this will be a Schema in the same
@@ -1347,7 +1349,7 @@ spec:
               settings:
                 additionalProperties:
                   type: string
-                description: Settings are the actual firmware settings stored as name/value
+                description: Settings are the firmware settings stored as name/value
                   pairs
                 type: object
             required:

controllers/metal3.io/baremetalhost_controller.go

@@ -56,6 +56,7 @@ const (
 	unmanagedRetryDelay           = time.Minute * 10
 	preprovImageRetryDelay        = time.Minute * 5
 	provisionerNotReadyRetryDelay = time.Second * 30
+	subResourceNotReadyRetryDelay = time.Second * 60
 	rebootAnnotationPrefix        = "reboot.metal3.io"
 	inspectAnnotationPrefix       = "inspect.metal3.io"
 	hardwareDetailsAnnotation     = inspectAnnotationPrefix + "/hardwaredetails"
@@ -808,6 +809,14 @@ func (r *BareMetalHostReconciler) registerHost(prov provisioner.Provisioner, inf
 		return result
 	}
 
+	// Create the hostFirmwareSettings resource with same host name/namespace if it doesn't exist
+	if info.host.Name != "" {
+		if err = r.createHostFirmwareSettings(info); err != nil {
+			info.log.Info("failed creating hostfirmwaresettings")
+			return actionError{errors.Wrap(err, "failed creating hostFirmwareSettings")}
+		}
+	}
+
 	// Reaching this point means the credentials are valid and worked,
 	// so clear any previous error and record the success in the
 	// status block.
@@ -944,7 +953,8 @@ func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner,
 	// Use settings in hostFirmwareSettings if available
 	hfs, err := r.getHostFirmwareSettings(info)
 	if err != nil {
-		info.log.Info("hostFirmwareSettings not available for cleaning")
+		// wait until hostFirmwareSettings are ready
+		return actionContinue{subResourceNotReadyRetryDelay}
 	}
 	if hfs != nil {
 		prepareData.ActualFirmwareSettings = hfs.Status.Settings.DeepCopy()
@@ -1318,6 +1328,38 @@ func saveHostProvisioningSettings(host *metal3v1alpha1.BareMetalHost) (dirty boo
 	return
 }
 
+func (r *BareMetalHostReconciler) createHostFirmwareSettings(info *reconcileInfo) error {
+
+	// Check if HostFirmwareSettings already exists
+	hfs := &metal3v1alpha1.HostFirmwareSettings{}
+	if err := r.Get(context.TODO(), info.request.NamespacedName, hfs); err != nil {
+		if k8serrors.IsNotFound(err) {
+			// A resource doesn't exist, create one
+			hfs.ObjectMeta = metav1.ObjectMeta{
+				Name:      info.host.Name,
+				Namespace: info.host.Namespace}
+			hfs.Status.Settings = make(metal3v1alpha1.SettingsMap)
+			hfs.Spec.Settings = make(metal3v1alpha1.DesiredSettingsMap)
+
+			// Set bmh as owner, this makes sure the resource is deleted when bmh is deleted
+			if err = controllerutil.SetControllerReference(info.host, hfs, r.Scheme()); err != nil {
+				return errors.Wrap(err, "could not set bmh as controller")
+			}
+			if err = r.Create(context.TODO(), hfs); err != nil {
+				return errors.Wrap(err, "failure creating hostFirmwareSettings resource")
+			}
+
+			info.log.Info("created new hostFirmwareSettings resource")
+
+		} else {
+			// Error reading the object
+			return errors.Wrap(err, "could not load hostFirmwareSettings resource")
+		}
+	}
+
+	return nil
+}
+
 // Get the stored firmware settings if there are valid changes
 func (r *BareMetalHostReconciler) getHostFirmwareSettings(info *reconcileInfo) (hfs *metal3v1alpha1.HostFirmwareSettings, err error) {
 
@@ -1335,9 +1377,9 @@ func (r *BareMetalHostReconciler) getHostFirmwareSettings(info *reconcileInfo) (
 	}
 
 	// Check if there are settings in the Spec that are different than the Status
-	if meta.IsStatusConditionTrue(hfs.Status.Conditions, string(metal3v1alpha1.UpdateRequested)) {
+	if meta.IsStatusConditionTrue(hfs.Status.Conditions, string(metal3v1alpha1.FirmwareSettingsChangeDetected)) {
 
-		if meta.IsStatusConditionTrue(hfs.Status.Conditions, string(metal3v1alpha1.SettingsValid)) {
+		if meta.IsStatusConditionTrue(hfs.Status.Conditions, string(metal3v1alpha1.FirmwareSettingsValid)) {
 			return hfs, nil
 		}