[WIP] Add multi manifest oci-archive support #921
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: umohnani8 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 |
umohnani8
force-pushed
the
multi
branch
4 times, most recently
from
5b1833d
to
c0f3141
Compare
umohnani8
force-pushed
the
multi
branch
2 times, most recently
from
c811431
to
1331f03
Compare
umohnani8
force-pushed
the
multi
branch
3 times, most recently
from
d4a38e5
to
f9d3e66
Compare
umohnani8
force-pushed
the
multi
branch
2 times, most recently
from
985c8a5
to
8779bc6
Compare
umohnani8
force-pushed
the
multi
branch
2 times, most recently
from
71a2345
to
219ab4e
Compare
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return r.copyFromOCIArchiveReader(ctx, reader, options) |
There was a problem hiding this comment.
If readerRef points at specific image, this seems to completely lose that choice.
Cc: @vrothberg . I thought the distinction is that podman pull oci-archive:… always pulls exactly the one image that c/image uses under that reference, while podman load can possibly pull all images from an archive.
(Compare also the discussion about NewReaderForReference / LoadManifestDescriptor in the c/image PR.)
There was a problem hiding this comment.
Cc: @vrothberg . I thought the distinction is that podman pull oci-archive:… always pulls exactly the one image that c/image uses under that reference, while podman load can possibly pull all images from an archive.
Yes, podman pull expects one image. You can check out the behavior via podman pull docker-archive:libimage/testdata/docker-two-images.tar.xz
Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
umohnani8
force-pushed
the
multi
branch
2 times, most recently
from
302dfba
to
c9cd8ba
Compare
| } | ||
| defer func() { | ||
| if err := reader.Close(); err != nil { | ||
| logrus.Errorf(err.Error()) |
There was a problem hiding this comment.
.Error() is not a format string; this should be something like logrus.Errorf("%s", err.Error())
|
@umohnani8: 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. |
Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
|
Can we close this one due to lack of activity? |
Pulling in containers/image#1381 and adding it to the load and save logic here.