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

feature: fullfill network scope field in network ls and inspect #2532

Merged
merged 1 commit into from Dec 6, 2018

Conversation

mathspanda
Copy link
Contributor

@mathspanda mathspanda commented Dec 5, 2018

Signed-off-by: mathspanda mathspanda826@gmail.com

Ⅰ. Describe what this PR did

Fullfill scope value returned in pouch network list/inpsect.

Ⅱ. Does this pull request fix one issue?

issue #2501

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Added.

Ⅳ. Describe how to verify it

root@lin-dev-vm2:~# pouch network list
NETWORK ID   NAME     DRIVER   SCOPE
058fce03b8   none     null     local
1bae6b8af5   bridge   bridge   local
d8684bf988   host     host     local
root@lin-dev-vm2:~# pouch network inspect 1bae6b8af5
[
    {
        "Driver": "bridge",
        "IPAM": {
            "Config": [
                {
                    "Gateway": "192.168.5.1",
                    "IPRange": "192.168.5.0/24",
                    "Subnet": "192.168.5.0/24"
                }
            ],
            "Driver": "default"
        },
        "Id": "1bae6b8af52ebd87b4f8a4a7b8b08ec6a096b0acef564f2bf27560ba4baa82da",
        "Name": "bridge",
        "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": "p0",
            "com.docker.network.driver.mtu": "1500"
        },
        "Scope": "local"
    }
]

Ⅴ. Special notes for reviews

None

@codecov
Copy link

codecov bot commented Dec 5, 2018

Codecov Report

Merging #2532 into master will decrease coverage by 0.04%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2532      +/-   ##
==========================================
- Coverage    69.2%   69.15%   -0.05%     
==========================================
  Files         278      278              
  Lines       18494    18508      +14     
==========================================
+ Hits        12798    12799       +1     
- Misses       4243     4246       +3     
- Partials     1453     1463      +10
Flag Coverage Δ
#criv1alpha1test 31.25% <0%> (-0.08%) ⬇️
#criv1alpha2test 35.51% <0%> (+0.23%) ⬆️
#integrationtest 40.57% <83.33%> (+0.03%) ⬆️
#nodee2etest 32.6% <0%> (-0.19%) ⬇️
#unittest 26.88% <0%> (-0.03%) ⬇️
Impacted Files Coverage Δ
apis/server/network_bridge.go 66.12% <83.33%> (+2.49%) ⬆️
cri/ocicni/cni_manager.go 58.82% <0%> (-11.77%) ⬇️
apis/server/utils.go 71.15% <0%> (-3.85%) ⬇️
pkg/meta/store.go 67.44% <0%> (-1.56%) ⬇️
daemon/mgr/container_utils.go 85.44% <0%> (-1.27%) ⬇️
daemon/mgr/container.go 58.69% <0%> (-0.65%) ⬇️
cri/v1alpha2/cri.go 69.55% <0%> (-0.38%) ⬇️
cri/v1alpha1/cri.go 60.59% <0%> (-0.34%) ⬇️
cri/v1alpha2/cri_wrapper.go 63.6% <0%> (ø) ⬆️
ctrd/container.go 58.41% <0%> (+0.39%) ⬆️
... and 2 more

@rudyfly
Copy link
Collaborator

rudyfly commented Dec 5, 2018

UT failed, you can rebase master, and rebuild ci

@allencloud
Copy link
Collaborator

Actually I do not think it is the UT test failure:

go: disabling cache (/home/travis/.cache/go-build) due to initialization failure: open /home/travis/.cache/go-build/log.txt: permission denied

It seems to be travis CI 's own issue. @mathspanda @rudyfly

I think we could ignore this.

fullfill scope value returned in pouch network list/inpsect

Signed-off-by: mathspanda <mathspanda826@gmail.com>
@allencloud allencloud changed the title feature: fullfill network scope field feature: fullfill network scope field in network ls and inspect Dec 5, 2018
@@ -48,10 +48,12 @@ func (suite *PouchNetworkSuite) TestNetworkInspectFormat(c *check.C) {
c.Errorf("failed to decode inspect output: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

just confirm, no network ls corresponding cli test case added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap. I haven't seen any cli test about network ls. Maybe needed if network ls support some flags.

@rudyfly
Copy link
Collaborator

rudyfly commented Dec 6, 2018

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Dec 6, 2018
@Ace-Tang Ace-Tang merged commit b8ce8e4 into AliyunContainerService:master Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/network kind/feature LGTM one maintainer or community participant agrees to merge the pull reuqest. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants