Add `publish` and `expose` filter for `docker ps --filter` by yongtang · Pull Request #27557 · moby/moby

yongtang · 2016-10-19T19:11:18Z

- What I did

This fix tries to address the enhancement proposal raised in #27178 for filtering based on published or exposed ports of docker ps --filter.

Note: this PR didn't implement the wildcard match initially, as protocol (tcp/udp) has to be taken into consideration. But is open to suggestions.

- How I did it

In this fix, two filter options, publish and expose have been added to take either <port>[/<protocol>] or <from>-<to>[/<protocol>] and filtering on containers.

- How to verify it

An integration test has been added to cover the changes.

- Description for the changelog

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

This fix fixes #27178.

Signed-off-by: Yong Tang [email protected]

vdemeester · 2016-10-20T12:13:34Z

Design SGTM 👼

LK4D4 · 2016-11-03T23:48:44Z

daemon/list.go

I wonder why it's never returning error

Thanks @LK4D4. I wasn't sure initially as some of the other filters didn't return the error. The PR has been updated and now expose and publish will return error.

LK4D4 · 2016-11-03T23:49:23Z

@yongtang would you mind to rebase? Thanks!

yongtang · 2016-11-04T00:39:40Z

Thanks @LK4D4 for the review. The PR has been updated and rebased. Please take a look and let me know if there are any issues.

mlaventure · 2016-12-01T18:22:45Z

integration-cli/docker_cli_ps_test.go

nit: you could add -q and use checker.Equals instead

Thanks @mlaventure. Done.

mlaventure

LGTM with inconsequential nit

mlaventure · 2016-12-01T18:23:41Z

ping @LK4D4

yongtang · 2016-12-01T19:05:42Z

Thanks all for the review. The PR has been updated. Please take a look and let me know if there are any other issues.

vdemeester

SGTM but I feel the logrus.Errorf shouldn't be there 👼

vdemeester · 2017-01-12T17:06:23Z

daemon/list.go

@yongtang debug message 😅

My bad. How could I have missed this one? 😳

yongtang · 2017-01-13T01:56:50Z

Thanks @vdemeester. The PR has been updated.

vdemeester

LGTM 🐸
/cc @thaJeztah for docs review 👼

mlaventure · 2017-01-17T21:45:44Z

ping @thaJeztah

AkihiroSuda · 2017-01-19T08:44:58Z

docs/reference/commandline/ps.md

port is 80, protocol is tcp

@AkihiroSuda The PR has been updated. Thanks.

LK4D4 · 2017-01-27T21:22:45Z

@yongtang needs rebase
@thaJeztah for docs-review

This fix tries to address the enhancement proposal raised in 27178 for filtering based on published or exposed ports of `docker ps --filter`. In this fix, two filter options, `publish` and `expose` have been added to take either `<port>[/<protocol>]` or `<from>-<to>[/<protocol>]` and filtering on containers. An integration test has been added to cover the changes. This fix fixes 27178. Signed-off-by: Yong Tang <[email protected]>

yongtang · 2017-01-27T21:26:36Z

@LK4D4 @thaJeztah The PR has been rebased. Thanks.

vdemeester

docs LGTM
/cc @thaJeztah for revisit 👼

thaJeztah · 2017-02-02T00:53:57Z

@yongtang can you;

add a mention of the new filters that are supported in the API history; https://github.com/docker/docker/blob/master/docs/api/version-history.md#v126-api-changes
update the man-pages accordingly?

yongtang · 2017-02-02T03:12:17Z

Thanks @thaJeztah. A PR #30661 has been created for the API history and man page. Please take a look.

This fix updates API history and man page for `docker ps --filter expose/publish`, from the feedback: moby#27557 (comment) Signed-off-by: Yong Tang <[email protected]>

This fix updates API history and man page for `docker ps --filter expose/publish`, from the feedback: moby/moby#27557 (comment) Signed-off-by: Yong Tang <[email protected]>

This fix updates API history and man page for `docker ps --filter expose/publish`, from the feedback: moby/moby#27557 (comment) Signed-off-by: Yong Tang <[email protected]> Upstream-commit: d77db0bd9ac80a3a2e7ad4d90b6aa33fe4fc0462 Component: cli

This fix updates API history and man page for `docker ps --filter expose/publish`, from the feedback: moby/moby#27557 (comment) Signed-off-by: Yong Tang <[email protected]>

Tusenka · 2020-01-24T07:06:15Z

