Suppressing image digest in docker ps by nishanttotla · Pull Request #30848 · moby/moby

nishanttotla · 2017-02-08T22:26:03Z

Signed-off-by: Nishant Totla nishanttotla@gmail.com

This PR suppresses digest in docker ps, requiring the use of --no-trunc if digest is to be seen.

Without `--no-trunc`

$ docker ps -a
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS                            PORTS               NAMES
67652ce46200        alpine              "sleep 1000"        10 minutes ago      Exited (137) About a minute ago                       nothing
001677952d7b        alpine:latest       "sleep 1000"        10 minutes ago      Exited (137) About a minute ago                       tag
d645f54a3b62        alpine              "sleep 1000"        10 minutes ago      Exited (137) About a minute ago                       digest
7e9e3fc29448        alpine:latest       "sleep 1000"        10 minutes ago      Exited (137) About a minute ago                       taganddigest

With `--no-trunc`

$ docker ps -a --no-trunc
CONTAINER ID                                                       IMAGE                                                                                   COMMAND             CREATED             STATUS                            PORTS               NAMES
67652ce46200bf406556b4220da0233bef229c5950899b2dabd81650677c7566   alpine                                                                                  "sleep 1000"        10 minutes ago      Exited (137) About a minute ago                       nothing
001677952d7b4e1a2b4dcff00db8e43c22bfc890a38d00393ce1fb0009e2e760   alpine:latest                                                                           "sleep 1000"        10 minutes ago      Exited (137) About a minute ago                       tag
d645f54a3b6287628389ac604eff553a8451c4ce038a6b137d15a56e8047f248   alpine@sha256:dfbd4a3a8ebca874ebd2474f044a0b33600d4523d03b0df76e5c5986cb02d7e8          "sleep 1000"        10 minutes ago      Exited (137) About a minute ago                       digest
7e9e3fc2944894097e8d168f735acf03e1c23ed50681d1576fff9aa0bd6dd56e   alpine:latest@sha256:dfbd4a3a8ebca874ebd2474f044a0b33600d4523d03b0df76e5c5986cb02d7e8   "sleep 1000"        11 minutes ago      Exited (137) About a minute ago                       taganddigest

This follows up from #28539 to make behavior consistent with service ls/ps.

nishanttotla · 2017-02-09T01:38:41Z

Ping @thaJeztah @cpuguy83

aaronlehmann · 2017-02-09T01:56:05Z

cli/command/formatter/container.go

should be "was not selected"

aaronlehmann · 2017-02-09T01:58:50Z

cli/command/formatter/container.go

Suggest else if instead of nested else and if

Actually, this check is not needed. ParseNormalizedNamed returns a reference.Named.

@aaronlehmann you're right, I removed the check.

aaronlehmann · 2017-02-10T19:55:40Z

cli/command/formatter/container.go

Thinking about this some more, I'm not sure we want to discard the digest in this case.

It makes sense for the digest+tag case, because the digest is primarily internal information that doesn't need to clutter the output.

But if there's only a digest, it feels like it might be misleading to only show the name.

Not sure what others think. If there's a good UX reason to never show digests here, I guess it's okay.

@aaronlehmann I see your point. Although, given that --no-trunc exists, it's not the case that there's no way to easily retrieve full information. Wouldn't the same reasoning apply to service ps/ls too? Anyhow, I'm happy to discuss the UX implications of this. Not sure who else to tag here.

