Conversation
jbtrystram
commented
Feb 4, 2026
jbtrystram
commented
Allow skiping the boot uuid stamp file writing in bootupd. When creating a golden image, we expect the UUIDS to be randomized at first boot, so always writing the stamp is odd. At least for Fedora CoreOS the stamp being already there at first boot short-circuit the partitions uuid randomization process. So let's make it optionnal. It will default to `false`, preserving existing behaviour when not specified. ref coreos/fedora-coreos-config#3898 Assisted-by: OpenCode (Opus 4.5) Signed-off-by: jbtrystram <jbtrystram@redhat.com>
Wiring-up this knob to the install config to allow it's use in bootc-image-builder. Assisted-by: OpenCode (Opus 4.5) Signed-off-by: jbtrystram <jbtrystram@redhat.com>
github-actions
bot
added
area/install
|
Oh snap it took the branch name as the PR title instead of the commit title, because it's two commits. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new option, skip_boot_uuid_stamp, to control whether bootupd writes the boot partition UUID. This is useful for creating golden images where UUIDs are randomized on first boot. The changes are implemented across the bootloader logic, installation configuration, and documentation. My review includes a couple of suggestions: one to fix a typo in a comment and another to simplify the logic for merging the new configuration option, improving code clarity.
| if !config_opts.skip_boot_uuid_stamp { | ||
| if let Some(skip) = config.skip_boot_uuid_stamp { | ||
| config_opts.skip_boot_uuid_stamp = skip; | ||
| } | ||
| } |
There was a problem hiding this comment.
This logic for merging the skip_boot_uuid_stamp option can be simplified. The nested if let can be replaced with unwrap_or to make the code more concise while preserving the same behavior.
if !config_opts.skip_boot_uuid_stamp {
config_opts.skip_boot_uuid_stamp = config.skip_boot_uuid_stamp.unwrap_or(false);
}| pub(crate) root_mount_spec: Option<String>, | ||
| /// Mount specification for the /boot filesystem. | ||
| pub(crate) boot_mount_spec: Option<String>, | ||
| /// Wether to skip writing the boot partition UUID to the bootloader configuration. |
There was a problem hiding this comment.
There's a typo in the comment. "Wether" should be "Whether".
| /// Wether to skip writing the boot partition UUID to the bootloader configuration. | |
| /// Whether to skip writing the boot partition UUID to the bootloader configuration. |
| - `boot-mount-spec`: A string specifying the /boot filesystem mount specification. | ||
| If not provided and /boot is a separate mount, its UUID will be used. | ||
| An empty string signals to omit boot mount kargs entirely. | ||
| - `skip-boot-uuid-stamp`: A boolean that controls whether to skip writing partition UUIDs |
There was a problem hiding this comment.
This is actually specific to bootupd and does not apply with e.g. sdboot and DPS in general. I think it's also meaningless with s390x. I wonder if we should have a dedicated bootupd section with this as the first option.
Now...then the next question is if we change things so that we error if bootupd isn't used. That would force cases like the FCOS image builds to conditionalize the install config per-arch/layout, but it does feel more right to me instead of silently ignoring it here, which is less compatible if we ever aded any other knobs
There was a problem hiding this comment.
Another approach suggested by @jlebon was to hide that logic behind the boot-mount-spec="" special case, since this is effectively signaling that the user want to own that process. But the kernel mount spec and a bootloader mount spec are still technically different stages, so I am not sure it's a better approach.
Now about having it fail when not applicable.. I am not really opposed to it. It make the pipeline slightly more complex, but we already have different manifests for each arch so I guess that isn't too much more work.
Final question: you said "bootupd", by that you mean grub ? Isn't bootupd support sd-boot at some point ?
There was a problem hiding this comment.
Final question: you said "bootupd", by that you mean grub ? Isn't bootupd support sd-boot at some point ?
Yes well the main argument is to support the shim + sdboot case actually but the uuid is really IMO a legacy for GRUB systems and newer things should use the DPS which we enforce with the sealeduki-composefs case.
(I have a WIP to allow DPS with ostree+grub but it requires newer Fedora)
| /// Don't pass --write-uuid to bootupd during bootloader installation. | ||
| #[clap(long)] | ||
| #[serde(default)] | ||
| pub(crate) skip_boot_uuid_stamp: bool, |
There was a problem hiding this comment.
Also per below how about --bootupd-skip-boot-uuid
2 participants
