From 056cf59b4dc974f3a6e6871eb61c5a4ed37af32b Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Sun, 10 May 2026 18:48:06 +0100 Subject: [PATCH 1/3] utils: add BOOTC_EXP_EXTERNAL_CONTAINER_TOOL env override Add a single environment variable that allows callers to substitute an alternative binary for both podman and skopeo without creating hard links or symlinks on the filesystem. The _EXP prefix makes clear that this interface is experimental and subject to change. BOOTC_EXP_EXTERNAL_CONTAINER_TOOL defaults to the conventional tool name ("podman" or "skopeo") when unset, preserving existing behaviour. Helper functions podman_bin() and skopeo_bin() are added to bootc-internal-utils and used at every call site across crates/lib and crates/ostree-ext. This unblocks downstream projects that ship a single alternative binary (e.g. dtool) in place of both tools by pointing the env var at that binary rather than hard-linking it into /usr/bin. Assisted-by: OpenCode (claude-sonnet-4-6) Signed-off-by: Eric Curtin --- crates/lib/src/cli.rs | 2 +- crates/lib/src/deploy.rs | 3 ++- crates/lib/src/image.rs | 2 +- crates/lib/src/podman.rs | 2 +- crates/lib/src/podman_client.rs | 4 ++-- crates/lib/src/podstorage.rs | 6 +++--- crates/ostree-ext/src/container/mod.rs | 2 +- crates/ostree-ext/src/container/skopeo.rs | 2 +- crates/utils/src/lib.rs | 22 ++++++++++++++++++++++ 9 files changed, 34 insertions(+), 11 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index f0bc8357b..730774daf 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -1856,7 +1856,7 @@ async fn run_from_opt(opt: Opt) -> Result<()> { ); } // And ensure we're finding the image in the host storage - let mut cmd = Command::new("skopeo"); + let mut cmd = Command::new(bootc_utils::skopeo_bin()); set_additional_image_store(&mut cmd, "/run/host-container-storage"); proxycfg.skopeo_cmd = Some(cmd); iid diff --git a/crates/lib/src/deploy.rs b/crates/lib/src/deploy.rs index c4c686eca..e82314629 100644 --- a/crates/lib/src/deploy.rs +++ b/crates/lib/src/deploy.rs @@ -9,6 +9,7 @@ use std::process::Command; use anyhow::{Context, Result, anyhow}; use bootc_kernel_cmdline::utf8::CmdlineOwned; +use bootc_utils::skopeo_bin; use cap_std::fs::{Dir, MetadataExt}; use cap_std_ext::cap_std; use cap_std_ext::dirext::CapStdExtDirExt; @@ -557,7 +558,7 @@ pub(crate) async fn prepare_for_pull_unified( // Configure the importer to use bootc storage as an additional image store let mut config = new_proxy_config(); - let mut cmd = Command::new("skopeo"); + let mut cmd = Command::new(skopeo_bin()); // Use the physical path to bootc storage from the Storage struct let storage_path = format!( "{}/{}", diff --git a/crates/lib/src/image.rs b/crates/lib/src/image.rs index 6cfd76109..8a6b17a54 100644 --- a/crates/lib/src/image.rs +++ b/crates/lib/src/image.rs @@ -30,7 +30,7 @@ pub(crate) const IMAGE_DEFAULT: &str = "localhost/bootc"; /// get structured responses. async fn image_exists_in_host_storage(image: &str) -> Result { use tokio::process::Command as AsyncCommand; - let mut cmd = AsyncCommand::new("podman"); + let mut cmd = AsyncCommand::new(bootc_utils::podman_bin()); cmd.args(["image", "exists", image]); Ok(cmd.status().await?.success()) } diff --git a/crates/lib/src/podman.rs b/crates/lib/src/podman.rs index 7337fbc89..9b287fb40 100644 --- a/crates/lib/src/podman.rs +++ b/crates/lib/src/podman.rs @@ -24,7 +24,7 @@ pub(crate) struct ImageListEntry { /// Given an image ID, return its manifest digest pub(crate) fn imageid_to_digest(imgid: &str) -> Result { use bootc_utils::CommandRunExt; - let o: Vec = crate::install::run_in_host_mountns("podman")? + let o: Vec = crate::install::run_in_host_mountns(bootc_utils::podman_bin())? .args(["inspect", imgid]) .run_and_parse_json()?; let i = o diff --git a/crates/lib/src/podman_client.rs b/crates/lib/src/podman_client.rs index 115b3c8bd..2e9e9924b 100644 --- a/crates/lib/src/podman_client.rs +++ b/crates/lib/src/podman_client.rs @@ -100,7 +100,7 @@ impl PodmanClient { std::fs::create_dir_all("/run/bootc/").ok(); let _ = std::fs::remove_file(&socket_path); - let mut cmd = std::process::Command::new("podman"); + let mut cmd = std::process::Command::new(bootc_utils::podman_bin()); let mut fds = CmdFds::new(); crate::podstorage::bind_storage_roots(&mut cmd, &mut fds, storage_root, run_root)?; crate::podstorage::setup_auth(&mut cmd, &mut fds, sysroot)?; @@ -252,7 +252,7 @@ impl PodmanClient { tracing::debug!( "Image uses non-docker transport, falling back to podman pull subprocess: {image}" ); - let mut cmd = Command::new("podman"); + let mut cmd = Command::new(bootc_utils::podman_bin()); let mut fds = CmdFds::new(); crate::podstorage::bind_storage_roots( &mut cmd, diff --git a/crates/lib/src/podstorage.rs b/crates/lib/src/podstorage.rs index e51d46f6b..de6c418ed 100644 --- a/crates/lib/src/podstorage.rs +++ b/crates/lib/src/podstorage.rs @@ -167,7 +167,7 @@ pub(crate) fn setup_auth(cmd: &mut Command, fds: &mut CmdFds, sysroot: &Dir) -> // - storage overridden to point to to storage_root // - Authentication (auth.json) using the bootc/ostree owned auth fn new_podman_cmd_in(sysroot: &Dir, storage_root: &Dir, run_root: &Dir) -> Result { - let mut cmd = Command::new("podman"); + let mut cmd = Command::new(bootc_utils::podman_bin()); let mut fds = CmdFds::new(); bind_storage_roots(&mut cmd, &mut fds, storage_root, run_root)?; let run_root = format!("/proc/self/fd/{STORAGE_RUN_FD}"); @@ -201,7 +201,7 @@ pub fn set_additional_image_store<'c>( /// /// Call this function any time we're going to write to containers-storage. pub(crate) fn ensure_floating_c_storage_initialized() { - if let Err(e) = Command::new("podman") + if let Err(e) = Command::new(bootc_utils::podman_bin()) .args(["system", "info"]) .stdout(Stdio::null()) .run_capture_stderr() @@ -447,7 +447,7 @@ impl CStorage { /// to this storage. #[context("Pulling from host storage: {image}")] pub(crate) async fn pull_from_host_storage(&self, image: &str) -> Result<()> { - let mut cmd = Command::new("podman"); + let mut cmd = Command::new(bootc_utils::podman_bin()); cmd.stdin(Stdio::null()); cmd.stdout(Stdio::null()); // An ephemeral place for the transient state; diff --git a/crates/ostree-ext/src/container/mod.rs b/crates/ostree-ext/src/container/mod.rs index 4b521690c..0fec7f757 100644 --- a/crates/ostree-ext/src/container/mod.rs +++ b/crates/ostree-ext/src/container/mod.rs @@ -588,7 +588,7 @@ pub fn merge_default_container_proxy_opts_with_isolation( if let Some(authfile) = config.authfile.take() { config.auth_data = Some(std::fs::File::open(authfile)?); } - let cmd = crate::isolation::unprivileged_subprocess("skopeo", user); + let cmd = crate::isolation::unprivileged_subprocess(bootc_utils::skopeo_bin(), user); config.skopeo_cmd = Some(cmd); } Ok(()) diff --git a/crates/ostree-ext/src/container/skopeo.rs b/crates/ostree-ext/src/container/skopeo.rs index f70ae4920..27c7ffea5 100644 --- a/crates/ostree-ext/src/container/skopeo.rs +++ b/crates/ostree-ext/src/container/skopeo.rs @@ -51,7 +51,7 @@ pub(crate) fn container_policy_is_default_insecure() -> Result { /// Create a Command builder for skopeo. pub(crate) fn new_cmd() -> std::process::Command { - let mut cmd = std::process::Command::new("skopeo"); + let mut cmd = std::process::Command::new(bootc_utils::skopeo_bin()); cmd.stdin(Stdio::null()); cmd } diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 39ae772c3..898ebf386 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -22,6 +22,28 @@ pub use tracing_util::*; /// The name of our binary pub const NAME: &str = "bootc"; +/// Return the podman binary path, honouring the `BOOTC_EXP_EXTERNAL_CONTAINER_TOOL` +/// environment variable so callers can substitute an alternative tool (e.g. `dtool`) +/// without hard-linking it as `/usr/bin/podman`. The _EXP prefix indicates this +/// interface is experimental and subject to change. +pub fn podman_bin() -> &'static str { + static BIN: std::sync::OnceLock = std::sync::OnceLock::new(); + BIN.get_or_init(|| { + std::env::var("BOOTC_EXP_EXTERNAL_CONTAINER_TOOL").unwrap_or_else(|_| "podman".to_string()) + }) +} + +/// Return the skopeo binary path, honouring the `BOOTC_EXP_EXTERNAL_CONTAINER_TOOL` +/// environment variable so callers can substitute an alternative tool (e.g. `dtool`) +/// without hard-linking it as `/usr/bin/skopeo`. The _EXP prefix indicates this +/// interface is experimental and subject to change. +pub fn skopeo_bin() -> &'static str { + static BIN: std::sync::OnceLock = std::sync::OnceLock::new(); + BIN.get_or_init(|| { + std::env::var("BOOTC_EXP_EXTERNAL_CONTAINER_TOOL").unwrap_or_else(|_| "skopeo".to_string()) + }) +} + /// Intended for use in `main`, calls an inner function and /// handles errors by printing them. pub fn run_main(f: F) From 0401db4953029522d562ad094781f6b299558351 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 21 May 2026 10:44:20 +0530 Subject: [PATCH 2/3] composefs/gc: Run GC for BLS/UKI binaries on finalize On finalize, after we are done atomically exchanging staged bootloader entries with the current one, run GC for bootloader binaries that are no longer referenced We only GC the bootloader binaries and we do not touch the composefs repository as that can turn out to be quite expensive. The repo will be pruned in the next update/switch Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/delete.rs | 2 +- crates/lib/src/bootc_composefs/finalize.rs | 6 +++++- crates/lib/src/bootc_composefs/gc.rs | 5 +++++ crates/lib/src/bootc_composefs/update.rs | 2 +- crates/lib/src/cli.rs | 7 ++++++- 5 files changed, 18 insertions(+), 4 deletions(-) diff --git a/crates/lib/src/bootc_composefs/delete.rs b/crates/lib/src/bootc_composefs/delete.rs index d6e9ea070..f92183de3 100644 --- a/crates/lib/src/bootc_composefs/delete.rs +++ b/crates/lib/src/bootc_composefs/delete.rs @@ -261,7 +261,7 @@ pub(crate) async fn delete_composefs_deployment( delete_depl_boot_entries(&depl_to_del, &storage, deleting_staged)?; - composefs_gc(storage, booted_cfs, true).await?; + composefs_gc(storage, booted_cfs, false, true).await?; Ok(()) } diff --git a/crates/lib/src/bootc_composefs/finalize.rs b/crates/lib/src/bootc_composefs/finalize.rs index ddc6d036f..18c40ccf4 100644 --- a/crates/lib/src/bootc_composefs/finalize.rs +++ b/crates/lib/src/bootc_composefs/finalize.rs @@ -1,6 +1,7 @@ use std::path::Path; use crate::bootc_composefs::boot::BootType; +use crate::bootc_composefs::gc::composefs_gc; use crate::bootc_composefs::rollback::{rename_exchange_bls_entries, rename_exchange_user_cfg}; use crate::bootc_composefs::status::get_composefs_status; use crate::composefs_consts::STATE_DIR_ABS; @@ -123,7 +124,6 @@ pub(crate) async fn composefs_backend_finalize( let boot_dir = storage.require_boot_dir()?; - // NOTE: Assuming here we won't have two bootloaders at the same time match booted_composefs.bootloader { Bootloader::Grub => match staged_composefs.boot_type { BootType::Bls => { @@ -141,6 +141,10 @@ pub(crate) async fn composefs_backend_finalize( Bootloader::None => unreachable!("Checked at install time"), }; + // Now that we have successfully updated bootloader entires, we can GC the unreferenced ones + // We do not prune the composefs repository here though + composefs_gc(storage, booted_cfs, false, false).await?; + Ok(()) } diff --git a/crates/lib/src/bootc_composefs/gc.rs b/crates/lib/src/bootc_composefs/gc.rs index 8ac54c010..828f66278 100644 --- a/crates/lib/src/bootc_composefs/gc.rs +++ b/crates/lib/src/bootc_composefs/gc.rs @@ -213,6 +213,7 @@ pub(crate) async fn composefs_gc( storage: &Storage, booted_cfs: &BootedComposefs, dry_run: bool, + prune_repo: bool, ) -> Result { const COMPOSEFS_GC_JOURNAL_ID: &str = "3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8e7"; @@ -288,6 +289,10 @@ pub(crate) async fn composefs_gc( } } + if !prune_repo { + return Ok(GcResult::default()); + } + // Identify orphaned deployments: state dirs or bootloader entries // that don't correspond to a live deployment. EROFS images in // composefs/images/ are NOT managed here — repo.gc() handles those diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index 0be2e430d..e88d1f3e9 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -324,7 +324,7 @@ pub(crate) async fn do_upgrade( // We take into account the staged bootloader entries so this won't remove // the currently staged entry - composefs_gc(storage, booted_cfs, false).await?; + composefs_gc(storage, booted_cfs, false, true).await?; apply_upgrade(storage, booted_cfs, &id.to_hex(), opts).await } diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 730774daf..77cf58dd8 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -725,6 +725,9 @@ pub(crate) enum InternalsOpts { /// Implies `--dry-run`. Intended for use in tests and health-checks. #[clap(long)] assert_no_op: bool, + /// Prune the composefs repository in addition to boot binaries + #[clap(long)] + prune_repo: bool, }, /// Block device inspection tools. #[clap(subcommand)] @@ -2214,6 +2217,7 @@ async fn run_from_opt(opt: Opt) -> Result<()> { InternalsOpts::ComposefsGC { dry_run, assert_no_op, + prune_repo, } => { let storage = &get_storage().await?; @@ -2225,7 +2229,8 @@ async fn run_from_opt(opt: Opt) -> Result<()> { BootedStorageKind::Composefs(booted_cfs) => { let effective_dry_run = dry_run || assert_no_op; let gc_result = - composefs_gc(storage, &booted_cfs, effective_dry_run).await?; + composefs_gc(storage, &booted_cfs, effective_dry_run, prune_repo) + .await?; if effective_dry_run { println!("Dry run (no files deleted)"); From e4dea2bbc41738cc3269351f4e786ebb2dd851b6 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 21 May 2026 11:50:37 +0530 Subject: [PATCH 3/3] test: Update composefs GC test Now that we immediately remove unreferenced bootloader binaries, update tests to test for the same Signed-off-by: Pragyan Poudyal --- tmt/tests/booted/test-composefs-gc-uki.nu | 38 +++-------------- tmt/tests/booted/test-composefs-gc.nu | 50 ++++++----------------- 2 files changed, 18 insertions(+), 70 deletions(-) diff --git a/tmt/tests/booted/test-composefs-gc-uki.nu b/tmt/tests/booted/test-composefs-gc-uki.nu index b82c449bd..e2efd561c 100644 --- a/tmt/tests/booted/test-composefs-gc-uki.nu +++ b/tmt/tests/booted/test-composefs-gc-uki.nu @@ -36,21 +36,12 @@ def first_boot [] { echo $containerfile | podman build -t localhost/bootc-first . -f - - let current_time = (date now) - bootc switch --transport containers-storage localhost/bootc-first # Find the large file's verity and save it - # nu has its own built in find which sucks, so we use the other one - # TODO: Replace this with some concrete API - # See: https://github.com/composefs/composefs-rs/pull/236 - let file_path = ( - /usr/bin/find /sysroot/composefs/objects -type f -size 1337k -newermt ($current_time | format date "%Y-%m-%d %H:%M:%S") - | xargs grep -lx "large-file-marker" - ) - - echo $file_path | save /var/large-file-marker-objpath - cat /var/large-file-marker-objpath + let new_st = bootc status --json | from json + let path = bootc internals cfs dump-files $new_st.status.staged.composefs.verity /usr/share/large-test-file --backing-path-only | awk '{print $2}' + echo $"/sysroot/composefs/objects/($path)" | save /var/large-file-marker-objpath echo $st.status.booted.composefs.verity | save /var/boot0-verity @@ -68,7 +59,6 @@ def second_boot [] { echo $st.status.booted.composefs.verity | save /var/boot1-verity let path = cat /var/large-file-marker-objpath - assert ($path | path exists) mut containerfile = echo " @@ -90,7 +80,7 @@ def third_boot [] { mount /dev/disk/by-partlabel/EFI-SYSTEM /var/tmp/efi assert equal $booted.image.image "localhost/bootc-second" - assert ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot0-verity).efi" | path exists) + assert (not ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot0-verity).efi" | path exists)) assert ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot1-verity).efi" | path exists) echo $st.status.booted.composefs.verity | save /var/boot2-verity @@ -119,12 +109,9 @@ def fourth_boot [] { mount /dev/disk/by-partlabel/EFI-SYSTEM /var/tmp/efi assert equal $booted.image.image "localhost/bootc-third" - assert (not ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot0-verity).efi" | path exists)) - assert ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot1-verity).efi" | path exists) + assert (not ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot1-verity).efi" | path exists)) assert ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot2-verity).efi" | path exists) - echo $st.status.booted.composefs.verity | save /var/boot3-verity - mut containerfile = " FROM localhost/bootc as base RUN echo 'another file' > /usr/share/another-one @@ -135,22 +122,10 @@ def fourth_boot [] { echo $containerfile | podman build -t localhost/bootc-final . -f - bootc switch --transport containers-storage localhost/bootc-final - tap ok -} - -def fifth_boot [] { - mkdir /var/tmp/efi - mount /dev/disk/by-partlabel/EFI-SYSTEM /var/tmp/efi - - assert equal $booted.image.image "localhost/bootc-final" - assert (not ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot1-verity).efi" | path exists)) - assert ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot2-verity).efi" | path exists) - assert ($"/var/tmp/efi/EFI/Linux/bootc/($uki_prefix)(cat /var/boot3-verity).efi" | path exists) - - # We had this in boot1 (second boot) let path = cat /var/large-file-marker-objpath assert (not ($path | path exists)) + tap ok } @@ -160,7 +135,6 @@ def main [] { "1" => second_boot, "2" => third_boot, "3" => fourth_boot, - "4" => fifth_boot, $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, } } diff --git a/tmt/tests/booted/test-composefs-gc.nu b/tmt/tests/booted/test-composefs-gc.nu index f1b5487b1..d19a01efe 100644 --- a/tmt/tests/booted/test-composefs-gc.nu +++ b/tmt/tests/booted/test-composefs-gc.nu @@ -30,21 +30,12 @@ def first_boot [] { RUN echo 'large-file-marker' | dd of=/usr/share/large-test-file conv=notrunc " | podman build -t localhost/bootc-derived . -f - - let current_time = (date now) - bootc switch --transport containers-storage localhost/bootc-derived # Find the large file's verity and save it - # nu has its own built in find which sucks, so we use the other one - # TODO: Replace this with some concrete API - # See: https://github.com/composefs/composefs-rs/pull/236 - let file_path = ( - /usr/bin/find /sysroot/composefs/objects -type f -size 1337k -newermt ($current_time | format date "%Y-%m-%d %H:%M:%S") - | xargs grep -lx "large-file-marker" - ) - - echo $file_path | save /var/large-file-marker-objpath - cat /var/large-file-marker-objpath + let new_st = bootc status --json | from json + let path = bootc internals cfs dump-files $new_st.status.staged.composefs.verity /usr/share/large-test-file --backing-path-only | awk '{print $2}' + echo $"/sysroot/composefs/objects/($path)" | save /var/large-file-marker-objpath echo $st.status.booted.composefs.verity | save /var/first-verity @@ -123,22 +114,6 @@ def third_boot [] { echo " FROM localhost/bootc-derived-initrd RUN echo 'another file' > /usr/share/another-one - " | podman build -t localhost/bootc-prefinal . -f - - - - bootc switch --transport containers-storage localhost/bootc-prefinal - - tmt-reboot -} - -def fourth_boot [] { - assert equal $booted.image.image "localhost/bootc-prefinal" - - # Now we create a new image derived from the current kernel + initrd - # Switching to this and rebooting should remove the old kernel + initrd - echo " - FROM localhost/bootc-derived-initrd - RUN echo 'another file 1' > /usr/share/another-one-1 " | podman build -t localhost/bootc-final . -f - @@ -147,7 +122,7 @@ def fourth_boot [] { tmt-reboot } -def fifth_boot [] { +def fourth_boot [] { let bootloader = (bootc status --json | from json).status.booted.composefs.bootloader if ($bootloader | str downcase) == "systemd" { @@ -156,10 +131,6 @@ def fifth_boot [] { mount /dev/disk/by-partlabel/EFI-SYSTEM /var/tmp/efi } - # The large file should be GC'd in the previous switch - let path = cat /var/large-file-marker-objpath - assert (not ($path | path exists)) - assert equal $booted.image.image "localhost/bootc-final" assert (not ((cat /var/to-be-deleted-kernel | path exists))) @@ -174,10 +145,14 @@ def fifth_boot [] { bootc switch --transport containers-storage localhost/bootc-shared-1 + # The large file should be GC'd in the previous switch + let path = cat /var/large-file-marker-objpath + assert (not ($path | path exists)) + tmt-reboot } -def sixth_boot [i: int] { +def fifth_boot [i: int] { assert equal $booted.image.image $"localhost/bootc-shared-($i)" # Just this being booted counts as success @@ -201,10 +176,9 @@ def main [] { "1" => second_boot, "2" => third_boot, "3" => fourth_boot, - "4" => fifth_boot, - "5" => { sixth_boot 1 }, - "6" => { sixth_boot 2 }, - "7" => { sixth_boot 3 }, + "4" => { fifth_boot 1 }, + "5" => { fifth_boot 2 }, + "6" => { fifth_boot 3 }, $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, } }