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 f0bc8357b..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)] @@ -1856,7 +1859,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 @@ -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)"); 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) 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)" } }, } }