-
Notifications
You must be signed in to change notification settings - Fork 619
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
agent: shutdown connection on node stop #1277
Conversation
I think @aaronlehmann meant another connection. I think that connection can be just cleaned up with defer in |
Current coverage is 54.80% (diff: 0.00%)@@ master #1277 diff @@
==========================================
Files 78 78
Lines 12440 12429 -11
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 6814 6812 -2
+ Misses 4683 4678 -5
+ Partials 943 939 -4
|
Indeed. Don't we still need to shut this one down? |
2950f77
to
9720c3a
Compare
@stevvooe At first I thought that yes, but now I believe that it will always be closed here https://github.com/stevvooe/swarmkit/blob/9720c3aff5db28dc5e46555b4e4c790bc9db392b/agent/node.go#L571 because context will be closed here https://github.com/stevvooe/swarmkit/blob/9720c3aff5db28dc5e46555b4e4c790bc9db392b/agent/node.go#L175 |
Signed-off-by: Stephen J Day <stephen.day@docker.com>
9720c3a
to
f6abeca
Compare
@LK4D4 Fair enough. |
LGTM |
1 similar comment
LGTM |
Closes #1274.
Signed-off-by: Stephen J Day stephen.day@docker.com
cc @LK4D4 @aaronlehmann @tonistiigi