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

enhance: display disconnect result for pouch network disconnect #1590

Conversation

shaloulcy
Copy link
Contributor

@shaloulcy shaloulcy commented Jun 24, 2018

Signed-off-by: Eric Li lcy041536@gmail.com

Ⅰ. Describe what this PR did

enhance: display disconnect result for pouch network disconnect

Now If we disconnect a container from a network, it will display nothing. This PR will print the success message like

container test is disconnected from network bridge

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Describe how you did it

NONE

Ⅳ. Describe how to verify it

if pouch network disconnect successfully, it will print success

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Jun 24, 2018

Codecov Report

Merging #1590 into master will decrease coverage by 3.51%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1590      +/-   ##
==========================================
- Coverage   41.84%   38.33%   -3.52%     
==========================================
  Files         272      272              
  Lines       17681    19291    +1610     
==========================================
- Hits         7399     7395       -4     
- Misses       9370    10985    +1615     
+ Partials      912      911       -1
Impacted Files Coverage Δ
cli/network.go 0% <0%> (ø) ⬆️
apis/server/exec_bridge.go 46.15% <0%> (-22.03%) ⬇️
daemon/mgr/container.go 33.16% <0%> (-17.04%) ⬇️
daemon/mgr/container_exec.go 51.63% <0%> (-16.11%) ⬇️
ctrd/image.go 63.13% <0%> (-14.89%) ⬇️
apis/server/server.go 35.39% <0%> (-12.8%) ⬇️
cri/v1alpha1/cri_utils.go 20.5% <0%> (-8.23%) ⬇️
cri/v1alpha2/cri_utils.go 19.81% <0%> (-8.05%) ⬇️
apis/server/router.go 86.98% <0%> (-4.39%) ⬇️
apis/server/utils.go 64.28% <0%> (-2.39%) ⬇️
... and 8 more

}
fmt.Printf("container %s is disconnected from network %s\n", container, network)

return nil
}

// networkDisconnectExample shows examples in 'disconnect' command, and is used in auto-generated cli docs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

need change the example command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fine. I have updated it. PTAL, thanks

@rudyfly
Copy link
Collaborator

rudyfly commented Jun 25, 2018

Can you add more information about this pr? Such as change format from "A" to "B"...

cli/network.go Outdated
if err != nil {
return err
}
fmt.Printf("container %s is disconnected from network %s\n", container, network)
Copy link
Contributor

Choose a reason for hiding this comment

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

how about successfully disconnected ? :)

Signed-off-by: Eric Li <lcy041536@gmail.com>
@HusterWan
Copy link
Contributor

lgtm

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

Successfully merging this pull request may close these issues.

5 participants