OCI-archive multi-manifest support POC#1178
Conversation
Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
…i-poc Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
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č <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
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č <mitr@redhat.com>
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č <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
[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č <mitr@redhat.com>
|
@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?)