(My personal opinion is that I find it cleaner to not have to look at digests unless I specifically ask for them. @cpuguy83 indicated the same earlier. This also came up in another issue that I can't find right now.)

That's fair, given that --no-trunc exists removing digests by default might be reasonable.

@cpuguy83: Any thoughts?

Yeah I see the digest as an ID, which is generally undesirable in ps unless that's exactly what was specified.

And even if I did specify it... digest is really big for docker ps.

I think it makes sense to omit it (alternatively, truncate the output, but we don't have a fixed length defined for these)

aaronlehmann · 2017-02-13T22:34:17Z

LGTM

cpuguy83 · 2017-02-17T02:02:05Z

$ docker swarm init
...
$ docker service create --name test busybox top
...
ERRO[0037] pulling image failed                          error="Cannot overwrite digest sha256:817a12c32a39bbe394944ba49de563e085f1d3c5266eb8e9723256bc4448680e" module="node/agent/taskmanager" task.id=8kzv0vh79rigpkz38uqc9okh4
ERRO[0037] fatal task error                              error="No such image: busybox:latest@sha256:817a12c32a39bbe394944ba49de563e085f1d3c5266eb8e9723256bc4448680e" module="node/agent/taskmanager" task.id=8kzv0vh79rigpkz38uqc9okh4

cpuguy83 · 2017-02-17T02:02:29Z

Really strange...

aaronlehmann · 2017-02-17T02:06:31Z

@cpuguy83: Have you rebased against the latest master? You may be experiencing #30889.

cpuguy83 · 2017-02-17T02:07:15Z

My master works, this PR is not.

aaronlehmann · 2017-02-17T02:09:37Z

It needs to be rebased.

Signed-off-by: Nishant Totla <nishanttotla@gmail.com>

nishanttotla · 2017-02-17T02:12:31Z

@cpuguy83 @aaronlehmann I just rebased this PR.

aaronlehmann · 2017-02-17T02:13:05Z

Thanks!

cpuguy83

LGTM

vdemeester · 2017-02-17T09:27:45Z

windows failure unrelated 😓

…r-ps Suppressing image digest in docker ps

GordonTheTurtle added the status/0-triage label Feb 8, 2017

nishanttotla changed the title ~~Suppressing digest in docker ps~~ Suppressing image digest in docker ps Feb 8, 2017

aaronlehmann reviewed Feb 9, 2017

View reviewed changes

cli/command/formatter/container.go Outdated

Copy link

aaronlehmann Feb 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be "was not selected"

aaronlehmann reviewed Feb 9, 2017

View reviewed changes

nishanttotla force-pushed the suppress-digest-docker-ps branch 2 times, most recently from 7b03e8a to 4dc55db Compare February 9, 2017 18:32

aaronlehmann reviewed Feb 10, 2017

View reviewed changes

thaJeztah added status/2-code-review and removed status/0-triage labels Feb 13, 2017

Suppressing image digest in docker ps

b04536f

Signed-off-by: Nishant Totla <nishanttotla@gmail.com>

nishanttotla force-pushed the suppress-digest-docker-ps branch from 4dc55db to b04536f Compare February 17, 2017 02:12

cpuguy83 approved these changes Feb 17, 2017

View reviewed changes

cpuguy83 added status/4-merge and removed status/2-code-review labels Feb 17, 2017

vdemeester merged commit 4053214 into moby:master Feb 17, 2017

GordonTheTurtle added this to the 1.14.0 milestone Feb 17, 2017

thaJeztah added impact/changelog impact/cli labels Feb 17, 2017

nishanttotla deleted the suppress-digest-docker-ps branch February 17, 2017 17:45

aaronlehmann mentioned this pull request Mar 9, 2017

docker stack deploy creates no tags #31574

Open

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#30848 from nishanttotla/suppress-digest-docke…

798ed3e

…r-ps Suppressing image digest in docker ps

Conversation

nishanttotla commented Feb 8, 2017

Without --no-trunc

With --no-trunc

Uh oh!

nishanttotla commented Feb 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronlehmann commented Feb 13, 2017

Uh oh!

cpuguy83 commented Feb 17, 2017

Uh oh!

cpuguy83 commented Feb 17, 2017

Uh oh!

aaronlehmann commented Feb 17, 2017

Uh oh!

cpuguy83 commented Feb 17, 2017

Uh oh!

aaronlehmann commented Feb 17, 2017

Uh oh!

nishanttotla commented Feb 17, 2017

Uh oh!

aaronlehmann commented Feb 17, 2017

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

vdemeester commented Feb 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Without `--no-trunc`

With `--no-trunc`