Composefs GC changes

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(())
 }

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(())
 }
 

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<GcResult> {
     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

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
 }

crates/lib/src/cli.rs

@@ -722,6 +722,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)]
@@ -2208,6 +2211,7 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
             InternalsOpts::ComposefsGC {
                 dry_run,
                 assert_no_op,
+                prune_repo,
             } => {
                 let storage = &get_storage().await?;
 
@@ -2219,7 +2223,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)");

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)" } },
     }
 }

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)" } },
     }
 }