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

Fixing network inspect for swarm #37045

Merged
merged 1 commit into from
May 14, 2018
Merged

Fixing network inspect for swarm #37045

merged 1 commit into from
May 14, 2018

Conversation

abhi
Copy link
Contributor

@abhi abhi commented May 11, 2018

Signed-off-by: Abhinandan Prativadi abhi@docker.com

docker network inspect of a swarm network on a manager node was not displaying the subnet information unless there was a task in that network on manager node.

- What I did
Fixed the GRPC conversion function
- How I did it

- How to verify it

$ docker network inspect net1
[
    {
        "Name": "net1",
        "Id": "6k4f6lg5xr34pndh33ikc9cwo",
        "Created": "2018-05-11T21:04:52.744708462Z",
        "Scope": "swarm",
        "Driver": "overlay",
        "EnableIPv6": false,
        "IPAM": {
            "Driver": "default",
            "Options": null,
            "Config": [
                {
                    "Subnet": "10.0.4.0/24",
                    "Gateway": "10.0.4.1"
                }
            ]
        },
        "Internal": false,
        "Attachable": false,
        "Ingress": false,
        "ConfigFrom": {
            "Network": ""
        },
        "ConfigOnly": false,
        "Containers": null,
        "Options": {
            "com.docker.network.driver.overlay.vxlanid_list": "4108"
        },
        "Labels": null

- Description for the changelog
Fix network inspect for overlay network

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

@thaJeztah
Copy link
Member

thaJeztah commented May 11, 2018

Perhaps the NetworkInspect test should be updated for this?

func TestInspectNetwork(t *testing.T) {

(or a new one added without containers attached)

@abhi
Copy link
Contributor Author

abhi commented May 11, 2018

@thaJeztah thanks. I added a check in there.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@selansen
Copy link
Contributor

FAiled at:
23:34:00 integration/network/inspect_test.go:1::warning: file is not gofmted with -s (gofmt)
23:34:00 integration/network/inspect_test.go:1::warning: file is not goimported (goimports)
23:34:01 Build step 'Execute shell' marked build as failure

}
}
return false

if network.IPAM.Config == nil {
Copy link
Member

Choose a reason for hiding this comment

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

Ah, looks like this is indented with spaces, likely this is where gofmt is complaining about

Signed-off-by: Abhinandan Prativadi <abhi@docker.com>
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

@codecov
Copy link

codecov bot commented May 14, 2018

Codecov Report

Merging #37045 into master will decrease coverage by 0.27%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #37045      +/-   ##
==========================================
- Coverage   35.37%   35.09%   -0.28%     
==========================================
  Files         615      615              
  Lines       45793    45793              
==========================================
- Hits        16198    16070     -128     
- Misses      27449    27614     +165     
+ Partials     2146     2109      -37

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.

5 participants