-
Notifications
You must be signed in to change notification settings - Fork 180
Description
When building a chunked image, one is supposed to be able to take a prior build's layer plan into account, minimizing layer changes with an update. However, if the image to be built has exclusive components (which should each be placed in a layer of their own), using a prior build's layer plan fails, making ostree-ext fall back to computing a new layer plan. (I did some local testing with rpm-ostree compose build-chunked-oci that seems to confirm this behavior.)
Steps to reproduce
- Build a bootc container image with some files marked as belonging to an exclusive component, for example using the
user.componentxattr. - Create a chunked version of this image with build-chunked-oci.
- Build a modified version of the image from step (2), for example by installing some additional packages.
- Set the environment variable
RUST_LOG=debug. - Use build-chunked-oci to created a chunked version of the modified image from step (3), outputting it to the same image reference in local storage that the image from step (2) was stored in. This makes build-chunked-oci try to reuse the layer plan of the image from step (2); however, the debug output shows that it fails to reuse the layer plan and falls back to creating a new layer plan.
Analysis
I believe the problem is that, in ostree_ext::chunking::basic_packing_with_prior_build, to use the prior build's layer plan, we require bin_size to be >= the number of layers of the prior build (excluding the ostree commit layer):
bootc/crates/ostree-ext/src/chunking.rs
Lines 640 to 643 in 99da62b
| if (bin_size.get() as usize) < curr_build.len() { | |
| tracing::debug!("bin_size = {bin_size} is too small to be compatible with the prior build"); | |
| return Ok(None); | |
| } |
But on the other hand, the bin_size input comes from here:
bootc/crates/ostree-ext/src/chunking.rs
Lines 381 to 384 in 99da62b
| // Process regular components with bin packing if we have remaining layers | |
| if let Some(remaining) = NonZeroU32::new(self.remaining()) { | |
| let start = Instant::now(); | |
| let packing = basic_packing(®ular_sizes, remaining, prior_build_metadata)?; |
And this is essentially computed as the maximum layer count minus the number of layers that have already been reserved at that point in the function... but this takes place after we've already made a layer for each exclusive component earlier in the same function:
bootc/crates/ostree-ext/src/chunking.rs
Lines 343 to 359 in 99da62b
| // Create exclusive chunks first if specified | |
| let mut processed_specific_components = BTreeSet::new(); | |
| if let Some(specific_meta) = specific_contentmeta { | |
| for (component, files) in specific_meta { | |
| let mut chunk = Chunk::new(&component); | |
| chunk.packages = vec![component.to_string()]; | |
| // Move all objects belonging to this exclusive component | |
| for (path, checksum) in files { | |
| self.remainder | |
| .move_path(&mut chunk, checksum.as_str(), path); | |
| } | |
| self.chunks.push(chunk); | |
| processed_specific_components.insert(component.clone()); | |
| } | |
| } |
So, I think this will always fail if there are exclusive components.
I haven't wrapped my head around the full packing algorithm well enough yet to understand exactly which part of this needs to be fixed and how best to fix it, but it seems like the problem is that it doesn't take into account the likely presence of the same exclusive components in the prior build. Either that or maybe the requirement for the bin size is wrong?