podman load/save: support multi-image docker archive#6811
podman load/save: support multi-image docker archive#6811openshift-merge-robot merged 1 commit intocontainers:masterfrom
Conversation
|
Need to get containers/image#975 in before. |
cda54b9 to
9efd4e8
Compare
mtrmac
left a comment
There was a problem hiding this comment.
Just a few notes while primarily reading the called c/image implementation.
9efd4e8 to
34bca45
Compare
aef021b to
e745b41
Compare
mtrmac
left a comment
There was a problem hiding this comment.
The changes to libpod/image/pull.go were dropped — is that intentional?
In a sense they really don’t belong into this PR, OTOH fixing the existing fairly broken podman pull docker-archive:… code would be nice.
That was intentional as I want us to focus on the multi-image part first. Once that's in, we can tackle the next issue - unless you feel strongly to tackle both at once. |
|
I’m fine with having the PR narrowly scoped; what is/isn’t necessary to keep the Podman repo maintainable is up to the judgment of Podman maintainers. |
e745b41 to
22a6361
Compare
22a6361 to
00c1f5a
Compare
40a2408 to
0a03352
Compare
mtrmac
left a comment
There was a problem hiding this comment.
More skeletons in the closet…
0041d39 to
b1a155f
Compare
064ce90 to
f06bea3
Compare
|
Back to c/storage v1.23.0 |
|
@edsantiago are https://github.com/containers/podman/pull/6811/checks?check_run_id=1007722626 flakes? @mtrmac, I am running out of time. If you find some cycles, feel free to take over this PR and push it over the finish line. |
|
@vrothberg those show every sign of being #7195 but I've never seen so many in one run. |
|
Restarted the last flaky job. The flakes are hitting this PR hard. |
|
Your getting hit by the flake I can not get by. |
8bd896e to
0c31a70
Compare
|
It's green, it's green! @rhatdan, let's merge before the CI gods stop shining upon me head. |
mtrmac
left a comment
There was a problem hiding this comment.
ACK (full review now), just some maintainability suggestions.
|
Also, this vendors an unreleased version of c/image. Is that OK? |
|
OK for now, but we'll need to get a c/image release out in the next several weeks for Podman 2.1.0 so we don't cut a full release with a prerelease c/image |
Support loading and saving tarballs with more than one image. Add a new `/libpod/images/export` endpoint to the rest API to allow for exporting/saving multiple images into an archive. Note that a non-release version of containers/image is vendored. A release version must be vendored before cutting a new Podman release. We force the containers/image version via a replace in the go.mod file; this way go won't try to match the versions. Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
0c31a70 to
7fea467
Compare
|
Final polish based on @mtrmac's comments pushed. |
| k8s.io/client-go v0.0.0-20190620085101-78d2af792bab | ||
| ) | ||
|
|
||
| replace github.com/containers/image/v5 => github.com/containers/image/v5 v5.5.2-0.20200902171422-1c313b2d23e0 |
There was a problem hiding this comment.
Isn't this a WIP until this is released? I think we should release containers/image so that @mheon can release pdoman 2.1.0
There was a problem hiding this comment.
@mheon acked above (see #6811 (comment)). We just need to release before v2.1.0 which is still a bit in the future.
|
/lgtm |
|
Is there any follow up work planned to bring the other formats into line with this? In particular I'm interested in |
The @nalind @mtrmac I haven't checked the code but assume that the |
|
A similar list/get-reference API seems natural; the thing I’m not currently sure about is multi-arch support; that somehow uses OCI indices, and I’m not quite sure how. IIRC it’s not a nested-index design, so the top-level index might be a multi-arch image set, not a multi-image archive. @nalind probably knows more. |
|
When we merged manifest list support, the Copying using Copying from another location into the same or When using an If there's more than one item in the |
Fixes: #2669
Signed-off-by: Valentin Rothberg rothberg@redhat.com