Suppressing digest for docker service ls/ps by nishanttotla · Pull Request #28539 · moby/moby

nishanttotla · 2016-11-17T06:43:31Z

This PR simplifies the UI for the changes introduced in #28173 (Pin images by digest).

In particular, this PR suppresses the digest for docker service ls and docker service ps, showing only image:tag. The full digest image:tag@digest is still shown as part of docker service inspect.

Fix #28502

nishanttotla · 2016-11-17T20:21:00Z

Looks like test failures are related, working on fixing them now.

aaronlehmann · 2016-11-18T00:29:48Z

cli/command/service/list.go

This type assertion may fail. Images do not always have a tag.

This is probably why some tests are failing.

You shouldn't ignore the second value from the type assertion:

namedTagged, ok := ref.(distreference.NamedTagged)

and the use of Name and Tag should be guarded by if ok.

@aaronlehmann At first I thought that the tests were checking against the spec which was causing failures (still confirming that). I've updated to include the ok value from the type assertion.

nishanttotla · 2016-11-18T06:59:54Z

Tests pass now. Ping @aaronlehmann @cpuguy83 @thaJeztah

cpuguy83 · 2016-11-18T13:46:26Z

Is there anything we can do for docker ps as well?

cpuguy83

LGTM

thaJeztah · 2016-11-18T15:49:38Z

Sorry for the late reply; looking at docker service ps we do have --no-trunc. Shouldn't we show the full image reference if I use that? Right now there's no difference (probably only ERROR isn't truncated)

docker service ps web
NAME                IMAGE         NODE          DESIRED STATE  CURRENT STATE               ERROR  PORTS
web.1.qk6sjgd0b0kq  nginx:alpine  d610c3c51568  Running        Running about a minute ago

docker service ps --no-trunc web
NAME                IMAGE         NODE          DESIRED STATE  CURRENT STATE               ERROR  PORTS
web.1.qk6sjgd0b0kq  nginx:alpine  d610c3c51568  Running        Running about a minute ago

nishanttotla · 2016-11-18T16:10:33Z

@thaJeztah that makes sense. Is --no-trunc only meant for docker service ps, or also makes sense for docker service ls?

aaronlehmann · 2016-11-18T17:33:48Z

cli/command/service/list.go

remove this comment

aaronlehmann · 2016-11-18T17:33:55Z

cli/command/task/print.go

remove this comment

Signed-off-by: Nishant Totla <[email protected]>

nishanttotla · 2016-11-18T17:40:06Z

@thaJeztah updated PR to print full image string if --no-trunc is supplied for docker service ps.

thaJeztah

LGTM, thanks!

vdemeester

LGTM 👼

vdemeester · 2016-11-21T13:02:06Z

There should be docs-update right ?

vdemeester · 2016-11-21T13:06:53Z

@thaJeztah will do a follow-up for the docs ⚓

cpuguy83 · 2016-11-21T13:34:01Z

@nishanttotla Did you have a chance to check if docker ps could get a similar fix?

nishanttotla · 2016-11-21T19:16:04Z

@cpuguy83 I thought docker ps already did this, but I will double check to confirm.

nishanttotla · 2016-11-21T23:53:32Z

@cpuguy83 you're right, this should be done for docker ps as well. I'll send out a PR.

Suppressing digest for docker service ls/ps

GordonTheTurtle added the status/0-triage label Nov 17, 2016

nishanttotla mentioned this pull request Nov 17, 2016

Pin images by digest #28173

Merged

vieux added status/2-code-review and removed status/0-triage labels Nov 17, 2016

vieux added this to the 1.13.0 milestone Nov 17, 2016

vieux added the process/cherry-pick label Nov 17, 2016

thaJeztah mentioned this pull request Nov 17, 2016

Get image tag from Services endpoint #28484

Closed

aaronlehmann reviewed Nov 18, 2016

View reviewed changes

nishanttotla force-pushed the pin-image-digest-ux branch from 90faafb to a4dc9df Compare November 18, 2016 00:39

cpuguy83 approved these changes Nov 18, 2016

View reviewed changes

aaronlehmann reviewed Nov 18, 2016

View reviewed changes

cli/command/service/list.go Outdated

Copy link

aaronlehmann Nov 18, 2016

Choose a reason for hiding this comment

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

remove this comment

aaronlehmann reviewed Nov 18, 2016

View reviewed changes

cli/command/task/print.go Outdated

Copy link

aaronlehmann Nov 18, 2016

Choose a reason for hiding this comment

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

remove this comment

nishanttotla force-pushed the pin-image-digest-ux branch 2 times, most recently from cac53ca to a717a64 Compare November 18, 2016 17:38

Suppressing digest for docker service ls/ps

e7d83fd

Signed-off-by: Nishant Totla <[email protected]>

nishanttotla force-pushed the pin-image-digest-ux branch from a717a64 to e7d83fd Compare November 18, 2016 17:39

thaJeztah approved these changes Nov 21, 2016

View reviewed changes

vdemeester approved these changes Nov 21, 2016

View reviewed changes

vdemeester merged commit 278e01d into moby:master Nov 21, 2016

vdemeester added the docs/revisit label Nov 21, 2016

thaJeztah mentioned this pull request Nov 21, 2016

Update service ps reference for "--no-trunc" #28664

Merged

nishanttotla deleted the pin-image-digest-ux branch November 21, 2016 19:15

vieux added process/cherry-picked and removed process/cherry-pick labels Nov 21, 2016

vieux mentioned this pull request Nov 21, 2016

1.13.0-rc2 cherry-picks : part 3 #28674

Merged

thaJeztah removed the docs/revisit label Nov 21, 2016

nishanttotla mentioned this pull request Feb 8, 2017

Suppressing image digest in docker ps #30848

Merged

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

Merge pull request moby#28539 from nishanttotla/pin-image-digest-ux

89703a0

Suppressing digest for docker service ls/ps

nishanttotla mentioned this pull request May 13, 2017

docker service ls/ps is not truncating digests anymore docker/cli#79

Closed

Conversation

nishanttotla commented Nov 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nishanttotla commented Nov 17, 2016

Uh oh!

aaronlehmann Nov 18, 2016

Choose a reason for hiding this comment

Uh oh!

nishanttotla Nov 18, 2016

Choose a reason for hiding this comment

Uh oh!

nishanttotla commented Nov 18, 2016

Uh oh!

cpuguy83 commented Nov 18, 2016

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Nov 18, 2016

Uh oh!

nishanttotla commented Nov 18, 2016

Uh oh!

aaronlehmann Nov 18, 2016

Choose a reason for hiding this comment

Uh oh!

aaronlehmann Nov 18, 2016

Choose a reason for hiding this comment

Uh oh!

nishanttotla commented Nov 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

vdemeester commented Nov 21, 2016

Uh oh!

vdemeester commented Nov 21, 2016

Uh oh!

cpuguy83 commented Nov 21, 2016

Uh oh!

nishanttotla commented Nov 21, 2016

Uh oh!

nishanttotla commented Nov 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

nishanttotla commented Nov 17, 2016 •

edited

Loading

nishanttotla commented Nov 18, 2016 •

edited

Loading