Skip to content
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

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

Closed
2 of 3 tasks
thaJeztah opened this issue Dec 5, 2019 · 4 comments · Fixed by #2220
Closed
2 of 3 tasks

Comments

@thaJeztah
Copy link
Member

thaJeztah commented Dec 5, 2019

Implementation:

The docker push command up until 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, 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 moby/moby#3411 and the pull request that implemented this: moby/moby#4948).

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"

For example, in the following situation;

docker images

REPOSITORY          TAG                        IMAGE ID            CREATED             SIZE
thajeztah/myimage   latest                     b534869c81f0        41 hours ago        1.22MB

Running docker push thajeztah/myimage seemingly does the expected behavior (it pushes thajeztah/myimage:latest to Docker Hub), however, it does not so for the reason expected (:latest being the default tag), but because :latest happens to be the only tag present for the thajeztah/myimage image.

If another tag exists for the image:

docker images

REPOSITORY          TAG                        IMAGE ID            CREATED             SIZE
thajeztah/myimage   latest                     b534869c81f0        41 hours ago        1.22MB
thajeztah/myimage   v1.0.0                     b534869c81f0        41 hours ago        1.22MB

Running the same command (docker push thajeztah/myimage) will push both images to Docker Hub.

Note that the behavior described above is currently not (clearly) documented; the docker push reference documentation (https://docs.docker.com/engine/reference/commandline/push/) does not mention that omitting the tag will push all tags

Proposed change

First of all, it should be noted that changing this behavior will be a breaking change, so should be clearly mentioned in the release notes.

I think changing the behavior should be reversed, and a flag should be added to opt-in to the behavior:

  • docker push myname/myimage will be the equivalent of docker push myname/myimage:latest
  • to push all images, the user needs to set a flag (e.g. --all-tags), so docker push --all-tags myname/myimage:latest

Alternatively, we can add support for pushing multiple tags to docker push. There is still discussion around this, as pushing multiple images or multiple tags should be optimised (instead of just looping through the provided tags). There's also a (slightly related) proposal to specify a source image on push; moby/moby#38880. That proposal was put on hold until the containerd integration was completed.

@thaJeztah
Copy link
Member Author

@silvin-lubecki @tianon @dmcgowan PTAL

@thaJeztah
Copy link
Member Author

/cc @nebuk89

@rokcarl
Copy link

rokcarl commented Dec 5, 2019

I was burnt by the current functionality today.

I had a Makefile with:

  • apush target that does docker push company/app,
  • a push-production target that does docker push company/app:production.

I was under the impression that production will never be overwritten unknowingly by a developer, even if they circumvent make and just do docker push.

But then we noticed the production instance running my development code. I was like "no way!" It took me quite a while to figure out how my code ended up on production.

Additionally, there's no great solution right now for me because our production system isn't ready yet to handle running immutable tags, e.g. the ones from the git-sha, which we compile as well. This means we need to run a hard-coded tag, which is production, but a developer could wreak havoc if they ran docker push company/app.

@tianon
Copy link
Contributor

tianon commented Dec 5, 2019

I'm +1 to bringing docker push back into symmetry with docker pull: 🎉

$ docker pull --help

Usage:	docker pull [OPTIONS] NAME[:TAG|@DIGEST]

Pull an image or a repository from a registry

Options:
  -a, --all-tags                Download all tagged images in the repository
      --disable-content-trust   Skip image verification (default true)

(This was swapped for docker pull in moby/moby#7759 👍)

thaJeztah added a commit to thaJeztah/cli that referenced this issue Jan 7, 2020
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 <github@gone.nl>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this issue Jan 9, 2020
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 <github@gone.nl>
Upstream-commit: 627a4cf7ccd0b7e92c6798c73de4dd4efc43175c
Component: cli
eiffel-fl pushed a commit to eiffel-fl/cli that referenced this issue Jul 28, 2020
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 <github@gone.nl>
elasticdog added a commit to elasticdog/tiddlywiki-docker that referenced this issue Feb 3, 2021
The default behavior in Docker 20.10 changed, so that `docker push`
will only push the `latest` tag rather than all tags. Upstream did add
an option flag to support the previous behavior (`--all-tags`), but
explicitly listing the desired tags is more understandable and ensures
backward compatibility.

See:
- docker/cli#2214
- https://www.docker.com/blog/introducing-docker-engine-20-10/
elasticdog added a commit to elasticdog/tiddlywiki-docker that referenced this issue Feb 3, 2021
The default behavior in Docker 20.10 changed, so that `docker push`
will only push the `latest` tag rather than all tags. Upstream did add
an option flag to support the previous behavior (`--all-tags`), but
explicitly listing the desired tags is more understandable and ensures
backward compatibility.

See:
- docker/cli#2214
- https://www.docker.com/blog/introducing-docker-engine-20-10/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants