diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types.go b/apis/metal3.io/v1alpha1/baremetalhost_types.go index 89525d4373..6c53c32f1c 100644 --- a/apis/metal3.io/v1alpha1/baremetalhost_types.go +++ b/apis/metal3.io/v1alpha1/baremetalhost_types.go @@ -495,6 +495,11 @@ type BareMetalHostSpec struct { // without hardware profiles. HardwareProfile string `json:"hardwareProfile,omitempty"` + // The value of the kernel commandline argument list that will be passed + // to the preprovisioning agent's kernel during boot. + // +optional + PreprovisioningExtraKernelParams string `json:"preprovisioningExtraKernelParams,omitempty"` + // Provide guidance about how to choose the device for the image // being provisioned. The default is currently to use /dev/sda as // the root device. diff --git a/config/base/crds/bases/metal3.io_baremetalhosts.yaml b/config/base/crds/bases/metal3.io_baremetalhosts.yaml index 6d5fa1a3da..2c309c0c37 100644 --- a/config/base/crds/bases/metal3.io_baremetalhosts.yaml +++ b/config/base/crds/bases/metal3.io_baremetalhosts.yaml @@ -344,6 +344,11 @@ spec: state (e.g. provisioned), its power state will be forced to match this value. type: boolean + preprovisioningExtraKernelParams: + description: |- + The value of the kernel commandline argument list that will be passed + to the preprovisioning agent's kernel during boot. + type: string preprovisioningNetworkDataName: description: |- PreprovisioningNetworkDataName is the name of the Secret in the diff --git a/config/render/capm3.yaml b/config/render/capm3.yaml index 5690bb64b9..539a7b133d 100644 --- a/config/render/capm3.yaml +++ b/config/render/capm3.yaml @@ -344,6 +344,11 @@ spec: state (e.g. provisioned), its power state will be forced to match this value. type: boolean + preprovisioningExtraKernelParams: + description: |- + The value of the kernel commandline argument list that will be passed + to the preprovisioning agent's kernel during boot. + type: string preprovisioningNetworkDataName: description: |- PreprovisioningNetworkDataName is the name of the Secret in the diff --git a/internal/controller/metal3.io/baremetalhost_controller.go b/internal/controller/metal3.io/baremetalhost_controller.go index d6501bd680..05a884521f 100644 --- a/internal/controller/metal3.io/baremetalhost_controller.go +++ b/internal/controller/metal3.io/baremetalhost_controller.go @@ -58,6 +58,7 @@ const ( clarifySoftPoweroffFailure = "Continuing with hard poweroff after soft poweroff fails. More details: " hardwareDataFinalizer = metal3api.BareMetalHostFinalizer + "/hardwareData" NotReady = "Not ready" + defaultPPImgFormat = "DEFAULT_BMH_PPI_IMG" ) // BareMetalHostReconciler reconciles a BareMetalHost object. @@ -84,6 +85,49 @@ func (info *reconcileInfo) publishEvent(reason, message string) { info.events = append(info.events, info.host.NewEvent(reason, message)) } +// return the PreprovisioningExtraKernelParams from the reconciliation info. +func (r *BareMetalHostReconciler) retrievePreprovisioningExtraKernelParamsSpec(info *reconcileInfo, preprovImg *provisioner.PreprovisioningImage) string { + if info == nil || info.host == nil { + return "" + } + kernelExtraPreprovParams := info.host.Spec.PreprovisioningExtraKernelParams + + if preprovImg == nil { + return kernelExtraPreprovParams + } + // If the execution reaches this point it means there is a PPI in the + // namespace with acceptable image format but it is managed by an external + // controller-manager. + // From this point onward there are 3 separate cases, if the image format + // is initrd then BMH and PPI kernel params are combined. In case + // the PPI is in ISO format then the BMH extra kernel params will be + // ignored and a warning will be logged to inform the user that in the + // specific use case only PPI kernel params matter. In any other scenarios + // the preprovisioningExtraKernelParams are taken only from BMH spec. + if preprovImg != nil { + format := preprovImg.Format + trimmedParams := strings.TrimSpace(kernelExtraPreprovParams) + switch format { + case metal3api.ImageFormatISO: + if trimmedParams != "" { + info.log.Info("Warning: PPI is in ISO format, BMH preprovisioningExtraKernelParams are ignored in favor of PPI extra kernel parameters!") + } + // Kernel params are already injected to the ISO. + kernelExtraPreprovParams = "" + case metal3api.ImageFormatInitRD: + trimmedImageParams := strings.TrimSpace(preprovImg.GeneratedImage.ExtraKernelParams) + if trimmedImageParams != "" { + kernelExtraPreprovParams += " " + trimmedImageParams + } + info.log.Info("BMH and external PPI (preprovisioning)ExtraKernelParams got combined!") + default: + // This scenario covers when the image format is internally set to defaultPPIImgFormat. + info.log.Info("PPI is generated by BMO or has unknown image format, preprovisioningExtraKernelParams are taken only from BMH spec.") + } + } + return kernelExtraPreprovParams +} + // +kubebuilder:rbac:groups=metal3.io,resources=baremetalhosts,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=metal3.io,resources=baremetalhosts/status,verbs=get;update;patch // +kubebuilder:rbac:groups=metal3.io,resources=baremetalhosts/finalizers,verbs=update @@ -776,6 +820,15 @@ func (r *BareMetalHostReconciler) getPreprovImage(ctx context.Context, info *rec return nil, fmt.Errorf("failed to retrieve pre-provisioning image data: %w", err) } + // Check who reconciles the given PPI, if it is not this BMO then it is + // and externally managed PPI. + isExternal := false + ctrlOwnerRef := metav1.GetControllerOf(&preprovImage) + if ctrlOwnerRef == nil || ctrlOwnerRef.Name != info.host.Name || ctrlOwnerRef.UID != info.host.UID { + info.log.Info("Using externally managed PreprovisioningImage") + isExternal = true + } + // If the PreprovisioningImage is being deleted, treat it as unavailable if !preprovImage.DeletionTimestamp.IsZero() { info.log.Info("PreprovisioningImage is being deleted, waiting for new one") @@ -807,13 +860,20 @@ func (r *BareMetalHostReconciler) getPreprovImage(ctx context.Context, info *rec return nil, err } + PPIFormat := preprovImage.Status.Format + if !isExternal { + // PPI was generated by the default PPI provider thus it has to be + // handled separately from regular formats. + PPIFormat = defaultPPImgFormat + } + image := provisioner.PreprovisioningImage{ GeneratedImage: imageprovider.GeneratedImage{ ImageURL: preprovImage.Status.ImageUrl, KernelURL: preprovImage.Status.KernelUrl, ExtraKernelParams: preprovImage.Status.ExtraKernelParams, }, - Format: preprovImage.Status.Format, + Format: PPIFormat, } info.log.Info("using PreprovisioningImage", "Image", image) return &image, nil @@ -833,6 +893,7 @@ func (r *BareMetalHostReconciler) registerHost(ctx context.Context, prov provisi dirty = true } + // Nil value is acceptable it means the provisioner doesn't support PPI. preprovImgFormats, err := prov.PreprovisioningImageFormats(ctx) if err != nil { return actionError{err} @@ -876,16 +937,17 @@ func (r *BareMetalHostReconciler) registerHost(ctx context.Context, prov provisi provResult, provID, err := prov.Register( ctx, provisioner.ManagementAccessData{ - BootMode: info.host.Status.Provisioning.BootMode, - AutomatedCleaningMode: info.host.Spec.AutomatedCleaningMode, - State: info.host.Status.Provisioning.State, - OperationalStatus: info.host.Status.OperationalStatus, - CurrentImage: getCurrentImage(info.host), - PreprovisioningImage: preprovImg, - PreprovisioningNetworkData: preprovisioningNetworkData, - HasCustomDeploy: hasCustomDeploy(info.host), - DisablePowerOff: info.host.Spec.DisablePowerOff, - CPUArchitecture: getHostArchitecture(info.host), + BootMode: info.host.Status.Provisioning.BootMode, + AutomatedCleaningMode: info.host.Spec.AutomatedCleaningMode, + State: info.host.Status.Provisioning.State, + OperationalStatus: info.host.Status.OperationalStatus, + CurrentImage: getCurrentImage(info.host), + PreprovisioningImage: preprovImg, + PreprovisioningNetworkData: preprovisioningNetworkData, + PreprovisioningExtraKernelParams: r.retrievePreprovisioningExtraKernelParamsSpec(info, preprovImg), + HasCustomDeploy: hasCustomDeploy(info.host), + DisablePowerOff: info.host.Spec.DisablePowerOff, + CPUArchitecture: getHostArchitecture(info.host), }, credsChanged, info.host.Status.ErrorType == metal3api.RegistrationError) diff --git a/internal/controller/metal3.io/baremetalhost_controller_test.go b/internal/controller/metal3.io/baremetalhost_controller_test.go index 0ff272ce7b..a1624445c4 100644 --- a/internal/controller/metal3.io/baremetalhost_controller_test.go +++ b/internal/controller/metal3.io/baremetalhost_controller_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "slices" "testing" @@ -11,6 +12,7 @@ import ( metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" "github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc" + "github.com/metal3-io/baremetal-operator/pkg/imageprovider" "github.com/metal3-io/baremetal-operator/pkg/provisioner" "github.com/metal3-io/baremetal-operator/pkg/provisioner/fixture" "github.com/metal3-io/baremetal-operator/pkg/secretutils" @@ -213,6 +215,412 @@ func tryReconcile(t *testing.T, r *BareMetalHostReconciler, host *metal3api.Bare } } +// Test fixture for PreprovisioningImageFormats support +// This extends the base fixture.Fixture to support custom PPI format configuration +// for testing different PreprovisioningImage scenarios. Fixture provisioner +// is used instead a mock provisioner as it is technically a fake provisioner +// implementation. +type testPPIFixture struct { + fixture.Fixture + ppiFormats []metal3api.ImageFormat + ppiError error +} + +func (f *testPPIFixture) NewTestProvisioner(ctx context.Context, hostData provisioner.HostData, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) { + p, err := f.Fixture.NewProvisioner(ctx, hostData, publisher) + if err != nil { + return nil, err + } + + return &testPPIProvisioner{ + Provisioner: p, + ppiFormats: f.ppiFormats, + ppiError: f.ppiError, + }, nil +} + +// Test provisioner that wraps the base fixture provisioner and adds +// PreprovisioningImageFormats support for testing. +type testPPIProvisioner struct { + provisioner.Provisioner + ppiFormats []metal3api.ImageFormat + ppiError error +} + +func (p *testPPIProvisioner) PreprovisioningImageFormats(ctx context.Context) ([]metal3api.ImageFormat, error) { + return p.ppiFormats, p.ppiError +} + +func TestRetrievePreprovisioningExtraKernelParamsSpecIntegration(t *testing.T) { + tests := []struct { + name string + bmhKernelParams string + ppiKernelParams string + ppiFormat metal3api.ImageFormat + ppiFormats []metal3api.ImageFormat + ppiFormatsError error + createPPI bool + defaultPPI bool + expectedResult string + expectWarningLog bool + }{ + { + name: "nil reconcile info returns empty string", + expectedResult: "", + }, + { + name: "provisioner no PPI support - use BMH params only", + bmhKernelParams: "console=ttyS0", + ppiFormatsError: errors.New("PPI is not supported"), + expectedResult: "console=ttyS0", + }, + { + name: "PPI controller disabled - getPreprovImage fails", + bmhKernelParams: "console=ttyS0", + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: false, // No PPI created, so getPreprovImage will fail + expectedResult: "console=ttyS0", + }, + { + name: "initrd format - combine BMH and PPI params", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatInitRD, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: true, + defaultPPI: false, + expectedResult: "console=ttyS0 debug=1", + }, + { + name: "initrd format - only PPI params", + bmhKernelParams: "", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatInitRD, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: true, + defaultPPI: false, + expectedResult: " debug=1", + }, + { + name: "initrd format - only BMH params", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "", + ppiFormat: metal3api.ImageFormatInitRD, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: true, + defaultPPI: false, + expectedResult: "console=ttyS0", + }, + { + name: "ISO format - ignore BMH params with warning", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatISO, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatISO}, + createPPI: true, + defaultPPI: false, + expectedResult: "", + expectWarningLog: true, + }, + { + name: "ISO format - no BMH params, no warning", + bmhKernelParams: "", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatISO, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatISO}, + createPPI: true, + defaultPPI: false, + expectedResult: "", + expectWarningLog: false, + }, + { + name: "complex kernel params combination", + bmhKernelParams: "console=ttyS0,115200 intel_iommu=on", + ppiKernelParams: "debug=1 systemd.log_level=debug", + ppiFormat: metal3api.ImageFormatInitRD, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: true, + defaultPPI: false, + expectedResult: "console=ttyS0,115200 intel_iommu=on debug=1 systemd.log_level=debug", + }, + { + name: "empty BMH params with whitespace", + bmhKernelParams: " ", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatInitRD, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: true, + defaultPPI: false, + expectedResult: " debug=1", + }, + { + name: "BMO-managed PPI (default case) - use BMH params only", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatInitRD, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: true, + defaultPPI: true, // This makes it BMO-managed, format becomes defaultPPImgFormat + expectedResult: "console=ttyS0", + }, + { + name: "BMO-managed PPI with empty BMH params", + bmhKernelParams: "", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatISO, // Original format in PPI status + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatISO}, + createPPI: true, + defaultPPI: true, // BMO-managed, so format becomes defaultPPImgFormat + expectedResult: "", + }, + { + name: "unknown external format - use BMH params only", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "debug=1", + ppiFormat: "unknown-format", // Unknown format should hit default case + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: true, + defaultPPI: false, + expectedResult: "console=ttyS0", + }, + { + name: "initrd format - whitespace-only PPI params trimmed", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: " \t ", // Whitespace-only PPI params should be trimmed out + ppiFormat: metal3api.ImageFormatInitRD, + ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD}, + createPPI: true, + defaultPPI: false, + expectedResult: "console=ttyS0", // No trailing space because PPI params are empty after trim + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // This test will be running with test reconciler using + // fixtrue and fixture provisioner instead of local mocks. + host := newDefaultHost(t) + host.Spec.PreprovisioningExtraKernelParams = tt.bmhKernelParams + + initObjs := []runtime.Object{host} + + // Create PreprovisioningImage if needed + if tt.createPPI { + ppi := &metal3api.PreprovisioningImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: host.Name, + Namespace: host.Namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "metal3.io/v1alpha1", + Kind: "BareMetalHost", + Name: host.Name, + UID: host.UID, + Controller: ptr.To(tt.defaultPPI), + }, + }, + }, + Spec: metal3api.PreprovisioningImageSpec{ + Architecture: "x86_64", + AcceptFormats: tt.ppiFormats, + }, + Status: metal3api.PreprovisioningImageStatus{ + Architecture: "x86_64", + ImageUrl: "http://example.com/image.iso", + Format: tt.ppiFormat, + ExtraKernelParams: tt.ppiKernelParams, + Conditions: []metav1.Condition{ + { + Type: string(metal3api.ConditionImageReady), + Status: metav1.ConditionTrue, + }, + { + Type: string(metal3api.ConditionImageError), + Status: metav1.ConditionFalse, + }, + }, + }, + } + initObjs = append(initObjs, ppi) + } + + // Setup custom fixture with PPI support + fix := &testPPIFixture{ + Fixture: fixture.Fixture{}, + ppiFormats: tt.ppiFormats, + ppiError: tt.ppiFormatsError, + } + + // Create reconciler with custom fixture + r := newTestReconcilerWithFixture(t, &fix.Fixture, initObjs...) + r.ProvisionerFactory = fix + + var info *reconcileInfo + if tt.name != "nil reconcile info returns empty string" { + info = makeReconcileInfo(host) + } + + prov, err := fix.NewTestProvisioner(context.TODO(), provisioner.HostData{ + ObjectMeta: host.ObjectMeta, + }, nil) + require.NoError(t, err) + preprovImgFormats, err := prov.PreprovisioningImageFormats(context.TODO()) + if tt.ppiFormatsError != nil { + // In current BMO implementation it is mandatory that the provisioner's + // PPI related calls return without error during host registration, + // but retrievePreprovisioningExtraKernelParamsSpec could + // work in an implementation where the faulty PPI calls would be handled + // differently and wouldn't cause immediate registration failure. + // This path is an example of a handled format error that originates from + // the provisioner. + require.Error(t, err, "Expected PPI formats error") + } else { + require.NoError(t, err) + } + preprovImg, err := r.getPreprovImage(context.TODO(), info, preprovImgFormats) + require.NoError(t, err) + + // Execute the function under test + result := r.retrievePreprovisioningExtraKernelParamsSpec(info, preprovImg) + assert.Equal(t, tt.expectedResult, result, "Unexpected kernel params result") + }) + } +} + +func TestRetrievePreprovisioningExtraKernelParamsSpec(t *testing.T) { + tests := []struct { + name string + bmhKernelParams string + ppiKernelParams string + ppiFormat metal3api.ImageFormat + expectedResult string + expectWarningLog bool + preprovImg *provisioner.PreprovisioningImage + }{ + { + name: "nil reconcile info returns empty string", + expectedResult: "", + }, + { + name: "no PPI - use BMH params only", + bmhKernelParams: "console=ttyS0", + expectedResult: "console=ttyS0", + preprovImg: nil, + }, + { + name: "initrd format - combine BMH and PPI params", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatInitRD, + expectedResult: "console=ttyS0 debug=1", + }, + { + name: "initrd format - only PPI params", + bmhKernelParams: "", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatInitRD, + expectedResult: " debug=1", + }, + { + name: "initrd format - only BMH params", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "", + ppiFormat: metal3api.ImageFormatInitRD, + expectedResult: "console=ttyS0", + }, + { + name: "ISO format - ignore BMH params with warning", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatISO, + expectedResult: "", + expectWarningLog: true, + }, + { + name: "ISO format - no BMH params, no warning", + bmhKernelParams: "", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatISO, + expectedResult: "", + expectWarningLog: false, + }, + { + name: "complex kernel params combination", + bmhKernelParams: "console=ttyS0,115200 intel_iommu=on", + ppiKernelParams: "debug=1 systemd.log_level=debug", + ppiFormat: metal3api.ImageFormatInitRD, + expectedResult: "console=ttyS0,115200 intel_iommu=on debug=1 systemd.log_level=debug", + }, + { + name: "empty BMH params with whitespace", + bmhKernelParams: " ", + ppiKernelParams: "debug=1", + ppiFormat: metal3api.ImageFormatInitRD, + expectedResult: " debug=1", + }, + { + name: "BMO-managed PPI - use only BMH params", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "debug=1", + ppiFormat: "defaultPPImgFormat", + expectedResult: "console=ttyS0", + }, + { + name: "BMO-managed PPI with empty BMH params", + bmhKernelParams: "", + ppiKernelParams: "debug=1", + ppiFormat: "defaultPPImgFormat", + expectedResult: "", + }, + { + name: "unknown external format - use BMH params only", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: "debug=1", + ppiFormat: "unknown-format", + expectedResult: "console=ttyS0", + }, + { + name: "initrd format - whitespace-only PPI params trimmed", + bmhKernelParams: "console=ttyS0", + ppiKernelParams: " \t ", // Whitespace-only PPI params should be trimmed out + ppiFormat: metal3api.ImageFormatInitRD, + expectedResult: "console=ttyS0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + host := newDefaultHost(t) + host.Spec.PreprovisioningExtraKernelParams = tt.bmhKernelParams + + var info *reconcileInfo + if tt.name != "nil reconcile info returns empty string" { + info = makeReconcileInfo(host) + } + + var preprovImg *provisioner.PreprovisioningImage + if tt.preprovImg != nil { + preprovImg = tt.preprovImg + } else if tt.ppiFormat != "" { + preprovImg = &provisioner.PreprovisioningImage{ + GeneratedImage: imageprovider.GeneratedImage{ + ImageURL: "http://example.com/image.iso", + KernelURL: "", + ExtraKernelParams: tt.ppiKernelParams, + }, + Format: tt.ppiFormat, + } + } + + r := &BareMetalHostReconciler{} + + // Execute the function under test + result := r.retrievePreprovisioningExtraKernelParamsSpec(info, preprovImg) + assert.Equal(t, tt.expectedResult, result, "Unexpected kernel params result") + }) + } +} + func waitForStatus(t *testing.T, r *BareMetalHostReconciler, host *metal3api.BareMetalHost, desiredStatus metal3api.OperationalStatus) { t.Helper() tryReconcile(t, r, host, @@ -2580,10 +2988,22 @@ func TestGetPreprovImage(t *testing.T) { imageURL := "http://example.test/image.iso" acceptFormats := []metal3api.ImageFormat{metal3api.ImageFormatISO, metal3api.ImageFormatInitRD} arch := getControllerArchitecture() + // In order to mock externally managed PPI the controller reference will, + // set to false even though this PPI is created by the test reconciler. This way + // the externally managed PPI can be mocked with just one reconciler. image := &metal3api.PreprovisioningImage{ ObjectMeta: metav1.ObjectMeta{ Name: host.Name, Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "metal3.io/v1alpha1", + Kind: "BareMetalHost", + Name: host.Name, + UID: host.UID, + Controller: ptr.To(false), + }, + }, }, Spec: metal3api.PreprovisioningImageSpec{ Architecture: arch, diff --git a/internal/controller/metal3.io/preprovisioningimage_controller.go b/internal/controller/metal3.io/preprovisioningimage_controller.go index 3cf6cf58e5..b708955d58 100644 --- a/internal/controller/metal3.io/preprovisioningimage_controller.go +++ b/internal/controller/metal3.io/preprovisioningimage_controller.go @@ -140,49 +140,69 @@ func configChanged(img *metal3api.PreprovisioningImage, format metal3api.ImageFo img.Status.NetworkData == networkDataStatus) } +// findOwnerBMH checks if the owner of a PreProvisioningImage is a BMH and +// in case the owner is a BMH then the BMH will be returned. +func (r *PreprovisioningImageReconciler) findOwnerBMH(ctx context.Context, ppi *metal3api.PreprovisioningImage) (*metal3api.BareMetalHost, error) { + ownerRef := metav1.GetControllerOf(ppi) + if ownerRef == nil { + return nil, fmt.Errorf("no controller owner found for PreprovisioningImage %s", ppi.Name) + } + + if ownerRef.Kind != "BareMetalHost" { + return nil, fmt.Errorf("controller owner is not a BareMetalHost, got %s", ownerRef.Kind) + } + bmh := &metal3api.BareMetalHost{} + key := client.ObjectKey{ + Name: ownerRef.Name, + Namespace: ppi.Namespace, + } + err := r.Get(ctx, key, bmh) + return bmh, err +} + // update updates the PreprovisioningImage resource with the latest image information. // It checks if the image configuration has changed since the last update, and if so, // it discards the existing image, sets up the new image data, and triggers a new // image build. If the image configuration has not changed, it simply updates the // image status with the latest information. -func (r *PreprovisioningImageReconciler) update(ctx context.Context, img *metal3api.PreprovisioningImage, log logr.Logger) (bool, error) { - generation := img.GetGeneration() +func (r *PreprovisioningImageReconciler) update(ctx context.Context, ppi *metal3api.PreprovisioningImage, log logr.Logger) (bool, error) { + generation := ppi.GetGeneration() - if !r.ImageProvider.SupportsArchitecture(img.Spec.Architecture) { - log.Info("image architecture not supported", "architecture", img.Spec.Architecture) - return setError(generation, &img.Status, reasonImageConfigurationError, "Architecture not supported"), nil + if !r.ImageProvider.SupportsArchitecture(ppi.Spec.Architecture) { + log.Info("image architecture not supported", "architecture", ppi.Spec.Architecture) + return setError(generation, &ppi.Status, reasonImageConfigurationError, "Architecture not supported"), nil } - format := r.getImageFormat(img.Spec, log) + format := r.getImageFormat(ppi.Spec, log) if format == "" { - return setError(generation, &img.Status, reasonImageConfigurationError, "No acceptable image format supported"), nil + return setError(generation, &ppi.Status, reasonImageConfigurationError, "No acceptable image format supported"), nil } secretManager := secretutils.NewSecretManager(log, r.Client, r.APIReader) - networkData, secretStatus, err := getNetworkData(ctx, secretManager, img) + networkData, secretStatus, err := getNetworkData(ctx, secretManager, ppi) if err != nil { if k8serrors.IsNotFound(err) { log.Info("network data Secret does not exist") - return setError(generation, &img.Status, reasonImageMissingNetworkData, "NetworkData secret not found"), err + return setError(generation, &ppi.Status, reasonImageMissingNetworkData, "NetworkData secret not found"), err } return false, err } - if configChanged(img, format, secretStatus) { + if configChanged(ppi, format, secretStatus) { reason := "Config changed" - if meta.IsStatusConditionTrue(img.Status.Conditions, string(metal3api.ConditionImageReady)) { + if meta.IsStatusConditionTrue(ppi.Status.Conditions, string(metal3api.ConditionImageReady)) { // Ensure we mark the status as not ready before we remove the build // from the image cache. - setUnready(generation, &img.Status, reason) + setUnready(generation, &ppi.Status, reason) } else { - if err = r.discardExistingImage(img, log); err != nil { + if err = r.discardExistingImage(ppi, log); err != nil { return false, err } // Set up all the data before building the image and adding the URL, // so that even if we fail to write the built image status and the // config subsequently changes, the image cache cannot leak. - setImage(generation, &img.Status, imageprovider.GeneratedImage{}, - format, secretStatus, img.Spec.Architecture, + setImage(generation, &ppi.Status, imageprovider.GeneratedImage{}, + format, secretStatus, ppi.Spec.Architecture, reason) } return true, nil @@ -193,23 +213,34 @@ func (r *PreprovisioningImageReconciler) update(ctx context.Context, img *metal3 networkDataContent = networkData.Data } image, err := r.ImageProvider.BuildImage(imageprovider.ImageData{ - ImageMetadata: img.ObjectMeta.DeepCopy(), + ImageMetadata: ppi.ObjectMeta.DeepCopy(), Format: format, - Architecture: img.Spec.Architecture, + Architecture: ppi.Spec.Architecture, NetworkDataStatus: secretStatus, }, networkDataContent, log) if err != nil { failure := imageprovider.ImageBuildInvalidError{} if errors.As(err, &failure) { - log.Info("image build failed", "error", "err") - return setError(generation, &img.Status, reasonImageBuildInvalid, failure.Error()), nil + log.Info("image build failed", "error", failure) + return setError(generation, &ppi.Status, reasonImageBuildInvalid, failure.Error()), nil } return false, err } + + if format == metal3api.ImageFormatInitRD { + // Potentially might be other PPI owners in the future when + // multi tenancy support is fully implemented + bmh, err := r.findOwnerBMH(ctx, ppi) + if err != nil { + return false, fmt.Errorf("failed to find BareMetalHost: %w", err) + } + image.ExtraKernelParams = bmh.Spec.PreprovisioningExtraKernelParams + log.Info("extra kernel parameters added to PPI", "params", image.ExtraKernelParams) + } log.Info("image URL available", "url", image, "format", format) - return setImage(generation, &img.Status, image, format, - secretStatus, img.Spec.Architecture, + return setImage(generation, &ppi.Status, image, format, + secretStatus, ppi.Spec.Architecture, "Generated image"), nil } @@ -227,16 +258,16 @@ func (r *PreprovisioningImageReconciler) getImageFormat(spec metal3api.Preprovis return } -func (r *PreprovisioningImageReconciler) discardExistingImage(img *metal3api.PreprovisioningImage, log logr.Logger) error { - if img.Status.Format == "" { +func (r *PreprovisioningImageReconciler) discardExistingImage(ppi *metal3api.PreprovisioningImage, log logr.Logger) error { + if ppi.Status.Format == "" { return nil } - log.Info("discarding existing image", "image_url", img.Status.ImageUrl) + log.Info("discarding existing image", "image_url", ppi.Status.ImageUrl) return r.ImageProvider.DiscardImage(imageprovider.ImageData{ - ImageMetadata: img.ObjectMeta.DeepCopy(), - Format: img.Status.Format, - Architecture: img.Status.Architecture, - NetworkDataStatus: img.Status.NetworkData, + ImageMetadata: ppi.ObjectMeta.DeepCopy(), + Format: ppi.Status.Format, + Architecture: ppi.Status.Architecture, + NetworkDataStatus: ppi.Status.NetworkData, }) } @@ -255,17 +286,17 @@ func getErrorRetryDelay(status metal3api.PreprovisioningImageStatus) time.Durati return delay } -func getNetworkData(ctx context.Context, secretManager secretutils.SecretManager, img *metal3api.PreprovisioningImage) (*corev1.Secret, metal3api.SecretStatus, error) { - networkDataSecret := img.Spec.NetworkDataName +func getNetworkData(ctx context.Context, secretManager secretutils.SecretManager, ppi *metal3api.PreprovisioningImage) (*corev1.Secret, metal3api.SecretStatus, error) { + networkDataSecret := ppi.Spec.NetworkDataName if networkDataSecret == "" { return nil, metal3api.SecretStatus{}, nil } secretKey := client.ObjectKey{ Name: networkDataSecret, - Namespace: img.ObjectMeta.Namespace, + Namespace: ppi.ObjectMeta.Namespace, } - secret, err := secretManager.AcquireSecret(ctx, secretKey, img, false) + secret, err := secretManager.AcquireSecret(ctx, secretKey, ppi, false) if err != nil { return nil, metal3api.SecretStatus{}, err } diff --git a/internal/controller/metal3.io/preprovisioningimage_controller_test.go b/internal/controller/metal3.io/preprovisioningimage_controller_test.go index fd1fbad2ab..3ffbfae4a9 100644 --- a/internal/controller/metal3.io/preprovisioningimage_controller_test.go +++ b/internal/controller/metal3.io/preprovisioningimage_controller_test.go @@ -1,13 +1,18 @@ package controllers import ( + "context" "testing" "github.com/go-logr/logr" metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" "github.com/metal3-io/baremetal-operator/pkg/imageprovider" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" ) type mockImageProvider struct { @@ -202,3 +207,103 @@ func TestConfigChanged(t *testing.T) { }) } } + +func TestPreprovisioningImageReconciler_FindOwnerBMH(t *testing.T) { + tests := []struct { + name string + ownerRefs []metav1.OwnerReference + expectedError string + setupBMH bool + }{ + { + name: "no owner reference", + ownerRefs: []metav1.OwnerReference{}, + expectedError: "no controller owner found", + }, + { + name: "wrong owner kind", + ownerRefs: []metav1.OwnerReference{ + { + Kind: "Secret", + Name: "test-secret", + Controller: &[]bool{true}[0], + }, + }, + expectedError: "controller owner is not a BareMetalHost", + }, + { + name: "valid BMH owner", + ownerRefs: []metav1.OwnerReference{ + { + Kind: "BareMetalHost", + Name: "test-bmh", + Controller: &[]bool{true}[0], + }, + }, + setupBMH: true, + }, + { + name: "BMH not found", + ownerRefs: []metav1.OwnerReference{ + { + Kind: "BareMetalHost", + Name: "missing-bmh", + Controller: &[]bool{true}[0], + }, + }, + expectedError: "not found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := runtime.NewScheme() + _ = metal3api.AddToScheme(scheme) + + clientBuilder := fakeclient.NewClientBuilder().WithScheme(scheme) + + if tt.setupBMH { + bmh := &metal3api.BareMetalHost{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bmh", + Namespace: "test-namespace", + }, + } + clientBuilder = clientBuilder.WithObjects(bmh) + } + + c := clientBuilder.Build() + + ppi := &metal3api.PreprovisioningImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ppi", + Namespace: "test-namespace", + OwnerReferences: tt.ownerRefs, + }, + } + + r := &PreprovisioningImageReconciler{ + Client: c, + } + + // Execute the test. + bmh, err := r.findOwnerBMH(context.Background(), ppi) + + if tt.expectedError != "" { + assert.Contains(t, err.Error(), tt.expectedError) + if tt.expectedError == "not found" { + // Function returns empty BMH object when Get() fails. + assert.NotNil(t, bmh) + assert.Empty(t, bmh.Name) + } else { + assert.Nil(t, bmh) + } + } else { + require.NoError(t, err) + assert.NotNil(t, bmh) + assert.Equal(t, "test-bmh", bmh.Name) + assert.Equal(t, "test-namespace", bmh.Namespace) + } + }) + } +} diff --git a/pkg/hardwareutils/bmc/access.go b/pkg/hardwareutils/bmc/access.go index d3860286d7..c1ed67f3e9 100644 --- a/pkg/hardwareutils/bmc/access.go +++ b/pkg/hardwareutils/bmc/access.go @@ -68,7 +68,7 @@ type AccessDetails interface { // pre-populated with the access information, and the caller is // expected to add any other information that might be needed // (such as the kernel and ramdisk locations). - DriverInfo(bmcCreds Credentials) map[string]interface{} + DriverInfo(bmcCreds Credentials, preprovExtraKernelArgs string) map[string]interface{} BIOSInterface() string diff --git a/pkg/hardwareutils/bmc/access_test.go b/pkg/hardwareutils/bmc/access_test.go index 2c90a27d65..853d04f4dc 100644 --- a/pkg/hardwareutils/bmc/access_test.go +++ b/pkg/hardwareutils/bmc/access_test.go @@ -464,12 +464,13 @@ func TestDriverInfo(t *testing.T) { Scenario: "ipmi default port", input: "ipmi://192.168.122.1", expects: map[string]interface{}{ - "ipmi_port": ipmiDefaultPort, - "ipmi_password": "", - "ipmi_username": "", - "ipmi_address": "192.168.122.1", - "ipmi_verify_ca": false, - "ipmi_priv_level": "ADMINISTRATOR", + "ipmi_port": ipmiDefaultPort, + "ipmi_password": "", + "ipmi_username": "", + "ipmi_address": "192.168.122.1", + "ipmi_verify_ca": false, + "ipmi_priv_level": "ADMINISTRATOR", + "kernel_append_params": "", }, }, @@ -477,12 +478,13 @@ func TestDriverInfo(t *testing.T) { Scenario: "ipmi setting privilege level", input: "ipmi://192.168.122.1?privilegelevel=OPERATOR", expects: map[string]interface{}{ - "ipmi_port": ipmiDefaultPort, - "ipmi_password": "", - "ipmi_username": "", - "ipmi_address": "192.168.122.1", - "ipmi_verify_ca": false, - "ipmi_priv_level": "OPERATOR", + "ipmi_port": ipmiDefaultPort, + "ipmi_password": "", + "ipmi_username": "", + "ipmi_address": "192.168.122.1", + "ipmi_verify_ca": false, + "ipmi_priv_level": "OPERATOR", + "kernel_append_params": "", }, }, @@ -490,11 +492,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish", input: "redfish://192.168.122.1/redfish/v1/foo/bar", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_system_id": "/redfish/v1/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_system_id": "/redfish/v1/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -502,11 +505,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish http", input: "redfish+http://192.168.122.1/foo/bar", expects: map[string]interface{}{ - "redfish_address": "http://192.168.122.1", - "redfish_system_id": "/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "http://192.168.122.1", + "redfish_system_id": "/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -514,11 +518,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish https", input: "redfish+https://192.168.122.1/foo/bar", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_system_id": "/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_system_id": "/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -526,11 +531,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish port", input: "redfish://192.168.122.1:8080/foo/bar", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1:8080", - "redfish_system_id": "/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1:8080", + "redfish_system_id": "/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -538,11 +544,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish ipv6", input: "redfish://[fe80::fc33:62ff:fe83:8a76]/foo/bar", expects: map[string]interface{}{ - "redfish_address": "https://[fe80::fc33:62ff:fe83:8a76]", - "redfish_system_id": "/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://[fe80::fc33:62ff:fe83:8a76]", + "redfish_system_id": "/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -550,11 +557,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish ipv6 port", input: "redfish://[fe80::fc33:62ff:fe83:8a76]:8080/foo", expects: map[string]interface{}{ - "redfish_address": "https://[fe80::fc33:62ff:fe83:8a76]:8080", - "redfish_system_id": "/foo", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://[fe80::fc33:62ff:fe83:8a76]:8080", + "redfish_system_id": "/foo", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -562,10 +570,11 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish no system ID", input: "redfish+https://192.168.122.1/", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -573,10 +582,11 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish wrong system ID", input: "redfish+https://192.168.122.1/redfish/v1/", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -584,11 +594,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish virtual media", input: "redfish-virtualmedia://192.168.122.1/redfish/v1/foo/bar", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_system_id": "/redfish/v1/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_system_id": "/redfish/v1/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -596,10 +607,11 @@ func TestDriverInfo(t *testing.T) { Scenario: "Redfish virtual media wrong system ID", input: "redfish-virtualmedia://192.168.122.1/redfish/v1/", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -607,11 +619,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "ilo5 virtual media", input: "ilo5-virtualmedia://192.168.122.1/foo/bar", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_system_id": "/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_system_id": "/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -619,11 +632,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "idrac redfish", input: "idrac-redfish://192.168.122.1/foo/bar", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_system_id": "/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_system_id": "/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -631,11 +645,12 @@ func TestDriverInfo(t *testing.T) { Scenario: "idrac virtual media", input: "idrac-virtualmedia://192.168.122.1/redfish/v1/foo/bar", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_system_id": "/redfish/v1/foo/bar", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_system_id": "/redfish/v1/foo/bar", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, @@ -643,10 +658,11 @@ func TestDriverInfo(t *testing.T) { Scenario: "idrac virtual media wrong system ID", input: "idrac-virtualmedia://192.168.122.1/redfish/v1/", expects: map[string]interface{}{ - "redfish_address": "https://192.168.122.1", - "redfish_password": "", - "redfish_username": "", - "redfish_verify_ca": false, + "redfish_address": "https://192.168.122.1", + "redfish_password": "", + "redfish_username": "", + "redfish_verify_ca": false, + "kernel_append_params": "", }, }, } { @@ -655,7 +671,7 @@ func TestDriverInfo(t *testing.T) { if err != nil { t.Fatalf("unexpected parse error: %v", err) } - di := acc.DriverInfo(Credentials{}) + di := acc.DriverInfo(Credentials{}, "") // If a key is present when it should not, this will catch it if len(di) != len(tc.expects) { t.Fatalf("Number of items do not match: %v and %v, %#v", len(di), diff --git a/pkg/hardwareutils/bmc/idrac_virtualmedia.go b/pkg/hardwareutils/bmc/idrac_virtualmedia.go index 838d62fa0f..53cc8113ed 100644 --- a/pkg/hardwareutils/bmc/idrac_virtualmedia.go +++ b/pkg/hardwareutils/bmc/idrac_virtualmedia.go @@ -50,8 +50,8 @@ func (a *redfishiDracVirtualMediaAccessDetails) DisableCertificateVerification() // pre-populated with the access information, and the caller is // expected to add any other information that might be needed (such as // the kernel and ramdisk locations). -func (a *redfishiDracVirtualMediaAccessDetails) DriverInfo(bmcCreds Credentials) map[string]interface{} { - return a.redfishAccessDetails.DriverInfo(bmcCreds) +func (a *redfishiDracVirtualMediaAccessDetails) DriverInfo(bmcCreds Credentials, preProvExtraKernParams string) map[string]interface{} { + return a.redfishAccessDetails.DriverInfo(bmcCreds, preProvExtraKernParams) } // iDrac Virtual Media Overrides diff --git a/pkg/hardwareutils/bmc/ipmi.go b/pkg/hardwareutils/bmc/ipmi.go index 1d66551793..aba81ef8c8 100644 --- a/pkg/hardwareutils/bmc/ipmi.go +++ b/pkg/hardwareutils/bmc/ipmi.go @@ -71,13 +71,14 @@ func (a *ipmiAccessDetails) DisableCertificateVerification() bool { // pre-populated with the access information, and the caller is // expected to add any other information that might be needed (such as // the kernel and ramdisk locations). -func (a *ipmiAccessDetails) DriverInfo(bmcCreds Credentials) map[string]interface{} { +func (a *ipmiAccessDetails) DriverInfo(bmcCreds Credentials, preProvExtraKernParams string) map[string]interface{} { result := map[string]interface{}{ - "ipmi_port": a.portNum, - "ipmi_username": bmcCreds.Username, - "ipmi_password": bmcCreds.Password, - "ipmi_address": a.hostname, - "ipmi_priv_level": a.privilegelevel, + "ipmi_port": a.portNum, + "ipmi_username": bmcCreds.Username, + "ipmi_password": bmcCreds.Password, + "ipmi_address": a.hostname, + "ipmi_priv_level": a.privilegelevel, + "kernel_append_params": preProvExtraKernParams, } if a.disableCertificateVerification { diff --git a/pkg/hardwareutils/bmc/redfish.go b/pkg/hardwareutils/bmc/redfish.go index bcf78cb7a3..9dcd473892 100644 --- a/pkg/hardwareutils/bmc/redfish.go +++ b/pkg/hardwareutils/bmc/redfish.go @@ -83,11 +83,12 @@ func getRedfishAddress(bmcType, host string) string { // pre-populated with the access information, and the caller is // expected to add any other information that might be needed (such as // the kernel and ramdisk locations). -func (a *redfishAccessDetails) DriverInfo(bmcCreds Credentials) map[string]interface{} { +func (a *redfishAccessDetails) DriverInfo(bmcCreds Credentials, preProvExtraKernParams string) map[string]interface{} { result := map[string]interface{}{ - "redfish_username": bmcCreds.Username, - "redfish_password": bmcCreds.Password, - "redfish_address": getRedfishAddress(a.bmcType, a.host), + "redfish_username": bmcCreds.Username, + "redfish_password": bmcCreds.Password, + "redfish_address": getRedfishAddress(a.bmcType, a.host), + "kernel_append_params": preProvExtraKernParams, } trimmedPath := strings.Trim(a.path, "/") if trimmedPath != "" && trimmedPath != "redfish/v1" { diff --git a/pkg/hardwareutils/bmc/redfish_https.go b/pkg/hardwareutils/bmc/redfish_https.go index b51a972ca3..cbea390b86 100644 --- a/pkg/hardwareutils/bmc/redfish_https.go +++ b/pkg/hardwareutils/bmc/redfish_https.go @@ -51,12 +51,13 @@ func (a *redfishHTTPBootMediaAccessDetails) DisableCertificateVerification() boo // pre-populated with the access information, and the caller is // expected to add any other information that might be needed (such as // the kernel and ramdisk locations). -func (a *redfishHTTPBootMediaAccessDetails) DriverInfo(bmcCreds Credentials) map[string]interface{} { +func (a *redfishHTTPBootMediaAccessDetails) DriverInfo(bmcCreds Credentials, preProvExtraKernParams string) map[string]interface{} { result := map[string]interface{}{ - "redfish_system_id": a.path, - "redfish_username": bmcCreds.Username, - "redfish_password": bmcCreds.Password, - "redfish_address": getRedfishAddress(a.bmcType, a.host), + "redfish_system_id": a.path, + "redfish_username": bmcCreds.Username, + "redfish_password": bmcCreds.Password, + "redfish_address": getRedfishAddress(a.bmcType, a.host), + "kernel_append_params": preProvExtraKernParams, } if a.disableCertificateVerification { diff --git a/pkg/hardwareutils/bmc/redfish_virtualmedia.go b/pkg/hardwareutils/bmc/redfish_virtualmedia.go index 1178a1f4e2..edb660f1cc 100644 --- a/pkg/hardwareutils/bmc/redfish_virtualmedia.go +++ b/pkg/hardwareutils/bmc/redfish_virtualmedia.go @@ -49,8 +49,8 @@ func (a *redfishVirtualMediaAccessDetails) DisableCertificateVerification() bool // pre-populated with the access information, and the caller is // expected to add any other information that might be needed (such as // the kernel and ramdisk locations). -func (a *redfishVirtualMediaAccessDetails) DriverInfo(bmcCreds Credentials) map[string]interface{} { - return a.redfishAccessDetails.DriverInfo(bmcCreds) +func (a *redfishVirtualMediaAccessDetails) DriverInfo(bmcCreds Credentials, preProvExtraKernParams string) map[string]interface{} { + return a.redfishAccessDetails.DriverInfo(bmcCreds, preProvExtraKernParams) } func (a *redfishVirtualMediaAccessDetails) BIOSInterface() string { diff --git a/pkg/provisioner/ironic/inspecthardware.go b/pkg/provisioner/ironic/inspecthardware.go index 45cc27fa17..e70cfaf4d6 100644 --- a/pkg/provisioner/ironic/inspecthardware.go +++ b/pkg/provisioner/ironic/inspecthardware.go @@ -39,6 +39,7 @@ func (p *ironicProvisioner) startInspection(ctx context.Context, data provisione SetPropertiesOpts(opts, ironicNode), ) if !started { + p.log.Info("node update not started", "node", ironicNode.UUID) return } diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go index 588a5736c0..fa56cde145 100644 --- a/pkg/provisioner/ironic/ironic.go +++ b/pkg/provisioner/ironic/ironic.go @@ -41,10 +41,11 @@ const ( nameSeparator = "~" customDeployPriority = 80 - deployKernelKey = "deploy_kernel" - deployRamdiskKey = "deploy_ramdisk" - deployISOKey = "deploy_iso" - kernelParamsKey = "kernel_append_params" + deployKernelKey = "deploy_kernel" + deployRamdiskKey = "deploy_ramdisk" + deployISOKey = "deploy_iso" + kernelParamsKey = "kernel_append_params" + defaultKernelParam = "%default%" ) type macAddressConflictError struct { @@ -316,6 +317,15 @@ func (p *ironicProvisioner) createPXEEnabledNodePort(ctx context.Context, uuid, return nil } +func fmtPreprovExtraKernParams(params string) string { + trimmedParams := strings.TrimSpace(params) + if trimmedParams == "" { + return defaultKernelParam + } + // spacing matters for kernel params + return defaultKernelParam + " " + trimmedParams +} + // configureNode configures Node properties that are not related to any specific provisioning phase. // It populates the AutomatedClean field, as well as capabilities and architecture in Properties. // It also calls setDeployImage to populate IPA parameters in DriverInfo and @@ -323,7 +333,7 @@ func (p *ironicProvisioner) createPXEEnabledNodePort(ctx context.Context, uuid, func (p *ironicProvisioner) configureNode(ctx context.Context, data provisioner.ManagementAccessData, ironicNode *nodes.Node, bmcAccess bmc.AccessDetails) (result provisioner.Result, err error) { updater := clients.UpdateOptsBuilder(p.log) - deployImageInfo := setDeployImage(p.config, bmcAccess, data.PreprovisioningImage) + deployImageInfo := setDeployImage(p.config, bmcAccess, data) updater.SetDriverInfoOpts(deployImageInfo, ironicNode) updater.SetTopLevelOpt("automated_clean", @@ -445,7 +455,8 @@ func setExternalURL(p *ironicProvisioner, driverInfo map[string]any) map[string] // setDeployImage configures the IPA ramdisk parameters in the Node's DriverInfo. // It can use either the provided PreprovisioningImage or the global configuration from ironicConfig. -func setDeployImage(config ironicConfig, accessDetails bmc.AccessDetails, hostImage *provisioner.PreprovisioningImage) clients.UpdateOptsData { +func setDeployImage(config ironicConfig, accessDetails bmc.AccessDetails, data provisioner.ManagementAccessData) clients.UpdateOptsData { + hostImage := data.PreprovisioningImage deployImageInfo := clients.UpdateOptsData{ deployKernelKey: nil, deployRamdiskKey: nil, @@ -471,9 +482,12 @@ func setDeployImage(config ironicConfig, accessDetails bmc.AccessDetails, hostIm deployImageInfo[deployKernelKey] = config.deployKernelURL } deployImageInfo[deployRamdiskKey] = hostImage.ImageURL - if hostImage.ExtraKernelParams != "" { - // Using %default% prevents overriding the config in ironic-image - deployImageInfo[kernelParamsKey] = "%default% " + hostImage.ExtraKernelParams + // In case the preprovImage format is initrd, the source of + // truth for extra kernel params is the BMH as the provisioner is + // in charge of constructing the pxe and grub config files + // TODO Set the status on the preprov image CR + if data.PreprovisioningExtraKernelParams != "" { + deployImageInfo[kernelParamsKey] = fmtPreprovExtraKernParams(data.PreprovisioningExtraKernelParams) } return deployImageInfo } @@ -487,6 +501,9 @@ func setDeployImage(config ironicConfig, accessDetails bmc.AccessDetails, hostIm if config.deployKernelURL != "" && config.deployRamdiskURL != "" { deployImageInfo[deployKernelKey] = config.deployKernelURL deployImageInfo[deployRamdiskKey] = config.deployRamdiskURL + if data.PreprovisioningExtraKernelParams != "" { + deployImageInfo[kernelParamsKey] = fmtPreprovExtraKernParams(data.PreprovisioningExtraKernelParams) + } return deployImageInfo } } @@ -689,8 +706,8 @@ func (p *ironicProvisioner) setCustomDeployUpdateOptsForNode(ironicNode *nodes.N SetTopLevelOpt("deploy_interface", "custom-agent", ironicNode.DeployInterface) } -// getInstanceUpdateOpts constructs InstanceInfo options required to provision a Node in Ironic. -func (p *ironicProvisioner) getInstanceUpdateOpts(ironicNode *nodes.Node, data provisioner.ProvisionData) *clients.NodeUpdater { +// getProvisioningInstanceUpdateOptsForNode constructs InstanceInfo and DriverInfo options required to provision a Node in Ironic. +func (p *ironicProvisioner) getProvisioningInstanceUpdateOptsForNode(ironicNode *nodes.Node, data provisioner.ProvisionData) *clients.NodeUpdater { updater := clients.UpdateOptsBuilder(p.log) hasCustomDeploy := data.CustomDeploy != nil && data.CustomDeploy.Method != "" @@ -809,9 +826,8 @@ func (p *ironicProvisioner) GetFirmwareComponents(ctx context.Context) ([]metal3 func (p *ironicProvisioner) setUpForProvisioning(ctx context.Context, ironicNode *nodes.Node, data provisioner.ProvisionData) (result provisioner.Result, err error) { p.log.Info("starting provisioning", "node properties", ironicNode.Properties) - ironicNode, success, result, err := p.tryUpdateNode(ctx, ironicNode, - p.getInstanceUpdateOpts(ironicNode, data)) + p.getProvisioningInstanceUpdateOptsForNode(ironicNode, data)) if !success { return result, err } @@ -1060,14 +1076,14 @@ func (p *ironicProvisioner) startManualCleaning(ctx context.Context, bmcAccess b // Set raid configuration result, err = setTargetRAIDCfg(ctx, p, bmcAccess.RAIDInterface(), ironicNode, data) if result.Dirty || result.ErrorMessage != "" || err != nil { - return + return success, result, err } // Build manual clean steps cleanSteps, err := p.buildManualCleaningSteps(bmcAccess, data) if err != nil { result, err = operationFailed(err.Error()) - return + return success, result, err } // Start manual clean @@ -1083,7 +1099,7 @@ func (p *ironicProvisioner) startManualCleaning(ctx context.Context, bmcAccess b ) } result, err = operationComplete() - return + return success, result, err } // Prepare remove existing configuration and set new configuration. diff --git a/pkg/provisioner/ironic/ironic_test.go b/pkg/provisioner/ironic/ironic_test.go index a8759c4466..f5cc418519 100644 --- a/pkg/provisioner/ironic/ironic_test.go +++ b/pkg/provisioner/ironic/ironic_test.go @@ -137,3 +137,51 @@ func TestNewNoBMCDetails(t *testing.T) { require.NoError(t, err) assert.NotNil(t, prov) } + +func TestFmtPreprovExtraKernParams(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "empty params", + input: "", + expected: defaultKernelParam, + }, + { + name: "whitespace only", + input: " \t\n ", + expected: defaultKernelParam, + }, + { + name: "single param", + input: "console=ttyS0", + expected: defaultKernelParam + " console=ttyS0", + }, + { + name: "multiple params", + input: "console=ttyS0,115200 debug=1", + expected: defaultKernelParam + " console=ttyS0,115200 debug=1", + }, + { + name: "params with leading/trailing spaces", + input: " console=ttyS0 debug=1 ", + expected: defaultKernelParam + " console=ttyS0 debug=1", + }, + { + name: "complex kernel params", + input: "console=ttyS0,115200n8 intel_iommu=on iommu=pt systemd.unified_cgroup_hierarchy=0", + expected: defaultKernelParam + " console=ttyS0,115200n8 intel_iommu=on iommu=pt systemd.unified_cgroup_hierarchy=0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := fmtPreprovExtraKernParams(tt.input) + if result != tt.expected { + t.Errorf("expected %+v, got %+v", tt.expected, result) + } + }) + } +} diff --git a/pkg/provisioner/ironic/prepare_test.go b/pkg/provisioner/ironic/prepare_test.go index 4f49cf47c8..0286469783 100644 --- a/pkg/provisioner/ironic/prepare_test.go +++ b/pkg/provisioner/ironic/prepare_test.go @@ -17,21 +17,21 @@ import ( type RAIDTestBMC struct{} -func (r *RAIDTestBMC) Type() string { return "raid-test" } -func (r *RAIDTestBMC) NeedsMAC() bool { return false } -func (r *RAIDTestBMC) Driver() string { return "raid-test" } -func (r *RAIDTestBMC) DisableCertificateVerification() bool { return false } -func (r *RAIDTestBMC) DriverInfo(bmc.Credentials) (i map[string]any) { return } -func (r *RAIDTestBMC) SupportsISOPreprovisioningImage() bool { return false } -func (r *RAIDTestBMC) BIOSInterface() string { return "" } -func (r *RAIDTestBMC) BootInterface() string { return "" } -func (r *RAIDTestBMC) FirmwareInterface() string { return "" } -func (r *RAIDTestBMC) ManagementInterface() string { return "" } -func (r *RAIDTestBMC) PowerInterface() string { return "" } -func (r *RAIDTestBMC) RAIDInterface() string { return "" } -func (r *RAIDTestBMC) VendorInterface() string { return "" } -func (r *RAIDTestBMC) SupportsSecureBoot() bool { return false } -func (r *RAIDTestBMC) RequiresProvisioningNetwork() bool { return true } +func (r *RAIDTestBMC) Type() string { return "raid-test" } +func (r *RAIDTestBMC) NeedsMAC() bool { return false } +func (r *RAIDTestBMC) Driver() string { return "raid-test" } +func (r *RAIDTestBMC) DisableCertificateVerification() bool { return false } +func (r *RAIDTestBMC) DriverInfo(bmc.Credentials, string) (i map[string]any) { return } +func (r *RAIDTestBMC) SupportsISOPreprovisioningImage() bool { return false } +func (r *RAIDTestBMC) BIOSInterface() string { return "" } +func (r *RAIDTestBMC) BootInterface() string { return "" } +func (r *RAIDTestBMC) FirmwareInterface() string { return "" } +func (r *RAIDTestBMC) ManagementInterface() string { return "" } +func (r *RAIDTestBMC) PowerInterface() string { return "" } +func (r *RAIDTestBMC) RAIDInterface() string { return "" } +func (r *RAIDTestBMC) VendorInterface() string { return "" } +func (r *RAIDTestBMC) SupportsSecureBoot() bool { return false } +func (r *RAIDTestBMC) RequiresProvisioningNetwork() bool { return true } func (r *RAIDTestBMC) BuildBIOSSettings(_ *bmc.FirmwareConfig) ([]map[string]string, error) { return nil, nil } diff --git a/pkg/provisioner/ironic/provision_test.go b/pkg/provisioner/ironic/provision_test.go index 8b4860de40..91d41b311e 100644 --- a/pkg/provisioner/ironic/provision_test.go +++ b/pkg/provisioner/ironic/provision_test.go @@ -859,7 +859,7 @@ func TestGetUpdateOptsForNodeWithRootHints(t *testing.T) { BootMode: metal3api.DefaultBootMode, RootDeviceHints: host.Status.Provisioning.RootDeviceHints, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -955,7 +955,7 @@ func TestGetUpdateOptsForNodeVirtual(t *testing.T) { BootMode: metal3api.DefaultBootMode, HardwareProfile: hwProf, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -1054,7 +1054,7 @@ func TestGetUpdateOptsForNodeDell(t *testing.T) { BootMode: metal3api.DefaultBootMode, HardwareProfile: hwProf, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -1118,7 +1118,7 @@ func TestGetUpdateOptsForNodeLiveIso(t *testing.T) { Image: *host.Spec.Image, BootMode: metal3api.DefaultBootMode, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -1187,7 +1187,7 @@ func TestGetUpdateOptsForNodeImageToLiveIso(t *testing.T) { Image: *host.Spec.Image, BootMode: metal3api.DefaultBootMode, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -1267,7 +1267,7 @@ func TestGetUpdateOptsForNodeLiveIsoToImage(t *testing.T) { Image: *host.Spec.Image, BootMode: metal3api.DefaultBootMode, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -1341,7 +1341,7 @@ func TestGetUpdateOptsForNodeCustomDeploy(t *testing.T) { BootMode: metal3api.DefaultBootMode, CustomDeploy: host.Spec.CustomDeploy, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -1400,7 +1400,7 @@ func TestGetUpdateOptsForNodeCustomDeployWithImage(t *testing.T) { BootMode: metal3api.DefaultBootMode, CustomDeploy: host.Spec.CustomDeploy, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -1469,7 +1469,7 @@ func TestGetUpdateOptsForNodeImageToCustomDeploy(t *testing.T) { BootMode: metal3api.DefaultBootMode, CustomDeploy: host.Spec.CustomDeploy, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) @@ -1564,7 +1564,7 @@ func TestGetUpdateOptsForNodeSecureBoot(t *testing.T) { BootMode: metal3api.UEFISecureBoot, HardwareProfile: hwProf, } - patches := prov.getInstanceUpdateOpts(ironicNode, provData).Updates + patches := prov.getProvisioningInstanceUpdateOptsForNode(ironicNode, provData).Updates t.Logf("patches: %v", patches) diff --git a/pkg/provisioner/ironic/register.go b/pkg/provisioner/ironic/register.go index 970909651c..c503b83106 100644 --- a/pkg/provisioner/ironic/register.go +++ b/pkg/provisioner/ironic/register.go @@ -102,7 +102,8 @@ func (p *ironicProvisioner) Register(ctx context.Context, data provisioner.Manag return result, "", err } - driverInfo := bmcAccess.DriverInfo(p.bmcCreds) + finalPreprovKernParams := fmtPreprovExtraKernParams(data.PreprovisioningExtraKernelParams) + driverInfo := bmcAccess.DriverInfo(p.bmcCreds, finalPreprovKernParams) driverInfo = setExternalURL(p, driverInfo) // If we have not found a node yet, we need to create one @@ -149,6 +150,9 @@ func (p *ironicProvisioner) Register(ctx context.Context, data provisioner.Manag if credentialsChanged || bmcAddressChanged { p.log.Info("Updating driver info because the credentials and/or the BMC address changed") updater.SetTopLevelOpt("driver_info", driverInfo, ironicNode.DriverInfo) + } else if fmtPreprovExtraKernParams("") != finalPreprovKernParams { + p.log.Info("Updating driver info because extra preprovisioning kernel arguments are in use!") + updater.SetTopLevelOpt("driver_info", driverInfo, ironicNode.DriverInfo) } // The updater only updates disable_power_off if it has changed diff --git a/pkg/provisioner/ironic/register_test.go b/pkg/provisioner/ironic/register_test.go index 3a3297c3cd..46494a1abb 100644 --- a/pkg/provisioner/ironic/register_test.go +++ b/pkg/provisioner/ironic/register_test.go @@ -995,6 +995,7 @@ func TestSetDeployImage(t *testing.T) { Config ironicConfig Driver bmc.AccessDetails Image *provisioner.PreprovisioningImage + Data provisioner.ManagementAccessData ExpectBuild bool ExpectISO bool ExpectPXE bool @@ -1021,10 +1022,11 @@ func TestSetDeployImage(t *testing.T) { deployKernelURL: localKernel, deployRamdiskURL: localRamdisk, }, - Driver: isoDriver, - ExpectBuild: false, - ExpectISO: false, - ExpectPXE: true, + Driver: isoDriver, + ExpectBuild: false, + ExpectISO: false, + ExpectPXE: true, + ExpectKernelParams: true, }, { Scenario: "pxe no imgbuilder", @@ -1034,10 +1036,11 @@ func TestSetDeployImage(t *testing.T) { deployRamdiskURL: localRamdisk, deployISOURL: localIso, }, - Driver: pxeDriver, - ExpectBuild: false, - ExpectISO: false, - ExpectPXE: true, + Driver: pxeDriver, + ExpectBuild: false, + ExpectISO: false, + ExpectPXE: true, + ExpectKernelParams: true, }, { Scenario: "iso no build", @@ -1085,9 +1088,10 @@ func TestSetDeployImage(t *testing.T) { }, Format: metal3api.ImageFormatInitRD, }, - ExpectBuild: true, - ExpectISO: false, - ExpectPXE: true, + ExpectBuild: true, + ExpectISO: false, + ExpectPXE: true, + ExpectKernelParams: true, }, { Scenario: "pxe build with new kernel and kernel params", @@ -1102,7 +1106,7 @@ func TestSetDeployImage(t *testing.T) { GeneratedImage: imageprovider.GeneratedImage{ ImageURL: buildRamdisk, KernelURL: buildKernel, - ExtraKernelParams: kernelParams, + ExtraKernelParams: "whatever", }, Format: metal3api.ImageFormatInitRD, }, @@ -1211,7 +1215,9 @@ func TestSetDeployImage(t *testing.T) { for _, tc := range testCases { t.Run(tc.Scenario, func(t *testing.T) { - opts := setDeployImage(tc.Config, tc.Driver, tc.Image) + tc.Data.PreprovisioningExtraKernelParams = kernelParams + tc.Data.PreprovisioningImage = tc.Image + opts := setDeployImage(tc.Config, tc.Driver, tc.Data) switch { case tc.ExpectISO: diff --git a/pkg/provisioner/ironic/servicing_test.go b/pkg/provisioner/ironic/servicing_test.go index bf8ff09d9a..838083851c 100644 --- a/pkg/provisioner/ironic/servicing_test.go +++ b/pkg/provisioner/ironic/servicing_test.go @@ -18,21 +18,21 @@ import ( type BIOSTestBMC struct{} -func (r *BIOSTestBMC) Type() string { return "bios-test" } -func (r *BIOSTestBMC) NeedsMAC() bool { return false } -func (r *BIOSTestBMC) Driver() string { return "bios-test" } -func (r *BIOSTestBMC) DisableCertificateVerification() bool { return false } -func (r *BIOSTestBMC) DriverInfo(bmc.Credentials) (i map[string]any) { return } -func (r *BIOSTestBMC) SupportsISOPreprovisioningImage() bool { return false } -func (r *BIOSTestBMC) BIOSInterface() string { return "" } -func (r *BIOSTestBMC) BootInterface() string { return "" } -func (r *BIOSTestBMC) FirmwareInterface() string { return "" } -func (r *BIOSTestBMC) ManagementInterface() string { return "" } -func (r *BIOSTestBMC) PowerInterface() string { return "" } -func (r *BIOSTestBMC) RAIDInterface() string { return "" } -func (r *BIOSTestBMC) VendorInterface() string { return "" } -func (r *BIOSTestBMC) SupportsSecureBoot() bool { return false } -func (r *BIOSTestBMC) RequiresProvisioningNetwork() bool { return true } +func (r *BIOSTestBMC) Type() string { return "bios-test" } +func (r *BIOSTestBMC) NeedsMAC() bool { return false } +func (r *BIOSTestBMC) Driver() string { return "bios-test" } +func (r *BIOSTestBMC) DisableCertificateVerification() bool { return false } +func (r *BIOSTestBMC) DriverInfo(bmc.Credentials, string) (i map[string]any) { return } +func (r *BIOSTestBMC) SupportsISOPreprovisioningImage() bool { return false } +func (r *BIOSTestBMC) BIOSInterface() string { return "" } +func (r *BIOSTestBMC) BootInterface() string { return "" } +func (r *BIOSTestBMC) FirmwareInterface() string { return "" } +func (r *BIOSTestBMC) ManagementInterface() string { return "" } +func (r *BIOSTestBMC) PowerInterface() string { return "" } +func (r *BIOSTestBMC) RAIDInterface() string { return "" } +func (r *BIOSTestBMC) VendorInterface() string { return "" } +func (r *BIOSTestBMC) SupportsSecureBoot() bool { return false } +func (r *BIOSTestBMC) RequiresProvisioningNetwork() bool { return true } func (r *BIOSTestBMC) BuildBIOSSettings(_ *bmc.FirmwareConfig) ([]map[string]string, error) { return nil, nil } diff --git a/pkg/provisioner/ironic/testbmc/testbmc.go b/pkg/provisioner/ironic/testbmc/testbmc.go index 362e0d8bd1..d92e341dd7 100644 --- a/pkg/provisioner/ironic/testbmc/testbmc.go +++ b/pkg/provisioner/ironic/testbmc/testbmc.go @@ -16,6 +16,7 @@ func NewTestBMCAccessDetails(parsedURL *url.URL, disableCertificateVerification bmcType: parsedURL.Scheme, hostname: parsedURL.Hostname(), disableCertificateVerification: disableCertificateVerification, + driverInfo: "", }, nil } @@ -23,6 +24,7 @@ type testAccessDetails struct { bmcType string hostname string disableCertificateVerification bool + driverInfo string } func (a *testAccessDetails) Type() string { @@ -48,12 +50,13 @@ func (a *testAccessDetails) DisableCertificateVerification() bool { // pre-populated with the access information, and the caller is // expected to add any other information that might be needed (such as // the kernel and ramdisk locations). -func (a *testAccessDetails) DriverInfo(bmcCreds bmc.Credentials) map[string]any { - result := map[string]any{ - "test_port": "42", - "test_username": bmcCreds.Username, - "test_password": bmcCreds.Password, - "test_address": a.hostname, +func (a *testAccessDetails) DriverInfo(bmcCreds bmc.Credentials, preProvExtraKernParams string) map[string]interface{} { + result := map[string]interface{}{ + "test_port": "42", + "test_username": bmcCreds.Username, + "test_password": bmcCreds.Password, + "test_address": a.hostname, + "kernel_append_params": preProvExtraKernParams, } if a.disableCertificateVerification { diff --git a/pkg/provisioner/provisioner.go b/pkg/provisioner/provisioner.go index ddd40f4a5b..b2759ebbc2 100644 --- a/pkg/provisioner/provisioner.go +++ b/pkg/provisioner/provisioner.go @@ -74,16 +74,17 @@ type PreprovisioningImage struct { } type ManagementAccessData struct { - BootMode metal3api.BootMode - AutomatedCleaningMode metal3api.AutomatedCleaningMode - State metal3api.ProvisioningState - OperationalStatus metal3api.OperationalStatus - CurrentImage *metal3api.Image - PreprovisioningImage *PreprovisioningImage - PreprovisioningNetworkData string - HasCustomDeploy bool - DisablePowerOff bool - CPUArchitecture string + BootMode metal3api.BootMode + AutomatedCleaningMode metal3api.AutomatedCleaningMode + State metal3api.ProvisioningState + OperationalStatus metal3api.OperationalStatus + CurrentImage *metal3api.Image + PreprovisioningImage *PreprovisioningImage + PreprovisioningNetworkData string + PreprovisioningExtraKernelParams string + HasCustomDeploy bool + DisablePowerOff bool + CPUArchitecture string } type AdoptData struct {