close
Skip to content

Include container names in network inspect#17615

Merged
LK4D4 merged 1 commit into
moby:masterfrom
WeiZhang555:17404-net-inspect-name
Nov 13, 2015
Merged

Include container names in network inspect#17615
LK4D4 merged 1 commit into
moby:masterfrom
WeiZhang555:17404-net-inspect-name

Conversation

@WeiZhang555

Copy link
Copy Markdown
Contributor

Fixes #17404

This commit makes docker network inspect print container names as
service discovery is based on container name.

Signed-off-by: Zhang Wei zhangwei555@huawei.com

Test cases are still on working. :)

@cpuguy83

cpuguy83 commented Nov 2, 2015

Copy link
Copy Markdown
Member

+1 to the container name here.

@WeiZhang555

Copy link
Copy Markdown
Contributor Author

And the final look of network inspect output:

$ docker network inspect bridge
[
    {
        "Name": "bridge",
        "Id": "e3a9b89b976b56bedf7c75fbbd704702e7f74779af6982b7b5f561666c27cb28",
        "Scope": "local",
        "Driver": "bridge",
        "IPAM": {
            "Driver": "default",
            "Config": [
                {
                    "Subnet": "172.17.0.1/16",
                    "Gateway": "172.17.0.1"
                }
            ]
        },
        "Containers": {
            "0b84bd9c71eade6dd1c4cc6513975e0b361fa10fe4fccd0d071c0a3de49d5521": {
                "Name": "amazing_bhabha",
                "EndpointID": "614560e0fd66aa7012eb38bd840d80fd7f0e8c94b80919466bb175539be28ddb",
                "MacAddress": "02:42:ac:11:00:02",
                "IPv4Address": "172.17.0.2/16",
                "IPv6Address": ""
            },
            "421a41f7402fe55cc327bc24a7686b5a8b29e13421dac07741436abdf26d7e44": {
                "Name": "mad_mclean",
                "EndpointID": "29a20fa20c5cd066e72d0db1b5c6f835cbd74ab90bfb50cc3aac63edf60fab4f",
                "MacAddress": "02:42:ac:11:00:03",
                "IPv4Address": "172.17.0.3/16",
                "IPv6Address": ""
            }
        },
        "Options": {
            "com.docker.network.bridge.default_bridge": "true",
            "com.docker.network.bridge.enable_icc": "true",
            "com.docker.network.bridge.enable_ip_masquerade": "true",
            "com.docker.network.bridge.host_binding_ipv4": "0.0.0.0",
            "com.docker.network.bridge.name": "docker0",
            "com.docker.network.driver.mtu": "1500"
        }
    }
]

@cpuguy83

cpuguy83 commented Nov 2, 2015

Copy link
Copy Markdown
Member

The one thing to think about is how aliasing will be handled on networks, and how it might be displayed in the inspect output in addition to the name.

ping @mavenugo

@thaJeztah

Copy link
Copy Markdown
Member

Nice! Thanks for picking this up. If this is agreed upon, do we need an integration test to check if it follows container rename? (Having a test may also prevent regressions)

@WeiZhang555

Copy link
Copy Markdown
Contributor Author

@thaJeztah @cpuguy83
Yep, integration test to check container rename is absolutely needed, I'm working on it.
For now it works fine for docker rename as I can see.
Thank you for help reviewing. :)

@WeiZhang555 WeiZhang555 force-pushed the 17404-net-inspect-name branch from fd9d451 to 97094ad Compare November 3, 2015 03:16
@WeiZhang555

Copy link
Copy Markdown
Contributor Author

Test cases are updated. 😄

@albers

albers commented Nov 3, 2015

Copy link
Copy Markdown
Member

Yes, please!
This feature would be very helpful for the bash completion of docker network disconnect (WIP), where I need a list of container names and their IDs that are connected to a specific network.
This would save me an additional per container lookup for the name.

@thaJeztah

Copy link
Copy Markdown
Member

Test cases are updated. 😄

Awesome!

@albers yes, I can definitely see that will make it easier. Unfortunately, this won't make it into 1.9.0 (too late for that)

@thaJeztah

Copy link
Copy Markdown
Member

ping @tiborvass do you think this can be considered for 1.9.1? I don't think this breaks backward compatibility, but it is a change in the API, so not sure we want to introduce that in a x.x.1 update

@calavera

calavera commented Nov 4, 2015

Copy link
Copy Markdown
Contributor

We don't want to add this in a X.X.1 release, since it's an improvement.

LGTM though. Maybe we should hear from @mavenugo, @mrjana and @aboch to make sure there are no further implications we've not considered.

@WeiZhang555 WeiZhang555 force-pushed the 17404-net-inspect-name branch from 97094ad to 35c3b68 Compare November 5, 2015 04:04
@WeiZhang555

Copy link
Copy Markdown
Contributor Author

Refresh and rebased. 😄

@thaJeztah

Copy link
Copy Markdown
Member

ping @mrjana @aboch could you give this a quick look?

(thanks for rebasing @WeiZhang555 !)

This commit makes `docker network inspect` print container names as
service discovery is based on container name.

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
@LK4D4

LK4D4 commented Nov 13, 2015

Copy link
Copy Markdown
Contributor

LGTM

LK4D4 added a commit that referenced this pull request Nov 13, 2015
Include container names in `network inspect`
@LK4D4 LK4D4 merged commit a4acb1d into moby:master Nov 13, 2015
@thaJeztah

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah added this to the 1.10 milestone Nov 14, 2015
@WeiZhang555

Copy link
Copy Markdown
Contributor Author

@thaJeztah
No problem, I'll do it ASAP :)

@thaJeztah

Copy link
Copy Markdown
Member

Thanks!

On 14 Nov 2015, at 13:47, zhangwei_cs notifications@github.com wrote:

@thaJeztah
No problem, I'll do it ASAP :)


Reply to this email directly or view it on GitHub.

@albers

albers commented Nov 15, 2015

Copy link
Copy Markdown
Member

@WeiZhang555 Thanks for the feature, I just used it in #17997.

@WeiZhang555

Copy link
Copy Markdown
Contributor Author

@albers I'm happy to hear that the feature is useful and meaningful, and glad to help! :)

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.

7 participants