Monitoring non-draining node exits #4380
Conversation
| @@ -215,6 +224,7 @@ func (n *Nodes) monitorDrainNode(ctx context.Context, nodeID string, index uint6 | |||
| msg = Messagef(MonitorMsgLevelInfo, "Node %q has marked all allocations for migration", nodeID) | |||
| } else { | |||
| msg = Messagef(MonitorMsgLevelInfo, "No drain strategy set for node %s", nodeID) | |||
| defer cancel() | |||
There was a problem hiding this comment.
Why here and not before the if statement?
There was a problem hiding this comment.
@nickethier So by cancelling the shared context we can cancel the alloc watcher goroutine as well. We only want to do that if we have no expectation of allocations ever changing state because of a drain. That is only true in the else statement because the node has no drain strategy set and it never did. In the other case, allocations can still be transitioning.
command/node_drain_test.go
Outdated
| if len(nodes) == 0 { | ||
| return false, fmt.Errorf("missing node") | ||
| } | ||
| nodeID = nodes[0].ID |
There was a problem hiding this comment.
I think this line and 270 can be removed.
api/nodes.go
Outdated
| @@ -177,6 +177,14 @@ func (n *Nodes) monitorDrainMultiplex(ctx context.Context, cancel func(), | |||
| select { | |||
| case outCh <- msg: | |||
| case <-ctx.Done(): | |||
|
|
|||
| // If we are exiting but we have a message, attempt to send it | |||
| // so we don't loose a message but do not block. | |||
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
No description provided.