diff --git a/command/agent/node_endpoint.go b/command/agent/node_endpoint.go index 4c30a2708..6b6f99750 100644 --- a/command/agent/node_endpoint.go +++ b/command/agent/node_endpoint.go @@ -2,7 +2,9 @@ package agent import ( "net/http" + "strconv" "strings" + "time" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/nomad/structs" @@ -106,8 +108,30 @@ func (s *HTTPServer) nodeToggleDrain(resp http.ResponseWriter, req *http.Request var drainRequest api.NodeUpdateDrainRequest - if err := decodeBody(req, &drainRequest); err != nil { - return nil, CodedError(400, err.Error()) + // COMPAT: Remove in 0.10. Allow the old style enable query param. + // Get the enable parameter + enableRaw := req.URL.Query().Get("enable") + var enable bool + if enableRaw != "" { + var err error + enable, err = strconv.ParseBool(enableRaw) + if err != nil { + return nil, CodedError(400, "invalid enable value") + } + + // Use the force drain to have it keep the same behavior as old clients. + if enable { + drainRequest.DrainSpec = &api.DrainSpec{ + Deadline: -1 * time.Second, + } + } else { + // If drain is disabled on an old client, mark the node as eligible for backwards compatibility + drainRequest.MarkEligible = true + } + } else { + if err := decodeBody(req, &drainRequest); err != nil { + return nil, CodedError(400, err.Error()) + } } args := structs.NodeUpdateDrainRequest{ diff --git a/command/agent/node_endpoint_test.go b/command/agent/node_endpoint_test.go index 8bc4dc95e..978c266f4 100644 --- a/command/agent/node_endpoint_test.go +++ b/command/agent/node_endpoint_test.go @@ -280,6 +280,7 @@ func TestHTTP_NodeDrain(t *testing.T) { state := s.Agent.server.State() out, err := state.NodeByID(nil, node.ID) require.Nil(err) + require.True(out.Drain) require.NotNil(out.DrainStrategy) require.Equal(10*time.Second, out.DrainStrategy.Deadline) @@ -296,10 +297,66 @@ func TestHTTP_NodeDrain(t *testing.T) { out, err = state.NodeByID(nil, node.ID) require.Nil(err) + require.False(out.Drain) require.Nil(out.DrainStrategy) }) } +// Tests backwards compatibility code to support pre 0.8 clients +func TestHTTP_NodeDrain_Compat(t *testing.T) { + t.Parallel() + require := require.New(t) + httpTest(t, nil, func(s *TestAgent) { + // Create the node + node := mock.Node() + args := structs.NodeRegisterRequest{ + Node: node, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + var resp structs.NodeUpdateResponse + require.Nil(s.Agent.RPC("Node.Register", &args, &resp)) + + // Make the HTTP request + req, err := http.NewRequest("POST", "/v1/node/"+node.ID+"/drain?enable=true", nil) + require.Nil(err) + respW := httptest.NewRecorder() + + // Make the request + obj, err := s.Server.NodeSpecificRequest(respW, req) + require.Nil(err) + + // Check for the index + require.NotZero(respW.HeaderMap.Get("X-Nomad-Index")) + + // Check the response + _, ok := obj.(structs.NodeDrainUpdateResponse) + require.True(ok) + + // Check that the node has been updated + state := s.Agent.server.State() + out, err := state.NodeByID(nil, node.ID) + require.Nil(err) + require.True(out.Drain) + require.NotNil(out.DrainStrategy) + require.Equal(-1*time.Second, out.DrainStrategy.Deadline) + + // Make the HTTP request to unset drain + req, err = http.NewRequest("POST", "/v1/node/"+node.ID+"/drain?enable=false", nil) + require.Nil(err) + respW = httptest.NewRecorder() + + // Make the request + _, err = s.Server.NodeSpecificRequest(respW, req) + require.Nil(err) + + out, err = state.NodeByID(nil, node.ID) + require.Nil(err) + require.False(out.Drain) + require.Nil(out.DrainStrategy) + require.Equal(structs.NodeSchedulingEligible, out.SchedulingEligibility) + }) +} + func TestHTTP_NodeEligible(t *testing.T) { t.Parallel() require := require.New(t) diff --git a/nomad/fsm_test.go b/nomad/fsm_test.go index c7873e912..ef79d1461 100644 --- a/nomad/fsm_test.go +++ b/nomad/fsm_test.go @@ -350,6 +350,7 @@ func TestFSM_BatchUpdateNodeDrain(t *testing.T) { ws := memdb.NewWatchSet() node, err = fsm.State().NodeByID(ws, req.Node.ID) require.Nil(err) + require.True(node.Drain) require.Equal(node.DrainStrategy, strategy) require.Len(node.Events, 2) } @@ -393,10 +394,46 @@ func TestFSM_UpdateNodeDrain(t *testing.T) { ws := memdb.NewWatchSet() node, err = fsm.State().NodeByID(ws, req.Node.ID) require.Nil(err) + require.True(node.Drain) require.Equal(node.DrainStrategy, strategy) require.Len(node.Events, 2) } +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/node_endpoint_test.go b/nomad/node_endpoint_test.go index d7d584434..6b7b90e2f 100644 --- a/nomad/node_endpoint_test.go +++ b/nomad/node_endpoint_test.go @@ -891,10 +891,10 @@ func TestClientEndpoint_UpdateDrain(t *testing.T) { // Check for the node in the FSM state := s1.fsm.State() - ws := memdb.NewWatchSet() out, err := state.NodeByID(ws, node.ID) require.Nil(err) + require.True(out.Drain) require.Equal(strategy.Deadline, out.DrainStrategy.Deadline) require.Len(out.Events, 2) require.Equal(NodeDrainEventDrainSet, out.Events[1].Message) @@ -2682,7 +2682,7 @@ func TestClientEndpoint_ListNodes_Blocking(t *testing.T) { if resp2.Index != 3 { t.Fatalf("Bad index: %d %d", resp2.Index, 3) } - if len(resp2.Nodes) != 1 { + if len(resp2.Nodes) != 1 || !resp2.Nodes[0].Drain { t.Fatalf("bad: %#v", resp2.Nodes) } diff --git a/nomad/state/state_store_test.go b/nomad/state/state_store_test.go index 7e3539f3e..d21e4bbde 100644 --- a/nomad/state/state_store_test.go +++ b/nomad/state/state_store_test.go @@ -935,6 +935,7 @@ func TestStateStore_BatchUpdateNodeDrain(t *testing.T) { for _, id := range []string{n1.ID, n2.ID} { out, err := state.NodeByID(ws, id) require.Nil(err) + require.True(out.Drain) require.NotNil(out.DrainStrategy) require.Equal(out.DrainStrategy, expectedDrain) require.Len(out.Events, 2) @@ -977,6 +978,7 @@ func TestStateStore_UpdateNodeDrain_Node(t *testing.T) { ws = memdb.NewWatchSet() out, err := state.NodeByID(ws, node.ID) require.Nil(err) + require.True(out.Drain) require.NotNil(out.DrainStrategy) require.Equal(out.DrainStrategy, expectedDrain) require.Len(out.Events, 2)