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

Add `publish` and `expose` filter for `docker ps --filter` #27557

Merged
merged 1 commit into from Feb 1, 2017

Conversation

@yongtang
Member

yongtang commented Oct 19, 2016

- 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 yong.tang.github@outlook.com

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester

vdemeester Oct 20, 2016

Member

Design SGTM 👼

Member

vdemeester commented Oct 20, 2016

Design SGTM 👼

Show outdated Hide outdated daemon/list.go Outdated
@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Nov 3, 2016

Contributor

@yongtang would you mind to rebase? Thanks!

Contributor

LK4D4 commented Nov 3, 2016

@yongtang would you mind to rebase? Thanks!

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Nov 4, 2016

Member

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.

Member

yongtang commented Nov 4, 2016

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

LGTM with inconsequential nit

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure

mlaventure Dec 1, 2016

Contributor

ping @LK4D4

Contributor

mlaventure commented Dec 1, 2016

ping @LK4D4

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Dec 1, 2016

Member

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

Member

yongtang commented Dec 1, 2016

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 vdemeester added this to the 1.14.0 milestone Dec 2, 2016

@vdemeester

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

Show outdated Hide outdated daemon/list.go Outdated
@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jan 13, 2017

Member

Thanks @vdemeester. The PR has been updated.

Member

yongtang commented Jan 13, 2017

Thanks @vdemeester. The PR has been updated.

@vdemeester

LGTM 🐸
/cc @thaJeztah for docs review 👼

@mlaventure

This comment has been minimized.

Show comment
Hide comment
@mlaventure
Contributor

mlaventure commented Jan 17, 2017

ping @thaJeztah

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Jan 27, 2017

Contributor

@yongtang needs rebase
@thaJeztah for docs-review

Contributor

LK4D4 commented Jan 27, 2017

@yongtang needs rebase
@thaJeztah for docs-review

Add `publish` and `expose` filter for `docker ps --filter`
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 <yong.tang.github@outlook.com>
@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Jan 27, 2017

Member

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

Member

yongtang commented Jan 27, 2017

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

@vdemeester

docs LGTM
/cc @thaJeztah for revisit 👼

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

4 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 30056 has succeeded
Details
janky Jenkins build Docker-PRs 38669 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 9695 has succeeded
Details

@yongtang yongtang deleted the yongtang:27178-ps-filter-publish-expose branch Feb 1, 2017

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Feb 2, 2017

Member

@yongtang can you;

Member

thaJeztah commented Feb 2, 2017

@yongtang can you;

yongtang added a commit to yongtang/docker that referenced this pull request Feb 2, 2017

Update API history and man page for `--filter expose/publish`
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 <yong.tang.github@outlook.com>
@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Feb 2, 2017

Member

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

Member

yongtang commented Feb 2, 2017

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

yongtang added a commit to yongtang/docker that referenced this pull request Feb 2, 2017

Update API history and man page for `--filter expose/publish`
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 <yong.tang.github@outlook.com>

yongtang added a commit to yongtang/docker that referenced this pull request Feb 7, 2017

Update API history and man page for `--filter expose/publish`
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 <yong.tang.github@outlook.com>

tiborvass added a commit to tiborvass/cli that referenced this pull request May 11, 2017

Update API history and man page for `--filter expose/publish`
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 <yong.tang.github@outlook.com>

andrewhsu pushed a commit to docker/docker-ce that referenced this pull request Jun 5, 2017

Update API history and man page for `--filter expose/publish`
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 <yong.tang.github@outlook.com>
Upstream-commit: d77db0bd9ac80a3a2e7ad4d90b6aa33fe4fc0462
Component: cli

dnephin pushed a commit to dnephin/cli that referenced this pull request Jun 14, 2017

Update API history and man page for `--filter expose/publish`
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 <yong.tang.github@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment