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

Add `--filter scope=swarm|local` for `docker network ls` #31529

Merged
merged 1 commit into from Mar 24, 2017

Conversation

@yongtang
Copy link
Member

commented Mar 3, 2017

- What I did

This fix tries to address the request in #31324 by adding --filter scope=swarm|local for docker network ls.

As docker network ls has a SCOPE column by default, it is natural to add the support of --filter scope=swarm|local.

- How I did it

This fix adds the scope=swarm|local support for docker network ls --filter.

Related docs has been updated.

- How to verify it

Additional unit test cases have been added.

- Description for the changelog

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

maxresdefault

This fix fixes #31324.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@allencloud

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2017

I think we still need to update swagger.yml. WDYT? @yongtang

@yongtang yongtang force-pushed the yongtang:31324-network-ls-filter-scope branch from a6fa0ea to 7744fbb Mar 5, 2017
@yongtang

This comment has been minimized.

Copy link
Member Author

commented Mar 5, 2017

Thanks @allencloud. The PR has been updated with changes in swagger.yaml added.

@yongtang

This comment has been minimized.

Copy link
Member Author

commented Mar 5, 2017

@allencloud The PR has been updated. Thanks.

@@ -6239,6 +6239,7 @@ paths:
- `label=<key>` or `label=<key>=<value>` of a network label.
- `name=<network-name>` Matches all or part of a network name.
- `type=["custom"|"builtin"]` Filters networks by type. The `custom` keyword returns all user-defined networks.
- `scope=["swarm"|"local"]` Filters networks by scope (either `swarm` or `local`).

This comment has been minimized.

Copy link
@allencloud

allencloud Mar 6, 2017

Contributor

I prefer to add scope before type since out of alphabetical order, WDYT?

@yongtang yongtang force-pushed the yongtang:31324-network-ls-filter-scope branch from 7744fbb to 49753b7 Mar 6, 2017
@yongtang

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2017

Thanks @allencloud. The swagger.yaml, docs, and man have been updated.

Copy link
Member

left a comment

design LGTM

ic6r88twuu92 swarmnet overlay swarm
```

The following example matches networks with the `swarm` scope:

This comment has been minimized.

Copy link
@allencloud

allencloud Mar 7, 2017

Contributor

I think this should be local scope. ^^

This comment has been minimized.

Copy link
@yongtang

yongtang Mar 7, 2017

Author Member

@allencloud My bad. The PR has been updated. Thanks.

ic6r88twuu92 swarmnet overlay swarm
```

The following example matches networks with the `swarm` scope:

This comment has been minimized.

Copy link
@allencloud

allencloud Mar 7, 2017

Contributor

I think this should be local scope. ^^

This comment has been minimized.

Copy link
@yongtang

yongtang Mar 7, 2017

Author Member

Done. Thanks.

@yongtang yongtang force-pushed the yongtang:31324-network-ls-filter-scope branch from 49753b7 to 46593c3 Mar 7, 2017
@thaJeztah

This comment has been minimized.

Copy link
Member

commented Mar 22, 2017

Design, and code looks good to me, but needs a rebase @yongtang 😅

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Mar 22, 2017

Don't forget to add a mention to the API history; https://github.com/docker/docker/blob/master/docs/api/version-history.md

@@ -6238,6 +6238,7 @@ paths:
- `id=<network-id>` Matches all or part of a network ID.
- `label=<key>` or `label=<key>=<value>` of a network label.
- `name=<network-name>` Matches all or part of a network name.
- `scope=["swarm"|"local"]` Filters networks by scope (either `swarm` or `local`).

This comment has been minimized.

Copy link
@cpuguy83

cpuguy83 Mar 22, 2017

Contributor

There is also global I think?

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Mar 22, 2017

Member

hm, is there 🤔

This comment has been minimized.

Copy link
@yongtang

yongtang Mar 23, 2017

Author Member

Thanks @cpuguy83 @thaJeztah. Yes I forgot libnetwork have the legacy external K-V store mode (global).

The PR has been updated and rebased. Please take a look.

@yongtang yongtang force-pushed the yongtang:31324-network-ls-filter-scope branch from 46593c3 to d93a710 Mar 23, 2017
Copy link
Contributor

left a comment

LGTM

@@ -29,6 +29,7 @@ keywords: "API, Docker, rcli, REST, documentation"
* `POST /services/create` and `POST /services/(id or name)/update` now accept a `rollback` value for `FailureAction`.
* `POST /services/create` and `POST /services/(id or name)/update` now accept an optional `RollbackConfig` object which specifies rollback options.
* `GET /services` now supports a `mode` filter to filter services based on the service mode (either `global` or `replicated`).
* `GET /networks/` now supports a `scope` filter to filter networks based on the network mode (`swarm`, `global`, or `local`).

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Mar 23, 2017

Member

I think this should go under API 1.29 now - we had to bump the API version for 17.03.1

This comment has been minimized.

Copy link
@yongtang

yongtang Mar 24, 2017

Author Member

Thanks @thaJeztah. The PR has been updated with version changed.

This fix tries to address the request in 31324 by adding
`--filter scope=swarm|local` for `docker network ls`.

As `docker network ls` has a `SCOPE` column by default,
it is natural to add the support of `--filter scope=swarm|local`.

This fix adds the `scope=swarm|local` support for
`docker network ls --filter`.

Related docs has been updated.

Additional unit test cases have been added.

This fix fixes 31324.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the yongtang:31324-network-ls-filter-scope branch from d93a710 to 704ea8f Mar 24, 2017
Copy link
Member

left a comment

LGTM 🐮
/cc @thaJeztah for docs-review 👼

Copy link
Member

left a comment

LGTM! thanks!

@thaJeztah

This comment has been minimized.

Copy link
Member

commented Mar 24, 2017

windows completed but failed to notify github

java.net.SocketTimeoutException: connect timed out
Finished: SUCCESS
@thaJeztah thaJeztah merged commit 4e290f7 into moby:master Mar 24, 2017
5 of 6 checks passed
5 of 6 checks passed
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 11704 is running
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 32006 has succeeded
Details
janky Jenkins build Docker-PRs 40628 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 751 has succeeded
Details
z Jenkins build Docker-PRs-s390x 615 has succeeded
Details
@GordonTheTurtle GordonTheTurtle added this to the 17.05.0 milestone Mar 24, 2017
@albers

This comment has been minimized.

Copy link
Member

commented Mar 24, 2017

Thanks @yongtang!

@yongtang yongtang deleted the yongtang:31324-network-ls-filter-scope branch Mar 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.