diff --git a/apis/metal3.io/v1alpha1/firmwareschema_types.go b/apis/metal3.io/v1alpha1/firmwareschema_types.go index 58b0247ba3..df76ff5dad 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) Validate(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,86 @@ 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 { - // return true if no settings to check validity - return true + if schema.LowerBound != nil && value.IntValue() < *schema.LowerBound { + return SchemaSettingError{name: name, message: fmt.Sprintf("integer %s is below minimum value %d", value.String(), *schema.LowerBound)} + } + if schema.UpperBound != nil && value.IntValue() > *schema.UpperBound { + return SchemaSettingError{name: name, message: fmt.Sprintf("integer %s is above maximum value %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 { - // return true if no settings to check validity - return true + strLen := len(value.String()) + if schema.MinLength != nil && strLen < *schema.MinLength { + return SchemaSettingError{name: name, message: fmt.Sprintf("string %s length is below minimum length %d", value.String(), *schema.MinLength)} } - return (len(value.String()) >= *schema.MinLength && len(value.String()) <= *schema.MaxLength) + if schema.MaxLength != nil && strLen > *schema.MaxLength { + return SchemaSettingError{name: name, message: fmt.Sprintf("string %s length is above maximum length %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: "passwords are immutable"} 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) ValidateSetting(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.Validate(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..8bca68d148 100644 --- a/apis/metal3.io/v1alpha1/hostfirmwaresettings_types.go +++ b/apis/metal3.io/v1alpha1/hostfirmwaresettings_types.go @@ -33,6 +33,16 @@ type SchemaReference struct { Name string `json:"name"` } +type SettingsConditionType string + +const ( + // Indicates that the settings in the Spec are different than Status + UpdateRequested SettingsConditionType = "UpdateRequested" + + // Indicates if the settings are valid and can be configured on the host + SettingsValid SettingsConditionType = "Valid" +) + // HostFirmwareSettingsSpec defines the desired state of HostFirmwareSettings type HostFirmwareSettingsSpec struct { @@ -53,9 +63,18 @@ type HostFirmwareSettingsStatus struct { // Settings are the actual firmware settings stored as name/value pairs Settings SettingsMap `json:"settings" required:"true"` + + // Track whether settings stored in the spec are valid based on the schema + // +patchMergeKey=type + // +patchStrategy=merge + // +listType=map + // +listMapKey=type + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` } //+kubebuilder:object:root=true +//+kubebuilder:resource:shortName=hfs //+kubebuilder:subresource:status // HostFirmwareSettings is the Schema for the hostfirmwaresettings API diff --git a/apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go b/apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go index 6d743f1a17..40463267c6 100644 --- a/apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go +++ b/apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go @@ -9,7 +9,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" ) -func TestCheckSettingIsValid(t *testing.T) { +func TestValidateSetting(t *testing.T) { lower_bound := 1 upper_bound := 20 @@ -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 maximum length 16", }, { Scenario: "StringTypeFailLower", Name: "AssetTag", Value: intstr.FromString(""), - Expected: false, + Expected: "Setting AssetTag is invalid, string length is below minimum length 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 maximum value 20", }, { Scenario: "IntegerTypeFailLower", Name: "NetworkBootRetryCount", Value: intstr.FromInt(0), - Expected: false, + Expected: "Setting NetworkBootRetryCount is invalid, integer 0 is below minimum value 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.ValidateSetting(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 2dd196bfe2..7ab1507ec3 100644 --- a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go +++ b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ package v1alpha1 import ( "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -583,6 +584,13 @@ func (in *HostFirmwareSettingsStatus) DeepCopyInto(out *HostFirmwareSettingsStat (*out)[key] = val } } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HostFirmwareSettingsStatus. @@ -786,6 +794,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..e506c90d7c 100644 --- a/config/crd/bases/metal3.io_hostfirmwaresettings.yaml +++ b/config/crd/bases/metal3.io_hostfirmwaresettings.yaml @@ -13,6 +13,8 @@ spec: kind: HostFirmwareSettings listKind: HostFirmwareSettingsList plural: hostfirmwaresettings + shortNames: + - hfs singular: hostfirmwaresettings scope: Namespaced versions: @@ -55,6 +57,80 @@ spec: description: HostFirmwareSettingsStatus defines the observed state of HostFirmwareSettings properties: + conditions: + description: Track whether settings stored in the spec are valid based + on the schema + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: + \"Available\", \"Progressing\", and \"Degraded\" // +patchMergeKey=type + \ // +patchStrategy=merge // +listType=map // +listMapKey=type + \ Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"` + \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map 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 2deabba153..42cdf0a7c3 100644 --- a/config/render/capm3.yaml +++ b/config/render/capm3.yaml @@ -1195,6 +1195,8 @@ spec: kind: HostFirmwareSettings listKind: HostFirmwareSettingsList plural: hostfirmwaresettings + shortNames: + - hfs singular: hostfirmwaresettings scope: Namespaced versions: @@ -1237,6 +1239,80 @@ spec: description: HostFirmwareSettingsStatus defines the observed state of HostFirmwareSettings properties: + conditions: + description: Track whether settings stored in the spec are valid based + on the schema + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: + \"Available\", \"Progressing\", and \"Degraded\" // +patchMergeKey=type + \ // +patchStrategy=merge // +listType=map // +listMapKey=type + \ Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"` + \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map 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 +1441,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..ef387dace5 100644 --- a/controllers/metal3.io/baremetalhost_controller.go +++ b/controllers/metal3.io/baremetalhost_controller.go @@ -32,6 +32,7 @@ import ( corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -89,6 +90,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) { @@ -786,7 +791,19 @@ func (r *BareMetalHostReconciler) actionPreparing(prov provisioner.Provisioner, prepareData.ActualRAIDConfig = nil dirty = true } + + // Use settings in hostFirmwareSettings if available + hfs, err := r.getHostFirmwareSettings(info) + if err != nil { + info.log.Info("hostFirmwareSettings not available for cleaning") + } + 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")} } @@ -1153,6 +1170,37 @@ func saveHostProvisioningSettings(host *metal3v1alpha1.BareMetalHost) (dirty boo return } +// Get the stored firmware settings if there are valid changes +func (r *BareMetalHostReconciler) getHostFirmwareSettings(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 + } + + // 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.SettingsValid)) { + return hfs, nil + } + + info.log.Info("hostFirmwareSettings not valid", "namespacename", info.request.NamespacedName) + return nil, nil + } + + info.log.Info("hostFirmwareSettings no updates", "namespacename", info.request.NamespacedName) + return nil, 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..566480e78a --- /dev/null +++ b/controllers/metal3.io/hostfirmwaresettings_controller.go @@ -0,0 +1,466 @@ +/* + + +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" + + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + 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/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/source" + + 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 + bmh *metal3v1alpha1.BareMetalHost + events []corev1.Event +} + +type conditionReason string + +const ( + reasonSuccess conditionReason = "Success" + reasonConfigurationError conditionReason = "ConfigurationError" +) + +func (info *rInfo) publishEvent(reason, message string) { + + t := metav1.Now() + hfsEvent := corev1.Event{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: reason + "-", + Namespace: info.hfs.ObjectMeta.Namespace, + }, + InvolvedObject: corev1.ObjectReference{ + Kind: "HostFirmwareSettings", + Namespace: info.hfs.Namespace, + Name: info.hfs.Name, + UID: info.hfs.UID, + APIVersion: metal3v1alpha1.GroupVersion.String(), + }, + Reason: reason, + Message: message, + Source: corev1.EventSource{ + Component: "metal3-hostfirmwaresettings-controller", + }, + FirstTimestamp: t, + LastTimestamp: t, + Count: 1, + Type: corev1.EventTypeNormal, + ReportingController: "metal3.io/hostfirmwaresettings-controller", + } + + info.events = append(info.events, hfsEvent) +} + +//+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") + + // Get the corresponding baremetalhost in this namespace, if one doesn't exist don't continue processing + bmh := &metal3v1alpha1.BareMetalHost{} + if err = r.Get(context.TODO(), req.NamespacedName, bmh); err != nil { + reqLogger.Info("could not get baremetalhost, not running reconciler") + // only run again if created + return ctrl.Result{}, nil + } + + // Fetch the HostFirmwareSettings + hfs := &metal3v1alpha1.HostFirmwareSettings{} + info := &rInfo{log: reqLogger, hfs: hfs, bmh: bmh} + if err = r.Get(ctx, req.NamespacedName, hfs); err != nil { + if k8serrors.IsNotFound(err) { + // A resource doesn't exist, create one + if err = r.newHostFirmwareSettings(info, req.NamespacedName); err != nil { + + return ctrl.Result{}, errors.Wrap(err, "could not create firmware settings") + } + } else { + // Error reading the object - requeue the request. + return ctrl.Result{}, errors.Wrap(err, "could not load host firmware settings") + } + } + + // Create a provisioner that can access Ironic API + prov, err := r.ProvisionerFactory.NewProvisioner(provisioner.BuildHostDataNoBMC(*bmh), info.publishEvent) + if err != nil { + return ctrl.Result{}, errors.Wrap(err, "failed to create provisioner") + } + + ready, err := prov.IsReady() + if err != nil || !ready { + reqLogger.Info("provisioner is not ready", "RequeueAfter:", provisionerNotReadyRetryDelay) + return ctrl.Result{Requeue: true, RequeueAfter: provisionerNotReadyRetryDelay}, nil + } + + // Get the data from Ironic and update HFS, the call to provisioner may fail if settings can't + // be retrieved for the bios_interface. + if err = r.updateHostFirmwareSettings(prov, info); err != nil { + return ctrl.Result{}, errors.Wrap(err, "Could not update hostFirmwareSettings") + } + + for _, e := range info.events { + r.publishEvent(req, e) + } + + // 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", "node", info.bmh.Status.Provisioning.ID) + + // 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.updateStatus(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) updateStatus(info *rInfo, settings metal3v1alpha1.SettingsMap, schema *metal3v1alpha1.FirmwareSchema) (err error) { + + if info.hfs.Status.Settings == nil { + info.hfs.Status.Settings = make(metal3v1alpha1.SettingsMap) + } + + // Update Status + 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 + } + + // Check if any Spec settings are different than Status + specMismatch := false + for k, v := range info.hfs.Spec.Settings { + if statusVal, ok := info.hfs.Status.Settings[k]; ok { + if v != intstr.FromString(statusVal) { + specMismatch = true + break + } + } + } + + // Set up the conditions which will be used by baremetalhost controller when determining whether to add settings during cleaning + reason := reasonSuccess + generation := info.hfs.GetGeneration() + + if specMismatch { + setCondition(generation, &info.hfs.Status, metal3v1alpha1.UpdateRequested, metav1.ConditionTrue, reason, "") + + } else { + setCondition(generation, &info.hfs.Status, metal3v1alpha1.UpdateRequested, metav1.ConditionFalse, reason, "") + } + + // Run validation on the Spec to detect invalid values entered by user, including Spec settings not in Status + // Eventually this will be handled by a webhook + errors := r.validateHostFirmwareSettings(info, schema) + if len(errors) == 0 { + setCondition(generation, &info.hfs.Status, metal3v1alpha1.SettingsValid, metav1.ConditionTrue, reason, "") + } else { + for _, error := range errors { + info.publishEvent("ValidationFailed", fmt.Sprintf("Invalid BIOS setting: %v", error)) + } + + reason = reasonConfigurationError + setCondition(generation, &info.hfs.Status, metal3v1alpha1.SettingsValid, metav1.ConditionFalse, reason, "Invalid BIOS setting") + } + + 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 info.bmh.Status.HardwareDetails != nil { + firmwareSchema.Spec.HardwareVendor = info.bmh.Status.HardwareDetails.SystemVendor.Manufacturer + firmwareSchema.Spec.HardwareModel = info.bmh.Status.HardwareDetails.SystemVendor.ProductName + } + + // 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 +} + +// Create a hostFirmwareSettings +func (r *HostFirmwareSettingsReconciler) newHostFirmwareSettings(info *rInfo, namespacedName types.NamespacedName) (err error) { + + info.hfs.ObjectMeta = metav1.ObjectMeta{ + Name: namespacedName.Name, Namespace: namespacedName.Namespace} + info.hfs.Status.Settings = make(metal3v1alpha1.SettingsMap) + info.hfs.Spec.Settings = make(metal3v1alpha1.DesiredSettingsMap) + + if err = r.Create(context.TODO(), info.hfs); err != nil { + return errors.Wrap(err, "failure creating hostFirmwareSettings resource") + } + + // Set bmh as owner, this makes sure the resource is deleted when bmh is deleted + if controllerutil.SetControllerReference(info.bmh, info.hfs, r.Scheme()); err != nil { + return errors.Wrap(err, "could not set bmh as controller") + } + if err = r.Update(context.TODO(), info.hfs); err != nil { + return errors.Wrap(err, "could not update hostfirmwaresettings") + } + + info.log.Info("created new hostFirmwareSettings resource") + + return nil +} + +// EventHandler for updates to both the baremetalhost and hostfirmwarettings +func (r *HostFirmwareSettingsReconciler) updateEventHandler(e event.UpdateEvent) bool { + + r.Log.Info("hostfirmwaresettings in event handler") + + // If this is a baremetalhost, only return true on certain state transitions + var oldState metal3v1alpha1.ProvisioningState = metal3v1alpha1.StateNone + newState := oldState + + oldHost, oldHostExists := e.ObjectOld.(*metal3v1alpha1.BareMetalHost) + if oldHostExists { + oldState = oldHost.Status.Provisioning.State + } + newHost, newHostExists := e.ObjectNew.(*metal3v1alpha1.BareMetalHost) + if newHostExists { + newState = newHost.Status.Provisioning.State + } + + if newHostExists && !oldHostExists { + // The baremetalhost has been created + return true + } + if oldHostExists || newHostExists { + // Data needs to be retrieved from Ironic when node is moved to Ready e.g. after cleaning and when + // provisioned, its not necessary on every state change as data from Ironic will be the same. + if (newState == metal3v1alpha1.StateReady || newState == metal3v1alpha1.StateProvisioned) && oldState != newState { + r.Log.Info("baremetalhost event state update", "oldstate", oldState, "newstate", newState) + return true + } + + return false + } + + _, oldHFS := e.ObjectOld.(*metal3v1alpha1.HostFirmwareSettings) + _, newHFS := e.ObjectNew.(*metal3v1alpha1.HostFirmwareSettings) + if !(oldHFS || newHFS) { + // Don't process if not a hostFirmwareSetting + return false + } + + // This is a hostfirmwaresettings resource, if the update increased the resource generation then process it + // changes to LastUpdated will not increase the resource generation + if e.ObjectNew.GetGeneration() != e.ObjectOld.GetGeneration() { + r.Log.Info("hostfirmwaresettings resource generation changed, updating") + return true + } + + return false +} + +// SetupWithManager sets up the controller with the Manager. +func (r *HostFirmwareSettingsReconciler) SetupWithManager(mgr ctrl.Manager) error { + + return ctrl.NewControllerManagedBy(mgr). + For(&metal3v1alpha1.HostFirmwareSettings{}). + WithEventFilter( + predicate.Funcs{ + UpdateFunc: r.updateEventHandler, + }). + Watches(&source.Kind{Type: &metal3v1alpha1.BareMetalHost{}}, &handler.EnqueueRequestForObject{}, builder.Predicates{}). + Complete(r) +} + +// Validate the HostFirmwareSetting Spec against the schema +func (r *HostFirmwareSettingsReconciler) validateHostFirmwareSettings(info *rInfo, schema *metal3v1alpha1.FirmwareSchema) []error { + + var errors []error + + for name, val := range info.hfs.Spec.Settings { + // Prohibit any Spec settings with "Password" + if strings.Contains(name, "Password") { + errors = append(errors, fmt.Errorf("Cannot set Password field")) + break + } + + // The setting must be in the Status + if _, ok := info.hfs.Status.Settings[name]; !ok { + errors = append(errors, fmt.Errorf("Setting %s is not in the Status field", name)) + break + } + + // check validity of updated value + if schema != nil { + if err := schema.ValidateSetting(name, val, schema.Spec.Schema); err != nil { + errors = append(errors, err) + } + } + } + + if len(errors) > 0 { + return errors + } + + return nil +} + +func (r *HostFirmwareSettingsReconciler) publishEvent(request ctrl.Request, event corev1.Event) { + reqLogger := r.Log.WithValues("hostfirmwaresettings", request.NamespacedName) + reqLogger.Info("publishing event", "reason", event.Reason, "message", event.Message) + err := r.Create(context.TODO(), &event) + if err != nil { + reqLogger.Info("failed to record event, ignoring", + "reason", event.Reason, "message", event.Message, "error", err) + } + return +} + +func setCondition(generation int64, status *metal3v1alpha1.HostFirmwareSettingsStatus, + cond metal3v1alpha1.SettingsConditionType, newStatus metav1.ConditionStatus, + reason conditionReason, message string) { + newCondition := metav1.Condition{ + Type: string(cond), + Status: newStatus, + ObservedGeneration: generation, + Reason: string(reason), + Message: message, + } + meta.SetStatusCondition(&status.Conditions, newCondition) +} + +// 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..126ca1671a --- /dev/null +++ b/controllers/metal3.io/hostfirmwaresettings_test.go @@ -0,0 +1,614 @@ +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/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 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 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 +} + +// Mock settings to return from provisioner +func getCurrentSettings() metal3v1alpha1.SettingsMap { + + return metal3v1alpha1.SettingsMap{ + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + "AssetTag": "X45672917", + } +} + +// Mock schema to return from provisioner +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, + }, + } +} + +// Create the baremetalhost reconciler and use that to create bmh in same namespace +func createBaremetalHost() *metal3v1alpha1.BareMetalHost { + + bmh := &metal3v1alpha1.BareMetalHost{} + bmh.ObjectMeta = metav1.ObjectMeta{Name: hostName, Namespace: hostNamespace} + c := fakeclient.NewFakeClient(bmh) + + reconciler := &BareMetalHostReconciler{ + Client: c, + ProvisionerFactory: nil, + Log: ctrl.Log.WithName("bmh_reconciler").WithName("BareMetalHost"), + } + + reconciler.Create(context.TODO(), bmh) + + return bmh +} + +func getExpectedSchemaResource() *metal3v1alpha1.FirmwareSchema { + firmwareSchema := getSchema() + firmwareSchema.Spec.Schema = getCurrentSchemaSettings() + + return firmwareSchema +} + +// Create an HFS with input spec settings +func getHFS(spec metal3v1alpha1.HostFirmwareSettingsSpec) *metal3v1alpha1.HostFirmwareSettings { + + hfs := &metal3v1alpha1.HostFirmwareSettings{} + + 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} + + hfs.Spec = spec + + return 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 + // the expected created or updated resource + ExpectedSettings *metal3v1alpha1.HostFirmwareSettings + // whether the spec values pass the validity test + SpecIsValid bool + }{ + { + Scenario: "initial 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, + ExpectedSettings: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{}, + }, + Status: metal3v1alpha1.HostFirmwareSettingsStatus{ + FirmwareSchema: &metal3v1alpha1.SchemaReference{ + Name: schemaName, + Namespace: hostNamespace, + }, + Settings: metal3v1alpha1.SettingsMap{ + "AssetTag": "X45672917", + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + }, + Conditions: []metav1.Condition{ + {Type: "UpdateRequested", Status: "False", Reason: "Success"}, + {Type: "Valid", Status: "True", Reason: "Success"}, + }, + }, + }, + SpecIsValid: true, + }, + { + Scenario: "initial 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, + ExpectedSettings: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{}, + }, + Status: metal3v1alpha1.HostFirmwareSettingsStatus{ + FirmwareSchema: &metal3v1alpha1.SchemaReference{ + Name: schemaName, + Namespace: hostNamespace, + }, + Settings: metal3v1alpha1.SettingsMap{ + "AssetTag": "X45672917", + "L2Cache": "10x512 KB", + "NetworkBootRetryCount": "20", + "ProcVirtualization": "Disabled", + "SecureBoot": "Enabled", + }, + Conditions: []metav1.Condition{ + {Type: "UpdateRequested", Status: "False", Reason: "Success"}, + {Type: "Valid", Status: "True", Reason: "Success"}, + }, + }, + }, + SpecIsValid: true, + }, + { + Scenario: "updated settings", + 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{ + "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, + 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", + }, + Conditions: []metav1.Condition{ + {Type: "UpdateRequested", Status: "True", Reason: "Success"}, + {Type: "Valid", Status: "True", Reason: "Success"}, + }, + }, + }, + SpecIsValid: true, + }, + { + Scenario: "spec updated with invalid setting", + 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{ + "NetworkBootRetryCount": intstr.FromString("1000"), + "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, + ExpectedSettings: &metal3v1alpha1.HostFirmwareSettings{ + Spec: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "NetworkBootRetryCount": intstr.FromString("1000"), + "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", + }, + Conditions: []metav1.Condition{ + {Type: "UpdateRequested", Status: "True", Reason: "Success"}, + {Type: "Valid", Status: "False", Reason: "ConfigurationError", Message: "Invalid BIOS setting"}, + }, + }, + }, + SpecIsValid: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.Scenario, func(t *testing.T) { + + ctx := context.TODO() + prov := getMockProvisioner(getCurrentSettings(), getCurrentSchemaSettings()) + + tc.ExpectedSettings.TypeMeta = metav1.TypeMeta{ + Kind: "HostFirmwareSettings", + APIVersion: "metal3.io/v1alpha1"} + tc.ExpectedSettings.ObjectMeta = metav1.ObjectMeta{ + Name: hostName, + Namespace: hostNamespace, + ResourceVersion: "2"} + + hfs := tc.CurrentHFSResource + r := getTestHFSReconciler(hfs) + // Create bmh resource needed by hfs reconciler + bmh := createBaremetalHost() + + info := &rInfo{ + log: logf.Log.WithName("controllers").WithName("HostFirmwareSettings"), + hfs: tc.CurrentHFSResource, + bmh: bmh, + } + + 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() + for i := 0; i < 2; i++ { + tc.ExpectedSettings.Status.Conditions[i].LastTransitionTime = currentTime + actualSettings.Status.Conditions[i].LastTransitionTime = currentTime + } + + 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 validate hostFirmwareSettings +func TestValidateHostFirmwareSettings(t *testing.T) { + + testCases := []struct { + Scenario string + SpecSettings metal3v1alpha1.HostFirmwareSettingsSpec + ExpectedError string + }{ + { + Scenario: "valid spec changes with schema", + SpecSettings: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + }, + ExpectedError: "", + }, + { + Scenario: "invalid string", + SpecSettings: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("A really long POST message"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + }, + ExpectedError: "Setting CustomPostMessage is invalid, string A really long POST message length is above maximum length 20", + }, + { + Scenario: "invalid int", + SpecSettings: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("2000"), + }, + }, + ExpectedError: "Setting NetworkBootRetryCount is invalid, integer 2000 is above maximum value 20", + }, + { + Scenario: "invalid enum", + SpecSettings: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Not enabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + }, + }, + ExpectedError: "Setting ProcVirtualization is invalid, unknown enumeration value - Not enabled", + }, + { + Scenario: "invalid name", + SpecSettings: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "SomeNewSetting": intstr.FromString("foo"), + }, + }, + ExpectedError: "Setting SomeNewSetting is not in the Status field", + }, + { + Scenario: "invalid password in spec", + SpecSettings: metal3v1alpha1.HostFirmwareSettingsSpec{ + Settings: metal3v1alpha1.DesiredSettingsMap{ + "CustomPostMessage": intstr.FromString("All tests passed"), + "ProcVirtualization": intstr.FromString("Disabled"), + "NetworkBootRetryCount": intstr.FromString("20"), + "SysPassword": intstr.FromString("Pa%$word"), + }, + }, + ExpectedError: "Cannot set Password field", + }, + } + + for _, tc := range testCases { + t.Run(tc.Scenario, func(t *testing.T) { + + hfs := getHFS(tc.SpecSettings) + r := getTestHFSReconciler(hfs) + info := &rInfo{ + log: logf.Log.WithName("controllers").WithName("HostFirmwareSettings"), + hfs: hfs, + } + + errors := r.validateHostFirmwareSettings(info, getExpectedSchemaResource()) + if len(errors) == 0 { + assert.Equal(t, tc.ExpectedError, "") + } else { + for _, error := range errors { + assert.Equal(t, tc.ExpectedError, error.Error()) + } + } + }) + } +} 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 81b8a6188d..924dd25d3c 100644 --- a/main.go +++ b/main.go @@ -168,6 +168,15 @@ func main() { os.Exit(1) } + 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) + } + setupChecks(mgr) if enableWebhook { 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..037807783c --- /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 node for BIOS settings: Host not registered", + }, + } + + 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 13075b7ada..ce70d2b292 100644 --- a/pkg/provisioner/ironic/ironic.go +++ b/pkg/provisioner/ironic/ironic.go @@ -840,6 +840,54 @@ 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) { + + ironicNode, err := p.getNode() + if err != nil { + return nil, nil, errors.Wrap(err, fmt.Sprintf("could not get node for BIOS settings")) + } + + // 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, ironicNode.UUID, opts).Extract() + } else { + settingsList, biosListErr = nodes.ListBIOSSettings(p.client, ironicNode.UUID, nil).Extract() + } + if biosListErr != nil { + return nil, nil, errors.Wrap(biosListErr, + fmt.Sprintf("could not get BIOS settings for node %s", ironicNode.UUID)) + } + p.log.Info("retrieved BIOS settings for node", "node", ironicNode.UUID, "size", len(settingsList)) + + 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 @@ -1001,19 +1049,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"], "node", p.nodeID) + } + } + + // 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, }, }, ) @@ -1024,6 +1103,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) @@ -1089,6 +1183,7 @@ func (p *ironicProvisioner) Prepare(data provisioner.PrepareData, unprepared boo started = true } // Automated clean finished + result, err = operationComplete() case nodes.Manageable: 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..639d1e1b3c 100644 --- a/pkg/provisioner/ironic/testserver/ironic.go +++ b/pkg/provisioner/ironic/testserver/ironic.go @@ -305,5 +305,104 @@ 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) + m.AddDefaultResponseJSON("/v1/nodes/"+nodeUUID, "", http.StatusOK, nodes.Node{ + UUID: nodeUUID, + }) + 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) + m.AddDefaultResponseJSON("/v1/nodes/"+nodeUUID, "", http.StatusOK, nodes.Node{ + UUID: nodeUUID, + }) + 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..914a512e10 100644 --- a/pkg/provisioner/provisioner.go +++ b/pkg/provisioner/provisioner.go @@ -39,6 +39,14 @@ func BuildHostData(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials) } } +// For controllers that do not need to manage the BMC just set the host and node ID to use with Ironic API +func BuildHostDataNoBMC(host metal3v1alpha1.BareMetalHost) HostData { + return HostData{ + ObjectMeta: *host.ObjectMeta.DeepCopy(), + ProvisionerID: host.Status.Provisioning.ID, + } +} + // Factory is the interface for creating new Provisioner objects. type Factory interface { NewProvisioner(hostData HostData, publish EventPublisher) (Provisioner, error) @@ -75,11 +83,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 +174,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.