Emit Node Events for draining #4284
Conversation
| node, err := state.NodeByID(nil, n1.ID) | ||
| require.NoError(err) | ||
| require.Len(node.Events, 3) | ||
| require.Equal(drainer.NodeDrainEventComplete, node.Events[2].Message) |
There was a problem hiding this comment.
what are the other two events expected and should they also be asserted here?
There was a problem hiding this comment.
It should be Register and Drain Enabled. I am not asserting it here since those aren't set by the node drainer system which this is testing.
nomad/node_endpoint.go
Outdated
| @@ -439,6 +444,9 @@ func (n *Node) UpdateDrain(args *structs.NodeUpdateDrainRequest, | |||
| if args.NodeID == "" { | |||
| return fmt.Errorf("missing node ID for drain update") | |||
| } | |||
| if args.NodeEvent != nil { | |||
| return fmt.Errorf("node event may not be set") | |||
| // Construct the node event | ||
| args.NodeEvent = structs.NewNodeEvent().SetSubsystem(structs.NodeEventSubsystemDrain) | ||
| if node.DrainStrategy == nil && args.DrainStrategy == nil { | ||
| return nil // Nothing to do |
There was a problem hiding this comment.
Shouldn't this go to line 492 where you apply the node update raft transaction? If there is no node drain event, seems like it should still apply the node update without the event.
There was a problem hiding this comment.
So I made this skip the raft transaction since it is a no-op. It is essentially saying, please update the node to have no drain strategy even when the node already doesn't have one.
|
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. |
This PR adds node events when a drain strategy is set, updated, unset and when
the drain is completed.