Add mounts to docker ps. #20017

calavera · 2016-02-05T03:10:31Z

Allow to filter containers by volume with --filter volume=name and filter volume=/dest.
Show their names in the list with the custom format {{ .Mounts }}.

This is consistent with the Network information we added recently.

Signed-off-by: David Calavera david.calavera@gmail.com

thaJeztah · 2016-02-15T13:55:14Z

ping @duglin I think this is something you were looking for?

duglin · 2016-02-15T14:09:32Z

compiling this (not rebasing) I see:

runconfig/opts/parse.go:411: undefined: container.IsolationLevel

duglin · 2016-02-15T14:20:09Z

rebase needed

thaJeztah · 2016-02-17T12:40:09Z

ping @calavera can you rebase this?

calavera · 2016-02-17T22:10:23Z

ping @calavera can you rebase this?

will do when we decide whether we want this change or not. There is no point to rebase before docker/engine-api#85 is merged, from my point of view.

calavera · 2016-02-18T22:28:50Z

I've rebased with master and vendored engine-api already. I also added documentation for the new filter and format.

thaJeztah · 2016-02-18T22:41:00Z

Can you add a note to the API changes; https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api.md#v123-api-changes

calavera · 2016-02-18T23:12:26Z

@thaJeztah done, thanks for the reminder.

lowenna · 2016-02-19T03:16:32Z

windowsTP4 failure is real - needs to have conditional code for Windows file path semantics. Also rather than running busybox top in the test, use runSleepingContainer.

16:50:14 ----------------------------------------------------------------------
16:50:14 FAIL: docker_cli_ps_test.go:738: DockerSuite.TestPsShowMounts
16:50:14 
16:50:14 docker_cli_ps_test.go:740:
16:50:14     dockerCmd(c, "run", "--name=volume-test-1", "-d", "--volume", "ps-volume-test:/test", "busybox", "top")
16:50:14 C:/gopath/src/github.com/docker/docker/pkg/integration/dockerCmd_utils.go:42:
16:50:14     c.Assert(err, check.IsNil, check.Commentf(quote+"%v"+quote+" failed with errors: %s, %v", strings.Join(args, " "), out, err))
16:50:14 ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc082505120)} ("exit status 125")
16:50:14 ... "run --name=volume-test-1 -d --volume ps-volume-test:/test busybox top" failed with errors: D:\CI\CI-cf50592\binary\docker.exe: Error response from daemon: Invalid bind mount spec "ps-volume-test:/test": volumeinvalid: Invalid volume specification: 'ps-volume-test:/test'.
16:50:14 See 'D:\CI\CI-cf50592\binary\docker.exe run --help'.
16:50:14 , exit status 125
16:50:14 
16:50:14 
16:50:14 ----------------------------------------------------------------------

duglin · 2016-02-19T15:53:30Z

docs/reference/api/docker_remote_api_v1.23.md

@@ -184,6 +195,10 @@ Query Parameters:
  -   `status=`(`created`|`restarting`|`running`|`paused`|`exited`|`dead`)
  -   `label=key` or `label="key=value"` of a container label
  -   `isolation=`(`default`|`process`|`hyperv`)   (Windows daemon only)