It doesn't work for me( I can filter only port to, not from

thaJeztah · 2020-01-24T09:58:13Z

Yes, something looks to be off.

Create some containers;

docker run -d --name test_no_ports nginx:alpine
docker run -d --name test_port_1080 -p 1080:80 nginx:alpine
docker run -d --name test_port_1090 -p 1090:80 nginx:alpine
docker run -d --name test_port_80_random -p 80 nginx:alpine
docker run -d --name test_port_all_random -P nginx:alpine


docker ps --filter name=test_

CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS                   NAMES
3ce8232cdd04        nginx:alpine        "nginx -g 'daemon of…"   6 seconds ago       Up 6 seconds        0.0.0.0:32775->80/tcp   test_port_all_random
7f8fad7a0eb9        nginx:alpine        "nginx -g 'daemon of…"   7 seconds ago       Up 6 seconds        0.0.0.0:32774->80/tcp   test_port_80_random
3870ccc2f8f7        nginx:alpine        "nginx -g 'daemon of…"   7 seconds ago       Up 6 seconds        0.0.0.0:1090->80/tcp    test_port_1090
aa565211513e        nginx:alpine        "nginx -g 'daemon of…"   8 seconds ago       Up 7 seconds        0.0.0.0:1080->80/tcp    test_port_1080
c4a1c6e0e99b        nginx:alpine        "nginx -g 'daemon of…"   8 seconds ago       Up 7 seconds        80/tcp                  test_no_ports

Filtering on the "exposed" port works:

docker ps --filter name=test_ --filter expose=80

CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS                   NAMES
3ce8232cdd04        nginx:alpine        "nginx -g 'daemon of…"   20 seconds ago      Up 19 seconds       0.0.0.0:32775->80/tcp   test_port_all_random
7f8fad7a0eb9        nginx:alpine        "nginx -g 'daemon of…"   21 seconds ago      Up 20 seconds       0.0.0.0:32774->80/tcp   test_port_80_random
3870ccc2f8f7        nginx:alpine        "nginx -g 'daemon of…"   21 seconds ago      Up 20 seconds       0.0.0.0:1090->80/tcp    test_port_1090
aa565211513e        nginx:alpine        "nginx -g 'daemon of…"   22 seconds ago      Up 20 seconds       0.0.0.0:1080->80/tcp    test_port_1080
c4a1c6e0e99b        nginx:alpine        "nginx -g 'daemon of…"   22 seconds ago      Up 21 seconds       80/tcp                  test_no_ports

docker ps --filter name=test_ --filter expose=90
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES

Filtering on the "published" port doesn't (trying some variations below):

docker ps --filter name=test_ --filter publish=1080
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS                 NAME


docker ps --filter name=test_ --filter publish=1080/tcp
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS                 NAMES

docker ps --filter name=test_ --filter publish=1080/udp
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS                 NAMES

However, using the "exposed" port number instead of the "published" port shows some results:

docker ps --filter name=test_ --filter publish=80
CONTAINER ID        IMAGE               COMMAND                  CREATED              STATUS              PORTS                   NAMES
7f8fad7a0eb9        nginx:alpine        "nginx -g 'daemon of…"   About a minute ago   Up About a minute   0.0.0.0:32774->80/tcp   test_port_80_random
3870ccc2f8f7        nginx:alpine        "nginx -g 'daemon of…"   About a minute ago   Up About a minute   0.0.0.0:1090->80/tcp    test_port_1090
aa565211513e        nginx:alpine        "nginx -g 'daemon of…"   About a minute ago   Up About a minute   0.0.0.0:1080->80/tcp    test_port_1080

It appears it's filtering on the wrong value; also, the container with "random" port-mapping for all ports is missing in this case (perhaps it's filtering on ports that were explicitly configured to be published?)

Note that the TestPsListContainersFilterPorts test-case doesn't appear to catch this issue because it appears to be filtering on port 80 (which is published to a random port);

docker run -d --publish 80 busybox top
537c87330e660cc4d7a7426dcddd771a3b1bb3f7c21c7750a4b1a3358e976b1c

docker ps --filter publish=80
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS                   NAMES
537c87330e66        busybox             "top"                    19 seconds ago      Up 18 seconds       0.0.0.0:32776->80/tcp   sleepy_khorana
...

thaJeztah · 2020-01-24T10:01:32Z

opened #40405

