Skip to content
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

[carry 22894] Adding network filter to docker ps command #23300

Merged
merged 2 commits into from Jun 7, 2016

Conversation

thaJeztah
Copy link
Member

closes #22894

- What I did

Carry of #22894, rebased, and adding filter by network id.

- How to verify it

docker run -d --net=bridge --name=onbridgenetwork busybox top
docker run -d --net=none --name=onnonenetwork busybox top

docker ps --filter network=bridge 

Output should show only the container with name "onbridgenetwork"

docker ps --filter network=bridge --filter network=none

Output should show both the containers "onbrigenetwork" and "onnonenetwork"

$ docker network inspect --format "{{.ID}}" net1
8c0b4110ae930dbe26b258de9bc34a03f98056ed6f27f991d32919bfe401d7c5

$ docker ps --filter network=8c0b4110ae930dbe26b258de9bc34a03f98056ed6f27f991d32919bfe401d7c5

Output should only show the onbridgenetwork container

- Description for the changelog

Add support for docker ps --filter network=<network-id | network-name> to filter containers by the network(s) they're connected to.

@thaJeztah
Copy link
Member Author

ping @vdemeester @cpuguy83; moved this back to code-review because of the latest changes

@thaJeztah thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Jun 6, 2016
@thaJeztah
Copy link
Member Author

thaJeztah commented Jun 6, 2016

Hm, guess Windows doesn't have "bridge" network, so I should make that dependent on platform

10:34:51 ----------------------------------------------------------------------
10:34:51 FAIL: docker_cli_ps_test.go:808: DockerSuite.TestPsListContainersFilterNetwork
10:34:51 
10:34:51 docker_cli_ps_test.go:810:
10:34:51     // create a container
10:34:51     runSleepingContainer(c, "--net=bridge", "--name=onbridgenetwork")
10:34:51 C:/gopath/src/github.com/docker/docker/pkg/integration/dockerCmd_utils.go:42:
10:34:51     c.Assert(err, check.IsNil, check.Commentf(quote+"%v"+quote+" failed with errors: %s, %v", strings.Join(args, " "), out, err))
10:34:51 ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc08247e980)} ("exit status 125")
10:34:51 ... "run -d --net=bridge --name=onbridgenetwork busybox sleep 240" failed with errors: D:\CI\CI-cbfb366\binary\docker.exe: Error response from daemon: network bridge not found.
10:34:51 See 'D:\CI\CI-cbfb366\binary\docker.exe run --help'.
10:34:51 , exit status 125

@cpuguy83
Copy link
Member

cpuguy83 commented Jun 6, 2016

Yeah, windows has a "nat" network.

@thaJeztah
Copy link
Member Author

I'll just create a custom network, then I don't have to make platform-specific changes

@thaJeztah thaJeztah force-pushed the carry-22894-ps_filter_network branch from 3dd6ebd to 0e6da8d Compare June 6, 2016 14:49
@thaJeztah thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Jun 6, 2016
@thaJeztah
Copy link
Member Author

Well, I'll be. Doesn't windows support dashes in network names?

15:27:19 FAIL: docker_cli_ps_test.go:808: DockerSuite.TestPsListContainersFilterNetwork
15:27:19 
15:27:19 docker_cli_ps_test.go:810:
15:27:19     // create a network, and some containers
15:27:19     dockerCmd(c, "network", "create", "network-a")
15:27:19 C:/gopath/src/github.com/docker/docker/pkg/integration/dockerCmd_utils.go:42:
15:27:19     c.Assert(err, check.IsNil, check.Commentf(quote+"%v"+quote+" failed with errors: %s, %v", strings.Join(args, " "), out, err))
15:27:19 ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc082400180)} ("exit status 1")
15:27:19 ... "network create network-a" failed with errors: Error response from daemon: plugin not found
15:27:19 , exit status 1
15:27:19 

@msabansal is that a known issue?

let me rename that network for now

@thaJeztah
Copy link
Member Author

Opened an issue for the Windows issue; #23314

@@ -804,3 +804,60 @@ func (s *DockerSuite) TestPsFormatSize(c *check.C) {
lines = strings.Split(out, "\n")
c.Assert(lines[8], checker.HasPrefix, "size:", check.Commentf("Size should be appended on a newline"))
}

func (s *DockerSuite) TestPsListContainersFilterNetwork(c *check.C) {
// create a network, and some containers
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a TODO and a requires DaemonIsLinux on that then 👼

Copy link
Member Author

Choose a reason for hiding this comment

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

Or make it conditional, and filter on "nat" for windows. Didn't come round to do this yet, will try to do so later today

Signed-off-by: Sainath Grandhi <sainath.grandhi@intel.com>
@thaJeztah thaJeztah force-pushed the carry-22894-ps_filter_network branch from 7b2dfa6 to e675f3f Compare June 7, 2016 14:49
This adds support for filtering by network ID, to be
consistent with other filter options.

Note that only *full* matches are returned; this is
consistent with other filters (e.g. volume), that
also return full matches only.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the carry-22894-ps_filter_network branch from e675f3f to 7c46ba0 Compare June 7, 2016 14:50
@thaJeztah
Copy link
Member Author

@vdemeester disabled the test for now on Windows, and added a TODO

@vdemeester
Copy link
Member

LGTM 🐮

@cpuguy83
Copy link
Member

cpuguy83 commented Jun 7, 2016

LGTM

@cpuguy83
Copy link
Member

cpuguy83 commented Jun 7, 2016

And win2lin finished but didn't notify. Merging.

@cpuguy83
Copy link
Member

cpuguy83 commented Jun 7, 2016

Actually, let me set it to docs-review :)

@vdemeester
Copy link
Member

vdemeester commented Jun 7, 2016

docs LGTM 🐮

@thaJeztah
Copy link
Member Author

author LGTM

@thaJeztah thaJeztah merged commit 06a204c into moby:master Jun 7, 2016
@thaJeztah thaJeztah deleted the carry-22894-ps_filter_network branch June 7, 2016 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants