Add support for cfsctl oci apply-delta#305
Conversation
alexlarsson
force-pushed
the
delta-apply
branch
2 times, most recently
from
f1c29de to
d7d70f2
Compare
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Just an initial style pass
alexlarsson
force-pushed
the
delta-apply
branch
2 times, most recently
from
d2ea945 to
0e43b30
Compare
|
Ok, i think I got all the comments fixed. |
|
Note: This doesn't actually run all the new tests in CI, as there is no oci-delta binary. However it passes locally here. We can build oci-delta ourselves, or we can just wait a bit until it gets packaged up and pick it up as an rpm. (This is in progress elsewhere). |
|
Ok, I redid this to be part of "oci pull", and it seems to work well. I can even pull a delta as an artifact: |
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Cool, this is looking nicer!
On tests, one thing we do in this project is copy over "fixtures" to ghcr.io/composefs. I think we could do the same with deltas. There's generic image mirroring logic.
That said it'd also help of course to have a static binary of oci-delta shipped from the upstream repo that we can use if installed (and install it in CI actions).
| let mut immediate_results: HashMap<usize, (OciDigest, ObjectID)> = HashMap::new(); | ||
| let mut stats = ImportStats::default(); | ||
|
|
||
| let threads = available_parallelism()?; |
There was a problem hiding this comment.
When we fetch OCI images today, we process the tar in a streaming fashion, writing objects into the composefs store as we go. Disk/memory usage is constant relative to the update size.
But one downside of deltas as designed/implemented today is that we have to download the entire delta layer to a file and regenerate the layer locally, so temporary disk space usage is O(delta size) plus O(layer).
I think for this reason I'd suggest we have at most 1 delta layer being downloaded, and 1 being processed at a time by default. WDYT?
There was a problem hiding this comment.
I'm not sure how common download-and-apply will be, but in the local-only (i.e. oci dir or oci archive) case we definitely want to parallelize the apply, as it greatly speeds up applying the delta (i saw times going from 40 to 10 secs). So, lets be careful of exactly how we limit this.
There was a problem hiding this comment.
Yes, in the oci: path we don't need to fetch anything at all, we should just parse from the existing files.
No harm to parallelizing the processing in that case. My concern is more about disk space usage spikes for nontrivial updates.
In the ociarchive: path...like ideally we can say that that file is not compressed (because the blobs will be, no point in double compressing them, and compressing the metadata brings almost no value).
Then we can do what this code is already doing I think, just walk the tar to find byte offsets.
I guess in the end I'm just arguing for the registry path to default to fetching one blob at a time, or maybe two.
Or maybe it's fine, and we can just make it configurable later if we need to.
(Some environment variable to control parallelism in composefs at a coarse grained level is probably enough, or perhaps just --jobserver-fd like https://make.mad-scientist.net/papers/jobserver-implementation/ )
There was a problem hiding this comment.
Ok, i will make the skopeo case do a limited parallelism, and keep the current parallelism in the oci_layout.rs case. Then I will make the oci_layout.rs case handle oci-archive as well as oci-dirs. That seems like a good approach to me.
There was a problem hiding this comment.
So, I did the parallelism limitation. However, I think the only reasonable approach to take the non-skopeo codepath for oci archives is to change ocidir-rs, which is gonna require some work. I think we can do that in a separate MR, oci-archive: works as is via skopeo anyway.
|
I added binaries (build on cs10) to: https://github.com/containers/oci-delta/releases/tag/v0.1.0 |
This allows applying an oci-delta oci-archive or registry artifact to create an oci image based on a delta and a previous oci image in the repo. For details on oci-delta, see: https://github.com/containers/oci-delta Signed-off-by: Alexander Larsson <alexl@redhat.com> Assisted-by: Claude Code (Opus 4.6)
| /// containing a valid OCI layout that can be read by `oci-delta`, | ||
| /// `import_oci_layout`, or any OCI-aware tool. | ||
| #[cfg(test)] | ||
| pub fn build_oci_layout(layers: &[&str]) -> tempfile::TempDir { |
There was a problem hiding this comment.
I have a gut feeling this is like the 6th copy of this, may overlap with #312
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
2 participants
This allows applying an oci-delta tarfile to create an oci image based on a delta and a previous oci image in the repo.
For details on oci-delta, see: https://github.com/containers/oci-delta