+  -   `ancestor`=(`<image-name>[:<tag>]`,  `<image id>` or `<image@digest>`)
+  -   `before`=(container's id or name)


minor thing, but for consistency, should these use < >, e.g. <container id> instead of "container's id" since you're using <image id> above?

duglin · 2016-02-19T16:04:44Z

one minor request on the test, but aside from that LGTM

calavera · 2016-02-19T17:46:57Z

@duglin I've addressed your comments.

@jhowardmsft ❤️ to know that when I break windows support the tests fail horribly 🙌

duglin · 2016-02-19T17:58:06Z

@calavera thanks! Still LGTM assuming janky is happy

lowenna · 2016-02-19T18:56:20Z

@calavera Looks like it's still broken, sorry!

10:24:51 ----------------------------------------------------------------------
10:24:51 PANIC: docker_cli_ps_test.go:738: DockerSuite.TestPsShowMounts
10:24:51 
10:24:51 ... Panic: runtime error: index out of range (PC=0x45DC15)
10:24:51 
10:24:51 c:/go/src/runtime/asm_amd64.s:437
10:24:51   in call32
10:24:51 c:/go/src/runtime/panic.go:423
10:24:51   in gopanic
10:24:51 c:/go/src/runtime/panic.go:12
10:24:51   in panicindex
10:24:51 docker_cli_ps_test.go:780
10:24:51   in DockerSuite.TestPsShowMounts
10:24:51 c:/go/src/runtime/asm_amd64.s:437
10:24:51   in call32
10:24:51 c:/go/src/reflect/value.go:432
10:24:51   in Value.call
10:24:51 c:/go/src/reflect/value.go:300
10:24:51   in Value.Call
10:24:51 C:/gopath/src/github.com/docker/docker/vendor/src/github.com/go-check/check/check.go:772
10:24:51   in suiteRunner.forkTest.func1
10:24:51 C:/gopath/src/github.com/docker/docker/vendor/src/github.com/go-check/check/check.go:666
10:24:51   in suiteRunner.forkCall.func1
10:24:51 c:/go/src/runtime/asm_amd64.s:1721
10:24:51   in goexit
10:24:51 
10:24:51 ----------------------------------------------------------------------

calavera · 2016-02-19T18:59:16Z

@jhowardmsft yeah, I'm doing some changes and I'll push an update shortly.

calavera · 2016-02-19T19:45:57Z

@jhowardmsft

https://jenkins.dockerproject.org/job/Docker-PRs-WoW-TP4/1250/console

19:34:50 PASS: docker_cli_ps_test.go:738: DockerSuite.TestPsShowMounts  10.944s

🎉

lowenna · 2016-02-20T02:01:56Z

@calavera Neat. LGTM from the Windows side. (I would have seen your message earlier if it wasn't for Gordon spamming my inbox and hiding 'real' messages 😈)

runcom · 2016-02-21T12:16:21Z

LGTM moving to doc review

- Allow to filter containers by volume with `--filter volume=name` and `filter volume=/dest`. - Show their names in the list with the custom format `{{ .Mounts }}`. Signed-off-by: David Calavera <david.calavera@gmail.com>

calavera · 2016-02-23T17:10:59Z

@thaJeztah, @vdemeester, @MHBauer can I have the docs reviewed here?

MHBauer · 2016-02-23T18:01:14Z

api/client/formatter/custom.go

@@ -142,6 +143,20 @@ func (c *containerContext) Label(name string) string {
 	return c.c.Labels[name]
 }

+func (c *containerContext) Mounts() string {


as a Capitalize Exported function, doesn't this need a comment? Shouldn't one of the validate passes catch this? Am I mistaken?

The function is exported because Go's template engine requires to export them, but the struct is not exported, so nobody else can really use it.

MHBauer · 2016-02-23T18:14:27Z

Only extra doc I could think of is the cli helptext, but that doesn't mention any specific filters at all. I think you're good for docs.

thaJeztah · 2016-02-23T20:50:48Z

oh! thought I reviewed this, lol.

yup, docs LGTM (thanks for the ping @calavera)

thaJeztah · 2016-02-23T22:50:18Z

@MHBauer is that a 'lgtm'?

vdemeester · 2016-02-23T23:01:17Z

LGTM 🦁

MHBauer · 2016-02-23T23:40:28Z

LGTM

thaJeztah · 2016-02-24T10:08:13Z

Janky, Experimental and WindowsTP4 is green, other CI is having lots of issues, so let's merge

Add mounts to docker ps.

GordonTheTurtle added the status/0-triage label Feb 5, 2016

cpuguy83 added status/1-design-review and removed status/0-triage labels Feb 5, 2016

calavera force-pushed the expose_volumes_in_ps branch 2 times, most recently from 77e0617 to d7450cd Compare February 5, 2016 16:31

calavera mentioned this pull request Feb 10, 2016

Expose container mounts in PS structure. docker/engine-api#85

Merged

calavera force-pushed the expose_volumes_in_ps branch from d7450cd to cf32be2 Compare February 10, 2016 23:40

thaJeztah added the status/needs-attention Calls for a collective discussion during a review session label Feb 15, 2016

thaJeztah added status/2-code-review and removed status/1-design-review status/needs-attention Calls for a collective discussion during a review session labels Feb 18, 2016

calavera force-pushed the expose_volumes_in_ps branch from cf32be2 to 864cc2f Compare February 18, 2016 22:27

calavera force-pushed the expose_volumes_in_ps branch from 864cc2f to cf50592 Compare February 18, 2016 23:12

thaJeztah added impact/api impact/changelog impact/cli labels Feb 18, 2016

duglin reviewed Feb 19, 2016
View reviewed changes

thaJeztah added this to the 1.11.0 milestone Feb 19, 2016

calavera force-pushed the expose_volumes_in_ps branch from 1cabb83 to 821f3f6 Compare February 19, 2016 19:02

calavera force-pushed the expose_volumes_in_ps branch from 821f3f6 to 5db6d74 Compare February 19, 2016 23:42

runcom added status/3-docs-review and removed status/2-code-review labels Feb 21, 2016

Add mounts to docker ps.

bd4fb00

- Allow to filter containers by volume with `--filter volume=name` and `filter volume=/dest`. - Show their names in the list with the custom format `{{ .Mounts }}`. Signed-off-by: David Calavera <david.calavera@gmail.com>

calavera force-pushed the expose_volumes_in_ps branch from 5db6d74 to bd4fb00 Compare February 23, 2016 17:10

MHBauer reviewed Feb 23, 2016
View reviewed changes

thaJeztah added status/4-merge and removed status/3-docs-review labels Feb 23, 2016

thaJeztah added a commit that referenced this pull request Feb 24, 2016

Merge pull request #20017 from calavera/expose_volumes_in_ps

034a1a8

Add mounts to docker ps.

thaJeztah merged commit 034a1a8 into moby:master Feb 24, 2016

thaJeztah mentioned this pull request Mar 15, 2016

Add the missed volume filter #21209

Merged

calavera deleted the expose_volumes_in_ps branch March 16, 2016 18:10

albers mentioned this pull request Mar 24, 2016

bash completion for docker ps -f volume #21481

Merged

sdurrheimer mentioned this pull request Mar 26, 2016

Add zsh completion for 'docker ps --filter' values #21546

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mounts to docker ps. #20017

Add mounts to docker ps. #20017

calavera commented Feb 5, 2016

thaJeztah commented Feb 15, 2016

duglin commented Feb 15, 2016

duglin commented Feb 15, 2016

thaJeztah commented Feb 17, 2016

calavera commented Feb 17, 2016

calavera commented Feb 18, 2016

thaJeztah commented Feb 18, 2016

calavera commented Feb 18, 2016

lowenna commented Feb 19, 2016

duglin Feb 19, 2016

duglin commented Feb 19, 2016

calavera commented Feb 19, 2016

duglin commented Feb 19, 2016

lowenna commented Feb 19, 2016

calavera commented Feb 19, 2016

calavera commented Feb 19, 2016

lowenna commented Feb 20, 2016

runcom commented Feb 21, 2016

calavera commented Feb 23, 2016

MHBauer Feb 23, 2016

calavera Feb 23, 2016

MHBauer commented Feb 23, 2016

thaJeztah commented Feb 23, 2016

thaJeztah commented Feb 23, 2016

vdemeester commented Feb 23, 2016

MHBauer commented Feb 23, 2016

thaJeztah commented Feb 24, 2016

Add mounts to docker ps. #20017

Add mounts to docker ps. #20017

Conversation

calavera commented Feb 5, 2016

thaJeztah commented Feb 15, 2016

duglin commented Feb 15, 2016

duglin commented Feb 15, 2016

thaJeztah commented Feb 17, 2016

calavera commented Feb 17, 2016

calavera commented Feb 18, 2016

thaJeztah commented Feb 18, 2016

calavera commented Feb 18, 2016

lowenna commented Feb 19, 2016

duglin Feb 19, 2016

Choose a reason for hiding this comment

duglin commented Feb 19, 2016

calavera commented Feb 19, 2016

duglin commented Feb 19, 2016

lowenna commented Feb 19, 2016

calavera commented Feb 19, 2016

calavera commented Feb 19, 2016

lowenna commented Feb 20, 2016

runcom commented Feb 21, 2016

calavera commented Feb 23, 2016

MHBauer Feb 23, 2016

Choose a reason for hiding this comment

calavera Feb 23, 2016

Choose a reason for hiding this comment

MHBauer commented Feb 23, 2016

thaJeztah commented Feb 23, 2016

thaJeztah commented Feb 23, 2016

vdemeester commented Feb 23, 2016

MHBauer commented Feb 23, 2016

thaJeztah commented Feb 24, 2016