[wip] Podman push --all-tags option #14949

byarbrough · 2022-07-15T15:00:59Z

Task for #14917

Podman will now push all tags for a given image
when -a or --all-tags is specified.

Signed-off-by: Brian Yarbrough[email protected]

Does this PR introduce a user-facing change?

Adds option to push all tags for a given image. When doing so, do not specify a tag after the destination.

Task for containers#14917 Podman will now push all tags for a given image when -a or --all-tags is specified. Signed-off-by: byarbrough <[email protected]>

byarbrough · 2022-07-15T15:06:08Z

This ended up being much more complicated than simply adding the flag. It required multiple modifications to some structs, as well as some new checks when a Push is happening. The preexisting All boolean didn't do anything.

IMHO this is the sort of complexity that should be left to tools like Skopeo or Buildah since they are purpose built for creating and moving a large numbers of tags. I really won't be offended if this is not merged. I couldn't find any issues that mention this as a requested feature.

But if we truly want to "alias docker" then I guess this flag would be required.

rhatdan · 2022-07-15T15:29:38Z

Yes we want this feature in Podman. We don't want to tell users to download and install another tool when they have Podman.

rhatdan · 2022-07-15T15:30:11Z

cmd/podman/images/push.go

@@ -73,8 +73,8 @@ func init() {
 func pushFlags(cmd *cobra.Command) {
 	flags := cmd.Flags()

-	// For now default All flag to true, for pushing of manifest lists
-	pushOptions.All = true
+	flags.BoolVarP(&pushOptions.All, "all-tags", "a", false, "Push all tagged images in the repository")


This should still be defaulted to true.

rhatdan · 2022-07-15T15:33:08Z

vendor/github.com/containers/common/libimage/push.go

@@ -36,6 +38,36 @@ func (r *Runtime) Push(ctx context.Context, source, destination string, options
 		return nil, err
 	}

+	// If specified to push all tags, look them up
+	if options.AllTags {


You can not make changes in vendored directory. You need to open a PR against containers/common to get this patch merged and then vendor it into Podman.

Ah, I was wondering what that was all about. I'll get that done.

openshift-ci · 2022-07-15T15:34:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: byarbrough, 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:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rhatdan · 2022-07-15T15:34:50Z

Your tests should be comparing --all=false to --all=true.
@vrothberg PTAL

vrothberg

Thanks for contributing!

A couple of additional comments:

Please add your real name to the Signed-off-by field in the commit message. The reasoning is explained in the contributing guidelines.
As Dan mentioned, the plumbing in libimage must be done in a separate PR in containers/common. Once the PR there is merged, you can vendor the changes along with this PR here.
The plumbing for the remote client must be done as well. This includes podman-remote and pkg/bindings for the client and pkg/api for the server (including the swagger docs in pkg/api/server and the endpoint in pkg/api/handlers).

Feel free to ping us here or on IRC/Matrix if you have any questions.

This seeks to mirror `docker push --all-tags IMAGE`. Because tags are appended to the destination, this will only work with the docker transport. Otherwise you'd get directories overwriting or with weird names. Also note that if AllTags is true then the user must provide the name of the image only; providing a tag will crash. Docker has this behavior: ``` tag can't be used with --all-tags/-a ``` Requirement for containers/podman#14949 ``` Signed-off-by: Brian Yarbrough [email protected] ```

This seeks to mirror `docker push --all-tags IMAGE`. Because tags are appended to the destination, this will only work with the docker transport. Otherwise you'd get directories overwriting or with weird names. Also note that if AllTags is true then the user must provide the name of the image only; providing a tag will crash. Docker has this behavior: ``` tag can't be used with --all-tags/-a ``` Requirement for containers/podman#14949 Signed-off-by: Brian Yarbrough <[email protected]>

openshift-merge-robot · 2022-07-23T10:59:37Z

@byarbrough: 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.

This seeks to mirror `docker push --all-tags IMAGE`. Because tags are appended to the destination, this will only work with the docker transport. Otherwise you'd get directories overwriting or with weird names. Also note that if AllTags is true then the user must provide the name of the image only; providing a tag will crash. Docker has this behavior: ``` tag can't be used with --all-tags/-a ``` Requirement for containers/podman#14949 Signed-off-by: Brian Yarbrough <[email protected]>

github-actions · 2022-08-23T00:07:55Z

A friendly reminder that this PR had no activity for 30 days.

rhatdan · 2022-08-23T13:56:42Z

@byarbrough Are you still working on this?
@vrothberg time to take this over?

byarbrough · 2022-08-23T20:05:44Z

This is blocked by the PR containers/common#1099
That one is turning out to be a larger lift than expected - especially now with the likely need to add some unit tests to verify that all the correct images are being identified.

jakecorrenti · 2022-09-23T03:08:02Z

This might be a bit over my head, but if it's not time sensitive I would like to take a whirl at this @rhatdan @vrothberg

rhatdan · 2022-09-23T13:54:04Z

SGTM

rhatdan · 2022-09-23T13:54:47Z

@jakecorrenti Take over the containers/common PR and get it merged first.

jakecorrenti · 2022-09-23T14:33:42Z

sounds good

Podman push --all-tags option

77e4b78

Task for containers#14917 Podman will now push all tags for a given image when -a or --all-tags is specified. Signed-off-by: byarbrough <[email protected]>

openshift-ci bot added the release-note label Jul 15, 2022

rhatdan changed the title ~~Podman push --all-tags option~~ [wip] Podman push --all-tags option Jul 15, 2022

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 15, 2022

rhatdan approved these changes Jul 15, 2022

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 15, 2022

vrothberg reviewed Jul 18, 2022

View reviewed changes

byarbrough mentioned this pull request Jul 21, 2022

[wip] Add option to push all tags containers/common#1099

Closed

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2022

github-actions bot added the stale-pr label Aug 23, 2022

github-actions bot removed the stale-pr label Aug 24, 2022

byarbrough closed this Sep 22, 2022

byarbrough mentioned this pull request Sep 22, 2022

man pages: docker differences #14917

Closed

69 tasks

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wip] Podman push --all-tags option #14949

[wip] Podman push --all-tags option #14949

byarbrough commented Jul 15, 2022

byarbrough commented Jul 15, 2022

rhatdan commented Jul 15, 2022

rhatdan Jul 15, 2022

rhatdan Jul 15, 2022

byarbrough Jul 15, 2022

openshift-ci bot commented Jul 15, 2022

rhatdan commented Jul 15, 2022

vrothberg left a comment

openshift-merge-robot commented Jul 23, 2022

github-actions bot commented Aug 23, 2022

rhatdan commented Aug 23, 2022

byarbrough commented Aug 23, 2022

jakecorrenti commented Sep 23, 2022

rhatdan commented Sep 23, 2022

rhatdan commented Sep 23, 2022

jakecorrenti commented Sep 23, 2022

[wip] Podman push --all-tags option #14949

[wip] Podman push --all-tags option #14949

Conversation

byarbrough commented Jul 15, 2022

Does this PR introduce a user-facing change?

byarbrough commented Jul 15, 2022

rhatdan commented Jul 15, 2022

rhatdan Jul 15, 2022

Choose a reason for hiding this comment

rhatdan Jul 15, 2022

Choose a reason for hiding this comment

byarbrough Jul 15, 2022

Choose a reason for hiding this comment

openshift-ci bot commented Jul 15, 2022

rhatdan commented Jul 15, 2022

vrothberg left a comment

Choose a reason for hiding this comment

openshift-merge-robot commented Jul 23, 2022

github-actions bot commented Aug 23, 2022

rhatdan commented Aug 23, 2022

byarbrough commented Aug 23, 2022

jakecorrenti commented Sep 23, 2022

rhatdan commented Sep 23, 2022

rhatdan commented Sep 23, 2022

jakecorrenti commented Sep 23, 2022