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

bugfix: del endpoint after failing to create endpoint #1069

Merged

Conversation

faycheng
Copy link
Contributor

@faycheng faycheng commented Apr 8, 2018

Signed-off-by: 程飞 fay.cheng.cn@gmail.com

Ⅰ. Describe what this PR did

del endpoint after failing to create endpoint

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Signed-off-by: 程飞 <fay.cheng.cn@gmail.com>
@pouchrobot pouchrobot added kind/bug This is bug report for project size/XS labels Apr 8, 2018
@allencloud
Copy link
Collaborator

Is it a bugfix for issue #1068?
If that, please add that in the pr description. Thanks a lot. @faycheng

@codecov-io
Copy link

Codecov Report

Merging #1069 into master will increase coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1069      +/-   ##
==========================================
+ Coverage   16.48%   16.49%   +0.01%     
==========================================
  Files         158      158              
  Lines        8841     8864      +23     
==========================================
+ Hits         1457     1462       +5     
- Misses       7281     7298      +17     
- Partials      103      104       +1
Impacted Files Coverage Δ
daemon/mgr/network.go 3.53% <0%> (-0.04%) ⬇️
client/client.go 28.57% <0%> (-4.77%) ⬇️

@faycheng
Copy link
Contributor Author

faycheng commented Apr 8, 2018

hi, @allencloud , i am afraid that this PR could not fix #1068 . And thanks for your comment.

@allencloud
Copy link
Collaborator

I am wondering if we could add an integration test to cover this. @faycheng

@faycheng
Copy link
Contributor Author

faycheng commented Apr 9, 2018

hi, @allencloud , i am afraid that i could not add integration test cases for this PR.
Actually, making a specific scene for verifying Endpoint cleanup works is too hard to me.
Sorry, maybe other genius contributors could help to do it.

@allencloud
Copy link
Collaborator

That is all right. To be honest, this is indeed some kind of complex. @faycheng

@rudyfly
Copy link
Collaborator

rudyfly commented Apr 12, 2018

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Apr 12, 2018
@rudyfly rudyfly merged commit 4e09ec5 into AliyunContainerService:master Apr 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is bug report for project LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants