client.ImagePush(): default to ":latest" instead of "all tags" by thaJeztah · Pull Request #40302 · moby/moby

thaJeztah · 2019-12-10T11:41:18Z

relates to docker/cli#2214 and docker/cli#2220

The docker push command up until docker v0.9.1 always pushed all tags of a given
image, so docker push foo/bar would push (e.g.) all of foo/bar:latest, foo:/bar:v1,
and foo/bar:v1.0.0.

Pushing all tags of an image was not desirable in many case, so docker v0.10.0
enhanced docker push to optionally specify a tag to push (docker push foo/bar:v1)
(see issue #3411 and #4948 (commit e648a18).

This behavior exists up until today, and is confusing, because unlike other commands,
docker push does not default to use the :latest tag when omitted, but instead
makes it push "all tags of the image".

docker pull had a similar behavior, but #7759 (9c08364)
changed the behavior to default to the :latest tag, and added a --all-tags flag
to the CLI to optionally pull all images.

This patch implements the API client changes to make docker push match the behavior
of docker pull, and default to pull a single image, unless the all option is passed.

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

The `docker push` command up until docker v0.9.1 always pushed all tags of a given image, so `docker push foo/bar` would push (e.g.) all of `foo/bar:latest`, `foo:/bar:v1`, and `foo/bar:v1.0.0`. Pushing all tags of an image was not desirable in many case, so docker v0.10.0 enhanced `docker push` to optionally specify a tag to push (`docker push foo/bar:v1`) (see issue 3411 and PR 4948 (commit e648a18). This behavior exists up until today, and is confusing, because unlike other commands, `docker push` does not default to use the `:latest` tag when omitted, but instead makes it push "all tags of the image". `docker pull` had a similar behavior, but PR 7759 (9c08364) changed the behavior to default to the `:latest` tag, and added a `--all-tags` flag to the CLI to optionally pull all images. This patch implements the API client changes to make `docker push` match the behavior of `docker pull`, and default to pull a single image, unless the `all` option is passed. Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2019-12-10T13:40:34Z

client/image_push.go

-
 	query := url.Values{}
-	query.Set("tag", tag)
+	if !options.All {


I was considering if we should produce an error if options.All is set and image contains a tag; currently this situation is handled in the CLI;

case opts.all && !reference.IsNameOnly(ref): return errors.New("tag can't be used with --all-tags/-a")

Which is the same as how it's handled for docker pull, but perhaps we want that case to be handled by the client instead.

Also happy to do that in a follow-up and do it for both Push and Pull if we want them to act consistent.

thaJeztah · 2019-12-10T13:41:26Z

ping @tianon @dmcgowan @tiborvass PTAL

tianon

IMO this is fine as-is (given the edge case is not likely common); LGTM

dmcgowan

LGTM

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]>

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker/cli#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker/cli#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker/cli#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]> Upstream-commit: 627a4cf7ccd0b7e92c6798c73de4dd4efc43175c Component: cli

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]>

Up until now, the build was relying on an undocumented--but well established--"feature" of the docker push command, namely the fact that if no tag was specified, all built tags would be pushed. This was recently changed [1], requiring the `--all-tags` flag to emulate the previous behavior. [1] moby/moby#40302

Up until now, the build was relying on an undocumented--but well established--"feature" of the docker push command, namely the fact that if no tag was specified, all built tags would be pushed. This was recently changed [1], requiring the `--all-tags` flag to emulate the previous behavior. [1] moby/moby#40302 Signed-off-by: Konstantinos Smanis <[email protected]>

Up until now, the build was relying on an undocumented--but well established--"feature" of the docker push command, namely the fact that if no tag was specified, all built tags would be pushed. This was recently changed [1], requiring the `--all-tags` flag to emulate the previous behavior. [1] moby/moby#40302 Signed-off-by: Konstantinos Smanis <[email protected]> Signed-off-by: Alexys Jacob <[email protected]>

thaJeztah added area/api API area/distribution Image Distribution status/2-code-review impact/changelog labels Dec 10, 2019

thaJeztah marked this pull request as ready for review December 10, 2019 13:37

thaJeztah commented Dec 10, 2019

View reviewed changes

tianon approved these changes Dec 10, 2019

View reviewed changes

tonistiigi requested a review from dmcgowan December 12, 2019 19:21

tonistiigi approved these changes Dec 12, 2019

View reviewed changes

dmcgowan approved these changes Dec 12, 2019

View reviewed changes

thaJeztah merged commit a0bbe36 into moby:master Dec 12, 2019

thaJeztah deleted the use_all_tags_option branch December 12, 2019 19:54

This was referenced Dec 19, 2019

[proposal] change "docker push" behavior to default to ":latest" instead of "all tags" docker/cli#2214

Closed

implement docker push -a/--all-tags docker/cli#2220

Merged

thaJeztah mentioned this pull request Jan 7, 2020

vendor: bump docker and dependencies a9507c6f76627fdc092edc542d5a7ef4a6df5eec docker/cli#2242

Merged

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

thaJeztah mentioned this pull request Apr 8, 2020

Bug or Misleading Docu: All docker images will be downloaded when the image tag is not specified docker/docker-py#2545

Closed

KSmanis mentioned this pull request Feb 13, 2021

Fix failing docker push gentoo/gentoo-docker-images#102

Closed

thaJeztah mentioned this pull request Feb 24, 2021

Don't allow pulling all tags by default for the ImageCreate API endpoint #42053

Open

andrewsomething mentioned this pull request Mar 1, 2021

Push to docker registry fails with tag does not exist registry.digitalocean.com/mailio/mailiofrontend:latest do-community/example-doctl-action#13

Closed

fawind mentioned this pull request Nov 23, 2021

Docker push pushes all tags palantir/gradle-docker#506

Merged

teliatko mentioned this pull request Jun 21, 2022

Extend docker push with --all-tags ps-aux/cibs#7

Merged

thaJeztah mentioned this pull request Mar 7, 2023

c8d: Implement push #44963

Merged

thaJeztah mentioned this pull request Oct 25, 2024

[RFC] consider changing meaning of "--all-tags" to "all tags for the same image" #48760

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client.ImagePush(): default to ":latest" instead of "all tags"#40302

client.ImagePush(): default to ":latest" instead of "all tags"#40302
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:use_all_tags_option

thaJeztah commented Dec 10, 2019

Uh oh!

thaJeztah Dec 10, 2019

Uh oh!

thaJeztah commented Dec 10, 2019

Uh oh!

tianon left a comment

Uh oh!

dmcgowan left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

thaJeztah commented Dec 10, 2019

Uh oh!

thaJeztah Dec 10, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Dec 10, 2019

Uh oh!

tianon left a comment

Choose a reason for hiding this comment

Uh oh!

dmcgowan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants