Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the purge API on node endpoints #3447

Merged
merged 4 commits into from
Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions command/agent/node_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ func (s *HTTPServer) NodeSpecificRequest(resp http.ResponseWriter, req *http.Req
case strings.HasSuffix(path, "/drain"):
nodeName := strings.TrimSuffix(path, "/drain")
return s.nodeToggleDrain(resp, req, nodeName)
case strings.HasSuffix(path, "/purge"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTP Documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

nodeName := strings.TrimSuffix(path, "/purge")
return s.nodePurge(resp, req, nodeName)
default:
return s.nodeQuery(resp, req, path)
}
Expand Down Expand Up @@ -142,3 +145,19 @@ func (s *HTTPServer) nodeQuery(resp http.ResponseWriter, req *http.Request,
}
return out.Node, nil
}

func (s *HTTPServer) nodePurge(resp http.ResponseWriter, req *http.Request, nodeID string) (interface{}, error) {
if req.Method != "POST" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to merge and also allow PUT

return nil, CodedError(405, ErrInvalidMethod)
}
args := structs.NodeDeregisterRequest{
NodeID: nodeID,
}
s.parseWriteRequest(req, &args.WriteRequest)
var out structs.NodeUpdateResponse
if err := s.agent.RPC("Node.Deregister", &args, &out); err != nil {
return nil, err
}
setIndex(resp, out.Index)
return out, nil
}
65 changes: 65 additions & 0 deletions command/agent/node_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,71 @@ func TestHTTP_NodeDrain(t *testing.T) {
})
}

func TestHTTP_NodePurge(t *testing.T) {
t.Parallel()
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
if err := s.Agent.RPC("Node.Register", &args, &resp); err != nil {
t.Fatalf("err: %v", err)
}

// Add some allocations to the node
state := s.Agent.server.State()
alloc1 := mock.Alloc()
alloc1.NodeID = node.ID
if err := state.UpsertJobSummary(999, mock.JobSummary(alloc1.JobID)); err != nil {
t.Fatal(err)
}
err := state.UpsertAllocs(1000, []*structs.Allocation{alloc1})
if err != nil {
t.Fatalf("err: %v", err)
}

// Make the HTTP request to purge it
req, err := http.NewRequest("POST", "/v1/node/"+node.ID+"/purge", nil)
if err != nil {
t.Fatalf("err: %v", err)
}
respW := httptest.NewRecorder()

// Make the request
obj, err := s.Server.NodeSpecificRequest(respW, req)
if err != nil {
t.Fatalf("err: %v", err)
}

// Check for the index
if respW.HeaderMap.Get("X-Nomad-Index") == "" {
t.Fatalf("missing index")
}

// Check the response
upd := obj.(structs.NodeUpdateResponse)
if len(upd.EvalIDs) == 0 {
t.Fatalf("bad: %v", upd)
}

// Ensure that the node is not present anymore
args1 := structs.NodeSpecificRequest{
NodeID: node.ID,
QueryOptions: structs.QueryOptions{Region: "global"},
}
var resp1 structs.SingleNodeResponse
if err := s.Agent.RPC("Node.GetNode", &args1, &resp1); err != nil {
t.Fatalf("err: %v", err)
}
if resp1.Node != nil {
t.Fatalf("node still exists after purging: %#v", resp1.Node)
}
})
}

func TestHTTP_NodeQuery(t *testing.T) {
t.Parallel()
httpTest(t, nil, func(s *TestAgent) {
Expand Down
15 changes: 14 additions & 1 deletion nomad/node_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,20 @@ func (n *Node) Deregister(args *structs.NodeDeregisterRequest, reply *structs.No
if args.NodeID == "" {
return fmt.Errorf("missing node ID for client deregistration")
}
// Look for the node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the // Verify the arguments section can you add:

	// Check node write permissions
	if aclObj, err := n.srv.ResolveToken(args.AuthToken); err != nil {
		return err
	} else if aclObj != nil && !aclObj.AllowNodeWrite() {
		return structs.ErrPermissionDenied
	}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to add a test similar to this: https://github.com/hashicorp/nomad/blob/master/nomad/node_endpoint_test.go#L613

What you need to test is that an ACL with node write/management token work and that another token does not.

snap, err := n.srv.fsm.State().Snapshot()
if err != nil {
return err
}

ws := memdb.NewWatchSet()
node, err := snap.NodeByID(ws, args.NodeID)
if err != nil {
return err
}
if node == nil {
return fmt.Errorf("node not found")
}

// Commit this update via Raft
_, index, err := n.srv.raftApply(structs.NodeDeregisterRequestType, args)
Expand All @@ -232,7 +246,6 @@ func (n *Node) Deregister(args *structs.NodeDeregisterRequest, reply *structs.No
}

// Determine if there are any Vault accessors on the node
ws := memdb.NewWatchSet()
accessors, err := n.srv.State().VaultAccessorsByNode(ws, args.NodeID)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dadgar Can we use the snap we created earlier?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you can switch it over

if err != nil {
n.srv.logger.Printf("[ERR] nomad.client: looking up accessors for node %q failed: %v", args.NodeID, err)
Expand Down