propose additional node specific pre-provisioning kernel args #552

Rozzii · 2025-06-24T09:54:29Z

This commit:

Introduces the proposal for BMH support for node specific extra kernel command line arguments for pre-provisioning images.

The motivation behind the commit can be found in the proposal.

metal3-io-bot · 2025-06-24T09:54:34Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rozzii for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

design/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Rozzii · 2025-06-25T06:11:46Z

Something that might be interesting to
/cc @dtantsur @elfosardo

lentzi90

Nice! I have just a few nits/typos and one suggestion to clarify how the strings are concatenated (with spaces?)

design/baremetal-operator/node-specific-kernel-args.md

lentzi90

/lgtm

design/baremetal-operator/node-specific-kernel-args.md

dtantsur · 2025-06-25T14:09:05Z

design/baremetal-operator/node-specific-kernel-args.md

+   `preProvisionKernelArgs`
+2. Extend the generic `Provisioner` interface to support the new optional field
+3. Implement support for the `preProvisionKernelArgs` in the Ironic provisioner
+


You also need to extend the PreprovisioningImage object to accept these params.

We can keep them separate, I didn't intend to make this new spec to be applied for the "PreprovisioningImage" I thought that already has a spec for this purpose.

That will create a wild inconsistency that will cause users file bugs for me with "preprovisioningExtraKernelParams does not work" :( Because they'll expect OpenShift's version of Metal3 to work the same way as upstream, and without PPI support it won't.

Btw it's fair if you ask me to follow-up with PPI support myself if you don't have cycles for that. I just want it to be in the design.

I have already looked into it, my original impression was that the user is already able to set the Kernel Args via the preprovisioningImage CR's spec but now I understand that it is not possible as on that CR the kernel args are only part of the status and not the spec. I have also checked https://github.com/openshift/image-customization-controller so I am more familiar with the use-case of the preprovisioningImage CR.

Based on what I know now It makes sense to include preprovisioningImage in the proposal.

design/baremetal-operator/node-specific-kernel-args.md

metal3-io-bot · 2025-07-22T12:24:23Z

New changes are detected. LGTM label has been removed.

Rozzii · 2025-07-22T12:44:55Z

WIP implementation: metal3-io/baremetal-operator#2576 at the moment without tests, but functionally this already worked .

EDIT: status and the preprovImage CR support is missing ATM

This commit: - Introduces the proposal for BMH support for additional node specific kernel command line arguments for non dynamically built pre-provisioning images. The motivation behind the commit can be found in the proposal. Signed-off-by: Adam Rozman <[email protected]>

fmuyassarov · 2025-07-28T13:28:46Z

design/baremetal-operator/node-specific-kernel-args.md

+5. Introducing a `preprovisioningExtraKernelParams` related status field that
+   would display the last applied `preprovisioningExtraKernelParams` value.
+   Let's call this field `status.lastAppliedPreprovisioningExtraKernelParams`


Perhaps something shorter would be easier but I don't have strong opinion on this
Example, status.lastAppliedKernelParams

I have been thinking about this, and our offline discussions and now I am on the opinion that I should separate this status topic to a different proposal/discussion, because right now we don't really provide status for applying provisioning or preprvisioning data to Ironic API so we might need a more comprehensive approach to give feedback about what, user/network/meta data and what preprovisioning data was applied. We might also need a status flag to show if the previously mentioned data references/values have changed since the last LCM operation (inspect,provision,deprovision,clean, service).

If I would leave the status topic out from the proposal I think it would be still aligned with our current feedback mechanism (or the lack of it) and then we can improve our feedback mechanism with the followup proposal.

fmuyassarov · 2025-07-28T13:47:34Z

design/baremetal-operator/node-specific-kernel-args.md

+4. Combine the values of the `preprovisioningExtraKernelParams` and the
+   `preprovisioningImage`'s `status.extraKernelParams` field if a
+   `preprovisioningImage` is refferences by the BMH and teh `status.extraKernelParams`
+   is not empty


Is it a good idea to construct a value of a field from two different sources? I imagine things might go wrong, for example how do we handle a case when prepovisioningImage object referenced by BMH doesn't exist for some reason?

Does this mean that a node might have different kernel parameters compared to what is asked via .spec.preprovisioningExtraKernelParams, in case preprovisioningImage status.extraKernelParams has something non-empty? Should not we have a single source of truth for defining the parameters and I guess the status sub-resource was never meant for defining the parameters but more for summarizing the current state right? Perhaps whoever writes into status.extraKernelParams should only rely on BMH spec and not something else? Or is this a transitional state that we eventually target to move to a single source?

@dtantsur thoughts ?

lentzi90 · 2025-08-19T06:24:51Z

design/baremetal-operator/node-specific-kernel-args.md

+3. Implement support for the `preprovisioningExtraKernelParams` in the Ironic provisioner
+4. Combine the values of the `preprovisioningExtraKernelParams` and the
+   `preprovisioningImage`'s `status.extraKernelParams` field if a
+   `preprovisioningImage` is refferences by the BMH and teh `status.extraKernelParams`


Suggested change

`preprovisioningImage` is refferences by the BMH and teh `status.extraKernelParams`

`preprovisioningImage` is referenced by the BMH and the `status.extraKernelParams`

lentzi90 · 2025-08-19T06:32:22Z

design/baremetal-operator/node-specific-kernel-args.md

+ bmc:
+   address: idrac://192.168.122.1:6230/
+   credentialsName: bm0-bmc-secret
+ preprovisioningExtraKernelParams: "--timeout 60000 systemd.journald.forward_to_console=yes ipa-debug=1"


Suggested change

preprovisioningExtraKernelParams: "--timeout 60000 systemd.journald.forward_to_console=yes ipa-debug=1"

preprovisioningExtraKernelParams: "--timeout 60000 systemd.journald.forward_to_console=yes ipa-debug=1"

lentzi90 · 2025-08-19T06:38:12Z

design/baremetal-operator/node-specific-kernel-args.md

+### Implementation Details/Notes/Constraints
+
+The `preprovisioningExtraKernelParams` field would be located directly under `spec`
+section. The format of the new field would be a list/array of individual


This has been changes elsewhere to be a string

Suggested change

section. The format of the new field would be a list/array of individual

section. The format of the new field would be a string.

The new field would be optional and it would be considered

Rozzii added this to Metal3 - Roadmap Jun 24, 2025

Rozzii added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 24, 2025

metal3-io-bot requested review from lentzi90 and zaneb June 24, 2025 09:54

metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 24, 2025

Rozzii force-pushed the node_specific_kernel_args branch 2 times, most recently from 5b3ba4c to 37d08d8 Compare June 25, 2025 05:49

metal3-io-bot requested review from dtantsur and elfosardo June 25, 2025 06:11

lentzi90 reviewed Jun 25, 2025

View reviewed changes

Rozzii force-pushed the node_specific_kernel_args branch from 37d08d8 to 86fa735 Compare June 25, 2025 07:21

lentzi90 reviewed Jun 25, 2025

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2025

Rozzii moved this to BMO WIP in Metal3 - Roadmap Jun 25, 2025

dtantsur reviewed Jun 25, 2025

View reviewed changes

design/baremetal-operator/node-specific-kernel-args.md Outdated Show resolved Hide resolved

dtantsur reviewed Jun 25, 2025

View reviewed changes

design/baremetal-operator/node-specific-kernel-args.md Outdated Show resolved Hide resolved

dtantsur reviewed Jun 25, 2025

View reviewed changes

design/baremetal-operator/node-specific-kernel-args.md Outdated Show resolved Hide resolved

Rozzii mentioned this pull request Jul 21, 2025

✨ introduce preprovisioningKernelParams to BMH metal3-io/baremetal-operator#2576

Open

Rozzii force-pushed the node_specific_kernel_args branch from 86fa735 to adaaa1c Compare July 22, 2025 12:24

metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2025

Rozzii force-pushed the node_specific_kernel_args branch from adaaa1c to 32dba0a Compare July 22, 2025 12:57

Rozzii changed the title ~~propose node specific pre-provisioning kernel args~~ propose additional node specific pre-provisioning kernel args Jul 22, 2025

Rozzii force-pushed the node_specific_kernel_args branch 3 times, most recently from 1da861b to f09084e Compare July 25, 2025 11:56

Rozzii force-pushed the node_specific_kernel_args branch from f09084e to 2b1740b Compare July 25, 2025 11:57

fmuyassarov reviewed Jul 28, 2025

View reviewed changes

Rozzii mentioned this pull request Aug 5, 2025

[New Feature] Add option to run "Automated Cleaning" only for initial provisioning metal3-io/baremetal-operator#2589

Open

lentzi90 reviewed Aug 19, 2025

View reviewed changes

Rozzii mentioned this pull request Oct 16, 2025

Redfish VirtualMedia never gets BOOTIF kernel parameter metal3-io/baremetal-operator#2712

Open

	`preprovisioningImage` is refferences by the BMH and teh `status.extraKernelParams`
	`preprovisioningImage` is referenced by the BMH and the `status.extraKernelParams`

	preprovisioningExtraKernelParams: "--timeout 60000 systemd.journald.forward_to_console=yes ipa-debug=1"
	preprovisioningExtraKernelParams: "--timeout 60000 systemd.journald.forward_to_console=yes ipa-debug=1"

	section. The format of the new field would be a list/array of individual
	section. The format of the new field would be a string.
	The new field would be optional and it would be considered

propose additional node specific pre-provisioning kernel args #552

Are you sure you want to change the base?

propose additional node specific pre-provisioning kernel args #552

Uh oh!

Conversation

Rozzii commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metal3-io-bot commented Jun 24, 2025

Uh oh!

Rozzii commented Jun 25, 2025

Uh oh!

lentzi90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lentzi90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dtantsur Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

metal3-io-bot commented Jul 22, 2025

Uh oh!

Rozzii commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmuyassarov Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Rozzii commented Jun 24, 2025 •

edited

Loading

dtantsur Jul 23, 2025 •

edited

Loading

Rozzii commented Jul 22, 2025 •

edited

Loading

fmuyassarov Jul 28, 2025 •

edited

Loading