From b2df34cf1889f50da0beff587ef0b29388c9423f Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 4 Aug 2017 22:18:49 +0000 Subject: [PATCH] further refactoring --- command/agent/http.go | 2 +- command/agent/resources_endpoint.go | 8 ++-- command/agent/resources_endpoint_test.go | 50 ++++++++++++------------ nomad/resources_endpoint.go | 35 ++++++++++------- nomad/resources_endpoint_test.go | 38 +++++++++--------- nomad/structs/structs.go | 16 +++++--- 6 files changed, 81 insertions(+), 68 deletions(-) diff --git a/command/agent/http.go b/command/agent/http.go index be570a9b8..aa6880a58 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -145,7 +145,7 @@ func (s *HTTPServer) registerHandlers(enableDebug bool) { s.mux.HandleFunc("/v1/evaluations", s.wrap(s.EvalsRequest)) s.mux.HandleFunc("/v1/evaluation/", s.wrap(s.EvalSpecificRequest)) - s.mux.HandleFunc("/v1/resources/", s.wrap(s.ResourcesRequest)) + s.mux.HandleFunc("/v1/resources/", s.wrap(s.ResourceListRequest)) s.mux.HandleFunc("/v1/deployments", s.wrap(s.DeploymentsRequest)) s.mux.HandleFunc("/v1/deployment/", s.wrap(s.DeploymentSpecificRequest)) diff --git a/command/agent/resources_endpoint.go b/command/agent/resources_endpoint.go index 36e0054fe..fadc98145 100644 --- a/command/agent/resources_endpoint.go +++ b/command/agent/resources_endpoint.go @@ -5,9 +5,9 @@ import ( "net/http" ) -// ResourcesRequest accepts a prefix and context and returns a list of matching +// ResourceListRequest accepts a prefix and context and returns a list of matching // IDs for that context. -func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { +func (s *HTTPServer) ResourceListRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { if req.Method == "POST" || req.Method == "PUT" { return s.resourcesRequest(resp, req) } @@ -15,13 +15,13 @@ func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Reques } func (s *HTTPServer) resourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - args := structs.ResourcesRequest{} + args := structs.ResourceListRequest{} if err := decodeBody(req, &args); err != nil { return nil, CodedError(400, err.Error()) } - var out structs.ResourcesResponse + var out structs.ResourceListResponse if err := s.agent.RPC("Resources.List", &args, &out); err != nil { return nil, err } diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index 375afb8a4..c40b39bdb 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -18,7 +18,7 @@ func TestHTTP_ResourcesWithIllegalMethod(t *testing.T) { assert.Nil(err) respW := httptest.NewRecorder() - _, err = s.Server.ResourcesRequest(respW, req) + _, err = s.Server.ResourceListRequest(respW, req) assert.NotNil(err, "HTTP DELETE should not be accepted for this endpoint") }) } @@ -44,16 +44,16 @@ func TestHTTP_Resources_POST(t *testing.T) { httpTest(t, nil, func(s *TestAgent) { createJobForTest(testJob, s, t) - data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} + data := structs.ResourceListRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) assert.Nil(err) respW := httptest.NewRecorder() - resp, err := s.Server.ResourcesRequest(respW, req) + resp, err := s.Server.ResourceListRequest(respW, req) assert.Nil(err) - res := resp.(structs.ResourcesResponse) + res := resp.(structs.ResourceListResponse) assert.Equal(1, len(res.Matches)) @@ -76,16 +76,16 @@ func TestHTTP_Resources_PUT(t *testing.T) { httpTest(t, nil, func(s *TestAgent) { createJobForTest(testJob, s, t) - data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} + data := structs.ResourceListRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("PUT", "/v1/resources", encodeReq(data)) assert.Nil(err) respW := httptest.NewRecorder() - resp, err := s.Server.ResourcesRequest(respW, req) + resp, err := s.Server.ResourceListRequest(respW, req) assert.Nil(err) - res := resp.(structs.ResourcesResponse) + res := resp.(structs.ResourceListResponse) assert.Equal(1, len(res.Matches)) @@ -114,16 +114,16 @@ func TestHTTP_Resources_MultipleJobs(t *testing.T) { createJobForTest(testJobB, s, t) createJobForTest(testJobC, s, t) - data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} + data := structs.ResourceListRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) assert.Nil(err) respW := httptest.NewRecorder() - resp, err := s.Server.ResourcesRequest(respW, req) + resp, err := s.Server.ResourceListRequest(respW, req) assert.Nil(err) - res := resp.(structs.ResourcesResponse) + res := resp.(structs.ResourceListResponse) assert.Equal(1, len(res.Matches)) @@ -152,16 +152,16 @@ func TestHTTP_ResoucesList_Evaluation(t *testing.T) { assert.Nil(err) prefix := eval1.ID[:len(eval1.ID)-2] - data := structs.ResourcesRequest{Prefix: prefix, Context: "evals"} + data := structs.ResourceListRequest{Prefix: prefix, Context: "evals"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) assert.Nil(err) respW := httptest.NewRecorder() - resp, err := s.Server.ResourcesRequest(respW, req) + resp, err := s.Server.ResourceListRequest(respW, req) assert.Nil(err) - res := resp.(structs.ResourcesResponse) + res := resp.(structs.ResourceListResponse) assert.Equal(1, len(res.Matches)) @@ -186,16 +186,16 @@ func TestHTTP_ResoucesList_Allocations(t *testing.T) { assert.Nil(err) prefix := alloc.ID[:len(alloc.ID)-2] - data := structs.ResourcesRequest{Prefix: prefix, Context: "allocs"} + data := structs.ResourceListRequest{Prefix: prefix, Context: "allocs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) assert.Nil(err) respW := httptest.NewRecorder() - resp, err := s.Server.ResourcesRequest(respW, req) + resp, err := s.Server.ResourceListRequest(respW, req) assert.Nil(err) - res := resp.(structs.ResourcesResponse) + res := resp.(structs.ResourceListResponse) assert.Equal(1, len(res.Matches)) @@ -219,16 +219,16 @@ func TestHTTP_ResoucesList_Nodes(t *testing.T) { assert.Nil(err) prefix := node.ID[:len(node.ID)-2] - data := structs.ResourcesRequest{Prefix: prefix, Context: "nodes"} + data := structs.ResourceListRequest{Prefix: prefix, Context: "nodes"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) assert.Nil(err) respW := httptest.NewRecorder() - resp, err := s.Server.ResourcesRequest(respW, req) + resp, err := s.Server.ResourceListRequest(respW, req) assert.Nil(err) - res := resp.(structs.ResourcesResponse) + res := resp.(structs.ResourceListResponse) assert.Equal(1, len(res.Matches)) @@ -246,16 +246,16 @@ func TestHTTP_Resources_NoJob(t *testing.T) { t.Parallel() httpTest(t, nil, func(s *TestAgent) { - data := structs.ResourcesRequest{Prefix: "12345", Context: "jobs"} + data := structs.ResourceListRequest{Prefix: "12345", Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) assert.Nil(err) respW := httptest.NewRecorder() - resp, err := s.Server.ResourcesRequest(respW, req) + resp, err := s.Server.ResourceListRequest(respW, req) assert.Nil(err) - res := resp.(structs.ResourcesResponse) + res := resp.(structs.ResourceListResponse) assert.Equal(1, len(res.Matches)) assert.Equal(0, len(res.Matches["jobs"])) @@ -279,16 +279,16 @@ func TestHTTP_Resources_NoContext(t *testing.T) { err := state.UpsertEvals(8000, []*structs.Evaluation{eval1}) assert.Nil(err) - data := structs.ResourcesRequest{Prefix: testJobPrefix} + data := structs.ResourceListRequest{Prefix: testJobPrefix} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) assert.Nil(err) respW := httptest.NewRecorder() - resp, err := s.Server.ResourcesRequest(respW, req) + resp, err := s.Server.ResourceListRequest(respW, req) assert.Nil(err) - res := resp.(structs.ResourcesResponse) + res := resp.(structs.ResourceListResponse) matchedJobs := res.Matches["jobs"] matchedEvals := res.Matches["evals"] diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 5681ef8b9..79c460722 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -9,7 +9,15 @@ import ( // truncateLimit is the maximum number of matches that will be returned for a // prefix for a specific context -const truncateLimit = 20 +const ( + truncateLimit = 20 +) + +// allContexts are the available contexts which searched to find matches for a +// given prefix +var ( + allContexts = []string{"allocs", "nodes", "jobs", "evals"} +) // Resource endpoint is used to lookup matches for a given prefix and context type Resources struct { @@ -39,6 +47,8 @@ func getMatches(iter memdb.ResultIterator) ([]string, bool) { case *structs.Node: id = raw.(*structs.Node).ID default: + //s.logger.Printf("[ERR] nomad: unexpected type for resources context; not a job, allocation, node, or evaluation") + // TODO continue } @@ -65,14 +75,14 @@ func getResourceIter(context, prefix string, ws memdb.WatchSet, state *state.Sta case "nodes": return state.NodesByIDPrefix(ws, prefix) default: - return nil, fmt.Errorf("invalid context") + return nil, fmt.Errorf("context must be one of %v; got %q", allContexts, context) } } // List is used to list the resouces registered in the system that matches the // given prefix. Resources are jobs, evaluations, allocations, and/or nodes. -func (r *Resources) List(args *structs.ResourcesRequest, - reply *structs.ResourcesResponse) error { +func (r *Resources) List(args *structs.ResourceListRequest, + reply *structs.ResourceListResponse) error { reply.Matches = make(map[string][]string) reply.Truncations = make(map[string]bool) @@ -84,20 +94,17 @@ func (r *Resources) List(args *structs.ResourcesRequest, iters := make(map[string]memdb.ResultIterator) + contexts := allContexts if args.Context != "" { - iter, err := getResourceIter(args.Context, args.Prefix, ws, state) + contexts = []string{args.Context} + } + + for _, e := range contexts { + iter, err := getResourceIter(e, args.Prefix, ws, state) if err != nil { return err } - iters[args.Context] = iter - } else { - for _, e := range []string{"allocs", "nodes", "jobs", "evals"} { - iter, err := getResourceIter(e, args.Prefix, ws, state) - if err != nil { - return err - } - iters[e] = iter - } + iters[e] = iter } // Return matches for the given prefix diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 2f67b2280..c9abbf241 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -39,12 +39,12 @@ func TestResourcesEndpoint_List(t *testing.T) { jobID := registerAndVerifyJob(s, t, prefix, 0) - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: prefix, Context: "jobs", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { t.Fatalf("err: %v", err) } @@ -72,12 +72,12 @@ func TestResourcesEndpoint_List_Truncate(t *testing.T) { registerAndVerifyJob(s, t, prefix, counter) } - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: prefix, Context: "jobs", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { t.Fatalf("err: %v", err) } @@ -103,12 +103,12 @@ func TestResourcesEndpoint_List_Evals(t *testing.T) { prefix := eval1.ID[:len(eval1.ID)-2] - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: prefix, Context: "evals", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { t.Fatalf("err: %v", err) } @@ -144,12 +144,12 @@ func TestResourcesEndpoint_List_Allocation(t *testing.T) { prefix := alloc.ID[:len(alloc.ID)-2] - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: prefix, Context: "allocs", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { t.Fatalf("err: %v", err) } @@ -181,12 +181,12 @@ func TestResourcesEndpoint_List_Node(t *testing.T) { prefix := node.ID[:len(node.ID)-2] - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: prefix, Context: "nodes", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { t.Fatalf("err: %v", err) } @@ -210,14 +210,14 @@ func TestResourcesEndpoint_List_InvalidContext(t *testing.T) { codec := rpcClient(t, s) testutil.WaitForLeader(t, s.RPC) - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: "anyPrefix", Context: "invalid", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp) - assert.Equal(err.Error(), "invalid context") + assert.Equal(err.Error(), "context must be one of [allocs nodes jobs evals]; got \"invalid\"") assert.Equal(uint64(0), resp.Index) } @@ -248,12 +248,12 @@ func TestResourcesEndpoint_List_NoContext(t *testing.T) { prefix := node.ID[:len(node.ID)-2] - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: prefix, Context: "", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { t.Fatalf("err: %v", err) } @@ -284,12 +284,12 @@ func TestResourcesEndpoint_List_NoPrefix(t *testing.T) { jobID := registerAndVerifyJob(s, t, prefix, 0) - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: "", Context: "jobs", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { t.Fatalf("err: %v", err) } @@ -315,12 +315,12 @@ func TestResourcesEndpoint_List_NoMatches(t *testing.T) { codec := rpcClient(t, s) testutil.WaitForLeader(t, s.RPC) - req := &structs.ResourcesRequest{ + req := &structs.ResourceListRequest{ Prefix: prefix, Context: "jobs", } - var resp structs.ResourcesResponse + var resp structs.ResourceListResponse if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { t.Fatalf("err: %v", err) } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 24a11298c..946f65a67 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -231,19 +231,25 @@ type NodeSpecificRequest struct { QueryOptions } -// ResourcesResponse is used to return matches and information about whether +// ResourceListResponse is used to return matches and information about whether // the match list is truncated specific to each type of context. -type ResourcesResponse struct { +type ResourceListResponse struct { + // Map of context types to resource ids which match a specified prefix Matches map[string][]string Truncations map[string]bool QueryMeta } -// ResourcesRequest is used to parameterize a resources request, and returns a +// ResourceListRequest is used to parameterize a resources request, and returns a // subset of information for jobs, allocations, evaluations, and nodes, along // with whether or not the information returned is truncated. -type ResourcesRequest struct { - Prefix string +type ResourceListRequest struct { + // Prefix is what resources are matched to. I.e, if the given prefix were + // "a", potential matches might be "abcd" or "aabb" + Prefix string + // Context is the resource that can be matched. A context can be a job, node, + // evaluation, allocation, or empty (indicated every context should be + // matched) Context string }