Composefs deleting deployment#1685

Johan-Liebert1 · 2025-10-13T06:08:10Z

Add a command to delete a composefs native deployment

Deleting a deployment would mean, deleting the EROFS image, the
bootloader entries for that deployment and deleting any objects in the
composefs repository that are only referenced by said deployment.

Also refactor some functions and add error contexts in some places

Draft PR for now as it requires some more adjustments

cgwalters · 2025-10-20T21:00:15Z

crates/lib/src/cli.rs

        Opt::ConfigDiff => get_etc_diff().await,
+
+        #[cfg(feature = "composefs-backend")]
+        Opt::DeleteDeployment { depl_id, delete } => {


Note this is also an RFE here #1276

cgwalters · 2025-10-20T21:00:54Z

crates/lib/src/spec.rs

    pub bootloader: Bootloader,
+    /// The sha256sum of vmlinuz + initrd
+    /// Only `Some` for Type1 boot entries
+    pub boot_digest: Option<String>,


Instead of String let's use say https://docs.rs/oci-spec/latest/oci_spec/image/struct.Sha256Digest.html for stuff like this?

Though that might be a bit harder with the jsonschema

Yeah, I think that's a good idea

With the jsonschema it's a bit tricky

cgwalters · 2025-10-20T21:41:07Z

crates/lib/src/bootc_composefs/delete.rs

+    let Some(depl_to_del) = depl_to_del else {
+        anyhow::bail!("Deployment {deployment_id} not found");
+    };
+


I think there's a missing check here that refuses to delete the booted one?

It's at https://github.com/bootc-dev/bootc/pull/1685/files/883110d50b9245951ba693b87aad02d132c73e8f#diff-12f8773f52c03d119c8cc2cb67374f160f918ff0baae4bd10306f1ce84120dd6R326

I have a delete param passed in to this function for debugging purposes. Might be helpful to just see what all will be deleted

I removed the param, as it doesn't really make much sense

crates/lib/src/bootc_composefs/delete.rs

cgwalters · 2025-10-20T22:02:37Z

crates/lib/src/bootc_composefs/delete.rs

+    grub_dir
+        .atomic_write(USER_CFG_STAGED, buffer)
+        .with_context(|| format!("Writing to {USER_CFG_STAGED}"))?;
+
+    rustix::fs::fsync(grub_dir.reopen_as_ownedfd().context("Reopening")?).context("fsync")?;


We could make an atomic_write_fsync helper

Also btw there's https://docs.rs/cap-std-ext/latest/cap_std_ext/dirext/trait.CapStdExtDirExt.html#tymethod.atomic_replace_with that supports writing-as-you-go instead of building up the in memory buffer.

Updated to use atomic_replace_with

We could make an atomic_write_fsync helper

looking at it again, we don't have any one show write + fsync. It's mostly a couple of writes then a final fsync

cgwalters · 2025-10-20T22:22:18Z

crates/lib/src/bootc_composefs/delete.rs

+        .remove_dir_all(&state_dir)
+        .with_context(|| format!("Removing dir {state_dir:?}"))?;
+
+    for sha in diff {


This should really be an api in composefs-rs right?

Also bigger picture...I think we want to very clearly separate two things (as libostree does): managing the "GC roots" vs a GC operation.

Deleting a deployment starts with unlinking its GC root: the bootloader entry.

But thereafter we should just invoke a generic "GC" operation which traverses all active roots.

The idea here is we must support being interrupted - and we want to be idempotent.

Agreed. There are a few failure cases that need to be handled

This should really be an api in composefs-rs right?

The deletion of unreferenced objects? It does make sense for it to be in composefs-rs. There's even a command for it, but right now it's basically a no-op.

Add a command to delete a composefs native deployment Deleting a deployment would mean, deleting the EROFS image, the bootloader entries for that deployment and deleting any objects in the composefs repository that are only referenced by said deployment. Also refactor some functions and add error contexts in some places Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> composefs-backend: Deleting staged deployment Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Delete the boot entries first, the image second and everything else afterwards. If we fail to delete the boot entry, then there's no point in deleting the image as the boot entry will still show, but there will be no image. We delete the objects at the end, as when we later perform a gc operation and don't find the image that references these objects, we can remove them then. The state directory shouldn't have any effect on boot if the image associated to it doesn't exist. If the staged file /run/composefs/staged-deployment does exist, but we have already deleted the staged image, the finalize service would fail but that wouldn't break anything Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Update the deletion of deployment to only simply delete the bootloader entries related to the deployment and then call a `gc` function, which will just get the difference between the states represented by the bootloader entries and the repository then try to reconcile everything by performing GC operation on the repository. Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Add debug logs for whatever is being deleted Remove the `delete` param from `delete_deployment` function Use `atomic_replace_with` instead of writing to a buffer then writing Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Johan-Liebert1 · 2025-10-28T11:41:47Z

Had some discussions with @allisonkarlitskaya on Matrix on how we'd want to handle gc for a composefs repo. The gc_objects function should not be in bootc and will be moved to composefs-rs

cgwalters

Looks generally OK, nothing blocking.

cgwalters · 2025-10-28T13:08:37Z

crates/lib/src/bootc_composefs/delete.rs

+        ""
+    };
+
+    tracing::info!("Deleting {kind}deployment '{deployment_id}'");


Minor but let's use structured logging for this instead like tracing::info!("Deleting deployment: {deployment_id}", kind = kind) or so

cgwalters · 2025-10-28T13:09:24Z

crates/lib/src/bootc_composefs/delete.rs

+
+    delete_depl_boot_entries(&depl_to_del, deleting_staged)?;
+
+    composefs_gc().await?;


I think the GC should be a clearly separate step in general so we can amortize its cost (e.g. "delete 3 deployments, then gc") and also skip the gc ("lazy prune storage later")

cgwalters · 2025-10-28T13:10:04Z

crates/lib/src/bootc_composefs/gc.rs

+#[fn_error_context::context("Listing EROFS images")]
+fn list_erofs_images(sysroot: &Dir) -> Result<Vec<String>> {
+    let images_dir = sysroot
+        .open_dir("composefs/images")


Hmm there isn't an API for this in Repository?

I think what would help here is if this API actually took a Repository instance for now at least - we can reference its fd and read things directly. Same elsewehere

cgwalters · 2025-10-28T15:42:16Z

Hmm it looks like we had a timeout during install in this run only for c9s. https://github.com/bootc-dev/bootc/actions/runs/18873507377/job/53857419924?pr=1685

bootc-bot bot requested a review from henrywang October 13, 2025 06:08

Johan-Liebert1 force-pushed the cfs-del-depl branch from de10dec to 883110d Compare October 13, 2025 06:24

cgwalters reviewed Oct 20, 2025

View reviewed changes

cgwalters requested changes Oct 20, 2025

View reviewed changes

Johan-Liebert1 added 2 commits October 28, 2025 15:26

Johan-Liebert1 force-pushed the cfs-del-depl branch from 883110d to 66d163e Compare October 28, 2025 09:56

github-actions bot added the area/install Issues related to `bootc install` label Oct 28, 2025

Johan-Liebert1 force-pushed the cfs-del-depl branch from 66d163e to 90b88f9 Compare October 28, 2025 10:15

Johan-Liebert1 added 2 commits October 28, 2025 17:07

composefs-backend/gc: Refactor and add logs

5afe838

Add debug logs for whatever is being deleted Remove the `delete` param from `delete_deployment` function Use `atomic_replace_with` instead of writing to a buffer then writing Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Bump composefs-rs

152ef25

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Johan-Liebert1 force-pushed the cfs-del-depl branch from fce95be to 152ef25 Compare October 28, 2025 11:38

Johan-Liebert1 marked this pull request as ready for review October 28, 2025 11:39

bootc-bot bot requested a review from gursewak1997 October 28, 2025 11:39

Johan-Liebert1 requested a review from cgwalters October 28, 2025 11:42

cgwalters approved these changes Oct 28, 2025

View reviewed changes

cgwalters enabled auto-merge (rebase) October 28, 2025 13:12

cgwalters merged commit 5daa432 into bootc-dev:main Oct 28, 2025
46 of 48 checks passed


		delete_depl_boot_entries(&depl_to_del, deleting_staged)?;

		composefs_gc().await?;

Conversation

Johan-Liebert1 commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Johan-Liebert1 commented Oct 28, 2025

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Oct 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Johan-Liebert1 commented Oct 13, 2025 •

edited

Loading