Merge pull request #4102 from hashicorp/b-drain-upgrade

Correctly handle the upgrade path of a node being drained when applying Raft logs

nomad/fsm.go

@@ -335,6 +335,20 @@ func (n *nomadFSM) applyDrainUpdate(buf []byte, index uint64) interface{} {
 		panic(fmt.Errorf("failed to decode request: %v", err))
 	}
 
+	// COMPAT Remove in version 0.10
+	// As part of Nomad 0.8 we have deprecated the drain boolean in favor of a
+	// drain strategy but we need to handle the upgrade path where the Raft log
+	// contains drain updates with just the drain boolean being manipulated.
+	if req.Drain && req.DrainStrategy == nil {
+		// Mark the drain strategy as a force to immitate the old style drain
+		// functionality.
+		req.DrainStrategy = &structs.DrainStrategy{
+			DrainSpec: structs.DrainSpec{
+				Deadline: -1 * time.Second,
+			},
+		}
+	}
+
 	if err := n.state.UpdateNodeDrain(index, req.NodeID, req.DrainStrategy, req.MarkEligible); err != nil {
 		n.logger.Printf("[ERR] nomad.fsm: UpdateNodeDrain failed: %v", err)
 		return err

nomad/fsm_test.go

@@ -386,6 +386,41 @@ func TestFSM_UpdateNodeDrain(t *testing.T) {
 	require.Equal(node.DrainStrategy, strategy)
 }
 
+func TestFSM_UpdateNodeDrain_Pre08_Compatibility(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+	fsm := testFSM(t)
+
+	// Force a node into the state store without eligiblity
+	node := mock.Node()
+	node.SchedulingEligibility = ""
+	require.Nil(fsm.State().UpsertNode(1, node))
+
+	// Do an old style drain
+	req := structs.NodeUpdateDrainRequest{
+		NodeID: node.ID,
+		Drain:  true,
+	}
+	buf, err := structs.Encode(structs.NodeUpdateDrainRequestType, req)
+	require.Nil(err)
+
+	resp := fsm.Apply(makeLog(buf))
+	require.Nil(resp)
+
+	// Verify we have upgraded to a force drain
+	ws := memdb.NewWatchSet()
+	node, err = fsm.State().NodeByID(ws, req.NodeID)
+	require.Nil(err)
+	require.True(node.Drain)
+
+	expected := &structs.DrainStrategy{
+		DrainSpec: structs.DrainSpec{
+			Deadline: -1 * time.Second,
+		},
+	}
+	require.Equal(expected, node.DrainStrategy)
+}
+
 func TestFSM_UpdateNodeEligibility(t *testing.T) {
 	t.Parallel()
 	require := require.New(t)

nomad/structs/structs.go

@@ -334,9 +334,14 @@ type NodeUpdateStatusRequest struct {
 // NodeUpdateDrainRequest is used for updating the drain strategy
 type NodeUpdateDrainRequest struct {
 	NodeID        string
-	Drain         bool // TODO Deprecate
 	DrainStrategy *DrainStrategy
 
+	// COMPAT Remove in version 0.10
+	// As part of Nomad 0.8 we have deprecated the drain boolean in favor of a
+	// drain strategy but we need to handle the upgrade path where the Raft log
+	// contains drain updates with just the drain boolean being manipulated.
+	Drain bool
+
 	// MarkEligible marks the node as eligible if removing the drain strategy.
 	MarkEligible bool
 	WriteRequest