-
Notifications
You must be signed in to change notification settings - Fork 225
OCI-archive multi-manifest support POC #1178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Urvashi Mohnani <[email protected]>
Signed-off-by: Urvashi Mohnani <[email protected]>
…i-poc Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
It can only accept a path, so don't round-trip through an ImageReference. (Alternatively, this could use a similar heuristic to loadMultiImageDockerArchive, stat()in the path. But even in that case it should first make a decision and _then_ potentially create a reference.) Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Model it more directly on the docker-archive logic. Pulls specify a single ref, and use all parts of that to choose a single image. Loads load all of the archive. Signed-off-by: Miloslav Trmač <[email protected]>
Introduce copyFromOCIArchiveReaderReferenceAndManifestDescriptor and storageReferenceFromOCIArchiveDescriptor , and use that so that we don't need to load manifest descriptors which we already have readily available. Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mtrmac The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
Signed-off-by: Miloslav Trmač <[email protected]>
|
@mtrmac: PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@mtrmac should this be closed or updated? |
|
The feature is 80–90 % done, so abandoning it seems like a waste. OTOH it has been a long time, and by now at least the c/image part requires a non-trivial rebase. |
|
Now that @flouthoc is not with Red Hat any longer, do you still think we are going to go forward with this, rather then just changing the default to zstd:chunked? |
|
@rhatdan This has no relationship to zstd (of any kind) at all. I think it’s a useful feature, but features get added one at a time depending on priorities. |
|
Hi, and thank you for your contribution! We’ve recently migrated this repository into a new monorepo: containers/container-libs along with other repositories As part of this migration, this repository is no longer accepting new Pull-Requests and therefore this Pull-Request is being closed. Thank you very much for your contribution. We would appreciate your continued help in migrating this PR to the new container-libs repository. Please let us know if you are facing any issues. You can read more about the migration and the reasoning behind it in our blog post: Upcoming migration of three containers repositories to monorepo. Thanks again for your work and for supporting the containers ecosystem! |
This is containers/image#1677 + #921, updated to merge on top of current main, + an attempt to resolve review comments, and a fairly intrusive set of changes to actually implement pulling as expected.
Note that this depends on
LoadManifestDescriptorbeing able to benefit fromarchive.Reader. Alternatively, we could introduce some other API with a similar effect (haveNewReaderForReferencedirectly return the manifest descriptor?)