UKI Addons Support - #2448
Conversation
50c71e7 to
ec3f9fb
Compare
Introduce a function to gather all currently installed addons, scoped and global. On upgrade/switch, gather all installed addons and if an addon with the same name is found in the upgrade image, update that particular addon automatically Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
We were partially supporting global addons, but they were lumped in with scoped/local addons. Add a new cli option to composefs installs called `--global-uki-addon` which would determine which global addon to install. Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Here is what we do now with UKI Addons - Accept `--uki-addon` and `--global-uki-addon` cli options for bootc switch/upgrade commands - If we find an installed addon with the same name as the one in the new image, we update it Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Assisted-by: AI Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Prefix global addon filenames with the bootc identifier in the ESP so we can distinguish bootc-managed global addons from third-party ones Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Add `bootc uki-addon` subcommand with three operations: - `bootc uki-addon list`: List installed UKI addons Supports `--json` for JSON output - `bootc uki-addon add <name> <global|scoped>`: Install an addon from the booted image onto the ESP - `bootc uki-addon remove <name> [deployment_id]`: Remove an addon Add Display and Serialize to UkiAddonType/UkiAddonsList Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
- Add GC tests for Addons - Add CLI tests Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Assisted-by: Claude-Code (Opus) Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
UkiAddonOpts is now flattened into both upgrade and switch commands, which don't have a --composefs-backend flag. The `requires = "composefs_backend"` constraint causes a panic at clap validation time because the referenced argument doesn't exist in those command contexts. This is generally safe as the options are ignored for ostree installs anyway Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Extract cmdline parsing from write_pe_to_esp into parse_uki_cmdline so it runs for both the UKI and UKI addons. The `composefs=` parameter can now be found in the main UKI or a scoped addon - At most one composefs= cmdline across all PE binaries (UKI + addons). A second one is rejected even if the digest matches. - Global UKI addons must never contain `composefs=` cmdline - At least one `composefs=` cmdline must be found or the install fails Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Before removing a scoped addon, parse its PE binary and check for a composefs= kernel parameter. If found, early exit as removing that addon would make the system unbootable Global addons are not checked because `composefs=` is rejected at install time for global addons Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Test that bootc rejects composefs= in the wrong places - Build a global addon containing composefs= from `bootc compute-composefs-digest`, attempt switch with --global-uki-addon, assert failure - Build a scoped addon containing composefs= alongside the UKI (which already has it), attempt switch with --uki-addon, assert failure due to duplicate composefs= It's a shame that we can't test UKI Addon only cmdline without piling on a bunch of hacks since `bootc container ukify` unconditionally puts the cmdline inside the UKI Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
2579e09 to
e31ce45
Compare
|
Can we install UKI addons as part of a deployment with |
| format!("{UKI_NAME_PREFIX}{depl_verity}{EFI_ADDON_FILE_EXT}") | ||
| /// Returns the name of a scoped/local UKI Addon directory given name | ||
| /// with or without the `.addon.efi` prefix | ||
| pub(crate) fn get_scoped_uki_addon_name(name: &str) -> String { |
There was a problem hiding this comment.
I think it's really "canonicalizing"?
But the use case seems weird; when would we ever see a name without the suffix?
| /// (installed addons are auto-updated when the new image has a matching filename), | ||
| /// but we still blindly overwrite same-named (global) files with no ownership tracking. | ||
| /// Before recommending this feature for wider use we should track which global addon | ||
| /// files are bootc-owned, and decide/document how this interacts with deployment |
There was a problem hiding this comment.
I think we can say a global addon is bootc-owned if any deployment ever installs it?
IOW when dropping a deployment/image we look at the set of all global addons and if any went from referenced ➡️ unreferenced then we drop it.
| let ent = ent?; | ||
| let filename = ent.file_name()?; | ||
|
|
||
| if let Some(addon_name) = filename.strip_suffix(EFI_ADDON_FILE_EXT) { |
There was a problem hiding this comment.
Minor nit I prefer let else .. {continue}
| let filename = ent.file_name()?; | ||
|
|
||
| if let Some(addon_name) = filename.strip_suffix(EFI_ADDON_FILE_EXT) { | ||
| match addon_name.strip_prefix(UKI_NAME_PREFIX) { |
There was a problem hiding this comment.
Also let else here would reduce indentation
| pub(crate) enum UkiAddonCliOpts { | ||
| /// List all installed UKI Addons | ||
| List { | ||
| /// Output in JSON format |
| @@ -0,0 +1,47 @@ | |||
| # NAME | |||
|
|
|||
| bootc-uki-addon-add - Add a UKI Addon to the current deployment | |||
There was a problem hiding this comment.
How about marking this as experimetnal first?
|
|
||
| ### Building Images with Addons | ||
|
|
||
| Addons are built with `ukify` in the same Containerfile stage that produces the sealed UKI. Each addon is a separate `ukify build` invocation: |
There was a problem hiding this comment.
Addons are built with
ukifyin the same Containerfile stage that produces the sealed UKI
There's no reason that has to be true?
| /// This option can be provided multiple times if multiple addons are to be installed. | ||
| #[clap(long = "uki-addon")] | ||
| #[serde(default)] | ||
| pub(crate) scoped: Option<Vec<String>>, |
There was a problem hiding this comment.
BTW ideally we validate these eagerly at install time
That's in the docs here right? |
|
Or well actually, I think what you may be asking about is external UKI addons (i.e. not included in the container image), in which case it would totally make sense to do. |
Yea I wasn't super clear but that was what I was thinking about 🙂 |
uki-addon: Update addons on update/switch
Introduce a function to gather all currently installed addons, scoped
and global. On upgrade/switch, gather all installed addons and if an
addon with the same name is found in the upgrade image, update that
particular addon automatically
uki/addon: Support global addons
We were partially supporting global addons, but they were lumped in with
scoped/local addons. Add a new cli option to composefs installs called
--global-uki-addonwhich would determine which global addon toinstall.
cfs/upgrade/switch: Handle UKI Addons
Here is what we do now with UKI Addons
Accept
--uki-addonand--global-uki-addoncli options for bootcswitch/upgrade commands
If we find an installed addon with the same name as the one in the new
image, we update it
global-uki-addons: Add prefix to name
Prefix global addon filenames with the bootc identifier in the ESP so
we can distinguish bootc-managed global addons from third-party ones
uki-addon: Add CLI for managing UKI Addons
Add
bootc uki-addonsubcommand with three operations:bootc uki-addon list: List installed UKI addonsSupports
--jsonfor JSON outputbootc uki-addon add <name> <global|scoped>: Install an addon fromthe booted image onto the ESP
bootc uki-addon remove <name> [deployment_id]: Remove an addonAdd Display and Serialize to UkiAddonType/UkiAddonsList