From 9617a13a2b3458e54542490e8439045e55359ce0 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Tue, 3 Apr 2018 15:24:20 -0700 Subject: [PATCH] Correctly handle the upgrade path of a node being drained when applying Raft logs --- nomad/fsm.go | 14 ++++++++++++++ nomad/fsm_test.go | 35 +++++++++++++++++++++++++++++++++++ nomad/structs/structs.go | 7 ++++++- 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/nomad/fsm.go b/nomad/fsm.go index 4d62835a5..46f786f34 100644 --- a/nomad/fsm.go +++ b/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 diff --git a/nomad/fsm_test.go b/nomad/fsm_test.go index e5bc2183e..5cace9d79 100644 --- a/nomad/fsm_test.go +++ b/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) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 328ff86a1..59ef5ff99 100644 --- a/nomad/structs/structs.go +++ b/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