Reuse existing structures and rely on json serialization to deep copy Container objects. Also consolidate all "save" operations on container.CheckpointTo, which now both saves a serialized json to disk, and replicates state to the ACID in-memory store. Signed-off-by: Fabio Kung <[email protected]> (cherry picked from commit edad527) Signed-off-by: Kir Kolyshkin <[email protected]> Conflicts: - daemon/container_operations.go: missing moby#30117 - daemon/daemon.go: missing moby#32821 - daemon/list.go: missing moby#27557, moby#33241 etc.

GordonTheTurtle added the status/0-triage label Oct 19, 2016

yongtang mentioned this pull request Oct 19, 2016

Feature request for docker --filter : ability to search by PORT number, exposed or published and all published #27178

Closed

vdemeester added status/1-design-review and removed status/0-triage labels Oct 20, 2016

LK4D4 reviewed Nov 3, 2016

View reviewed changes

LK4D4 added status/2-code-review and removed status/1-design-review labels Nov 3, 2016

yongtang force-pushed the 27178-ps-filter-publish-expose branch 2 times, most recently from 143e04b to 07ccfde Compare November 4, 2016 00:19

yongtang force-pushed the 27178-ps-filter-publish-expose branch from 07ccfde to 902335e Compare November 8, 2016 03:42

mlaventure reviewed Dec 1, 2016

View reviewed changes

mlaventure approved these changes Dec 1, 2016

View reviewed changes

yongtang force-pushed the 27178-ps-filter-publish-expose branch from 902335e to 91e1432 Compare December 1, 2016 19:04

vdemeester added this to the 1.14.0 milestone Dec 2, 2016

vdemeester suggested changes Jan 12, 2017

View reviewed changes

yongtang force-pushed the 27178-ps-filter-publish-expose branch from 91e1432 to a548a44 Compare January 13, 2017 01:56

yongtang force-pushed the 27178-ps-filter-publish-expose branch from a548a44 to 3257ff7 Compare January 13, 2017 04:14

vdemeester approved these changes Jan 13, 2017

View reviewed changes

vdemeester added the status/3-docs-review label Jan 13, 2017

AkihiroSuda removed the status/2-code-review label Jan 19, 2017

AkihiroSuda reviewed Jan 19, 2017

View reviewed changes

yongtang force-pushed the 27178-ps-filter-publish-expose branch from 3257ff7 to 4ba69bd Compare January 19, 2017 11:08

yongtang force-pushed the 27178-ps-filter-publish-expose branch from 4ba69bd to 743943f Compare January 27, 2017 21:26

vdemeester approved these changes Feb 1, 2017

View reviewed changes

vdemeester merged commit 1b6a15e into moby:master Feb 1, 2017

vdemeester assigned thaJeztah Feb 1, 2017

vdemeester added the docs/revisit label Feb 1, 2017

yongtang deleted the 27178-ps-filter-publish-expose branch February 1, 2017 16:03

albers mentioned this pull request Feb 1, 2017

Add bash completion for docker ps --filter expose|publish #30638

Merged

thaJeztah added impact/api impact/changelog impact/cli labels Feb 2, 2017

yongtang mentioned this pull request Feb 2, 2017

Update API history and man page for --filter expose/publish #30661

Merged

sdurrheimer mentioned this pull request Feb 12, 2017

Add zsh completion for 'docker {ps,service ls} --filter expose|publish' #30949

Merged

thaJeztah mentioned this pull request Jan 24, 2020

docker ps --filter publish=... filters on wrong values #40405

Closed

kolyshkin mentioned this pull request May 2, 2020

backport: No container locks on docker ps projectatomic/docker#375

Merged

Conversation

yongtang commented Oct 19, 2016

Uh oh!

vdemeester commented Oct 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LK4D4 commented Nov 3, 2016

Uh oh!

yongtang commented Nov 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlaventure left a comment

Choose a reason for hiding this comment

Uh oh!

mlaventure commented Dec 1, 2016

Uh oh!

yongtang commented Dec 1, 2016

Uh oh!

vdemeester left a comment

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!

yongtang commented Jan 13, 2017

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

mlaventure commented Jan 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LK4D4 commented Jan 27, 2017

Uh oh!

yongtang commented Jan 27, 2017

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Feb 2, 2017

Uh oh!

yongtang commented Feb 2, 2017

Uh oh!

Tusenka commented Jan 24, 2020

Uh oh!

thaJeztah commented Jan 24, 2020

Uh oh!

thaJeztah commented Jan 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants