Skip to content

Commit f447d3c

Browse files
RozziiRozzii
committed
introduce preprovisioningKernelParams to BMH
This commit: - Implements support for additional kernel parameters during preprovisioning - Add PreprovisioningExtraKernelParams spec field to BareMetalHost CRD - Implement kernel parameter combination logic between BMH and PreprovisioningImage CRs - Add comprehensive unit tests for parameter combination scenarios - Extend Ironic node updates to servicing and preparing states - Rename getInstanceUpdateOpts to getProvisioningInstanceUpdateOptsForNode for consistency - Update BMC access utilities to support extra kernel parameters - Handle format-specific logic (ISO ignores BMH params, initrd combines both) Notes: - The new spec field is optional and maintains backward compatibility - ISO format PreprovisioningImages ignore BMH kernel params with warning - Initrd format PreprovisioningImages combine both BMH and PPI kernel params - Kernel parameters cannot be changed during fast-track deployments Signed-off-by: Adam Rozman <adam.rozman@est.tech>
1 parent 5a65e4a commit f447d3c

24 files changed

Lines changed: 736 additions & 229 deletions

‎apis/metal3.io/v1alpha1/baremetalhost_types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,11 @@ type BareMetalHostSpec struct {
495495
// without hardware profiles.
496496
HardwareProfile string `json:"hardwareProfile,omitempty"`
497497

498+
// The value of the kernel commandline argument list that will be passed
499+
// to the pre provisioning agent's kernel during boot.
500+
// +optional
501+
PreprovisioningExtraKernelParams string `json:"preprovisioningExtraKernelParams,omitempty"`
502+
498503
// Provide guidance about how to choose the device for the image
499504
// being provisioned. The default is currently to use /dev/sda as
500505
// the root device.

‎config/base/crds/bases/metal3.io_baremetalhosts.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,11 @@ spec:
344344
state (e.g. provisioned), its power state will be forced to match
345345
this value.
346346
type: boolean
347+
preprovisioningExtraKernelParams:
348+
description: |-
349+
The value of the kernel commandline argument list that will be passed
350+
to the pre provisioning agent's kernel during boot.
351+
type: string
347352
preprovisioningNetworkDataName:
348353
description: |-
349354
PreprovisioningNetworkDataName is the name of the Secret in the

‎config/render/capm3.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,11 @@ spec:
344344
state (e.g. provisioned), its power state will be forced to match
345345
this value.
346346
type: boolean
347+
preprovisioningExtraKernelParams:
348+
description: |-
349+
The value of the kernel commandline argument list that will be passed
350+
to the pre provisioning agent's kernel during boot.
351+
type: string
347352
preprovisioningNetworkDataName:
348353
description: |-
349354
PreprovisioningNetworkDataName is the name of the Secret in the

‎internal/controller/metal3.io/baremetalhost_controller.go‎

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,49 @@ func (info *reconcileInfo) publishEvent(reason, message string) {
8484
info.events = append(info.events, info.host.NewEvent(reason, message))
8585
}
8686

87+
// return the PreprovisioningExtraKernelParams from the reconciliation info.
88+
func (r *BareMetalHostReconciler) retrievePreprovisioningExtraKernelParamsSpec(ctx context.Context, info *reconcileInfo, prov provisioner.Provisioner) string {
89+
if info == nil || info.host == nil {
90+
return ""
91+
}
92+
kernelExtraPreprovParams := info.host.Spec.PreprovisioningExtraKernelParams
93+
94+
// This is a multy purpose check, this both checks whether BMO is running
95+
// with the default PPI provider and PPI CR reconciler and it also checks
96+
// whether the provisioner in use has support for PPI or not.
97+
preprovImgFormats, err := prov.PreprovisioningImageFormats()
98+
if err != nil {
99+
return kernelExtraPreprovParams
100+
}
101+
preprovImg, err := r.getPreprovImage(ctx, info, preprovImgFormats)
102+
if err != nil {
103+
return kernelExtraPreprovParams
104+
}
105+
// If the execution reaches this point it means there is a PPI in the
106+
// namespace with acceptable image fromat but it is managed by an external
107+
// controller-manager.
108+
// From this point onward there are 2 separate cases, if the image fromat
109+
// is initrd then BMH and PPI kernel params are combined. In case
110+
// the PPI is in ISO format then the BMH extra kernel params will be
111+
// ignored and the spec field will be cleaned up and a warning will be
112+
// logged to inform the user that in the specific use case only PPI
113+
// kernel params matter.
114+
if preprovImg != nil {
115+
format := preprovImg.Format
116+
trimmedParams := strings.TrimSpace(kernelExtraPreprovParams)
117+
if format == metal3api.ImageFormatISO {
118+
if trimmedParams != "" {
119+
// log warning
120+
info.log.Info("Warning: PPI is in ISO fromat, BMH preprovisioningExtraKernelParams are ignored in favor of PPI extra kernel parameters!")
121+
}
122+
// Kernel params are already injected to the ISO
123+
return ""
124+
}
125+
kernelExtraPreprovParams += " " + preprovImg.GeneratedImage.ExtraKernelParams
126+
}
127+
return kernelExtraPreprovParams
128+
}
129+
87130
// +kubebuilder:rbac:groups=metal3.io,resources=baremetalhosts,verbs=get;list;watch;create;update;patch;delete
88131
// +kubebuilder:rbac:groups=metal3.io,resources=baremetalhosts/status,verbs=get;update;patch
89132
// +kubebuilder:rbac:groups=metal3.io,resources=baremetalhosts/finalizers,verbs=update
@@ -871,16 +914,17 @@ func (r *BareMetalHostReconciler) registerHost(ctx context.Context, prov provisi
871914
provResult, provID, err := prov.Register(
872915
ctx,
873916
provisioner.ManagementAccessData{
874-
BootMode: info.host.Status.Provisioning.BootMode,
875-
AutomatedCleaningMode: info.host.Spec.AutomatedCleaningMode,
876-
State: info.host.Status.Provisioning.State,
877-
OperationalStatus: info.host.Status.OperationalStatus,
878-
CurrentImage: getCurrentImage(info.host),
879-
PreprovisioningImage: preprovImg,
880-
PreprovisioningNetworkData: preprovisioningNetworkData,
881-
HasCustomDeploy: hasCustomDeploy(info.host),
882-
DisablePowerOff: info.host.Spec.DisablePowerOff,
883-
CPUArchitecture: getHostArchitecture(info.host),
917+
BootMode: info.host.Status.Provisioning.BootMode,
918+
AutomatedCleaningMode: info.host.Spec.AutomatedCleaningMode,
919+
State: info.host.Status.Provisioning.State,
920+
OperationalStatus: info.host.Status.OperationalStatus,
921+
CurrentImage: getCurrentImage(info.host),
922+
PreprovisioningImage: preprovImg,
923+
PreprovisioningNetworkData: preprovisioningNetworkData,
924+
PreprovisioningExtraKernelParams: r.retrievePreprovisioningExtraKernelParamsSpec(ctx, info, prov),
925+
HasCustomDeploy: hasCustomDeploy(info.host),
926+
DisablePowerOff: info.host.Spec.DisablePowerOff,
927+
CPUArchitecture: getHostArchitecture(info.host),
884928
},
885929
credsChanged,
886930
info.host.Status.ErrorType == metal3api.RegistrationError)

‎internal/controller/metal3.io/baremetalhost_controller_test.go‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/base64"
66
"encoding/json"
7+
"errors"
78
"fmt"
89
"slices"
910
"testing"
@@ -213,6 +214,219 @@ func tryReconcile(t *testing.T, r *BareMetalHostReconciler, host *metal3api.Bare
213214
}
214215
}
215216

217+
// Test fixture for PreprovisioningImageFormats support
218+
// This extends the base fixture.Fixture to support custom PPI format configuration
219+
// for testing different PreprovisioningImage scenarios. Fixture provisioner
220+
// is used instead a mock provisioner as it is technically a fake provisioner
221+
// implementation.
222+
type testPPIFixture struct {
223+
fixture.Fixture
224+
ppiFormats []metal3api.ImageFormat
225+
ppiError error
226+
}
227+
228+
func (f *testPPIFixture) NewTestProvisioner(ctx context.Context, hostData provisioner.HostData, publisher provisioner.EventPublisher) (provisioner.Provisioner, error) {
229+
p, err := f.Fixture.NewProvisioner(ctx, hostData, publisher)
230+
if err != nil {
231+
return nil, err
232+
}
233+
234+
return &testPPIProvisioner{
235+
Provisioner: p,
236+
ppiFormats: f.ppiFormats,
237+
ppiError: f.ppiError,
238+
}, nil
239+
}
240+
241+
// Test provisioner that wraps the base fixture provisioner and adds
242+
// PreprovisioningImageFormats support for testing.
243+
type testPPIProvisioner struct {
244+
provisioner.Provisioner
245+
ppiFormats []metal3api.ImageFormat
246+
ppiError error
247+
}
248+
249+
func (p *testPPIProvisioner) PreprovisioningImageFormats() ([]metal3api.ImageFormat, error) {
250+
return p.ppiFormats, p.ppiError
251+
}
252+
253+
func TestRetrievePreprovisioningExtraKernelParamsSpec(t *testing.T) {
254+
tests := []struct {
255+
name string
256+
bmhKernelParams string
257+
ppiKernelParams string
258+
ppiFormat metal3api.ImageFormat
259+
ppiFormats []metal3api.ImageFormat
260+
ppiFormatsError error
261+
createPPI bool
262+
expectedResult string
263+
expectWarningLog bool
264+
}{
265+
{
266+
name: "nil reconcile info returns empty string",
267+
expectedResult: "",
268+
},
269+
{
270+
name: "provisioner no PPI support - use BMH params only",
271+
bmhKernelParams: "console=ttyS0",
272+
ppiFormatsError: errors.New("PPI is not supported"),
273+
expectedResult: "console=ttyS0",
274+
},
275+
{
276+
name: "PPI controller disabled - getPreprovImage fails",
277+
bmhKernelParams: "console=ttyS0",
278+
ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD},
279+
createPPI: false, // No PPI created, so getPreprovImage will fail
280+
expectedResult: "console=ttyS0",
281+
},
282+
{
283+
name: "initrd format - combine BMH and PPI params",
284+
bmhKernelParams: "console=ttyS0",
285+
ppiKernelParams: "debug=1",
286+
ppiFormat: metal3api.ImageFormatInitRD,
287+
ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD},
288+
createPPI: true,
289+
expectedResult: "console=ttyS0 debug=1",
290+
},
291+
{
292+
name: "initrd format - only PPI params",
293+
bmhKernelParams: "",
294+
ppiKernelParams: "debug=1",
295+
ppiFormat: metal3api.ImageFormatInitRD,
296+
ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD},
297+
createPPI: true,
298+
expectedResult: " debug=1",
299+
},
300+
{
301+
name: "initrd format - only BMH params",
302+
bmhKernelParams: "console=ttyS0",
303+
ppiKernelParams: "",
304+
ppiFormat: metal3api.ImageFormatInitRD,
305+
ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD},
306+
createPPI: true,
307+
expectedResult: "console=ttyS0 ",
308+
},
309+
{
310+
name: "ISO format - ignore BMH params with warning",
311+
bmhKernelParams: "console=ttyS0",
312+
ppiKernelParams: "debug=1",
313+
ppiFormat: metal3api.ImageFormatISO,
314+
ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatISO},
315+
createPPI: true,
316+
expectedResult: "",
317+
expectWarningLog: true,
318+
},
319+
{
320+
name: "ISO format - no BMH params, no warning",
321+
bmhKernelParams: "",
322+
ppiKernelParams: "debug=1",
323+
ppiFormat: metal3api.ImageFormatISO,
324+
ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatISO},
325+
createPPI: true,
326+
expectedResult: "",
327+
expectWarningLog: false,
328+
},
329+
{
330+
name: "complex kernel params combination",
331+
bmhKernelParams: "console=ttyS0,115200 intel_iommu=on",
332+
ppiKernelParams: "debug=1 systemd.log_level=debug",
333+
ppiFormat: metal3api.ImageFormatInitRD,
334+
ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD},
335+
createPPI: true,
336+
expectedResult: "console=ttyS0,115200 intel_iommu=on debug=1 systemd.log_level=debug",
337+
},
338+
{
339+
name: "empty BMH params with whitespace",
340+
bmhKernelParams: " ",
341+
ppiKernelParams: "debug=1",
342+
ppiFormat: metal3api.ImageFormatInitRD,
343+
ppiFormats: []metal3api.ImageFormat{metal3api.ImageFormatInitRD},
344+
createPPI: true,
345+
expectedResult: " debug=1",
346+
},
347+
}
348+
349+
for _, tt := range tests {
350+
t.Run(tt.name, func(t *testing.T) {
351+
// This test will be running with test reconciler using
352+
// fixtrue and fixture provisioner instead of local mocks.
353+
host := newDefaultHost(t)
354+
host.Spec.PreprovisioningExtraKernelParams = tt.bmhKernelParams
355+
356+
initObjs := []runtime.Object{host}
357+
358+
// Create PreprovisioningImage if needed
359+
if tt.createPPI {
360+
ppi := &metal3api.PreprovisioningImage{
361+
ObjectMeta: metav1.ObjectMeta{
362+
Name: host.Name,
363+
Namespace: host.Namespace,
364+
OwnerReferences: []metav1.OwnerReference{
365+
{
366+
APIVersion: "metal3.io/v1alpha1",
367+
Kind: "BareMetalHost",
368+
Name: host.Name,
369+
UID: host.UID,
370+
Controller: ptr.To(true),
371+
},
372+
},
373+
},
374+
Spec: metal3api.PreprovisioningImageSpec{
375+
Architecture: "x86_64",
376+
AcceptFormats: tt.ppiFormats,
377+
},
378+
Status: metal3api.PreprovisioningImageStatus{
379+
Architecture: "x86_64",
380+
ImageUrl: "http://example.com/image.iso",
381+
Format: tt.ppiFormat,
382+
ExtraKernelParams: tt.ppiKernelParams,
383+
Conditions: []metav1.Condition{
384+
{
385+
Type: string(metal3api.ConditionImageReady),
386+
Status: metav1.ConditionTrue,
387+
},
388+
{
389+
Type: string(metal3api.ConditionImageError),
390+
Status: metav1.ConditionFalse,
391+
},
392+
},
393+
},
394+
}
395+
initObjs = append(initObjs, ppi)
396+
}
397+
398+
// Setup custom fixture with PPI support
399+
fix := &testPPIFixture{
400+
Fixture: fixture.Fixture{},
401+
ppiFormats: tt.ppiFormats,
402+
ppiError: tt.ppiFormatsError,
403+
}
404+
405+
// Create reconciler with custom fixture
406+
r := newTestReconcilerWithFixture(t, &fix.Fixture, initObjs...)
407+
r.ProvisionerFactory = fix
408+
409+
var info *reconcileInfo
410+
if tt.name != "nil reconcile info returns empty string" {
411+
info = makeReconcileInfo(host)
412+
}
413+
414+
prov, err := fix.NewTestProvisioner(context.TODO(), provisioner.HostData{
415+
ObjectMeta: host.ObjectMeta,
416+
}, nil)
417+
require.NoError(t, err)
418+
419+
// Execute the function under test
420+
result := r.retrievePreprovisioningExtraKernelParamsSpec(context.TODO(), info, prov)
421+
422+
assert.Equal(t, tt.expectedResult, result, "Unexpected kernel params result")
423+
424+
// TODO: Add log verification if expectWarningLog is true
425+
// This would require capturing log output during test execution
426+
})
427+
}
428+
}
429+
216430
func waitForStatus(t *testing.T, r *BareMetalHostReconciler, host *metal3api.BareMetalHost, desiredStatus metal3api.OperationalStatus) {
217431
t.Helper()
218432
tryReconcile(t, r, host,

0 commit comments

Comments
 (0)