Composefs deleting deployment #1685
Conversation
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 <[email protected]>
Signed-off-by: Pragyan Poudyal <[email protected]>
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 <[email protected]>
Johan-Liebert1
force-pushed
the
cfs-del-depl
branch
from
de10dec to
883110d
Compare
| Opt::ConfigDiff => get_etc_diff().await, | ||
|
|
||
| #[cfg(feature = "composefs-backend")] | ||
| Opt::DeleteDeployment { depl_id, delete } => { |
There was a problem hiding this comment.
Note this is also an RFE here #1276
| pub bootloader: Bootloader, | ||
| /// The sha256sum of vmlinuz + initrd | ||
| /// Only `Some` for Type1 boot entries | ||
| pub boot_digest: Option<String>, |
There was a problem hiding this comment.
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
| let Some(depl_to_del) = depl_to_del else { | ||
| anyhow::bail!("Deployment {deployment_id} not found"); | ||
| }; | ||
|
|
There was a problem hiding this comment.
I think there's a missing check here that refuses to delete the booted one?
| let sysroot = | ||
| Dir::open_ambient_dir("/sysroot", ambient_authority()).context("Opening sysroot")?; | ||
|
|
||
| let repo = open_composefs_repo(&sysroot)?; |
There was a problem hiding this comment.
I'd still like this stuff to key off Storage but maybe we can just do a mass conversion later
| let device = get_sysroot_parent_dev()?; | ||
| let (esp_part, ..) = get_esp_partition(&device)?; | ||
|
|
||
| let esp_mount = TempMount::mount_dev(&esp_part)?; |
There was a problem hiding this comment.
This will need updating for #1691
| rename_exchange_user_cfg(&grub_dir) | ||
| } | ||
|
|
||
| fn delete_depl_boot_entries(deployment: &DeploymentEntry, deleting_staged: bool) -> Result<()> { |
There was a problem hiding this comment.
Hmm entries plural? There's a 1:1 mapping with the UKI case, right?
I guess with type1 we're trying to handle multiple?
| 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")?; |
There was a problem hiding this comment.
We could make an atomic_write_fsync helper
There was a problem hiding this comment.
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.
| .remove_dir_all(&state_dir) | ||
| .with_context(|| format!("Removing dir {state_dir:?}"))?; | ||
|
|
||
| for sha in diff { |
There was a problem hiding this comment.
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.
2 participants
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 containers/composefs-rs#188