-
Notifications
You must be signed in to change notification settings - Fork 201
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
OCI-archive multi-manifest support POC #1178
base: main
Are you sure you want to change the base?
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.
Needs 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. Instructions 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. |
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
LoadManifestDescriptor
being able to benefit fromarchive.Reader
. Alternatively, we could introduce some other API with a similar effect (haveNewReaderForReference
directly return the manifest descriptor?)