-
Notifications
You must be signed in to change notification settings - Fork 787
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
Make buildah push support pushing manifests lists and digests #2895
Conversation
cmd/buildah/manifest.go
Outdated
return manifestsPush(systemContext, store, listImageSpec, destSpec, opts) | ||
} | ||
|
||
func manifestsPush(systemContext *types.SystemContext, store storage.Store, listImageSpec, destSpec string, opts pushOptions) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to manifestPush
. The stray "s" isn't consistent with anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange I had it their originally and then mistakenly made this change. C hanged back.
cmd/buildah/push.go
Outdated
@@ -76,6 +79,7 @@ func init() { | |||
flags.BoolVarP(&opts.disableCompression, "disable-compression", "D", false, "don't compress layers") | |||
flags.StringVarP(&opts.format, "format", "f", "", "manifest type (oci, v2s1, or v2s2) to use when saving image using the 'dir:' transport (default is manifest type of source)") | |||
flags.BoolVarP(&opts.quiet, "quiet", "q", false, "don't output progress information when pushing images") | |||
flags.BoolVar(&opts.purge, "purge", false, "remove the manifest list if push succeeds") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why purge
and not rm
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because manifest used --purge. Moved both to use --rm, and aliases purge to rm.
cmd/buildah/push.go
Outdated
@@ -195,6 +199,10 @@ func pushCmd(c *cobra.Command, args []string, iopts pushOptions) error { | |||
|
|||
ref, digest, err := buildah.Push(getContext(), src, dest, options) | |||
if err != nil { | |||
// Attempt to manfest push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: "manfest".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
7e8bbad
to
810c11a
Compare
LGTM |
cmd/buildah/push.go
Outdated
@@ -68,6 +71,7 @@ func init() { | |||
|
|||
flags := pushCommand.Flags() | |||
flags.SetInterspersed(false) | |||
flags.BoolVar(&opts.all, "all", false, "add all of the lists' images if the images to add are lists") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy/paste error in the description of the option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is how the option is described in manfest list.
unhappy red tests @rhatdan |
Currently manifests just look like images in container storage. It is surprising to the user when they go to push the images that they end up failing, and have to use the buildah manifest push. This patch causes buildah push to failover to buildah manifest push if the image is a manifest. Signed-off-by: Daniel J Walsh <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Currently manifests just look like images in container storage.
It is surprising to the user when they go to push the images
that they end up failing, and have to use the buildah manifest push.
This patch causes buildah push to failover to buildah manifest push
if the image is a manifest.
Signed-off-by: Daniel J Walsh [email protected]
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?