From 5daafe7ff48d74208f9981184fdadb8585dae233 Mon Sep 17 00:00:00 2001 From: Bob Fournier Date: Mon, 31 May 2021 08:39:52 -0400 Subject: [PATCH 1/2] Add new CRD for BIOS configuration Add a new CRD for the FirmwareSettings for BIOS configuration as described in the spec - https://github.com/metal3-io/metal3-docs/pull/173. A new CRD is necessary since its not practical to include the number of settings in BareMetalHost. --- apis/metal3.io/v1alpha1/zz_generated.deepcopy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go index 44ab715ee5..b86df8312f 100644 --- a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go +++ b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go @@ -23,6 +23,7 @@ package v1alpha1 import ( "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. From 7fb1b6dc12744262eeb93fb177d64dc93988ceb8 Mon Sep 17 00:00:00 2001 From: Bob Fournier Date: Mon, 28 Jun 2021 14:28:27 -0400 Subject: [PATCH 2/2] Support for getting firmware settings and creating resources Add support in the provision to retrieve the BIOS settings from Ironic. Changes to baremetalhost_controller to get the settings and create resources for the hostFirmwareSettings and firmwareSchema. --- .../v1alpha1/firmwareschema_types.go | 115 +-- .../v1alpha1/hostfirmwaresettings_types.go | 18 + .../hostfirmwaresettings_types_test.go | 38 +- .../v1alpha1/zz_generated.deepcopy.go | 36 +- .../bases/metal3.io_hostfirmwaresettings.yaml | 16 + config/rbac/role.yaml | 40 ++ config/render/capm3.yaml | 56 ++ .../metal3.io/baremetalhost_controller.go | 118 ++++ .../metal3.io/host_state_machine_test.go | 36 +- .../hostfirmwaresettings_controller.go | 295 ++++++++ .../metal3.io/hostfirmwaresettings_test.go | 654 ++++++++++++++++++ docs/configuration.md | 2 +- main.go | 8 + pkg/provisioner/demo/demo.go | 6 + pkg/provisioner/fixture/fixture.go | 5 + pkg/provisioner/ironic/bios_test.go | 135 ++++ pkg/provisioner/ironic/clients/client.go | 4 +- pkg/provisioner/ironic/ironic.go | 103 ++- pkg/provisioner/ironic/provision_test.go | 241 +++++++ pkg/provisioner/ironic/testbmc/testbmc.go | 55 +- pkg/provisioner/ironic/testserver/ironic.go | 93 +++ pkg/provisioner/provisioner.go | 26 +- 22 files changed, 2019 insertions(+), 81 deletions(-) create mode 100644 controllers/metal3.io/hostfirmwaresettings_controller.go create mode 100644 controllers/metal3.io/hostfirmwaresettings_test.go create mode 100644 pkg/provisioner/ironic/bios_test.go diff --git a/apis/metal3.io/v1alpha1/firmwareschema_types.go b/apis/metal3.io/v1alpha1/firmwareschema_types.go index 58b0247ba3..c1f6477343 100644 --- a/apis/metal3.io/v1alpha1/firmwareschema_types.go +++ b/apis/metal3.io/v1alpha1/firmwareschema_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1alpha1 import ( + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -54,42 +56,19 @@ type SettingSchema struct { Unique *bool `json:"unique,omitempty"` } -// FirmwareSchemaSpec defines the desired state of FirmwareSchema -type FirmwareSchemaSpec struct { - - // The hardware vendor associated with this schema - // +optional - HardwareVendor string `json:"hardwareVendor,omitempty"` - - // The hardware model associated with this schema - // +optional - HardwareModel string `json:"hardwareModel,omitempty"` - - // Map of firmware name to schema - Schema map[string]SettingSchema `json:"schema" required:"true"` +type SchemaSettingError struct { + name string + message string } -//+kubebuilder:object:root=true - -// FirmwareSchema is the Schema for the firmwareschemas API -type FirmwareSchema struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec FirmwareSchemaSpec `json:"spec,omitempty"` +func (e SchemaSettingError) Error() string { + return fmt.Sprintf("Setting %s is invalid, %s", e.name, e.message) } -// Check whether the setting's name and value is valid using the schema -func (host *FirmwareSchema) CheckSettingIsValid(name string, value intstr.IntOrString, schemas map[string]SettingSchema) bool { - - schema, ok := schemas[name] - if !ok { - // The setting must exist in the status - return false - } +func (schema *SettingSchema) IsValid(name string, value intstr.IntOrString) error { if schema.ReadOnly != nil && *schema.ReadOnly == true { - return false + return SchemaSettingError{name: name, message: "it is ReadOnly"} } // Check if valid based on type @@ -97,42 +76,94 @@ func (host *FirmwareSchema) CheckSettingIsValid(name string, value intstr.IntOrS case "Enumeration": for _, av := range schema.AllowableValues { if value.String() == av { - return true + return nil } } - return false + return SchemaSettingError{name: name, message: fmt.Sprintf("unknown enumeration value - %s", value.String())} case "Integer": - if schema.LowerBound == nil || schema.UpperBound == nil { + if schema.LowerBound == nil && schema.UpperBound == nil { // return true if no settings to check validity - return true + return nil + } + if schema.LowerBound != nil && value.IntValue() < *schema.LowerBound { + return SchemaSettingError{name: name, message: fmt.Sprintf("integer %s is below range %d", value.String(), *schema.LowerBound)} + } + if schema.UpperBound != nil && value.IntValue() > *schema.UpperBound { + return SchemaSettingError{name: name, message: fmt.Sprintf("integer %s is above range %d", value.String(), *schema.UpperBound)} } - return (value.IntValue() >= *schema.LowerBound && value.IntValue() <= *schema.UpperBound) + return nil case "String": - if schema.MinLength == nil || schema.MaxLength == nil { + if schema.MinLength == nil && schema.MaxLength == nil { // return true if no settings to check validity - return true + return nil } - return (len(value.String()) >= *schema.MinLength && len(value.String()) <= *schema.MaxLength) + strLen := len(value.String()) + if schema.MinLength != nil && strLen < *schema.MinLength { + return SchemaSettingError{name: name, message: fmt.Sprintf("string %s length is below range %d", value.String(), *schema.MinLength)} + } + if schema.MaxLength != nil && strLen > *schema.MaxLength { + return SchemaSettingError{name: name, message: fmt.Sprintf("string %s length is above range %d", value.String(), *schema.MaxLength)} + } + return nil case "Boolean": - return (value.String() == "true" || value.String() == "false") + if value.String() == "true" || value.String() == "false" { + return nil + } + return SchemaSettingError{name: name, message: fmt.Sprintf("%s is not a boolean", value.String())} case "Password": // Prevent sets of password types - return false + return SchemaSettingError{name: name, message: "it is a Password type"} case "": // allow the set as BIOS registry fields may not have been available - return true + return nil default: // Unexpected attribute type - return false + return SchemaSettingError{name: name, message: fmt.Sprintf("unexpected attribute type %s", schema.AttributeType)} } } +// FirmwareSchemaSpec defines the desired state of FirmwareSchema +type FirmwareSchemaSpec struct { + + // The hardware vendor associated with this schema + // +optional + HardwareVendor string `json:"hardwareVendor,omitempty"` + + // The hardware model associated with this schema + // +optional + HardwareModel string `json:"hardwareModel,omitempty"` + + // Map of firmware name to schema + Schema map[string]SettingSchema `json:"schema" required:"true"` +} + +//+kubebuilder:object:root=true + +// FirmwareSchema is the Schema for the firmwareschemas API +type FirmwareSchema struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec FirmwareSchemaSpec `json:"spec,omitempty"` +} + +// Check whether the setting's name and value is valid using the schema +func (host *FirmwareSchema) CheckSettingIsValid(name string, value intstr.IntOrString, schemas map[string]SettingSchema) error { + + schema, ok := schemas[name] + if !ok { + return SchemaSettingError{name: name, message: "it is not in the associated schema"} + } + + return schema.IsValid(name, value) +} + //+kubebuilder:object:root=true // FirmwareSchemaList contains a list of FirmwareSchema diff --git a/apis/metal3.io/v1alpha1/hostfirmwaresettings_types.go b/apis/metal3.io/v1alpha1/hostfirmwaresettings_types.go index 1d95589958..7c229d172b 100644 --- a/apis/metal3.io/v1alpha1/hostfirmwaresettings_types.go +++ b/apis/metal3.io/v1alpha1/hostfirmwaresettings_types.go @@ -26,6 +26,13 @@ import ( type SettingsMap map[string]string type DesiredSettingsMap map[string]intstr.IntOrString +const ( + // strings for storing Annotations + ProvisionerIdAnnotation = "provisionierID" + HardwareVendorAnnotation = "hardwareVendor" + HardwareModelAnnotation = "hardwareModel" +) + type SchemaReference struct { // `namespace` is the namespace of the where the schema is stored. Namespace string `json:"namespace"` @@ -33,6 +40,14 @@ type SchemaReference struct { Name string `json:"name"` } +type ProvisioningInfo struct { + // Indicates that the hostfirmwaresettings_controller should read the settings and update resource + Update bool `json:"update"` + + // Stores the time that resource was last updated by hostfirmwaresettings_controller + LastUpdated *metav1.Time `json:"lastUpdate,omitempty"` +} + // HostFirmwareSettingsSpec defines the desired state of HostFirmwareSettings type HostFirmwareSettingsSpec struct { @@ -53,6 +68,9 @@ type HostFirmwareSettingsStatus struct { // Settings are the actual firmware settings stored as name/value pairs Settings SettingsMap `json:"settings" required:"true"` + + // Fields used to manage reading Firmware settings from provisioner + ProvStatus ProvisioningInfo `json:"provStatus,omitempty"` } //+kubebuilder:object:root=true diff --git a/apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go b/apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go index 6d743f1a17..38cfc550af 100644 --- a/apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go +++ b/apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go @@ -42,96 +42,100 @@ func TestCheckSettingIsValid(t *testing.T) { Scenario string Name string Value intstr.IntOrString - Expected bool + Expected string }{ { Scenario: "StringTypePass", Name: "AssetTag", Value: intstr.FromString("NewServer"), - Expected: true, + Expected: "", }, { Scenario: "StringTypeFailUpper", Name: "AssetTag", Value: intstr.FromString("NewServerPutInServiceIn2021"), - Expected: false, + Expected: "Setting AssetTag is invalid, string NewServerPutInServiceIn2021 length is above range 16", }, { Scenario: "StringTypeFailLower", Name: "AssetTag", Value: intstr.FromString(""), - Expected: false, + Expected: "Setting AssetTag is invalid, string length is below range 1", }, { Scenario: "EnumerationTypePass", Name: "ProcVirtualization", Value: intstr.FromString("Disabled"), - Expected: true, + Expected: "", }, { Scenario: "EnumerationTypeFail", Name: "ProcVirtualization", Value: intstr.FromString("Foo"), - Expected: false, + Expected: "Setting ProcVirtualization is invalid, unknown enumeration value - Foo", }, { Scenario: "IntegerTypePassAsString", Name: "NetworkBootRetryCount", Value: intstr.FromString("10"), - Expected: true, + Expected: "", }, { Scenario: "IntegerTypePassAsInt", Name: "NetworkBootRetryCount", Value: intstr.FromInt(10), - Expected: true, + Expected: "", }, { Scenario: "IntegerTypeFailUpper", Name: "NetworkBootRetryCount", Value: intstr.FromString("42"), - Expected: false, + Expected: "Setting NetworkBootRetryCount is invalid, integer 42 is above range 20", }, { Scenario: "IntegerTypeFailLower", Name: "NetworkBootRetryCount", Value: intstr.FromInt(0), - Expected: false, + Expected: "Setting NetworkBootRetryCount is invalid, integer 0 is below range 1", }, { Scenario: "BooleanTypePass", Name: "QuietBoot", Value: intstr.FromString("true"), - Expected: true, + Expected: "", }, { Scenario: "BooleanTypeFail", Name: "QuietBoot", Value: intstr.FromString("Enabled"), - Expected: false, + Expected: "Setting QuietBoot is invalid, Enabled is not a boolean", }, { Scenario: "ReadOnlyTypeFail", Name: "SerialNumber", Value: intstr.FromString("42"), - Expected: false, + Expected: "Setting SerialNumber is invalid, it is ReadOnly", }, { Scenario: "MissingEnumerationField", Name: "SriovEnable", Value: intstr.FromString("Disabled"), - Expected: true, + Expected: "", }, { Scenario: "UnknownSettingFail", Name: "IceCream", Value: intstr.FromString("Vanilla"), - Expected: false, + Expected: "Setting IceCream is invalid, it is not in the associated schema", }, } { t.Run(tc.Scenario, func(t *testing.T) { - actual := fwSchema.CheckSettingIsValid(tc.Name, tc.Value, fwSchema.Spec.Schema) - assert.Equal(t, tc.Expected, actual) + err := fwSchema.CheckSettingIsValid(tc.Name, tc.Value, fwSchema.Spec.Schema) + if err == nil { + assert.Equal(t, tc.Expected, "") + } else { + assert.Equal(t, tc.Expected, err.Error()) + } }) } } diff --git a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go index b86df8312f..eb6b5df1a8 100644 --- a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go +++ b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go @@ -23,7 +23,6 @@ package v1alpha1 import ( "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/intstr" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -584,6 +583,7 @@ func (in *HostFirmwareSettingsStatus) DeepCopyInto(out *HostFirmwareSettingsStat (*out)[key] = val } } + in.ProvStatus.DeepCopyInto(&out.ProvStatus) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HostFirmwareSettingsStatus. @@ -708,6 +708,25 @@ func (in *ProvisionStatus) DeepCopy() *ProvisionStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ProvisioningInfo) DeepCopyInto(out *ProvisioningInfo) { + *out = *in + if in.LastUpdated != nil { + in, out := &in.LastUpdated, &out.LastUpdated + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProvisioningInfo. +func (in *ProvisioningInfo) DeepCopy() *ProvisioningInfo { + if in == nil { + return nil + } + out := new(ProvisioningInfo) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RAIDConfig) DeepCopyInto(out *RAIDConfig) { *out = *in @@ -787,6 +806,21 @@ func (in *SchemaReference) DeepCopy() *SchemaReference { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SchemaSettingError) DeepCopyInto(out *SchemaSettingError) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SchemaSettingError. +func (in *SchemaSettingError) DeepCopy() *SchemaSettingError { + if in == nil { + return nil + } + out := new(SchemaSettingError) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SettingSchema) DeepCopyInto(out *SettingSchema) { *out = *in diff --git a/config/crd/bases/metal3.io_hostfirmwaresettings.yaml b/config/crd/bases/metal3.io_hostfirmwaresettings.yaml index b738802e5c..7299666e08 100644 --- a/config/crd/bases/metal3.io_hostfirmwaresettings.yaml +++ b/config/crd/bases/metal3.io_hostfirmwaresettings.yaml @@ -55,6 +55,22 @@ spec: description: HostFirmwareSettingsStatus defines the observed state of HostFirmwareSettings properties: + provStatus: + description: Fields used to manage reading Firmware settings from + provisioner + properties: + lastUpdate: + description: Stores the time that resource was last updated by + hostfirmwaresettings_controller + format: date-time + type: string + update: + description: Indicates that the hostfirmwaresettings_controller + should read the settings and update resource + type: boolean + required: + - update + type: object schema: description: FirmwareSchema is a reference to the Schema used to describe each FirmwareSetting. By default, this will be a Schema in the same diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index a98be9ed33..27b97fa4e4 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -46,3 +46,43 @@ rules: - get - patch - update +- apiGroups: + - metal3.io + resources: + - firmwareschemas + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - metal3.io + resources: + - firmwareschemas/status + verbs: + - get + - patch + - update +- apiGroups: + - metal3.io + resources: + - hostfirmwaresettings + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - metal3.io + resources: + - hostfirmwaresettings/status + verbs: + - get + - patch + - update diff --git a/config/render/capm3.yaml b/config/render/capm3.yaml index 9fca532b60..379b2c6149 100644 --- a/config/render/capm3.yaml +++ b/config/render/capm3.yaml @@ -1237,6 +1237,22 @@ spec: description: HostFirmwareSettingsStatus defines the observed state of HostFirmwareSettings properties: + provStatus: + description: Fields used to manage reading Firmware settings from + provisioner + properties: + lastUpdate: + description: Stores the time that resource was last updated by + hostfirmwaresettings_controller + format: date-time + type: string + update: + description: Indicates that the hostfirmwaresettings_controller + should read the settings and update resource + type: boolean + required: + - update + type: object schema: description: FirmwareSchema is a reference to the Schema used to describe each FirmwareSetting. By default, this will be a Schema in the same @@ -1365,6 +1381,46 @@ rules: - get - patch - update +- apiGroups: + - metal3.io + resources: + - firmwareschemas + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - metal3.io + resources: + - firmwareschemas/status + verbs: + - get + - patch + - update +- apiGroups: + - metal3.io + resources: + - hostfirmwaresettings + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - metal3.io + resources: + - hostfirmwaresettings/status + verbs: + - get + - patch + - update --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go index 51e8447d7a..febb4703e7 100644 --- a/controllers/metal3.io/baremetalhost_controller.go +++ b/controllers/metal3.io/baremetalhost_controller.go @@ -89,6 +89,10 @@ func (info *reconcileInfo) publishEvent(reason, message string) { // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;update // +kubebuilder:rbac:groups="",resources=events,verbs=get;list;watch;create;update;patch +// Allow for managing hostfirmwaresettings and firmwareschema +//+kubebuilder:rbac:groups=metal3.io,resources=hostfirmwaresettings,verbs=get;list;watch;create;update;patch +//+kubebuilder:rbac:groups=metal3.io,resources=firmwareschemas,verbs=get;list;watch;create;update;patch + // Reconcile handles changes to BareMetalHost resources func (r *BareMetalHostReconciler) Reconcile(ctx context.Context, request ctrl.Request) (result ctrl.Result, err error) { @@ -717,6 +721,11 @@ func (r *BareMetalHostReconciler) actionInspecting(prov provisioner.Provisioner, return result } + // Indicate that the hostFirmwareSettings resource should be updated by its controller + if err = r.markHostFirmwareSettingsForUpdate(info); err != nil { + info.log.Error(err, "could not hostFirmwareSettings resource for update") + } + clearError(info.host) info.host.Status.HardwareDetails = details return actionComplete{} @@ -786,7 +795,20 @@ func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner, prepareData.ActualRAIDConfig = nil dirty = true } + + // Use settings in hostFirmwareSettings if available + hfs, err := r.getValidHostFirmwareSettings(info) + if err != nil { + // Return error if settings available but invalid + return actionError{errors.Wrap(err, "error with hostFirmwareSettings")} + } + if hfs != nil { + prepareData.ActualFirmwareSettings = hfs.Status.Settings.DeepCopy() + prepareData.TargetFirmwareSettings = hfs.Spec.Settings.DeepCopy() + } + provResult, started, err := prov.Prepare(prepareData, dirty) + if err != nil { return actionError{errors.Wrap(err, "error preparing host")} } @@ -815,6 +837,11 @@ func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner, return result } + // Indicate that the hostFirmwareSettings resource should be updated by its controller + if err = r.markHostFirmwareSettingsForUpdate(info); err != nil { + info.log.Error(err, "could not hostFirmwareSettings resource for update") + } + return actionComplete{} } @@ -1153,6 +1180,97 @@ func saveHostProvisioningSettings(host *metal3v1alpha1.BareMetalHost) (dirty boo return } +// Get the stored firmware settings and validate the Spec settings against the schema +func (r *BareMetalHostReconciler) getValidHostFirmwareSettings(info *reconcileInfo) (hfs *metal3v1alpha1.HostFirmwareSettings, err error) { + + hfs = &metal3v1alpha1.HostFirmwareSettings{} + if err = r.Get(context.TODO(), info.request.NamespacedName, hfs); err != nil { + + if !k8serrors.IsNotFound(err) { + // Error reading the object + return nil, errors.Wrap(err, "could not load host firmware settings") + } + + // 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 hostFirmwareSettings", "namespacename", info.request.NamespacedName) + return nil, nil + } + + schema := &metal3v1alpha1.FirmwareSchema{} + if hfs.Status.FirmwareSchema != nil { + if err = r.Get(context.TODO(), client.ObjectKey{ + Namespace: hfs.Status.FirmwareSchema.Namespace, + Name: hfs.Status.FirmwareSchema.Name}, schema); err != nil { + + info.log.Error(err, "failed to get schema for hostFirmwareSettings") + return nil, err + } + } else { + // No schema is available + schema = nil + } + + // Validate the Spec setting using schema, if available + if err = ValidateHostFirmwareSettings(hfs, schema); err != nil { + return nil, err + } + + return hfs, nil +} + +// Get or create HostFirmwareSettings resource and indicate that its controller should get Firmware Settings from provisioner +func (r *BareMetalHostReconciler) markHostFirmwareSettingsForUpdate(info *reconcileInfo) (err error) { + + // Check if there is a hostFirmwareSettings resource for this host + hfs := &metal3v1alpha1.HostFirmwareSettings{} + if err = r.Get(context.TODO(), info.request.NamespacedName, hfs); err == nil { + info.log.Info("found existing hostFirmwareSettings resource") + } else if !k8serrors.IsNotFound(err) { + // Error reading the object + return errors.Wrap(err, "could not load host firmware settings") + } else { + + return r.newHostFirmwareSettings(info) + } + + // Set flag for hostFirmwareSetting_controller to update settings + if hfs.Status.ProvStatus.Update == false { + hfs.Status.ProvStatus.Update = true + r.Status().Update(context.TODO(), hfs) + info.log.Info("set hostFirmwareSettings resource update flag") + } + + return nil +} + +func (r *BareMetalHostReconciler) newHostFirmwareSettings(info *reconcileInfo) (err error) { + + hfs := &metal3v1alpha1.HostFirmwareSettings{} + // Create an empty hostFirmwareSetting with same name and namespace as host + hfs.ObjectMeta = metav1.ObjectMeta{ + Name: info.host.ObjectMeta.Name, + Namespace: info.host.ObjectMeta.Namespace} + hfs.Status.Settings = make(metal3v1alpha1.SettingsMap) + hfs.Spec.Settings = make(metal3v1alpha1.DesiredSettingsMap) + hfs.Annotations = make(map[string]string) + // Store info in annotation that will persist if settings is cleared during pivot + // Provisioner ID is needed to access Ironic API + hfs.Annotations[metal3v1alpha1.ProvisionerIdAnnotation] = info.host.Status.Provisioning.ID + // Hardware info is optional + if info.host.Status.HardwareDetails != nil { + hfs.Annotations[metal3v1alpha1.HardwareVendorAnnotation] = info.host.Status.HardwareDetails.SystemVendor.Manufacturer + hfs.Annotations[metal3v1alpha1.HardwareModelAnnotation] = info.host.Status.HardwareDetails.SystemVendor.ProductName + } + hfs.Status.ProvStatus.Update = true + + if err = r.Create(context.TODO(), hfs); err != nil { + return errors.Wrap(err, "could not create hostFirmwareSettings resource") + } + info.log.Info("created new hostFirmwareSettings resource") + + return nil +} + func (r *BareMetalHostReconciler) saveHostStatus(host *metal3v1alpha1.BareMetalHost) error { t := metav1.Now() host.Status.LastUpdated = &t diff --git a/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go index 70ac5a90ed..476e8badf2 100644 --- a/controllers/metal3.io/host_state_machine_test.go +++ b/controllers/metal3.io/host_state_machine_test.go @@ -16,6 +16,7 @@ import ( promutil "github.com/prometheus/client_golang/prometheus/testutil" corev1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" logf "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -26,6 +27,19 @@ func testStateMachine(host *metal3v1alpha1.BareMetalHost) *hostStateMachine { return newHostStateMachine(host, r, p, true) } +// Create a reconciler with a fake client to satisfy states that use the client +func testNewReconciler(host *metal3v1alpha1.BareMetalHost) *BareMetalHostReconciler { + + c := fakeclient.NewFakeClient(host) + reconciler := &BareMetalHostReconciler{ + Client: c, + ProvisionerFactory: nil, + Log: ctrl.Log.WithName("host_state_machine").WithName("BareMetalHost"), + } + + return reconciler +} + func TestProvisioningCapacity(t *testing.T) { testCases := []struct { Scenario string @@ -124,7 +138,8 @@ func TestProvisioningCapacity(t *testing.T) { t.Run(tc.Scenario, func(t *testing.T) { prov := newMockProvisioner() prov.setHasCapacity(tc.HasProvisioningCapacity) - hsm := newHostStateMachine(tc.Host, &BareMetalHostReconciler{}, prov, true) + reconciler := testNewReconciler(tc.Host) + hsm := newHostStateMachine(tc.Host, reconciler, prov, true) info := makeDefaultReconcileInfo(tc.Host) delayedProvisioningHostCounters.Reset() @@ -178,7 +193,8 @@ func TestDeprovisioningCapacity(t *testing.T) { t.Run(tc.Scenario, func(t *testing.T) { prov := newMockProvisioner() prov.setHasCapacity(tc.HasDeprovisioningCapacity) - hsm := newHostStateMachine(tc.Host, &BareMetalHostReconciler{}, prov, true) + reconciler := testNewReconciler(tc.Host) + hsm := newHostStateMachine(tc.Host, reconciler, prov, true) info := makeDefaultReconcileInfo(tc.Host) delayedDeprovisioningHostCounters.Reset() @@ -363,7 +379,8 @@ func TestDetach(t *testing.T) { } } prov := newMockProvisioner() - hsm := newHostStateMachine(tc.Host, &BareMetalHostReconciler{}, prov, true) + reconciler := testNewReconciler(tc.Host) + hsm := newHostStateMachine(tc.Host, reconciler, prov, true) info := makeDefaultReconcileInfo(tc.Host) result := hsm.ReconcileState(info) @@ -405,7 +422,8 @@ func TestDetachError(t *testing.T) { metal3v1alpha1.DetachedAnnotation: "true", } prov := newMockProvisioner() - hsm := newHostStateMachine(tc.Host, &BareMetalHostReconciler{}, prov, true) + reconciler := testNewReconciler(tc.Host) + hsm := newHostStateMachine(tc.Host, reconciler, prov, true) info := makeDefaultReconcileInfo(tc.Host) prov.setNextError("Detach", "some error") @@ -853,7 +871,8 @@ func TestErrorCountIncreasedOnActionFailure(t *testing.T) { for _, tt := range tests { t.Run(tt.Scenario, func(t *testing.T) { prov := newMockProvisioner() - hsm := newHostStateMachine(tt.Host, &BareMetalHostReconciler{}, prov, true) + reconciler := testNewReconciler(tt.Host) + hsm := newHostStateMachine(tt.Host, reconciler, prov, true) info := makeDefaultReconcileInfo(tt.Host) prov.setNextError(tt.ProvisionerErrorOn, "some error") @@ -913,7 +932,8 @@ func TestErrorCountClearedOnStateTransition(t *testing.T) { for _, tt := range tests { t.Run(tt.Scenario, func(t *testing.T) { prov := newMockProvisioner() - hsm := newHostStateMachine(tt.Host, &BareMetalHostReconciler{}, prov, true) + reconciler := testNewReconciler(tt.Host) + hsm := newHostStateMachine(tt.Host, reconciler, prov, true) info := makeDefaultReconcileInfo(tt.Host) info.host.Status.ErrorCount = 1 @@ -1187,6 +1207,10 @@ func (m *mockProvisioner) IsReady() (result bool, err error) { return } +func (m *mockProvisioner) GetFirmwareSettings(includeSchema bool) (settings metal3v1alpha1.SettingsMap, schema map[string]metal3v1alpha1.SettingSchema, err error) { + return +} + func TestUpdateBootModeStatus(t *testing.T) { testCases := []struct { Scenario string diff --git a/controllers/metal3.io/hostfirmwaresettings_controller.go b/controllers/metal3.io/hostfirmwaresettings_controller.go new file mode 100644 index 0000000000..8fc6f9d625 --- /dev/null +++ b/controllers/metal3.io/hostfirmwaresettings_controller.go @@ -0,0 +1,295 @@ +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "crypto/sha256" + "fmt" + "sort" + "strings" + + "github.com/go-logr/logr" + "github.com/pkg/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" + "github.com/metal3-io/baremetal-operator/pkg/provisioner" +) + +// HostFirmwareSettingsReconciler reconciles a HostFirmwareSettings object +type HostFirmwareSettingsReconciler struct { + client.Client + Log logr.Logger + ProvisionerFactory provisioner.Factory +} + +type rInfo struct { + log logr.Logger + hfs *metal3v1alpha1.HostFirmwareSettings +} + +//+kubebuilder:rbac:groups=metal3.io,resources=hostfirmwaresettings,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=metal3.io,resources=hostfirmwaresettings/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=metal3.io,resources=firmwareschemas,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=metal3.io,resources=firmwareschemas/status,verbs=get;update;patch + +// Reconcile is part of the main kubernetes reconciliation loop which aims to +// move the current state of the cluster closer to the desired state. +// +// For more details, check Reconcile and its Result here: +// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.6.4/pkg/reconcile +func (r *HostFirmwareSettingsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { + + reqLogger := r.Log.WithValues("hostfirmwaresettings", req.NamespacedName) + reqLogger.Info("start") + + // Fetch the HostFirmwareSettings + hfs := &metal3v1alpha1.HostFirmwareSettings{} + if err = r.Get(ctx, req.NamespacedName, hfs); err != nil { + if k8serrors.IsNotFound(err) { + // Request object not found, could have been deleted after + // reconcile request. Return and don't requeue + reqLogger.Info("could not find host firmware settings") + return ctrl.Result{}, nil + } + // Error reading the object - requeue the request. + return ctrl.Result{}, errors.Wrap(err, "could not load host firmware settings") + } + + // Get settings from provisioner if update requested or settings are empty + if hfs.Status.ProvStatus.Update == true || hfs.Status.Settings == nil { + + // Use the provisioner ID stored in annotation to access Ironic API + if _, ok := hfs.Annotations[metal3v1alpha1.ProvisionerIdAnnotation]; ok { + provID := hfs.Annotations[metal3v1alpha1.ProvisionerIdAnnotation] + + info := &rInfo{log: reqLogger, hfs: hfs} + + prov, err := r.ProvisionerFactory.NewProvisioner(provisioner.BuildEmptyHostData(provID), func(reason, message string) {}) + if err != nil { + return ctrl.Result{}, errors.Wrap(err, "failed to create provisioner") + } + + if err = r.updateHostFirmwareSettings(prov, info); err != nil { + return ctrl.Result{}, errors.Wrap(err, "Could not update hostFirmwareSettings") + } + } else { + // This is unexpected but retrying won't help + reqLogger.Info("could not get provisioner ID from annotation") + } + } + + // only run again on changes + return ctrl.Result{}, nil +} + +// Get the firmware settings from the provisioner and update hostFirmwareSettings +func (r *HostFirmwareSettingsReconciler) updateHostFirmwareSettings(prov provisioner.Provisioner, info *rInfo) (err error) { + + info.log.Info("retrieving firmware settings and saving to resource") + + // Get the current settings and schema + currentSettings, schema, err := prov.GetFirmwareSettings(true) + if err != nil { + return errors.Wrap(err, "could not get firmware settings from provisioner") + } + + // get or create a firmwareSchema to hold schema + firmwareSchema, err := r.getOrCreateFirmwareSchema(info, schema) + if err != nil { + return errors.Wrap(err, "could not get/create firmware schema") + } + + // Set hostFirmwareSetting to use this schema + info.hfs.Status.FirmwareSchema = &metal3v1alpha1.SchemaReference{ + Namespace: firmwareSchema.ObjectMeta.Namespace, + Name: firmwareSchema.ObjectMeta.Name} + + if err = r.updateResource(info, currentSettings, firmwareSchema); err != nil { + return errors.Wrap(err, "could not update hostFirmwareSettings") + } + + return nil +} + +// Update the HostFirmwareSettings resource using the settings and schema from provisioner +func (r *HostFirmwareSettingsReconciler) updateResource(info *rInfo, settings metal3v1alpha1.SettingsMap, schema *metal3v1alpha1.FirmwareSchema) (err error) { + + if info.hfs.Status.Settings == nil { + info.hfs.Status.Settings = make(metal3v1alpha1.SettingsMap) + } + + // Update Spec and Status + specDirty := false + for k, v := range settings { + // Some vendors include encrypted password fields, don't add these + if strings.Contains(k, "Password") { + continue + } + info.hfs.Status.Settings[k] = v + + // Don't include setting in Spec if it cannot be changed (ReadOnly is set) + // or if it unique to this particular host, in order to prevent the unique + // settings from being copied to other hosts + if schema != nil { + if schemaSetting, ok := schema.Spec.Schema[k]; ok { + if (schemaSetting.ReadOnly != nil && *schemaSetting.ReadOnly == true) || + (schemaSetting.Unique != nil && *schemaSetting.Unique == true) { + continue + } + } + } + // Copy to Spec only if doesn't exist as setting may have been set by user + if _, ok := info.hfs.Spec.Settings[k]; !ok { + info.hfs.Spec.Settings[k] = intstr.FromString(v) + specDirty = true + } + } + + if specDirty { + if err = r.Update(context.TODO(), info.hfs); err != nil { + return errors.Wrap(err, "could not update host firmware settings") + } + } + + info.hfs.Status.ProvStatus.Update = false + t := metav1.Now() + info.hfs.Status.ProvStatus.LastUpdated = &t + info.log.Info("clearing hostFirmwareSettings update flag after updating resource") + + return r.Status().Update(context.TODO(), info.hfs) +} + +// Get a firmware schema that matches the host vendor or create one if it doesn't exist +func (r *HostFirmwareSettingsReconciler) getOrCreateFirmwareSchema(info *rInfo, schema map[string]metal3v1alpha1.SettingSchema) (fSchema *metal3v1alpha1.FirmwareSchema, err error) { + + info.log.Info("getting firmwareSchema") + + schemaName := GetSchemaName(schema) + firmwareSchema := &metal3v1alpha1.FirmwareSchema{} + + // If a schema exists that matches, use that, otherwise create a new one + if err = r.Get(context.TODO(), client.ObjectKey{Namespace: info.hfs.ObjectMeta.Namespace, Name: schemaName}, + firmwareSchema); err == nil { + + info.log.Info("found existing firmwareSchema resource") + + return firmwareSchema, nil + } + if !k8serrors.IsNotFound(err) { + // Error reading the object + return nil, err + + } + + firmwareSchema = &metal3v1alpha1.FirmwareSchema{ + ObjectMeta: metav1.ObjectMeta{ + Name: schemaName, + Namespace: info.hfs.ObjectMeta.Namespace, + }, + } + + // If available, store hardware details in schema for additional info + if _, ok := info.hfs.Annotations[metal3v1alpha1.HardwareVendorAnnotation]; ok { + firmwareSchema.Spec.HardwareVendor = info.hfs.Annotations[metal3v1alpha1.HardwareVendorAnnotation] + } + if _, ok := info.hfs.Annotations[metal3v1alpha1.HardwareModelAnnotation]; ok { + firmwareSchema.Spec.HardwareModel = info.hfs.Annotations[metal3v1alpha1.HardwareModelAnnotation] + } + + // Copy in the schema from provisioner + firmwareSchema.Spec.Schema = make(map[string]metal3v1alpha1.SettingSchema) + for k, v := range schema { + firmwareSchema.Spec.Schema[k] = v + } + + if err = r.Create(context.TODO(), firmwareSchema); err != nil { + return nil, err + } + // Set hfs as owner after the create + if controllerutil.SetOwnerReference(info.hfs, firmwareSchema, r.Scheme()); err != nil { + return nil, errors.Wrap(err, "could not set owner of firmwareSchema") + } + if err = r.Update(context.TODO(), firmwareSchema); err != nil { + return nil, err + } + + info.log.Info("created new firmwareSchema resource") + + return firmwareSchema, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *HostFirmwareSettingsReconciler) SetupWithManager(mgr ctrl.Manager) error { + + return ctrl.NewControllerManagedBy(mgr). + For(&metal3v1alpha1.HostFirmwareSettings{}). + Complete(r) +} + +// Validate the HostFirmwareSetting Spec +func ValidateHostFirmwareSettings(hfs *metal3v1alpha1.HostFirmwareSettings, schema *metal3v1alpha1.FirmwareSchema) error { + + if hfs == nil { + return fmt.Errorf("Missing parameter to ValidateHostFirmwareSettings") + } + + for name, val := range hfs.Spec.Settings { + // Prohibit any Spec settings with "Password" + if strings.Contains(name, "Password") { + return fmt.Errorf("Cannot set Password field") + } + + // The setting must be in the Status + if _, ok := hfs.Status.Settings[name]; !ok { + return fmt.Errorf("Setting %s is not in the Status field", name) + } + + // check validity of updated value + if schema != nil { + if err := schema.CheckSettingIsValid(name, val, schema.Spec.Schema); err != nil { + return err + } + } + } + + return nil +} + +// Generate a name based on the schema keys which should be the same for similar hardware +func GetSchemaName(schema map[string]metal3v1alpha1.SettingSchema) string { + + // Schemas from the same vendor and model should be identical for both keys and values. + // Hash the keys of the map to get a identifier unique to this schema. + keys := make([]string, 0, len(schema)) + for k := range schema { + keys = append(keys, k) + } + sort.Strings(keys) + + h := sha256.New() + h.Write([]byte(fmt.Sprintf("%v", keys))) + hash := fmt.Sprintf("%x", h.Sum(nil))[:8] + + return "schema-" + hash +} diff --git a/controllers/metal3.io/hostfirmwaresettings_test.go b/controllers/metal3.io/hostfirmwaresettings_test.go new file mode 100644 index 0000000000..73d4a17aae --- /dev/null +++ b/controllers/metal3.io/hostfirmwaresettings_test.go @@ -0,0 +1,654 @@ +package controllers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" + "github.com/metal3-io/baremetal-operator/pkg/provisioner" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +const ( + hostName string = "myHostName" + hostNamespace string = "myHostNamespace" + schemaName string = "schema-17e7ebad" // Hash generated from schema, change this if the schema is changed +) + +var ( + iTrue bool = true + iFalse bool = false + minLength int = 0 + maxLength int = 20 + lowerBound int = 0 + upperBound int = 20 +) + +// Test support for HostFirmwareSettings in the BareMetalHostReconciler +func getTestHostReconciler(host *metal3v1alpha1.BareMetalHost) *BareMetalHostReconciler { + + c := fakeclient.NewFakeClient(host) + reconciler := &BareMetalHostReconciler{ + Client: c, + ProvisionerFactory: nil, + Log: ctrl.Log.WithName("host_state_machine").WithName("BareMetalHost"), + } + + return reconciler +} + +// Test support for HostFirmwareSettings in the HostFirmwareSettingsReconciler +func getTestHFSReconciler(host *metal3v1alpha1.HostFirmwareSettings) *HostFirmwareSettingsReconciler { + + c := fakeclient.NewFakeClient(host) + reconciler := &HostFirmwareSettingsReconciler{ + Client: c, + Log: ctrl.Log.WithName("test_reconciler").WithName("HostFirmwareSettings"), + } + + return reconciler +} + +func getDefaultHostReconcileInfo(host *metal3v1alpha1.BareMetalHost, name string, namespace string) *reconcileInfo { + r := &reconcileInfo{ + log: logf.Log.WithName("controllers").WithName("HostFirmwareSettings"), + host: host, + request: ctrl.Request{}, + } + r.request.NamespacedName = types.NamespacedName{Namespace: namespace, Name: name} + + return r +} + +func getMockProvisioner(settings metal3v1alpha1.SettingsMap, schema map[string]metal3v1alpha1.SettingSchema) *hsfMockProvisioner { + return &hsfMockProvisioner{ + Settings: settings, + Schema: schema, + Error: nil, + } +} + +type hsfMockProvisioner struct { + Settings metal3v1alpha1.SettingsMap + Schema map[string]metal3v1alpha1.SettingSchema + Error error +} + +func (m *hsfMockProvisioner) HasCapacity() (result bool, err error) { + return +} + +func (m *hsfMockProvisioner) ValidateManagementAccess(data provisioner.ManagementAccessData, credentialsChanged, force bool) (result provisioner.Result, provID string, err error) { + return +} + +func (m *hsfMockProvisioner) InspectHardware(data provisioner.InspectData, force, refresh bool) (result provisioner.Result, started bool, details *metal3v1alpha1.HardwareDetails, err error) { + return +} + +func (m *hsfMockProvisioner) UpdateHardwareState() (hwState provisioner.HardwareState, err error) { + return +} + +func (m *hsfMockProvisioner) Prepare(data provisioner.PrepareData, unprepared bool) (result provisioner.Result, started bool, err error) { + return +} + +func (m *hsfMockProvisioner) Adopt(data provisioner.AdoptData, force bool) (result provisioner.Result, err error) { + return +} + +func (m *hsfMockProvisioner) Provision(data provisioner.ProvisionData) (result provisioner.Result, err error) { + return +} + +func (m *hsfMockProvisioner) Deprovision(force bool) (result provisioner.Result, err error) { + return +} + +func (m *hsfMockProvisioner) Delete() (result provisioner.Result, err error) { + return +} + +func (m *hsfMockProvisioner) Detach() (result provisioner.Result, err error) { + return +} + +func (m *hsfMockProvisioner) PowerOn(force bool) (result provisioner.Result, err error) { + return +} + +func (m *hsfMockProvisioner) PowerOff(rebootMode metal3v1alpha1.RebootMode, force bool) (result provisioner.Result, err error) { + return +} + +func (m *hsfMockProvisioner) IsReady() (result bool, err error) { + return +} + +func (m *hsfMockProvisioner) GetFirmwareSettings(includeSchema bool) (settings metal3v1alpha1.SettingsMap, schema map[string]metal3v1alpha1.SettingSchema, err error) { + + return m.Settings, m.Schema, m.Error +} + +func getSchema() *metal3v1alpha1.FirmwareSchema { + + schema := &metal3v1alpha1.FirmwareSchema{ + TypeMeta: metav1.TypeMeta{ + Kind: "FirmwareSchema", + APIVersion: "metal3.io/v1alpha1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: schemaName, + Namespace: hostNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "metal3.io/v1alpha1", + Kind: "HostFirmwareSettings", + Name: hostName, + }, + }, + }, + } + + return schema +} + +func getCurrentSchemaSettings() map[string]metal3v1alpha1.SettingSchema { + + return map[string]metal3v1alpha1.SettingSchema{ + "AssetTag": { + AttributeType: "String", + MinLength: &minLength, + MaxLength: &maxLength, + Unique: &iTrue, + }, + "CustomPostMessage": { + AttributeType: "String", + MinLength: &minLength, + MaxLength: &maxLength, + Unique: &iFalse, + ReadOnly: &iFalse, + }, + "L2Cache": { + AttributeType: "String", + MinLength: &minLength, + MaxLength: &maxLength, + ReadOnly: &iTrue, + }, + "NetworkBootRetryCount": { + AttributeType: "Integer", + LowerBound: &lowerBound, + UpperBound: &upperBound, + ReadOnly: &iFalse, + }, + "ProcVirtualization": { + AttributeType: "Enumeration", + AllowableValues: []string{"Enabled", "Disabled"}, + ReadOnly: &iFalse, + }, + "SecureBoot": { + AttributeType: "Enumeration", + AllowableValues: []string{"Enabled", "Disabled"}, + ReadOnly: &iTrue, + }, + } +} + +func createSchemaResource(ctx context.Context, r *BareMetalHostReconciler) { + firmwareSchema := getSchema() + firmwareSchema.Spec.Schema = getCurrentSchemaSettings() + + r.Client.Create(ctx, firmwareSchema) + r.Client.Update(ctx, firmwareSchema) // needed to make sure ResourceVersion matches existing schemas +} + +func getExpectedSchemaResource() *metal3v1alpha1.FirmwareSchema { + firmwareSchema := getSchema() + firmwareSchema.Spec.Schema = getCurrentSchemaSettings() + + return firmwareSchema +} + +func createHFSResource(ctx context.Context, r *BareMetalHostReconciler, hfs *metal3v1alpha1.HostFirmwareSettings, createSchema bool) { + + hfs.Status = metal3v1alpha1.HostFirmwareSettingsStatus{ + Settings: metal3v1alpha1.SettingsMap{ + "CustomPostMessage": "All tests passed", + "L2Cache": "10x256 KB", + "NetworkBootRetryCount": "10", + "ProcVirtualization": "Enabled", + "SecureBoot": "Enabled", + "AssetTag": "X45672917", + }, + } + hfs.TypeMeta = metav1.TypeMeta{ + Kind: "HostFirmwareSettings", + APIVersion: "metal3.io/v1alpha1"} + hfs.ObjectMeta = metav1.ObjectMeta{ + Name: hostName, + Namespace: hostNamespace} + + if createSchema { + hfs.Status.FirmwareSchema = + &metal3v1alpha1.SchemaReference{ + Name: schemaName, + Namespace: hostNamespace} + } + + r.Client.Create(ctx, hfs) + r.Client.Update(ctx, hfs) +} + +// Test the hostfirmwaresettings reconciler functions +func TestStoreHostFirmwareSettings(t *testing.T) { + + testCases := []struct { + Scenario string + // the resource that the reconciler is managing + CurrentHFSResource *metal3v1alpha1.HostFirmwareSettings + // whether to create a schema resource before calling reconciler + CreateSchemaResource bool + // mock data returned from Ironic via the provisioner + CurrentSettings metal3v1alpha1.SettingsMap + // the expected created or updated resource + ExpectedSettings *metal3v1alpha1.HostFirmwareSettings + }{ + { + Scenario: "inital hfs resource with no schema", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + TypeMeta: metav1.TypeMeta{ + Kind: "HostFirmwareSettings", + APIVersion: "metal3.io/v1alpha1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: hostName, + Namespace: hostNamespace, + ResourceVersion: "1"}, + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{}, + }, + Status: metal3v1alpha1.HostFirmwareSettingsStatus{}, + }, + CreateSchemaResource: false, + CurrentSettings: metal3v1alpha1.SettingsMap{ + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + "AssetTag": "X45672917", + }, + ExpectedSettings: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "NetworkBootRetryCount": intstr.FromString("20"), + "ProcVirtualization": intstr.FromString("Disabled"), + }, + }, + Status: metal3v1alpha1.HostFirmwareSettingsStatus{ + FirmwareSchema: &metal3v1alpha1.SchemaReference{ + Name: schemaName, + Namespace: hostNamespace, + }, + Settings: metal3v1alpha1.SettingsMap{ + "AssetTag": "X45672917", + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + }, + }, + }, + }, + { + Scenario: "inital hfs resource with existing schema", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + TypeMeta: metav1.TypeMeta{ + Kind: "HostFirmwareSettings", + APIVersion: "metal3.io/v1alpha1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: hostName, + Namespace: hostNamespace, + ResourceVersion: "1"}, + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{}, + }, + Status: metal3v1alpha1.HostFirmwareSettingsStatus{}, + }, + CreateSchemaResource: true, + CurrentSettings: metal3v1alpha1.SettingsMap{ + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + "AssetTag": "X45672917", + }, + ExpectedSettings: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "NetworkBootRetryCount": intstr.FromString("20"), + "ProcVirtualization": intstr.FromString("Disabled"), + }, + }, + Status: metal3v1alpha1.HostFirmwareSettingsStatus{ + FirmwareSchema: &metal3v1alpha1.SchemaReference{ + Name: schemaName, + Namespace: hostNamespace, + }, + Settings: metal3v1alpha1.SettingsMap{ + "AssetTag": "X45672917", + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + }, + }, + }, + }, + { + Scenario: "updated settings", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + TypeMeta: metav1.TypeMeta{ + Kind: "HostFirmwareSettings", + APIVersion: "metal3.io/v1alpha1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: hostName, + Namespace: hostNamespace, + ResourceVersion: "2"}, + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "NetworkBootRetryCount": intstr.FromString("10"), + "ProcVirtualization": intstr.FromString("Enabled"), + }, + }, + Status: metal3v1alpha1.HostFirmwareSettingsStatus{ + FirmwareSchema: &metal3v1alpha1.SchemaReference{ + Name: schemaName, + Namespace: hostNamespace, + }, + Settings: metal3v1alpha1.SettingsMap{ + "L2Cache": "10x256 KB", + "NetworkBootRetryCount": "10", + "ProcVirtualization": "Enabled", + }, + }, + }, + CreateSchemaResource: true, + CurrentSettings: metal3v1alpha1.SettingsMap{ + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + "AssetTag": "X45672917", + }, + ExpectedSettings: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "NetworkBootRetryCount": intstr.FromString("10"), + "ProcVirtualization": intstr.FromString("Enabled"), + }, + }, + Status: metal3v1alpha1.HostFirmwareSettingsStatus{ + FirmwareSchema: &metal3v1alpha1.SchemaReference{ + Name: schemaName, + Namespace: hostNamespace, + }, + Settings: metal3v1alpha1.SettingsMap{ + "AssetTag": "X45672917", + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.Scenario, func(t *testing.T) { + + ctx := context.TODO() + prov := getMockProvisioner(tc.CurrentSettings, getCurrentSchemaSettings()) + + tc.ExpectedSettings.TypeMeta = metav1.TypeMeta{ + Kind: "HostFirmwareSettings", + APIVersion: "metal3.io/v1alpha1"} + tc.ExpectedSettings.ObjectMeta = metav1.ObjectMeta{ + Name: hostName, + Namespace: hostNamespace, + ResourceVersion: "3"} + + hfs := tc.CurrentHFSResource + r := getTestHFSReconciler(hfs) + info := &rInfo{ + log: logf.Log.WithName("controllers").WithName("HostFirmwareSettings"), + hfs: tc.CurrentHFSResource, + } + + if tc.CreateSchemaResource { + firmwareSchema := getSchema() + firmwareSchema.Spec.Schema = getCurrentSchemaSettings() + + r.Client.Create(ctx, firmwareSchema) + r.Client.Update(ctx, firmwareSchema) // in order to set resource version + } + + err := r.updateHostFirmwareSettings(prov, info) + assert.Equal(t, nil, err) + + // Check that resources get created or updated + key := client.ObjectKey{ + Namespace: hfs.ObjectMeta.Namespace, Name: hfs.ObjectMeta.Name} + actualSettings := &metal3v1alpha1.HostFirmwareSettings{} + err = r.Client.Get(ctx, key, actualSettings) + assert.Equal(t, nil, err) + + // Use the same time for expected and actual + currentTime := metav1.Now() + actualSettings.Status.ProvStatus.LastUpdated = ¤tTime + tc.ExpectedSettings.Status.ProvStatus.LastUpdated = ¤tTime + + assert.Equal(t, tc.ExpectedSettings, actualSettings) + + key = client.ObjectKey{ + Namespace: hfs.ObjectMeta.Namespace, Name: schemaName} + actualSchema := &metal3v1alpha1.FirmwareSchema{} + err = r.Client.Get(ctx, key, actualSchema) + assert.Equal(t, nil, err) + expectedSchema := getExpectedSchemaResource() + expectedSchema.ObjectMeta.ResourceVersion = "2" + assert.Equal(t, expectedSchema, actualSchema) + }) + } +} + +// Test the function to get hostFirmwareSettings for cleaning in the baremtalhost_controller +func TestGetValidHostFirmwareSettings(t *testing.T) { + + testCases := []struct { + Scenario string + // the existing resources + CurrentHFSResource *metal3v1alpha1.HostFirmwareSettings + CreateSchemaResource bool + // the expected updated resource + ExpectedStatusSettings metal3v1alpha1.SettingsMap + ExpectedSpecSettings metal3v1alpha1.DesiredSettingsMap + ExpectedError string + }{ + { + Scenario: "valid spec changes no schema", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "L2Cache": intstr.FromString("10x512 KB"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + }, + }, + CreateSchemaResource: false, + ExpectedStatusSettings: metal3v1alpha1.SettingsMap{ + "CustomPostMessage": "All tests passed", + "L2Cache": "10x256 KB", + "NetworkBootRetryCount": "10", + "ProcVirtualization": "Enabled", + "SecureBoot": "Enabled", + "AssetTag": "X45672917", + }, + ExpectedSpecSettings: metal3v1alpha1.DesiredSettingsMap{ + "L2Cache": intstr.FromString("10x512 KB"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + ExpectedError: "", + }, + { + Scenario: "valid spec changes with schema", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + }, + }, + CreateSchemaResource: true, + ExpectedStatusSettings: metal3v1alpha1.SettingsMap{ + "CustomPostMessage": "All tests passed", + "L2Cache": "10x256 KB", + "NetworkBootRetryCount": "10", + "ProcVirtualization": "Enabled", + "SecureBoot": "Enabled", + "AssetTag": "X45672917", + }, + ExpectedSpecSettings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + ExpectedError: "", + }, + { + Scenario: "invalid string", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("A really long POST message"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + }, + }, + CreateSchemaResource: true, + ExpectedStatusSettings: nil, + ExpectedSpecSettings: nil, + ExpectedError: "Setting CustomPostMessage is invalid, string A really long POST message length is above range 20", + }, + { + Scenario: "invalid int", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("2000"), + }, + }, + }, + CreateSchemaResource: true, + ExpectedStatusSettings: nil, + ExpectedSpecSettings: nil, + ExpectedError: "Setting NetworkBootRetryCount is invalid, integer 2000 is above range 20", + }, + { + Scenario: "invalid enum", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Not enabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + }, + }, + CreateSchemaResource: true, + ExpectedStatusSettings: nil, + ExpectedSpecSettings: nil, + ExpectedError: "Setting ProcVirtualization is invalid, unknown enumeration value - Not enabled", + }, + { + Scenario: "invalid name", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "SomeNewSetting": intstr.FromString("foo"), + }, + }, + }, + CreateSchemaResource: true, + ExpectedStatusSettings: nil, + ExpectedSpecSettings: nil, + ExpectedError: "Setting SomeNewSetting is not in the Status field", + }, + { + Scenario: "invalid password in spec", + CurrentHFSResource: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + "SysPassword": intstr.FromString("Pa%$word"), + }, + }, + }, + CreateSchemaResource: true, + ExpectedStatusSettings: nil, + ExpectedSpecSettings: nil, + ExpectedError: "Cannot set Password field", + }, + } + + for _, tc := range testCases { + t.Run(tc.Scenario, func(t *testing.T) { + + ctx := context.TODO() + host := &metal3v1alpha1.BareMetalHost{} + host.ObjectMeta.Name = hostName + host.ObjectMeta.Namespace = hostNamespace + + r := getTestHostReconciler(host) + info := getDefaultHostReconcileInfo(host, hostName, hostNamespace) + + // Create the resources using fakeclient + if tc.CurrentHFSResource != nil { + + createHFSResource(ctx, r, tc.CurrentHFSResource, tc.CreateSchemaResource) + } + if tc.CreateSchemaResource { + createSchemaResource(ctx, r) + } + + hfs, err := r.getValidHostFirmwareSettings(info) + if err == nil { + assert.Equal(t, tc.ExpectedError, "") + } else { + assert.Equal(t, tc.ExpectedError, err.Error()) + } + if tc.ExpectedStatusSettings != nil { + assert.Equal(t, tc.ExpectedStatusSettings, hfs.Status.Settings) + assert.Equal(t, tc.ExpectedSpecSettings, hfs.Spec.Settings) + } + }) + } +} diff --git a/docs/configuration.md b/docs/configuration.md index 20dcb68038..f0a57b99f2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -65,4 +65,4 @@ When an external Ironic is used, the following requirements must be met: * Either HTTP basic or no-auth authentication must be used (Keystone is not supported). -* API version 1.69 (Wallaby release cycle) or newer must be available. +* API version 1.74 (Xena release cycle) or newer must be available. diff --git a/main.go b/main.go index 06831ffaf0..f614bfe810 100644 --- a/main.go +++ b/main.go @@ -158,6 +158,14 @@ func main() { setupChecks(mgr) + if err = (&metal3iocontroller.HostFirmwareSettingsReconciler{ + Client: mgr.GetClient(), + Log: ctrl.Log.WithName("controllers").WithName("HostFirmwareSettings"), + ProvisionerFactory: provisionerFactory, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "HostFirmwareSettings") + os.Exit(1) + } // +kubebuilder:scaffold:builder setupLog.Info("starting manager") diff --git a/pkg/provisioner/demo/demo.go b/pkg/provisioner/demo/demo.go index be79a6f464..c1ded2200f 100644 --- a/pkg/provisioner/demo/demo.go +++ b/pkg/provisioner/demo/demo.go @@ -349,3 +349,9 @@ func (p *demoProvisioner) PowerOff(rebootMode metal3v1alpha1.RebootMode, force b func (p *demoProvisioner) IsReady() (result bool, err error) { return true, nil } + +func (p *demoProvisioner) GetFirmwareSettings(includeSchema bool) (settings metal3v1alpha1.SettingsMap, schema map[string]metal3v1alpha1.SettingSchema, err error) { + + p.log.Info("getting BIOS settings") + return +} diff --git a/pkg/provisioner/fixture/fixture.go b/pkg/provisioner/fixture/fixture.go index b3eee1cf4b..ea34fd138d 100644 --- a/pkg/provisioner/fixture/fixture.go +++ b/pkg/provisioner/fixture/fixture.go @@ -331,3 +331,8 @@ func (p *fixtureProvisioner) IsReady() (result bool, err error) { return p.state.BecomeReadyCounter == 0, nil } + +func (p *fixtureProvisioner) GetFirmwareSettings(includeSchema bool) (settings metal3v1alpha1.SettingsMap, schema map[string]metal3v1alpha1.SettingSchema, err error) { + p.log.Info("getting BIOS settings") + return +} diff --git a/pkg/provisioner/ironic/bios_test.go b/pkg/provisioner/ironic/bios_test.go new file mode 100644 index 0000000000..865ebb9439 --- /dev/null +++ b/pkg/provisioner/ironic/bios_test.go @@ -0,0 +1,135 @@ +package ironic + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" + "github.com/metal3-io/baremetal-operator/pkg/bmc" + "github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/clients" + "github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/testserver" +) + +func TestGetFirmwareSettings(t *testing.T) { + + nodeUUID := "158c5d59-9ace-9631-ed51-d842a45f1c52" + iTrue := true + iFalse := false + minLength := 0 + maxLength := 16 + lowerBound := 0 + upperBound := 20 + + cases := []struct { + name string + expectedSettingsMap metal3v1alpha1.SettingsMap + expectedSchemaMap map[string]metal3v1alpha1.SettingSchema + includeSchema bool + ironic *testserver.IronicMock + expectedError string + }{ + { + name: "no-schema", + expectedSettingsMap: metal3v1alpha1.SettingsMap{ + "L2Cache": "10x256 KB", + "NumCores": "10", + "ProcVirtualization": "Enabled", + }, + expectedSchemaMap: map[string]metal3v1alpha1.SettingSchema{}, + ironic: testserver.NewIronic(t).BIOSSettings(nodeUUID), + includeSchema: false, + expectedError: "", + }, + { + name: "include-schema", + expectedSettingsMap: metal3v1alpha1.SettingsMap{ + "L2Cache": "10x256 KB", + "NumCores": "10", + "ProcVirtualization": "Enabled", + }, + expectedSchemaMap: map[string]metal3v1alpha1.SettingSchema{ + "L2Cache": { + AttributeType: "String", + AllowableValues: []string{}, + LowerBound: nil, + UpperBound: nil, + MinLength: &minLength, + MaxLength: &maxLength, + ReadOnly: &iTrue, + ResetRequired: nil, + Unique: nil, + }, + "NumCores": { + AttributeType: "Integer", + AllowableValues: []string{}, + LowerBound: &lowerBound, + UpperBound: &upperBound, + MinLength: nil, + MaxLength: nil, + ReadOnly: &iTrue, + ResetRequired: nil, + Unique: nil, + }, + "ProcVirtualization": { + AttributeType: "Enumeration", + AllowableValues: []string{"Enabled", "Disabled"}, + LowerBound: nil, + UpperBound: nil, + MinLength: nil, + MaxLength: nil, + ReadOnly: &iFalse, + ResetRequired: nil, + Unique: nil, + }, + }, + ironic: testserver.NewIronic(t).BIOSDetailSettings(nodeUUID), + includeSchema: true, + expectedError: "", + }, + { + name: "error404", + expectedSettingsMap: metal3v1alpha1.SettingsMap(nil), + expectedSchemaMap: map[string]metal3v1alpha1.SettingSchema(nil), + ironic: testserver.NewIronic(t).NoBIOS(nodeUUID), + includeSchema: false, + expectedError: "could not get BIOS settings for node", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + + tc.ironic.Start() + defer tc.ironic.Stop() + + inspector := testserver.NewInspector(t).Start() + defer inspector.Stop() + + host := makeHost() + host.Name = "node-1" + host.Status.Provisioning.ID = nodeUUID + + auth := clients.AuthConfig{Type: clients.NoAuth} + + prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nullEventPublisher, + tc.ironic.Endpoint(), auth, inspector.Endpoint(), auth, + ) + if err != nil { + t.Fatalf("could not create provisioner: %s", err) + } + + settingsMap, schemaMap, err := prov.GetFirmwareSettings(tc.includeSchema) + + assert.Equal(t, tc.expectedSettingsMap, settingsMap) + assert.Equal(t, tc.expectedSchemaMap, schemaMap) + + if tc.expectedError == "" { + assert.NoError(t, err) + } else { + assert.Error(t, err) + assert.Regexp(t, tc.expectedError, err.Error()) + } + }) + } +} diff --git a/pkg/provisioner/ironic/clients/client.go b/pkg/provisioner/ironic/clients/client.go index 348ac480a7..4b0c2046a9 100644 --- a/pkg/provisioner/ironic/clients/client.go +++ b/pkg/provisioner/ironic/clients/client.go @@ -95,8 +95,8 @@ func IronicClient(ironicEndpoint string, auth AuthConfig, tls TLSConfig) (client // Ensure we have a microversion high enough to get the features // we need. Update docs/configuration.md when updating the version. - // Version 1.69 introduces deploySteps argument to provisioning. - client.Microversion = "1.69" + // Version 1.74 allows retrival of the BIOS Registry + client.Microversion = "1.74" return updateHTTPClient(client, tls) } diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go index 282363e302..bb3d58d340 100644 --- a/pkg/provisioner/ironic/ironic.go +++ b/pkg/provisioner/ironic/ironic.go @@ -826,6 +826,53 @@ func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node, data pr return updater } +// GetFirmwareSettings gets the BIOS settings and optional schema from the host and returns maps +func (p *ironicProvisioner) GetFirmwareSettings(includeSchema bool) (settings metal3v1alpha1.SettingsMap, schema map[string]metal3v1alpha1.SettingSchema, err error) { + + if p.nodeID == "" { + return nil, nil, provisioner.ErrNeedsRegistration + } + + // Get the settings from Ironic via Gophercloud + var settingsList []nodes.BIOSSetting + var biosListErr error + if includeSchema { + opts := nodes.ListBIOSSettingsOpts{Detail: true} + settingsList, biosListErr = nodes.ListBIOSSettings(p.client, p.nodeID, opts).Extract() + } else { + settingsList, biosListErr = nodes.ListBIOSSettings(p.client, p.nodeID, nil).Extract() + } + if biosListErr != nil { + return nil, nil, errors.Wrap(biosListErr, + fmt.Sprintf("could not get BIOS settings for node %s", p.nodeID)) + } + p.log.Info("retrieved BIOS settings for node", "node", p.nodeID) + + settings = make(map[string]string) + schema = make(map[string]metal3v1alpha1.SettingSchema) + + for _, v := range settingsList { + settings[v.Name] = v.Value + + if includeSchema { + // add to schema + schema[v.Name] = metal3v1alpha1.SettingSchema{ + AttributeType: v.AttributeType, + AllowableValues: v.AllowableValues, + LowerBound: v.LowerBound, + UpperBound: v.UpperBound, + MinLength: v.MinLength, + MaxLength: v.MaxLength, + ReadOnly: v.ReadOnly, + ResetRequired: v.ResetRequired, + Unique: v.Unique, + } + } + } + + return settings, schema, nil +} + // We can't just replace the capabilities because we need to keep the // values provided by inspection. We can't replace only the boot_mode // because the API isn't fine-grained enough for that. So we have to @@ -987,19 +1034,50 @@ func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails } cleanSteps = append(cleanSteps, raidCleanSteps...) - // Build bios clean steps - settings, err := bmcAccess.BuildBIOSSettings(data.FirmwareConfig) + // Get the subset (currently 3) of vendor specific BIOS settings converted from common names + bmcsettings, err := bmcAccess.BuildBIOSSettings(data.FirmwareConfig) if err != nil { return nil, err } - if len(settings) != 0 { + + var newSettings []map[string]string + if data.ActualFirmwareSettings != nil { + // If we have the current settings from Ironic, update the settings to contain: + // 1. settings converted by BMC drivers that are different than current settings + for _, bmcsetting := range bmcsettings { + if val, exists := data.ActualFirmwareSettings[bmcsetting["name"]]; exists { + if bmcsetting["value"] != val { + newSettings = buildFirmwareSettings(newSettings, bmcsetting["name"], bmcsetting["value"]) + } + } else { + p.log.Info("name converted from bmc driver not found in firmware settings", "name", bmcsetting["name"]) + } + } + + // 2. target settings that are different than current settings + if data.TargetFirmwareSettings != nil { + for k, v := range data.TargetFirmwareSettings { + if data.ActualFirmwareSettings[k] != v.String() { + newSettings = buildFirmwareSettings(newSettings, k, v.String()) + } + } + } + } else { + // use only the settings converted by bmc driver + for _, bmcsetting := range bmcsettings { + newSettings = append(newSettings, bmcsetting) + } + } + + if len(newSettings) != 0 { + p.log.Info("Applying BIOS config clean steps", "settings", newSettings) cleanSteps = append( cleanSteps, nodes.CleanStep{ Interface: "bios", Step: "apply_configuration", Args: map[string]interface{}{ - "settings": settings, + "settings": newSettings, }, }, ) @@ -1010,6 +1088,21 @@ func (p *ironicProvisioner) buildManualCleaningSteps(bmcAccess bmc.AccessDetails return } +func buildFirmwareSettings(settings []map[string]string, name string, value string) []map[string]string { + // if name already exists, don't add it + for _, setting := range settings { + if setting["name"] == name { + return settings + } + } + + return append(settings, + map[string]string{ + "name": name, + "value": value}, + ) +} + func (p *ironicProvisioner) startManualCleaning(bmcAccess bmc.AccessDetails, ironicNode *nodes.Node, data provisioner.PrepareData) (success bool, result provisioner.Result, err error) { // Set raid configuration result, err = setTargetRAIDCfg(p, bmcAccess.RAIDInterface(), ironicNode, data) @@ -1075,6 +1168,7 @@ func (p *ironicProvisioner) Prepare(data provisioner.PrepareData, unprepared boo started = true } // Automated clean finished + result, err = operationComplete() case nodes.Manageable: @@ -1087,6 +1181,7 @@ func (p *ironicProvisioner) Prepare(data provisioner.PrepareData, unprepared boo started = true } // Manual clean finished + result, err = p.changeNodeProvisionState( ironicNode, nodes.ProvisionStateOpts{Target: nodes.TargetProvide}, diff --git a/pkg/provisioner/ironic/provision_test.go b/pkg/provisioner/ironic/provision_test.go index b0581e746f..52a67a6b92 100644 --- a/pkg/provisioner/ironic/provision_test.go +++ b/pkg/provisioner/ironic/provision_test.go @@ -1,6 +1,7 @@ package ironic import ( + "net/url" "testing" "time" @@ -13,7 +14,12 @@ import ( "github.com/metal3-io/baremetal-operator/pkg/provisioner" "github.com/metal3-io/baremetal-operator/pkg/provisioner/fixture" "github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/clients" + "github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/testbmc" "github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/testserver" + + "k8s.io/apimachinery/pkg/util/intstr" + + _ "github.com/metal3-io/baremetal-operator/pkg/bmc" ) func TestProvision(t *testing.T) { @@ -392,3 +398,238 @@ func TestIronicHasSameImage(t *testing.T) { }) } } + +func TestBuildCleanSteps(t *testing.T) { + + var True bool = true + var False bool = false + + nodeUUID := "33ce8659-7400-4c68-9535-d10766f07a58" + cases := []struct { + name string + ironic *testserver.IronicMock + currentSettings v1alpha1.SettingsMap + desiredSettings v1alpha1.DesiredSettingsMap + firmwareConfig *v1alpha1.FirmwareConfig + expectedSettings []map[string]string + }{ + { + name: "no current settings", + ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{ + ProvisionState: string(nodes.DeployFail), + UUID: nodeUUID, + }), + currentSettings: nil, + desiredSettings: nil, + firmwareConfig: &v1alpha1.FirmwareConfig{ + VirtualizationEnabled: &True, + SimultaneousMultithreadingEnabled: &False, + }, + expectedSettings: []map[string]string{ + { + "name": "ProcVirtualization", + "value": "Enabled", + }, + { + "name": "ProcHyperthreading", + "value": "Disabled", + }, + }, + }, + { + name: "current settings same as bmc", + ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{ + ProvisionState: string(nodes.DeployFail), + UUID: nodeUUID, + }), + currentSettings: map[string]string{ + "L2Cache": "10x256 KB", + "NumCores": "10", + "ProcVirtualization": "Enabled", + "ProcHyperthreading": "Disabled", + }, + desiredSettings: nil, + firmwareConfig: &v1alpha1.FirmwareConfig{ + VirtualizationEnabled: &True, + SimultaneousMultithreadingEnabled: &False, + }, + expectedSettings: nil, + }, + { + name: "current settings different than bmc", + ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{ + ProvisionState: string(nodes.DeployFail), + UUID: nodeUUID, + }), + currentSettings: v1alpha1.SettingsMap{ + "L2Cache": "10x256 KB", + "NumCores": "10", + "ProcVirtualization": "Disabled", + "ProcHyperthreading": "Enabled", + }, + desiredSettings: nil, + firmwareConfig: &v1alpha1.FirmwareConfig{ + VirtualizationEnabled: &True, + SimultaneousMultithreadingEnabled: &False, + }, + expectedSettings: []map[string]string{ + { + "name": "ProcVirtualization", + "value": "Enabled", + }, + { + "name": "ProcHyperthreading", + "value": "Disabled", + }, + }, + }, + { + name: "current settings same as bmc different than desired", + ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{ + ProvisionState: string(nodes.DeployFail), + UUID: nodeUUID, + }), + currentSettings: v1alpha1.SettingsMap{ + "L2Cache": "10x256 KB", + "NumCores": "10", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Enabled", + "ProcHyperthreading": "Disabled", + }, + desiredSettings: v1alpha1.DesiredSettingsMap{ + "NetworkBootRetryCount": intstr.FromString("10"), + "ProcVirtualization": intstr.FromString("Disabled"), + "ProcHyperthreading": intstr.FromString("Enabled"), + }, + firmwareConfig: &v1alpha1.FirmwareConfig{ + VirtualizationEnabled: &True, + SimultaneousMultithreadingEnabled: &False, + }, + expectedSettings: []map[string]string{ + { + "name": "NetworkBootRetryCount", + "value": "10", + }, + { + "name": "ProcVirtualization", + "value": "Disabled", + }, + { + "name": "ProcHyperthreading", + "value": "Enabled", + }, + }, + }, + { + name: "current settings different than bmc and desired", + ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{ + ProvisionState: string(nodes.DeployFail), + UUID: nodeUUID, + }), + currentSettings: v1alpha1.SettingsMap{ + "L2Cache": "10x256 KB", + "NumCores": "10", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Enabled", + "ProcHyperthreading": "Disabled", + }, + desiredSettings: v1alpha1.DesiredSettingsMap{ + "NetworkBootRetryCount": intstr.FromString("5"), + "ProcVirtualization": intstr.FromString("Enabled"), + "ProcHyperthreading": intstr.FromString("Disabled"), + }, + firmwareConfig: &v1alpha1.FirmwareConfig{ + VirtualizationEnabled: &False, + SimultaneousMultithreadingEnabled: &True, + }, + expectedSettings: []map[string]string{ + { + "name": "ProcVirtualization", + "value": "Disabled", + }, + { + "name": "ProcHyperthreading", + "value": "Enabled", + }, + { + "name": "NetworkBootRetryCount", + "value": "5", + }, + }, + }, + { + name: "bmc and desired duplicate settings", + ironic: testserver.NewIronic(t).WithDefaultResponses().Node(nodes.Node{ + ProvisionState: string(nodes.DeployFail), + UUID: nodeUUID, + }), + currentSettings: v1alpha1.SettingsMap{ + "L2Cache": "10x256 KB", + "NumCores": "10", + "ProcVirtualization": "Enabled", + "ProcHyperthreading": "Disabled", + }, + desiredSettings: v1alpha1.DesiredSettingsMap{ + "ProcVirtualization": intstr.FromString("Disabled"), + "ProcHyperthreading": intstr.FromString("Enabled"), + }, + firmwareConfig: &v1alpha1.FirmwareConfig{ + VirtualizationEnabled: &False, + SimultaneousMultithreadingEnabled: &True, + }, + expectedSettings: []map[string]string{ + { + "name": "ProcHyperthreading", + "value": "Enabled", + }, + { + "name": "ProcVirtualization", + "value": "Disabled", + }, + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if tc.ironic != nil { + tc.ironic.Start() + defer tc.ironic.Stop() + } + + inspector := testserver.NewInspector(t).Ready().WithIntrospection(nodeUUID, introspection.Introspection{ + Finished: false, + }) + inspector.Start() + defer inspector.Stop() + + host := makeHost() + host.Status.Provisioning.ID = nodeUUID + publisher := func(reason, message string) {} + auth := clients.AuthConfig{Type: clients.NoAuth} + prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, publisher, + tc.ironic.Endpoint(), auth, inspector.Endpoint(), auth, + ) + if err != nil { + t.Fatalf("could not create provisioner: %s", err) + } + + parsedURL := &url.URL{Scheme: "redfish", Host: "10.1.1.1"} + + testBMC, _ := testbmc.NewTestBMCAccessDetails(parsedURL, false) + + cleanSteps, err := prov.buildManualCleaningSteps(testBMC, provisioner.PrepareData{ + FirmwareConfig: tc.firmwareConfig, + ActualFirmwareSettings: tc.currentSettings, + TargetFirmwareSettings: tc.desiredSettings, + }) + + assert.Equal(t, nil, err) + if cleanSteps == nil { + assert.Equal(t, tc.expectedSettings, []map[string]string(nil)) + } else { + settings := cleanSteps[0].Args["settings"] + assert.ElementsMatch(t, tc.expectedSettings, settings) + } + }) + } +} diff --git a/pkg/provisioner/ironic/testbmc/testbmc.go b/pkg/provisioner/ironic/testbmc/testbmc.go index ae7a627d0f..a02ec22a42 100644 --- a/pkg/provisioner/ironic/testbmc/testbmc.go +++ b/pkg/provisioner/ironic/testbmc/testbmc.go @@ -8,11 +8,11 @@ import ( ) func init() { - bmc.RegisterFactory("test", newTestBMCAccessDetails, []string{}) - bmc.RegisterFactory("test-needs-mac", newTestBMCAccessDetails, []string{}) + bmc.RegisterFactory("test", NewTestBMCAccessDetails, []string{}) + bmc.RegisterFactory("test-needs-mac", NewTestBMCAccessDetails, []string{}) } -func newTestBMCAccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (bmc.AccessDetails, error) { +func NewTestBMCAccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (bmc.AccessDetails, error) { return &testAccessDetails{ bmcType: parsedURL.Scheme, hostname: parsedURL.Hostname(), @@ -92,5 +92,52 @@ func (a *testAccessDetails) SupportsSecureBoot() bool { } func (a *testAccessDetails) BuildBIOSSettings(firmwareConfig *metal3v1alpha1.FirmwareConfig) (settings []map[string]string, err error) { - return nil, nil + + // Return sample BMC data for test purposes + if firmwareConfig == nil { + return nil, nil + } + + var value string + + if firmwareConfig.VirtualizationEnabled != nil { + value = "Disabled" + if *firmwareConfig.VirtualizationEnabled { + value = "Enabled" + } + settings = append(settings, + map[string]string{ + "name": "ProcVirtualization", + "value": value, + }, + ) + } + + if firmwareConfig.SimultaneousMultithreadingEnabled != nil { + value = "Disabled" + if *firmwareConfig.SimultaneousMultithreadingEnabled { + value = "Enabled" + } + settings = append(settings, + map[string]string{ + "name": "ProcHyperthreading", + "value": value, + }, + ) + } + + if firmwareConfig.SriovEnabled != nil { + value = "Disabled" + if *firmwareConfig.SriovEnabled { + value = "Enabled" + } + settings = append(settings, + map[string]string{ + "name": "Sriov", + "value": value, + }, + ) + } + + return } diff --git a/pkg/provisioner/ironic/testserver/ironic.go b/pkg/provisioner/ironic/testserver/ironic.go index 6120a388e9..b250a412fe 100644 --- a/pkg/provisioner/ironic/testserver/ironic.go +++ b/pkg/provisioner/ironic/testserver/ironic.go @@ -305,5 +305,98 @@ func (m *IronicMock) ClearDatabase() { } } } +} + +// BIOSSettings configure the server with a valid response for /v1/nodes//bios +func (m *IronicMock) BIOSSettings(nodeUUID string) *IronicMock { + settings := []nodes.BIOSSetting{ + { + Name: "L2Cache", + Value: "10x256 KB", + }, + { + Name: "NumCores", + Value: "10", + }, + { + Name: "ProcVirtualization", + Value: "Enabled", + }, + } + + resp := struct { + Settings []nodes.BIOSSetting `json:"bios"` + }{ + Settings: settings, + } + + m.ResponseJSON(m.buildURL("/v1/nodes/"+nodeUUID+"/bios", http.MethodGet), resp) + return m +} + +// BIOSDetailSettings configure the server with a valid response for /v1/nodes//bios?detail=True +func (m *IronicMock) BIOSDetailSettings(nodeUUID string) *IronicMock { + + iTrue := true + iFalse := false + minLength := 0 + maxLength := 16 + lowerBound := 0 + upperBound := 20 + + settings := []nodes.BIOSSetting{ + { + Name: "L2Cache", + Value: "10x256 KB", + AttributeType: "String", + AllowableValues: []string{}, + LowerBound: nil, + UpperBound: nil, + MinLength: &minLength, + MaxLength: &maxLength, + ReadOnly: &iTrue, + ResetRequired: nil, + Unique: nil, + }, + { + Name: "NumCores", + Value: "10", + AttributeType: "Integer", + AllowableValues: []string{}, + LowerBound: &lowerBound, + UpperBound: &upperBound, + MinLength: nil, + MaxLength: nil, + ReadOnly: &iTrue, + ResetRequired: nil, + Unique: nil, + }, + { + Name: "ProcVirtualization", + Value: "Enabled", + AttributeType: "Enumeration", + AllowableValues: []string{"Enabled", "Disabled"}, + LowerBound: nil, + UpperBound: nil, + MinLength: nil, + MaxLength: nil, + ReadOnly: &iFalse, + ResetRequired: nil, + Unique: nil, + }, + } + + resp := struct { + Settings []nodes.BIOSSetting `json:"bios"` + }{ + Settings: settings, + } + + m.ResponseJSON(m.buildURL("/v1/nodes/"+nodeUUID+"/bios", http.MethodGet), resp) + return m +} +// NoBIOS configures the server so /v1/node//bios returns a 404 +func (m *IronicMock) NoBIOS(nodeUUID string) *IronicMock { + return m.NodeError(nodeUUID, http.StatusNotFound) } diff --git a/pkg/provisioner/provisioner.go b/pkg/provisioner/provisioner.go index 2c209e12cf..1c42f77b02 100644 --- a/pkg/provisioner/provisioner.go +++ b/pkg/provisioner/provisioner.go @@ -39,6 +39,13 @@ func BuildHostData(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials) } } +// For controllers that do not need to manage the BMC just set the node ID to use with Ironic API +func BuildEmptyHostData(provisionerID string) HostData { + return HostData{ + ProvisionerID: provisionerID, + } +} + // Factory is the interface for creating new Provisioner objects. type Factory interface { NewProvisioner(hostData HostData, publish EventPublisher) (Provisioner, error) @@ -75,11 +82,19 @@ type InspectData struct { BootMode metal3v1alpha1.BootMode } +// FirmwareConfig and FirmwareSettings are used for implementation of similar functionality +// FirmwareConfig contains a small subset of common names/values for the BIOS settings and the BMC +// driver converts them to vendor specific name/values. +// ActualFirmwareSettings are the complete settings retrieved from the BMC, the names and +// values are vendor specific. +// TargetFirmwareSettings contains values that the user has changed. type PrepareData struct { - TargetRAIDConfig *metal3v1alpha1.RAIDConfig - ActualRAIDConfig *metal3v1alpha1.RAIDConfig - RootDeviceHints *metal3v1alpha1.RootDeviceHints - FirmwareConfig *metal3v1alpha1.FirmwareConfig + TargetRAIDConfig *metal3v1alpha1.RAIDConfig + ActualRAIDConfig *metal3v1alpha1.RAIDConfig + RootDeviceHints *metal3v1alpha1.RootDeviceHints + FirmwareConfig *metal3v1alpha1.FirmwareConfig + TargetFirmwareSettings metal3v1alpha1.DesiredSettingsMap + ActualFirmwareSettings metal3v1alpha1.SettingsMap } type ProvisionData struct { @@ -158,6 +173,9 @@ type Provisioner interface { // HasCapacity checks if the backend has a free (de)provisioning slot for the current host HasCapacity() (result bool, err error) + + // GetFirmwareSettings gets the BIOS settings and optional schema from the host and returns maps + GetFirmwareSettings(includeSchema bool) (settings metal3v1alpha1.SettingsMap, schema map[string]metal3v1alpha1.SettingSchema, err error) } // Result holds the response from a call in the Provsioner API.