GC UKI and UKI Addons from Composefs Repository - #2380
Conversation
c507311 to
1bcce9f
Compare
|
Why can't we just read the non-bootable tree? That's already supported in composefs-rs directly - the same filesystem tree one would get to run as a container image. Yes, we won't have e.g. SELinux labeling, but that's not a problem. |
I didn't even realise we saved this :| |
|
Okay, going through the code a bit more, we're doing things extremely inefficiently. We're recreating the filesystem multiple times, one for non-bootable fs and once for bootable. I had some optimizations here edb2ef0, but the current stuff requires some more refactoring |
What's inefficient about that? It's just O(metadata) so I'd be surprised if it was taking a noticeable amount of CPU etc. |
|
|
Hmm, but we only need to process the tar metadata not the data, that seems like a plain bug to fix in composefs-rs - like we want a tar reading path that only gives headers from split stream.
I don't understand this one. But at this point can you spawn an agent on this and let's move this to composefs-rs? All that needs to be done on the bootc side I think is to read the non-bootable image to access |
Here https://github.com/bootc-dev/bootc/blob/main/crates/lib/src/bootc_composefs/boot.rs#L1486, we pass config verity as
This would be a combined effort really. I think the easiest solution is to simply return the created fs from composefs APIs and have other functions take in an |
|
Also, regarding just this PR, I don't believe there's any way we link the bootable and non-bootable EROFS-es? I see we have them as GC links in the config splitstream, but that requires reading the splitstream metadata. Should we create a ref in bootc for easier accessibility, similar to a ref I'm creating here 6660f94? |
|
Hmm...yes we may be missing APIs for this, will look |
Signed-off-by: Pragyan Poudyal <[email protected]>
Make use of the non-bootable EROFS to GC everything in the boot directory of the deployment being GC'd. This is only done for UKI boots as for normal Type1 booted systems, there should be nothing in /boot Signed-off-by: Pragyan Poudyal <[email protected]>
Test whether UKI assets are being cleaned up from the objects directory during GC Signed-off-by: Pragyan Poudyal <[email protected]>
There are two reasons to do this 1. We skip the expensive operation of reading every layer and computing its hash to preserve integrity. 2. We end up also reading whiteouts in `/boot` which end up erroring out since those objects don't exist anymore Signed-off-by: Pragyan Poudyal <[email protected]>
1bcce9f to
86f2f34
Compare
If we find an orphan state directory that has invalid fs-verity, which could've resulted from the user snooping around or one of our operations messing things up, we do not want to block on GC and allow user to switch/upgrade to a new deployment. Signed-off-by: Pragyan Poudyal <[email protected]>
|
@cgwalters I think this should be good now |
Johan-Liebert1
changed the title
/boot structure as an image
|
|
||
| let (mut objects_bytes, mut objects_removed) = (0, 0); | ||
|
|
||
| // Now GC the UKI/UKI Addons from `.boot` EROFS if we have them |
There was a problem hiding this comment.
We don't have any integration tests for this though...a bit of a risk
There was a problem hiding this comment.
| tracing::debug!( | ||
| "{}: objects/{path:?}", | ||
| if gc_opts.dry_run { | ||
| "would remove" | ||
| } else { | ||
| "removing" | ||
| }, | ||
| ); |
There was a problem hiding this comment.
This construction is a bit awkward I think it'd be simpler to use structured logging and pass dry_run=gc_opts.dry_run or so
|
|
||
| objects_removed += 1; | ||
|
|
||
| unlinkat(&objects_dir, path.as_ref(), AtFlags::empty()) |
There was a problem hiding this comment.
Here and elsewhere we need to be able to recover from corrupted state - in this particular case should gracefully handle ENOENT
There was a problem hiding this comment.
This is something that I'm unsure about. I think the GC op should be aware of what exists and what does not and we should definitely error out if something we absolutely expected does not exist. I think it's worth discussing in the weekly meet
Related #2442
2 participants
cfs/gc: GC UKI and UKI Addons
Make use of the non-bootable EROFS to GC everything in the boot
directory of the deployment being GC'd. This is only done for UKI boots
as for normal Type1 booted systems, there should be nothing in /boot
tmt: Add tests for .boot images and UKI asset cleanup
Test whether UKI assets are being cleaned up from the objects directory
during GC
cfs/filesystem: Pass manifest verity to create_filesystem
There are two reasons to do this
We skip the expensive operation of reading every layer and computing
its hash to preserve integrity.
We end up also reading whiteouts in
/bootwhich end up erroring outsince those objects don't exist anymore