From 1b24ae599e84f808ac14573486ff8286fecc47c0 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 28 Jul 2017 21:48:15 +0000 Subject: [PATCH 01/27] Retrieve job information for resources endpoint requires further refactoring and logic for more contexts --- command/agent/http.go | 2 + command/agent/resources_endpoint.go | 30 +++++++++ command/agent/resources_endpoint_test.go | 77 ++++++++++++++++++++++++ nomad/resources_endpoint.go | 55 +++++++++++++++++ nomad/resources_endpoint_test.go | 72 ++++++++++++++++++++++ nomad/server.go | 3 + nomad/structs/structs.go | 18 ++++++ 7 files changed, 257 insertions(+) create mode 100644 command/agent/resources_endpoint.go create mode 100644 command/agent/resources_endpoint_test.go create mode 100644 nomad/resources_endpoint.go create mode 100644 nomad/resources_endpoint_test.go diff --git a/command/agent/http.go b/command/agent/http.go index 3891ef37f6c..be570a9b866 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -145,6 +145,8 @@ 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/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 new file mode 100644 index 00000000000..42d33942507 --- /dev/null +++ b/command/agent/resources_endpoint.go @@ -0,0 +1,30 @@ +package agent + +import ( + "github.com/hashicorp/nomad/nomad/structs" + "net/http" +) + +func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + switch req.Method { + case "GET": + return s.resourcesRequest(resp, req) + default: + return nil, CodedError(405, ErrInvalidMethod) + } +} + +func (s *HTTPServer) resourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + // TODO test a failure case for this? + args := structs.ResourcesRequest{} + if s.parse(resp, req, &args.Region, &args.QueryOptions) { + return nil, nil + } + + var out structs.ResourcesResponse + if err := s.agent.RPC("Resources.List", &args, &out); err != nil { + return nil, err + } + + return &out.Resources, nil +} diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go new file mode 100644 index 00000000000..3e97749af7c --- /dev/null +++ b/command/agent/resources_endpoint_test.go @@ -0,0 +1,77 @@ +package agent + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/hashicorp/nomad/nomad/mock" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/stretchr/testify/assert" +) + +func TestHTTP_ResourcesWithIllegalMethod(t *testing.T) { + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + req, err := http.NewRequest("DELETE", "/v1/resources", nil) + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + _, err = s.Server.ResourcesRequest(respW, req) + assert.NotNil(t, err, "HTTP DELETE should not be accepted for this endpoint") + }) +} + +func createJobForTest(jobID string, s *TestAgent, t *testing.T) { + job := mock.Job() + job.ID = jobID + job.TaskGroups[0].Count = 1 + args := structs.JobRegisterRequest{ + Job: job, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + var resp structs.JobRegisterResponse + if err := s.Agent.RPC("Job.Register", &args, &resp); err != nil { + t.Fatalf("err: %v", err) + } +} + +func TestHTTP_ResourcesWithSingleJob(t *testing.T) { + testJob := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + createJobForTest(testJob, s, t) + + endpoint := fmt.Sprintf("/v1/resources?context=job&prefix=%s", testJob) + req, err := http.NewRequest("GET", endpoint, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + resp, err := s.Server.ResourcesRequest(respW, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + res := resp.(*structs.ResourcesListStub) + if len(res.Matches) != 1 { + t.Fatalf("No expected key values in resources list") + } + + j := res.Matches["jobs"] + if j == nil || len(j) != 1 { + t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (1)", j) + } + + // TODO verify that the job we are getting is the same that we created + // assert.Equal(t, j[0], testJob) + }) +} + +// +//func TestHTTP_ResourcesWithNoJob(t *testing.T) { +//} diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go new file mode 100644 index 00000000000..87d3cc03047 --- /dev/null +++ b/nomad/resources_endpoint.go @@ -0,0 +1,55 @@ +package nomad + +import ( + "github.com/hashicorp/go-memdb" + "github.com/hashicorp/nomad/nomad/state" + "github.com/hashicorp/nomad/nomad/structs" +) + +type Resources struct { + srv *Server +} + +// List is used to list the jobs registered in the system +// TODO logic to determine context, to return only that context if needed +// TODO if no context, return all +// TODO return top n matches +// TODO refactor to prevent duplication +func (r *Resources) List(args *structs.ResourcesRequest, + reply *structs.ResourcesResponse) error { + + resources := structs.ResourcesListStub{} + resources.Matches = make(map[string][]string) + + // Setup the blocking query + opts := blockingOptions{ + queryOpts: &args.QueryOptions, + queryMeta: &reply.QueryMeta, + run: func(ws memdb.WatchSet, state *state.StateStore) error { + + // return jobs matching given prefix + var err error + var iter memdb.ResultIterator + iter, err = state.JobsByIDPrefix(ws, args.QueryOptions.Prefix) + if err != nil { + return err + } + + var jobs []string + for { + raw := iter.Next() + if raw == nil { + break + } + + job := raw.(*structs.Job) + jobs = append(jobs, job.ID) + } + + resources.Matches["jobs"] = jobs + reply.Resources = resources + + return nil + }} + return r.srv.blockingRPC(&opts) +} diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go new file mode 100644 index 00000000000..66fe15a977b --- /dev/null +++ b/nomad/resources_endpoint_test.go @@ -0,0 +1,72 @@ +package nomad + +import ( + "fmt" + "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/hashicorp/nomad/nomad/mock" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/assert" + "net/rpc" + "testing" +) + +func registerAndVerifyJob(codec rpc.ClientCodec, s *Server, t *testing.T) string { + // Create the register request + job := mock.Job() + state := s.fsm.State() + err := state.UpsertJob(1000, job) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Verify the job was created + get := &structs.JobListRequest{ + QueryOptions: structs.QueryOptions{Region: "global"}, + } + var resp2 structs.JobListResponse + if err := msgpackrpc.CallWithCodec(codec, "Job.List", get, &resp2); err != nil { + t.Fatalf("err: %v", err) + } + if resp2.Index != 1000 { + t.Fatalf("Bad index: %d %d", resp2.Index, 1000) + } + + if len(resp2.Jobs) != 1 { + t.Fatalf("bad: %#v", resp2.Jobs) + } + if resp2.Jobs[0].ID != job.ID { + t.Fatalf("bad: %#v", resp2.Jobs[0]) + } + + return job.ID +} + +func TestResourcesEndpoint_List(t *testing.T) { + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + jobID := registerAndVerifyJob(codec, s, t) + + req := &structs.ResourcesRequest{ + QueryOptions: structs.QueryOptions{Region: "global", Prefix: jobID}, + } + + var resp structs.ResourcesResponse + if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + num_matches := len(resp.Resources.Matches["jobs"]) + if num_matches != 1 { + t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs registered %d", 1, num_matches)) + } + + assert.Equal(t, jobID, resp.Resources.Matches["jobs"][0]) +} diff --git a/nomad/server.go b/nomad/server.go index 19d8a6d054b..a18f697722e 100644 --- a/nomad/server.go +++ b/nomad/server.go @@ -174,6 +174,7 @@ type endpoints struct { Alloc *Alloc Deployment *Deployment Region *Region + Resources *Resources Periodic *Periodic System *System Operator *Operator @@ -725,6 +726,7 @@ func (s *Server) setupRPC(tlsWrap tlsutil.RegionWrapper) error { s.endpoints.Region = &Region{s} s.endpoints.Status = &Status{s} s.endpoints.System = &System{s} + s.endpoints.Resources = &Resources{s} // Register the handlers s.rpcServer.Register(s.endpoints.Alloc) @@ -738,6 +740,7 @@ func (s *Server) setupRPC(tlsWrap tlsutil.RegionWrapper) error { s.rpcServer.Register(s.endpoints.Region) s.rpcServer.Register(s.endpoints.Status) s.rpcServer.Register(s.endpoints.System) + s.rpcServer.Register(s.endpoints.Resources) list, err := net.ListenTCP("tcp", s.config.RPCAddr) if err != nil { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 8143e3c9caa..7b0cc3dd08c 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -231,6 +231,18 @@ type NodeSpecificRequest struct { QueryOptions } +type ResourcesResponse struct { + Resources ResourcesListStub + QueryMeta +} + +// TODO can there be a more generic Request object, if a specific one is not +// needed? +// ResourcesRequest is used to parameterize a resources request +type ResourcesRequest struct { + QueryOptions +} + // JobRegisterRequest is used for Job.Register endpoint // to register a job as being a schedulable entity. type JobRegisterRequest struct { @@ -1871,6 +1883,12 @@ func (j *Job) SetSubmitTime() { j.SubmitTime = time.Now().UTC().UnixNano() } +// ResourcesListStub is used to return a subset of information +// for jobs, allocations, evaluations, and nodes +type ResourcesListStub struct { + Matches map[string][]string +} + // JobListStub is used to return a subset of job information // for the job list type JobListStub struct { From cb1e898eeac516c63f91548375026d9aa398816b Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Tue, 1 Aug 2017 19:33:40 +0000 Subject: [PATCH 02/27] adding test validation that received resources matches requested --- command/agent/resources_endpoint_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index 3e97749af7c..49d6cfc5e0c 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -41,11 +41,12 @@ func createJobForTest(jobID string, s *TestAgent, t *testing.T) { func TestHTTP_ResourcesWithSingleJob(t *testing.T) { testJob := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" + testJobPrefix := "aaaaaaaa-e8f7-fd38" t.Parallel() httpTest(t, nil, func(s *TestAgent) { createJobForTest(testJob, s, t) - endpoint := fmt.Sprintf("/v1/resources?context=job&prefix=%s", testJob) + endpoint := fmt.Sprintf("/v1/resources?context=job&prefix=%s", testJobPrefix) req, err := http.NewRequest("GET", endpoint, nil) if err != nil { t.Fatalf("err: %v", err) @@ -67,8 +68,7 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (1)", j) } - // TODO verify that the job we are getting is the same that we created - // assert.Equal(t, j[0], testJob) + assert.Equal(t, j[0], testJob) }) } From c3e0bd82b76e1badfcf39d4729d2ee59604b31a8 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Tue, 1 Aug 2017 19:59:19 +0000 Subject: [PATCH 03/27] test resources endpoint will return matching prefixes --- command/agent/resources_endpoint_test.go | 43 +++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index 49d6cfc5e0c..dff6e5da7ca 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -46,7 +46,7 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { httpTest(t, nil, func(s *TestAgent) { createJobForTest(testJob, s, t) - endpoint := fmt.Sprintf("/v1/resources?context=job&prefix=%s", testJobPrefix) + endpoint := fmt.Sprintf("/v1/resources?context=job&prefix=%s&context=job", testJobPrefix) req, err := http.NewRequest("GET", endpoint, nil) if err != nil { t.Fatalf("err: %v", err) @@ -72,6 +72,47 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { }) } +func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { + testJobA := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" + testJobB := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89707" + testJobC := "bbbbbbbb-e8f7-fd38-c855-ab94ceb89707" + + testJobPrefix := "aaaaaaaa-e8f7-fd38" + + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + createJobForTest(testJobA, s, t) + createJobForTest(testJobB, s, t) + createJobForTest(testJobC, s, t) + + endpoint := fmt.Sprintf("/v1/resources?context=job&prefix=%s&context=job", testJobPrefix) + req, err := http.NewRequest("GET", endpoint, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + resp, err := s.Server.ResourcesRequest(respW, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + res := resp.(*structs.ResourcesListStub) + if len(res.Matches) != 1 { + t.Fatalf("No expected key values in resources list") + } + + j := res.Matches["jobs"] + if j == nil || len(j) != 2 { + t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (2)", j) + } + + assert.Contains(t, j, testJobA) + assert.Contains(t, j, testJobB) + assert.NotContains(t, j, testJobC) + }) +} + // //func TestHTTP_ResourcesWithNoJob(t *testing.T) { //} From 0bd974272d0ca892d6599431a0460a4b288873c9 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Tue, 1 Aug 2017 20:42:23 +0000 Subject: [PATCH 04/27] remove unnecessary validations; these are tested elsewhere --- nomad/resources_endpoint_test.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 66fe15a977b..74eb45a6cfb 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -20,25 +20,6 @@ func registerAndVerifyJob(codec rpc.ClientCodec, s *Server, t *testing.T) string t.Fatalf("err: %v", err) } - // Verify the job was created - get := &structs.JobListRequest{ - QueryOptions: structs.QueryOptions{Region: "global"}, - } - var resp2 structs.JobListResponse - if err := msgpackrpc.CallWithCodec(codec, "Job.List", get, &resp2); err != nil { - t.Fatalf("err: %v", err) - } - if resp2.Index != 1000 { - t.Fatalf("Bad index: %d %d", resp2.Index, 1000) - } - - if len(resp2.Jobs) != 1 { - t.Fatalf("bad: %#v", resp2.Jobs) - } - if resp2.Jobs[0].ID != job.ID { - t.Fatalf("bad: %#v", resp2.Jobs[0]) - } - return job.ID } From 5435773dbb3b3d06e3b8030ef06823f4c173fa87 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Tue, 1 Aug 2017 21:06:52 +0000 Subject: [PATCH 05/27] refactor test helper --- nomad/resources_endpoint_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 74eb45a6cfb..f37abb58530 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -7,13 +7,14 @@ import ( "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/assert" - "net/rpc" + "strconv" "testing" ) -func registerAndVerifyJob(codec rpc.ClientCodec, s *Server, t *testing.T) string { - // Create the register request +func registerAndVerifyJob(s *Server, t *testing.T, prefix string, counter int) string { job := mock.Job() + + job.ID = prefix + strconv.Itoa(counter) state := s.fsm.State() err := state.UpsertJob(1000, job) if err != nil { @@ -24,6 +25,8 @@ func registerAndVerifyJob(codec rpc.ClientCodec, s *Server, t *testing.T) string } func TestResourcesEndpoint_List(t *testing.T) { + prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" + t.Parallel() s := testServer(t, func(c *Config) { c.NumSchedulers = 0 // Prevent automatic dequeue @@ -33,7 +36,7 @@ func TestResourcesEndpoint_List(t *testing.T) { codec := rpcClient(t, s) testutil.WaitForLeader(t, s.RPC) - jobID := registerAndVerifyJob(codec, s, t) + jobID := registerAndVerifyJob(s, t, prefix, 0) req := &structs.ResourcesRequest{ QueryOptions: structs.QueryOptions{Region: "global", Prefix: jobID}, From 4945b98a7fe7361d2c0f403a0c9ba956d9166efe Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Tue, 1 Aug 2017 21:28:53 +0000 Subject: [PATCH 06/27] limit resources results to 20 --- nomad/resources_endpoint.go | 3 +-- nomad/resources_endpoint_test.go | 33 +++++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 87d3cc03047..83cdf4970c2 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -13,7 +13,6 @@ type Resources struct { // List is used to list the jobs registered in the system // TODO logic to determine context, to return only that context if needed // TODO if no context, return all -// TODO return top n matches // TODO refactor to prevent duplication func (r *Resources) List(args *structs.ResourcesRequest, reply *structs.ResourcesResponse) error { @@ -36,7 +35,7 @@ func (r *Resources) List(args *structs.ResourcesRequest, } var jobs []string - for { + for i := 0; i < 20; i++ { raw := iter.Next() if raw == nil { break diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index f37abb58530..8bb04009b4d 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -39,7 +39,7 @@ func TestResourcesEndpoint_List(t *testing.T) { jobID := registerAndVerifyJob(s, t, prefix, 0) req := &structs.ResourcesRequest{ - QueryOptions: structs.QueryOptions{Region: "global", Prefix: jobID}, + QueryOptions: structs.QueryOptions{Region: "global", Prefix: prefix}, } var resp structs.ResourcesResponse @@ -54,3 +54,34 @@ func TestResourcesEndpoint_List(t *testing.T) { assert.Equal(t, jobID, resp.Resources.Matches["jobs"][0]) } + +func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { + prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" + + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + for counter := 0; counter < 25; counter++ { + registerAndVerifyJob(s, t, prefix, counter) + } + + req := &structs.ResourcesRequest{ + QueryOptions: structs.QueryOptions{Region: "global", Prefix: prefix}, + } + + var resp structs.ResourcesResponse + if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + num_matches := len(resp.Resources.Matches["jobs"]) + if num_matches != 20 { + t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs returned %d", 20, num_matches)) + } +} From 1ec5010f0bb0bf8cec5811d92c460341e410a7f7 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Tue, 1 Aug 2017 22:20:03 +0000 Subject: [PATCH 07/27] remove resourceliststub, no need for another layer of abstraction --- command/agent/resources_endpoint.go | 2 +- command/agent/resources_endpoint_test.go | 4 ++-- nomad/resources_endpoint.go | 7 +++---- nomad/resources_endpoint_test.go | 6 +++--- nomad/structs/structs.go | 15 +++++---------- 5 files changed, 14 insertions(+), 20 deletions(-) diff --git a/command/agent/resources_endpoint.go b/command/agent/resources_endpoint.go index 42d33942507..2921bee7f4d 100644 --- a/command/agent/resources_endpoint.go +++ b/command/agent/resources_endpoint.go @@ -26,5 +26,5 @@ func (s *HTTPServer) resourcesRequest(resp http.ResponseWriter, req *http.Reques return nil, err } - return &out.Resources, nil + return out, nil } diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index dff6e5da7ca..8cc405d45a9 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -58,7 +58,7 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { t.Fatalf("err: %v", err) } - res := resp.(*structs.ResourcesListStub) + res := resp.(structs.ResourcesResponse) if len(res.Matches) != 1 { t.Fatalf("No expected key values in resources list") } @@ -97,7 +97,7 @@ func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { t.Fatalf("err: %v", err) } - res := resp.(*structs.ResourcesListStub) + res := resp.(structs.ResourcesResponse) if len(res.Matches) != 1 { t.Fatalf("No expected key values in resources list") } diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 83cdf4970c2..e5e33339c22 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -17,8 +17,7 @@ type Resources struct { func (r *Resources) List(args *structs.ResourcesRequest, reply *structs.ResourcesResponse) error { - resources := structs.ResourcesListStub{} - resources.Matches = make(map[string][]string) + matches := make(map[string][]string) // Setup the blocking query opts := blockingOptions{ @@ -45,8 +44,8 @@ func (r *Resources) List(args *structs.ResourcesRequest, jobs = append(jobs, job.ID) } - resources.Matches["jobs"] = jobs - reply.Resources = resources + matches["jobs"] = jobs + reply.Matches = matches return nil }} diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 8bb04009b4d..7082b2f25bf 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -47,12 +47,12 @@ func TestResourcesEndpoint_List(t *testing.T) { t.Fatalf("err: %v", err) } - num_matches := len(resp.Resources.Matches["jobs"]) + num_matches := len(resp.Matches["jobs"]) if num_matches != 1 { t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs registered %d", 1, num_matches)) } - assert.Equal(t, jobID, resp.Resources.Matches["jobs"][0]) + assert.Equal(t, jobID, resp.Matches["jobs"][0]) } func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { @@ -80,7 +80,7 @@ func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { t.Fatalf("err: %v", err) } - num_matches := len(resp.Resources.Matches["jobs"]) + num_matches := len(resp.Matches["jobs"]) if num_matches != 20 { t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs returned %d", 20, num_matches)) } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 7b0cc3dd08c..f15044ea3c1 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -232,13 +232,14 @@ type NodeSpecificRequest struct { } type ResourcesResponse struct { - Resources ResourcesListStub + Matches map[string][]string + Truncations map[string]bool QueryMeta } -// TODO can there be a more generic Request object, if a specific one is not -// needed? -// ResourcesRequest is used to parameterize a resources request +// ResourcesRequest 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 { QueryOptions } @@ -1883,12 +1884,6 @@ func (j *Job) SetSubmitTime() { j.SubmitTime = time.Now().UTC().UnixNano() } -// ResourcesListStub is used to return a subset of information -// for jobs, allocations, evaluations, and nodes -type ResourcesListStub struct { - Matches map[string][]string -} - // JobListStub is used to return a subset of job information // for the job list type JobListStub struct { From 9e06777613286ae19fe9e4dc1d11251f1bc83e91 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 2 Aug 2017 15:57:19 +0000 Subject: [PATCH 08/27] change resources endpoint from http get to post --- command/agent/resources_endpoint.go | 8 +++++--- command/agent/resources_endpoint_test.go | 13 +++++++------ nomad/resources_endpoint.go | 4 ++-- nomad/resources_endpoint_test.go | 6 ++++-- nomad/structs/structs.go | 3 ++- 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/command/agent/resources_endpoint.go b/command/agent/resources_endpoint.go index 2921bee7f4d..1c1c581b73b 100644 --- a/command/agent/resources_endpoint.go +++ b/command/agent/resources_endpoint.go @@ -7,7 +7,7 @@ import ( func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { switch req.Method { - case "GET": + case "POST": return s.resourcesRequest(resp, req) default: return nil, CodedError(405, ErrInvalidMethod) @@ -17,8 +17,10 @@ func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Reques func (s *HTTPServer) resourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { // TODO test a failure case for this? args := structs.ResourcesRequest{} - if s.parse(resp, req, &args.Region, &args.QueryOptions) { - return nil, nil + + // TODO this should be tested + if err := decodeBody(req, &args); err != nil { + return nil, CodedError(400, err.Error()) } var out structs.ResourcesResponse diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index 8cc405d45a9..e19b7d411cd 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -1,7 +1,6 @@ package agent import ( - "fmt" "net/http" "net/http/httptest" "testing" @@ -46,8 +45,9 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { httpTest(t, nil, func(s *TestAgent) { createJobForTest(testJob, s, t) - endpoint := fmt.Sprintf("/v1/resources?context=job&prefix=%s&context=job", testJobPrefix) - req, err := http.NewRequest("GET", endpoint, nil) + data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} + req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + if err != nil { t.Fatalf("err: %v", err) } @@ -85,8 +85,9 @@ func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { createJobForTest(testJobB, s, t) createJobForTest(testJobC, s, t) - endpoint := fmt.Sprintf("/v1/resources?context=job&prefix=%s&context=job", testJobPrefix) - req, err := http.NewRequest("GET", endpoint, nil) + data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} + req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + if err != nil { t.Fatalf("err: %v", err) } @@ -113,6 +114,6 @@ func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { }) } -// +// TODO //func TestHTTP_ResourcesWithNoJob(t *testing.T) { //} diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index e5e33339c22..eabc9e4c84a 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -21,14 +21,14 @@ func (r *Resources) List(args *structs.ResourcesRequest, // Setup the blocking query opts := blockingOptions{ - queryOpts: &args.QueryOptions, queryMeta: &reply.QueryMeta, + queryOpts: &structs.QueryOptions{}, run: func(ws memdb.WatchSet, state *state.StateStore) error { // return jobs matching given prefix var err error var iter memdb.ResultIterator - iter, err = state.JobsByIDPrefix(ws, args.QueryOptions.Prefix) + iter, err = state.JobsByIDPrefix(ws, args.Prefix) if err != nil { return err } diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 7082b2f25bf..73b7a743a6c 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -39,7 +39,8 @@ func TestResourcesEndpoint_List(t *testing.T) { jobID := registerAndVerifyJob(s, t, prefix, 0) req := &structs.ResourcesRequest{ - QueryOptions: structs.QueryOptions{Region: "global", Prefix: prefix}, + Prefix: prefix, + Context: "job", } var resp structs.ResourcesResponse @@ -72,7 +73,8 @@ func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { } req := &structs.ResourcesRequest{ - QueryOptions: structs.QueryOptions{Region: "global", Prefix: prefix}, + Prefix: prefix, + Context: "job", } var resp structs.ResourcesResponse diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index f15044ea3c1..572536749fb 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -241,7 +241,8 @@ type ResourcesResponse struct { // subset of information for jobs, allocations, evaluations, and nodes, along // with whether or not the information returned is truncated. type ResourcesRequest struct { - QueryOptions + Prefix string + Context string } // JobRegisterRequest is used for Job.Register endpoint From dfd8c1d832931871e41ff00a257818a7a2c54e03 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 2 Aug 2017 17:21:58 +0000 Subject: [PATCH 09/27] adds evaluations makes context singular --- command/agent/resources_endpoint_test.go | 49 ++++++++++++++++++++++-- nomad/resources_endpoint.go | 46 ++++++++++++++++------ nomad/resources_endpoint_test.go | 42 ++++++++++++++++++-- 3 files changed, 119 insertions(+), 18 deletions(-) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index e19b7d411cd..b0652e6db6d 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -45,7 +45,7 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { httpTest(t, nil, func(s *TestAgent) { createJobForTest(testJob, s, t) - data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} + data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "job"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) if err != nil { @@ -63,7 +63,7 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { t.Fatalf("No expected key values in resources list") } - j := res.Matches["jobs"] + j := res.Matches["job"] if j == nil || len(j) != 1 { t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (1)", j) } @@ -85,7 +85,7 @@ func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { createJobForTest(testJobB, s, t) createJobForTest(testJobC, s, t) - data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} + data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "job"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) if err != nil { @@ -103,7 +103,7 @@ func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { t.Fatalf("No expected key values in resources list") } - j := res.Matches["jobs"] + j := res.Matches["job"] if j == nil || len(j) != 2 { t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (2)", j) } @@ -114,6 +114,47 @@ func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { }) } +func TestHTTP_ResoucesListForEvaluations(t *testing.T) { + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + state := s.Agent.server.State() + eval1 := mock.Eval() + eval2 := mock.Eval() + err := state.UpsertEvals(1000, + []*structs.Evaluation{eval1, eval2}) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Make the HTTP request + evalPrefix := eval1.ID[:len(eval1.ID)-2] + data := structs.ResourcesRequest{Prefix: evalPrefix, Context: "eval"} + req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + resp, err := s.Server.ResourcesRequest(respW, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + res := resp.(structs.ResourcesResponse) + if len(res.Matches) != 1 { + t.Fatalf("No expected key values in resources list") + } + + j := res.Matches["eval"] + if len(j) != 1 { + t.Fatalf("The number of evaluations that were returned does not equal the number we expected (1)", j) + } + + assert.Contains(t, j, eval1.ID) + assert.NotContains(t, j, eval2.ID) + }) +} + // TODO //func TestHTTP_ResourcesWithNoJob(t *testing.T) { //} diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index eabc9e4c84a..7414fe9aa46 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -28,23 +28,47 @@ func (r *Resources) List(args *structs.ResourcesRequest, // return jobs matching given prefix var err error var iter memdb.ResultIterator - iter, err = state.JobsByIDPrefix(ws, args.Prefix) - if err != nil { - return err + + if args.Context == "job" { + iter, err = state.JobsByIDPrefix(ws, args.Prefix) + if err != nil { + return err + } + + var jobs []string + for i := 0; i < 20; i++ { + raw := iter.Next() + if raw == nil { + break + } + + job := raw.(*structs.Job) + jobs = append(jobs, job.ID) + } + + matches["job"] = jobs } - var jobs []string - for i := 0; i < 20; i++ { - raw := iter.Next() - if raw == nil { - break + if args.Context == "eval" { + iter, err = state.EvalsByIDPrefix(ws, args.Prefix) + if err != nil { + return err + } + + var evals []string + for i := 0; i < 20; i++ { // TODO extract magic number + raw := iter.Next() + if raw == nil { + break + } + + eval := raw.(*structs.Evaluation) + evals = append(evals, eval.ID) } - job := raw.(*structs.Job) - jobs = append(jobs, job.ID) + matches["eval"] = evals } - matches["jobs"] = jobs reply.Matches = matches return nil diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 73b7a743a6c..5e64c015648 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -48,12 +48,12 @@ func TestResourcesEndpoint_List(t *testing.T) { t.Fatalf("err: %v", err) } - num_matches := len(resp.Matches["jobs"]) + num_matches := len(resp.Matches["job"]) if num_matches != 1 { t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs registered %d", 1, num_matches)) } - assert.Equal(t, jobID, resp.Matches["jobs"][0]) + assert.Equal(t, jobID, resp.Matches["job"][0]) } func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { @@ -82,8 +82,44 @@ func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { t.Fatalf("err: %v", err) } - num_matches := len(resp.Matches["jobs"]) + num_matches := len(resp.Matches["job"]) if num_matches != 20 { t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs returned %d", 20, num_matches)) } } + +func TestResourcesEndpoint_List_ShouldReturnEvals(t *testing.T) { + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 // Prevent automatic dequeue + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + eval1 := mock.Eval() + s.fsm.State().UpsertEvals(1000, []*structs.Evaluation{eval1}) + + prefix := eval1.ID[:len(eval1.ID)-2] + + req := &structs.ResourcesRequest{ + Prefix: prefix, + Context: "eval", + } + + var resp structs.ResourcesResponse + if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + numMatches := len(resp.Matches["eval"]) + if numMatches != 1 { + t.Fatalf(fmt.Sprintf("err: the number of evaluations expected %d does not match the number expected %d", 1, numMatches)) + } + + recEval := resp.Matches["eval"][0] + if recEval != eval1.ID { + t.Fatalf(fmt.Sprintf("err: expected %s evaluation but received %s", eval1.ID, recEval)) + } +} From e14b279bc073e4cfbb52b3e98913f7b2acae2204 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 2 Aug 2017 19:26:50 +0000 Subject: [PATCH 10/27] add truncation boolean to response --- command/agent/resources_endpoint_test.go | 2 ++ nomad/resources_endpoint.go | 10 ++++++++++ nomad/resources_endpoint_test.go | 4 ++++ 3 files changed, 16 insertions(+) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index b0652e6db6d..a150011c13c 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -69,6 +69,7 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { } assert.Equal(t, j[0], testJob) + assert.Equal(t, res.Truncations["job"], false) }) } @@ -152,6 +153,7 @@ func TestHTTP_ResoucesListForEvaluations(t *testing.T) { assert.Contains(t, j, eval1.ID) assert.NotContains(t, j, eval2.ID) + assert.Equal(t, res.Truncations["eval"], false) }) } diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 7414fe9aa46..c7d1e1cec68 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -28,6 +28,7 @@ func (r *Resources) List(args *structs.ResourcesRequest, // return jobs matching given prefix var err error var iter memdb.ResultIterator + truncations := make(map[string]bool) if args.Context == "job" { iter, err = state.JobsByIDPrefix(ws, args.Prefix) @@ -46,6 +47,10 @@ func (r *Resources) List(args *structs.ResourcesRequest, jobs = append(jobs, job.ID) } + if iter.Next() != nil { + truncations["job"] = true + } + matches["job"] = jobs } @@ -66,10 +71,15 @@ func (r *Resources) List(args *structs.ResourcesRequest, evals = append(evals, eval.ID) } + if iter.Next() != nil { + truncations["eval"] = true + } + matches["eval"] = evals } reply.Matches = matches + reply.Truncations = truncations return nil }} diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 5e64c015648..fa97a9ed6c1 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -86,6 +86,8 @@ func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { if num_matches != 20 { t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs returned %d", 20, num_matches)) } + + assert.Equal(t, resp.Truncations["job"], true) } func TestResourcesEndpoint_List_ShouldReturnEvals(t *testing.T) { @@ -122,4 +124,6 @@ func TestResourcesEndpoint_List_ShouldReturnEvals(t *testing.T) { if recEval != eval1.ID { t.Fatalf(fmt.Sprintf("err: expected %s evaluation but received %s", eval1.ID, recEval)) } + + assert.Equal(t, resp.Truncations["job"], false) } From a9ed750a99bff315d0c1b1afdb83baacb5dde377 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 2 Aug 2017 20:05:10 +0000 Subject: [PATCH 11/27] use upsert as a test helper --- command/agent/resources_endpoint_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index a150011c13c..5bed0dd0cf5 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -28,12 +28,10 @@ func createJobForTest(jobID string, s *TestAgent, t *testing.T) { job := mock.Job() job.ID = jobID job.TaskGroups[0].Count = 1 - args := structs.JobRegisterRequest{ - Job: job, - WriteRequest: structs.WriteRequest{Region: "global"}, - } - var resp structs.JobRegisterResponse - if err := s.Agent.RPC("Job.Register", &args, &resp); err != nil { + + state := s.Agent.server.State() + err := state.UpsertJob(1000, job) + if err != nil { t.Fatalf("err: %v", err) } } From ed8e707659ce30bd7e7144256a181985924f10e4 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 2 Aug 2017 21:55:48 +0000 Subject: [PATCH 12/27] refactor to remove duplication for types of resources --- nomad/resources_endpoint.go | 97 ++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 50 deletions(-) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index c7d1e1cec68..9c7fa0582ba 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -10,14 +10,48 @@ type Resources struct { srv *Server } +func getMatches(iter memdb.ResultIterator, context, prefix string) ([]string, bool) { + var matches []string + isTruncated := false + + for i := 0; i < 20; i++ { + raw := iter.Next() + if raw == nil { + break + } + + getID := func(i interface{}) string { + switch i.(type) { + case *structs.Job: + return i.(*structs.Job).ID + case *structs.Evaluation: + return i.(*structs.Evaluation).ID + default: + return "" + } + } + + id := getID(raw) + if id == "" { + continue + } + + matches = append(matches, id) + } + + if iter.Next() != nil { + isTruncated = true + } + + return matches, isTruncated +} + // List is used to list the jobs registered in the system -// TODO logic to determine context, to return only that context if needed // TODO if no context, return all -// TODO refactor to prevent duplication func (r *Resources) List(args *structs.ResourcesRequest, reply *structs.ResourcesResponse) error { - - matches := make(map[string][]string) + reply.Matches = make(map[string][]string) + reply.Truncations = make(map[string]bool) // Setup the blocking query opts := blockingOptions{ @@ -28,58 +62,21 @@ func (r *Resources) List(args *structs.ResourcesRequest, // return jobs matching given prefix var err error var iter memdb.ResultIterator - truncations := make(map[string]bool) + res := make([]string, 0) + isTrunc := false if args.Context == "job" { iter, err = state.JobsByIDPrefix(ws, args.Prefix) - if err != nil { - return err - } - - var jobs []string - for i := 0; i < 20; i++ { - raw := iter.Next() - if raw == nil { - break - } - - job := raw.(*structs.Job) - jobs = append(jobs, job.ID) - } - - if iter.Next() != nil { - truncations["job"] = true - } - - matches["job"] = jobs - } - - if args.Context == "eval" { + } else if args.Context == "eval" { iter, err = state.EvalsByIDPrefix(ws, args.Prefix) - if err != nil { - return err - } - - var evals []string - for i := 0; i < 20; i++ { // TODO extract magic number - raw := iter.Next() - if raw == nil { - break - } - - eval := raw.(*structs.Evaluation) - evals = append(evals, eval.ID) - } - - if iter.Next() != nil { - truncations["eval"] = true - } - - matches["eval"] = evals } - reply.Matches = matches - reply.Truncations = truncations + if err != nil { + return err + } + res, isTrunc = getMatches(iter, args.Context, args.Prefix) + reply.Matches[args.Context] = res + reply.Truncations[args.Context] = isTrunc return nil }} From a9728c39856bd8dd5cefe25ad12292750a2026ea Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 3 Aug 2017 14:28:38 +0000 Subject: [PATCH 13/27] adding allocations to resouces list endpoint adding nodes to resources list endpoint --- nomad/resources_endpoint.go | 9 ++++ nomad/resources_endpoint_test.go | 88 ++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 9c7fa0582ba..1d459071ab3 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -26,6 +26,10 @@ func getMatches(iter memdb.ResultIterator, context, prefix string) ([]string, bo return i.(*structs.Job).ID case *structs.Evaluation: return i.(*structs.Evaluation).ID + case *structs.Allocation: + return i.(*structs.Allocation).ID + case *structs.Node: + return i.(*structs.Node).ID default: return "" } @@ -69,11 +73,16 @@ func (r *Resources) List(args *structs.ResourcesRequest, iter, err = state.JobsByIDPrefix(ws, args.Prefix) } else if args.Context == "eval" { iter, err = state.EvalsByIDPrefix(ws, args.Prefix) + } else if args.Context == "alloc" { + iter, err = state.AllocsByIDPrefix(ws, args.Prefix) + } else if args.Context == "node" { + iter, err = state.NodesByIDPrefix(ws, args.Prefix) } if err != nil { return err } + res, isTrunc = getMatches(iter, args.Context, args.Prefix) reply.Matches[args.Context] = res reply.Truncations[args.Context] = isTrunc diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index fa97a9ed6c1..75e90a42c32 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -127,3 +127,91 @@ func TestResourcesEndpoint_List_ShouldReturnEvals(t *testing.T) { assert.Equal(t, resp.Truncations["job"], false) } + +func TestResourcesEndpoint_List_Allocation(t *testing.T) { + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + alloc := mock.Alloc() + summary := mock.JobSummary(alloc.JobID) + state := s.fsm.State() + + if err := state.UpsertJobSummary(999, summary); err != nil { + t.Fatalf("err: %v", err) + } + if err := state.UpsertAllocs(1000, []*structs.Allocation{alloc}); err != nil { + t.Fatalf("err: %v", err) + } + + prefix := alloc.ID[:len(alloc.ID)-2] + + req := &structs.ResourcesRequest{ + Prefix: prefix, + Context: "alloc", + } + + var resp structs.ResourcesResponse + if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + numMatches := len(resp.Matches["alloc"]) + if numMatches != 1 { + t.Fatalf(fmt.Sprintf("err: the number of allocations expected %d does not match the number expected %d", 1, numMatches)) + } + + recAlloc := resp.Matches["alloc"][0] + if recAlloc != alloc.ID { + t.Fatalf(fmt.Sprintf("err: expected %s allocation but received %s", alloc.ID, recAlloc)) + } + + assert.Equal(t, resp.Truncations["alloc"], false) +} + +func TestResourcesEndpoint_List_Node(t *testing.T) { + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + state := s.fsm.State() + node := mock.Node() + + if err := state.UpsertNode(100, node); err != nil { + t.Fatalf("err: %v", err) + } + + prefix := node.ID[:len(node.ID)-2] + + req := &structs.ResourcesRequest{ + Prefix: prefix, + Context: "node", + } + + var resp structs.ResourcesResponse + if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + numMatches := len(resp.Matches["node"]) + if numMatches != 1 { + t.Fatalf(fmt.Sprintf("err: the number of nodes expected %d does not match the number expected %d", 1, numMatches)) + } + + recNode := resp.Matches["node"][0] + if recNode != node.ID { + t.Fatalf(fmt.Sprintf("err: expected %s node but received %s", node.ID, recNode)) + } + + assert.Equal(t, resp.Truncations["node"], false) +} From a3234b2317d52d188e902d88b85ec63a0c8c1b6b Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 3 Aug 2017 14:47:20 +0000 Subject: [PATCH 14/27] refactor and add error handling for invalid context type --- command/agent/resources_endpoint_test.go | 6 ++--- nomad/resources_endpoint.go | 12 ++++++--- nomad/resources_endpoint_test.go | 31 ++++++++++++++++++++---- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index 5bed0dd0cf5..b07a192af99 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -36,7 +36,7 @@ func createJobForTest(jobID string, s *TestAgent, t *testing.T) { } } -func TestHTTP_ResourcesWithSingleJob(t *testing.T) { +func TestHTTP_Resources_SingleJob(t *testing.T) { testJob := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" testJobPrefix := "aaaaaaaa-e8f7-fd38" t.Parallel() @@ -71,7 +71,7 @@ func TestHTTP_ResourcesWithSingleJob(t *testing.T) { }) } -func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { +func TestHTTP_Resources_MultipleJobs(t *testing.T) { testJobA := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" testJobB := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89707" testJobC := "bbbbbbbb-e8f7-fd38-c855-ab94ceb89707" @@ -113,7 +113,7 @@ func TestHTTP_ResourcesWithMultipleJobs(t *testing.T) { }) } -func TestHTTP_ResoucesListForEvaluations(t *testing.T) { +func TestHTTP_ResoucesList_Evaluation(t *testing.T) { t.Parallel() httpTest(t, nil, func(s *TestAgent) { state := s.Agent.server.State() diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 1d459071ab3..4056bb48d9b 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -1,6 +1,7 @@ package nomad import ( + "fmt" "github.com/hashicorp/go-memdb" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" @@ -69,14 +70,17 @@ func (r *Resources) List(args *structs.ResourcesRequest, res := make([]string, 0) isTrunc := false - if args.Context == "job" { + switch args.Context { + case "job": iter, err = state.JobsByIDPrefix(ws, args.Prefix) - } else if args.Context == "eval" { + case "eval": iter, err = state.EvalsByIDPrefix(ws, args.Prefix) - } else if args.Context == "alloc" { + case "alloc": iter, err = state.AllocsByIDPrefix(ws, args.Prefix) - } else if args.Context == "node" { + case "node": iter, err = state.NodesByIDPrefix(ws, args.Prefix) + default: + return fmt.Errorf("invalid context") } if err != nil { diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 75e90a42c32..5dfeb597862 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -29,7 +29,7 @@ func TestResourcesEndpoint_List(t *testing.T) { t.Parallel() s := testServer(t, func(c *Config) { - c.NumSchedulers = 0 // Prevent automatic dequeue + c.NumSchedulers = 0 }) defer s.Shutdown() @@ -56,12 +56,13 @@ func TestResourcesEndpoint_List(t *testing.T) { assert.Equal(t, jobID, resp.Matches["job"][0]) } -func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { +// truncate should limit results to 20 +func TestResourcesEndpoint_List_Truncate(t *testing.T) { prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" t.Parallel() s := testServer(t, func(c *Config) { - c.NumSchedulers = 0 // Prevent automatic dequeue + c.NumSchedulers = 0 }) defer s.Shutdown() @@ -90,10 +91,10 @@ func TestResourcesEndpoint_List_ShouldTruncateResultsToUnder20(t *testing.T) { assert.Equal(t, resp.Truncations["job"], true) } -func TestResourcesEndpoint_List_ShouldReturnEvals(t *testing.T) { +func TestResourcesEndpoint_List_Evals(t *testing.T) { t.Parallel() s := testServer(t, func(c *Config) { - c.NumSchedulers = 0 // Prevent automatic dequeue + c.NumSchedulers = 0 }) defer s.Shutdown() @@ -215,3 +216,23 @@ func TestResourcesEndpoint_List_Node(t *testing.T) { assert.Equal(t, resp.Truncations["node"], false) } + +func TestResourcesEndpoint_List_InvalidContext(t *testing.T) { + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + req := &structs.ResourcesRequest{ + Prefix: "anyPrefix", + Context: "invalid", + } + + var resp structs.ResourcesResponse + err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp) + assert.Equal(t, err.Error(), "invalid context") +} From 760f4299166ac99d7f5f97c3a1ece0d37accb922 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 3 Aug 2017 15:12:14 +0000 Subject: [PATCH 15/27] resources list endpoint accepts http POST and PUT set the index for a resources response --- command/agent/resources_endpoint.go | 6 ++-- command/agent/resources_endpoint_test.go | 37 +++++++++++++++++++++++- nomad/resources_endpoint.go | 7 +++++ 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/command/agent/resources_endpoint.go b/command/agent/resources_endpoint.go index 1c1c581b73b..93f610209be 100644 --- a/command/agent/resources_endpoint.go +++ b/command/agent/resources_endpoint.go @@ -6,12 +6,10 @@ import ( ) func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - switch req.Method { - case "POST": + if req.Method == "POST" || req.Method == "PUT" { return s.resourcesRequest(resp, req) - default: - return nil, CodedError(405, ErrInvalidMethod) } + return nil, CodedError(405, ErrInvalidMethod) } func (s *HTTPServer) resourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index b07a192af99..c6ba32ce7bc 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -36,7 +36,7 @@ func createJobForTest(jobID string, s *TestAgent, t *testing.T) { } } -func TestHTTP_Resources_SingleJob(t *testing.T) { +func TestHTTP_Resources_POST(t *testing.T) { testJob := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" testJobPrefix := "aaaaaaaa-e8f7-fd38" t.Parallel() @@ -71,6 +71,41 @@ func TestHTTP_Resources_SingleJob(t *testing.T) { }) } +func TestHTTP_Resources_PUT(t *testing.T) { + testJob := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" + testJobPrefix := "aaaaaaaa-e8f7-fd38" + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + createJobForTest(testJob, s, t) + + data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "job"} + req, err := http.NewRequest("PUT", "/v1/resources", encodeReq(data)) + + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + resp, err := s.Server.ResourcesRequest(respW, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + res := resp.(structs.ResourcesResponse) + if len(res.Matches) != 1 { + t.Fatalf("No expected key values in resources list") + } + + j := res.Matches["job"] + if j == nil || len(j) != 1 { + t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (1)", j) + } + + assert.Equal(t, j[0], testJob) + assert.Equal(t, res.Truncations["job"], false) + }) +} + func TestHTTP_Resources_MultipleJobs(t *testing.T) { testJobA := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" testJobB := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89707" diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 4056bb48d9b..0cea578068e 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -91,6 +91,13 @@ func (r *Resources) List(args *structs.ResourcesRequest, reply.Matches[args.Context] = res reply.Truncations[args.Context] = isTrunc + // Use the last index that affected the table + index, err := state.Index(args.Context) + if err != nil { + return err + } + reply.Index = index + return nil }} return r.srv.blockingRPC(&opts) From c9c7b1914c0069e12c30ae77a09791f20d32e01d Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 3 Aug 2017 15:59:27 +0000 Subject: [PATCH 16/27] refactor rpc endpoint and tests add test for when no prefixes are matched add test for no context at HTTP api --- command/agent/resources_endpoint_test.go | 103 +++++++++++++++++------ nomad/resources_endpoint.go | 78 +++++++++-------- nomad/resources_endpoint_test.go | 91 +++++++++++--------- 3 files changed, 171 insertions(+), 101 deletions(-) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index c6ba32ce7bc..60662f9ca6e 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -57,15 +57,12 @@ func TestHTTP_Resources_POST(t *testing.T) { } res := resp.(structs.ResourcesResponse) - if len(res.Matches) != 1 { - t.Fatalf("No expected key values in resources list") - } + + assert.Equal(t, 1, len(res.Matches)) j := res.Matches["job"] - if j == nil || len(j) != 1 { - t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (1)", j) - } + assert.Equal(t, 1, len(j)) assert.Equal(t, j[0], testJob) assert.Equal(t, res.Truncations["job"], false) }) @@ -92,15 +89,12 @@ func TestHTTP_Resources_PUT(t *testing.T) { } res := resp.(structs.ResourcesResponse) - if len(res.Matches) != 1 { - t.Fatalf("No expected key values in resources list") - } + + assert.Equal(t, 1, len(res.Matches)) j := res.Matches["job"] - if j == nil || len(j) != 1 { - t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (1)", j) - } + assert.Equal(t, 1, len(j)) assert.Equal(t, j[0], testJob) assert.Equal(t, res.Truncations["job"], false) }) @@ -133,15 +127,12 @@ func TestHTTP_Resources_MultipleJobs(t *testing.T) { } res := resp.(structs.ResourcesResponse) - if len(res.Matches) != 1 { - t.Fatalf("No expected key values in resources list") - } + + assert.Equal(t, 1, len(res.Matches)) j := res.Matches["job"] - if j == nil || len(j) != 2 { - t.Fatalf("The number of jobs that were returned does not equal the number of jobs we expected (2)", j) - } + assert.Equal(t, 2, len(j)) assert.Contains(t, j, testJobA) assert.Contains(t, j, testJobB) assert.NotContains(t, j, testJobC) @@ -175,21 +166,79 @@ func TestHTTP_ResoucesList_Evaluation(t *testing.T) { } res := resp.(structs.ResourcesResponse) - if len(res.Matches) != 1 { - t.Fatalf("No expected key values in resources list") - } + + assert.Equal(t, 1, len(res.Matches)) j := res.Matches["eval"] - if len(j) != 1 { - t.Fatalf("The number of evaluations that were returned does not equal the number we expected (1)", j) - } + assert.Equal(t, 1, len(j)) assert.Contains(t, j, eval1.ID) assert.NotContains(t, j, eval2.ID) assert.Equal(t, res.Truncations["eval"], false) }) } -// TODO -//func TestHTTP_ResourcesWithNoJob(t *testing.T) { -//} +func TestHTTP_Resources_NoJob(t *testing.T) { + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + data := structs.ResourcesRequest{Prefix: "12345", Context: "job"} + req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + resp, err := s.Server.ResourcesRequest(respW, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + res := resp.(structs.ResourcesResponse) + + assert.Equal(t, 1, len(res.Matches)) + assert.Equal(t, 0, len(res.Matches["jobs"])) + }) +} + +func TestHTTP_Resources_NoContext(t *testing.T) { + testJobID := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" + testJobPrefix := "aaaaaaaa-e8f7-fd38" + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + createJobForTest(testJobID, s, t) + + state := s.Agent.server.State() + eval1 := mock.Eval() + eval1.ID = testJobID + err := state.UpsertEvals(1000, + []*structs.Evaluation{eval1}) + if err != nil { + t.Fatalf("err: %v", err) + } + + data := structs.ResourcesRequest{Prefix: testJobPrefix} + req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + + if err != nil { + t.Fatalf("err: %v", err) + } + respW := httptest.NewRecorder() + + resp, err := s.Server.ResourcesRequest(respW, req) + if err != nil { + t.Fatalf("err: %v", err) + } + + res := resp.(structs.ResourcesResponse) + + matchedJobs := res.Matches["job"] + matchedEvals := res.Matches["eval"] + + assert.Equal(t, 1, len(matchedJobs)) + assert.Equal(t, 1, len(matchedEvals)) + + assert.Equal(t, matchedJobs[0], testJobID) + assert.Equal(t, matchedEvals[0], eval1.ID) + }) +} diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 0cea578068e..db0f7073b93 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -11,7 +11,7 @@ type Resources struct { srv *Server } -func getMatches(iter memdb.ResultIterator, context, prefix string) ([]string, bool) { +func getMatches(iter memdb.ResultIterator) ([]string, bool) { var matches []string isTruncated := false @@ -21,23 +21,23 @@ func getMatches(iter memdb.ResultIterator, context, prefix string) ([]string, bo break } - getID := func(i interface{}) string { + getID := func(i interface{}) (string, error) { switch i.(type) { case *structs.Job: - return i.(*structs.Job).ID + return i.(*structs.Job).ID, nil case *structs.Evaluation: - return i.(*structs.Evaluation).ID + return i.(*structs.Evaluation).ID, nil case *structs.Allocation: - return i.(*structs.Allocation).ID + return i.(*structs.Allocation).ID, nil case *structs.Node: - return i.(*structs.Node).ID + return i.(*structs.Node).ID, nil default: - return "" + return "", fmt.Errorf("invalid context") } } - id := getID(raw) - if id == "" { + id, err := getID(raw) + if err != nil { continue } @@ -51,8 +51,22 @@ func getMatches(iter memdb.ResultIterator, context, prefix string) ([]string, bo return matches, isTruncated } +func getResourceIter(context, prefix string, ws memdb.WatchSet, state *state.StateStore) (memdb.ResultIterator, error) { + switch context { + case "job": + return state.JobsByIDPrefix(ws, prefix) + case "eval": + return state.EvalsByIDPrefix(ws, prefix) + case "alloc": + return state.AllocsByIDPrefix(ws, prefix) + case "node": + return state.NodesByIDPrefix(ws, prefix) + default: + return nil, fmt.Errorf("invalid context") + } +} + // List is used to list the jobs registered in the system -// TODO if no context, return all func (r *Resources) List(args *structs.ResourcesRequest, reply *structs.ResourcesResponse) error { reply.Matches = make(map[string][]string) @@ -64,33 +78,31 @@ func (r *Resources) List(args *structs.ResourcesRequest, queryOpts: &structs.QueryOptions{}, run: func(ws memdb.WatchSet, state *state.StateStore) error { - // return jobs matching given prefix - var err error - var iter memdb.ResultIterator - res := make([]string, 0) - isTrunc := false - - switch args.Context { - case "job": - iter, err = state.JobsByIDPrefix(ws, args.Prefix) - case "eval": - iter, err = state.EvalsByIDPrefix(ws, args.Prefix) - case "alloc": - iter, err = state.AllocsByIDPrefix(ws, args.Prefix) - case "node": - iter, err = state.NodesByIDPrefix(ws, args.Prefix) - default: - return fmt.Errorf("invalid context") + iters := make(map[string]memdb.ResultIterator) + + if args.Context != "" { + iter, err := getResourceIter(args.Context, args.Prefix, ws, state) + if err != nil { + return err + } + iters[args.Context] = iter + } else { + for _, e := range []string{"alloc", "node", "job", "eval"} { + iter, err := getResourceIter(e, args.Prefix, ws, state) + if err != nil { + return err + } + iters[e] = iter + } } - if err != nil { - return err + // return jobs matching given prefix + for k, v := range iters { + res, isTrunc := getMatches(v) + reply.Matches[k] = res + reply.Truncations[k] = isTrunc } - res, isTrunc = getMatches(iter, args.Context, args.Prefix) - reply.Matches[args.Context] = res - reply.Truncations[args.Context] = isTrunc - // Use the last index that affected the table index, err := state.Index(args.Context) if err != nil { diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 5dfeb597862..cf54846c7ad 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -1,7 +1,6 @@ package nomad import ( - "fmt" "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -48,11 +47,7 @@ func TestResourcesEndpoint_List(t *testing.T) { t.Fatalf("err: %v", err) } - num_matches := len(resp.Matches["job"]) - if num_matches != 1 { - t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs registered %d", 1, num_matches)) - } - + assert.Equal(t, 1, len(resp.Matches["job"])) assert.Equal(t, jobID, resp.Matches["job"][0]) } @@ -83,11 +78,7 @@ func TestResourcesEndpoint_List_Truncate(t *testing.T) { t.Fatalf("err: %v", err) } - num_matches := len(resp.Matches["job"]) - if num_matches != 20 { - t.Fatalf(fmt.Sprintf("err: the number of jobs expected %d does not match the number of jobs returned %d", 20, num_matches)) - } - + assert.Equal(t, 20, len(resp.Matches["job"])) assert.Equal(t, resp.Truncations["job"], true) } @@ -116,16 +107,8 @@ func TestResourcesEndpoint_List_Evals(t *testing.T) { t.Fatalf("err: %v", err) } - numMatches := len(resp.Matches["eval"]) - if numMatches != 1 { - t.Fatalf(fmt.Sprintf("err: the number of evaluations expected %d does not match the number expected %d", 1, numMatches)) - } - - recEval := resp.Matches["eval"][0] - if recEval != eval1.ID { - t.Fatalf(fmt.Sprintf("err: expected %s evaluation but received %s", eval1.ID, recEval)) - } - + assert.Equal(t, 1, len(resp.Matches["eval"])) + assert.Equal(t, eval1.ID, resp.Matches["eval"][0]) assert.Equal(t, resp.Truncations["job"], false) } @@ -162,16 +145,8 @@ func TestResourcesEndpoint_List_Allocation(t *testing.T) { t.Fatalf("err: %v", err) } - numMatches := len(resp.Matches["alloc"]) - if numMatches != 1 { - t.Fatalf(fmt.Sprintf("err: the number of allocations expected %d does not match the number expected %d", 1, numMatches)) - } - - recAlloc := resp.Matches["alloc"][0] - if recAlloc != alloc.ID { - t.Fatalf(fmt.Sprintf("err: expected %s allocation but received %s", alloc.ID, recAlloc)) - } - + assert.Equal(t, 1, len(resp.Matches["alloc"])) + assert.Equal(t, alloc.ID, resp.Matches["alloc"][0]) assert.Equal(t, resp.Truncations["alloc"], false) } @@ -204,16 +179,8 @@ func TestResourcesEndpoint_List_Node(t *testing.T) { t.Fatalf("err: %v", err) } - numMatches := len(resp.Matches["node"]) - if numMatches != 1 { - t.Fatalf(fmt.Sprintf("err: the number of nodes expected %d does not match the number expected %d", 1, numMatches)) - } - - recNode := resp.Matches["node"][0] - if recNode != node.ID { - t.Fatalf(fmt.Sprintf("err: expected %s node but received %s", node.ID, recNode)) - } - + assert.Equal(t, 1, len(resp.Matches["node"])) + assert.Equal(t, node.ID, resp.Matches["node"][0]) assert.Equal(t, resp.Truncations["node"], false) } @@ -236,3 +203,45 @@ func TestResourcesEndpoint_List_InvalidContext(t *testing.T) { err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp) assert.Equal(t, err.Error(), "invalid context") } + +func TestResourcesEndpoint_List_NoContext(t *testing.T) { + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + state := s.fsm.State() + node := mock.Node() + + if err := state.UpsertNode(100, node); err != nil { + t.Fatalf("err: %v", err) + } + + eval1 := mock.Eval() + eval1.ID = node.ID + if err := state.UpsertEvals(1000, []*structs.Evaluation{eval1}); err != nil { + t.Fatalf("err: %v", err) + } + + prefix := node.ID[:len(node.ID)-2] + + req := &structs.ResourcesRequest{ + Prefix: prefix, + Context: "", + } + + var resp structs.ResourcesResponse + if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + assert.Equal(t, 1, len(resp.Matches["node"])) + assert.Equal(t, 1, len(resp.Matches["eval"])) + + assert.Equal(t, node.ID, resp.Matches["node"][0]) + assert.Equal(t, eval1.ID, resp.Matches["eval"][0]) +} From 9947939ca7b7298a01fd5377c5c4748097b3e36c Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 3 Aug 2017 20:34:56 +0000 Subject: [PATCH 17/27] add documentation extract magic number into variable --- command/agent/resources_endpoint.go | 5 +++-- nomad/resources_endpoint.go | 14 ++++++++++++-- nomad/structs/structs.go | 2 ++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/command/agent/resources_endpoint.go b/command/agent/resources_endpoint.go index 93f610209be..40106d3c541 100644 --- a/command/agent/resources_endpoint.go +++ b/command/agent/resources_endpoint.go @@ -5,6 +5,8 @@ import ( "net/http" ) +// ResourcesRequest accepts a prefix and context and returns a list of matching +// prefixes for that context. func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { if req.Method == "POST" || req.Method == "PUT" { return s.resourcesRequest(resp, req) @@ -13,10 +15,8 @@ func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Reques } func (s *HTTPServer) resourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - // TODO test a failure case for this? args := structs.ResourcesRequest{} - // TODO this should be tested if err := decodeBody(req, &args); err != nil { return nil, CodedError(400, err.Error()) } @@ -26,5 +26,6 @@ func (s *HTTPServer) resourcesRequest(resp http.ResponseWriter, req *http.Reques return nil, err } + setMeta(resp, &out.QueryMeta) return out, nil } diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index db0f7073b93..350eff3c837 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -7,15 +7,22 @@ import ( "github.com/hashicorp/nomad/nomad/structs" ) +// truncateLimit is the maximum number of matches that will be returned for a +// prefix for a specific context +const truncateLimit = 20 + +// Resource endpoint is used to lookup matches for a given prefix and context type Resources struct { srv *Server } +// getMatches extracts matches for an iterator, and returns a list of ids for +// these matches. func getMatches(iter memdb.ResultIterator) ([]string, bool) { var matches []string isTruncated := false - for i := 0; i < 20; i++ { + for i := 0; i < truncateLimit; i++ { raw := iter.Next() if raw == nil { break @@ -51,6 +58,8 @@ func getMatches(iter memdb.ResultIterator) ([]string, bool) { return matches, isTruncated } +// getResourceIter takes a context and returns a memdb iterator specific to +// that context func getResourceIter(context, prefix string, ws memdb.WatchSet, state *state.StateStore) (memdb.ResultIterator, error) { switch context { case "job": @@ -66,7 +75,8 @@ func getResourceIter(context, prefix string, ws memdb.WatchSet, state *state.Sta } } -// List is used to list the jobs registered in the system +// 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 { reply.Matches = make(map[string][]string) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 572536749fb..774cccf5317 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -231,6 +231,8 @@ type NodeSpecificRequest struct { QueryOptions } +// ResourcesResponse is used to return matches and information about whether +// the match list is truncated, specific to each type of context. type ResourcesResponse struct { Matches map[string][]string Truncations map[string]bool From 01640d879153dceafe2a8a3657cbbc47cfa7af70 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 3 Aug 2017 22:39:56 +0000 Subject: [PATCH 18/27] set response index and meta information add tests for edge cases --- nomad/resources_endpoint.go | 18 +++++++-- nomad/resources_endpoint_test.go | 68 ++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 350eff3c837..f0c165d8736 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -106,20 +106,32 @@ func (r *Resources) List(args *structs.ResourcesRequest, } } - // return jobs matching given prefix + // Return jobs matching given prefix for k, v := range iters { res, isTrunc := getMatches(v) reply.Matches[k] = res reply.Truncations[k] = isTrunc } - // Use the last index that affected the table - index, err := state.Index(args.Context) + // Set the index of the context if it is specified. Otherwise, set the + // index of the first non-empty match set. + var index uint64 + var err error + if args.Context != "" { + index, err = state.Index(args.Context) + } else { + for k, v := range reply.Matches { + if len(v) != 0 { + index, err = state.Index(k) + } + } + } if err != nil { return err } reply.Index = index + r.srv.setQueryMeta(&reply.QueryMeta) return nil }} return r.srv.blockingRPC(&opts) diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index cf54846c7ad..4b1e75343a0 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -49,6 +49,7 @@ func TestResourcesEndpoint_List(t *testing.T) { assert.Equal(t, 1, len(resp.Matches["job"])) assert.Equal(t, jobID, resp.Matches["job"][0]) + assert.NotEqual(t, 0, resp.Index) } // truncate should limit results to 20 @@ -80,6 +81,7 @@ func TestResourcesEndpoint_List_Truncate(t *testing.T) { assert.Equal(t, 20, len(resp.Matches["job"])) assert.Equal(t, resp.Truncations["job"], true) + assert.NotEqual(t, 0, resp.Index) } func TestResourcesEndpoint_List_Evals(t *testing.T) { @@ -110,6 +112,8 @@ func TestResourcesEndpoint_List_Evals(t *testing.T) { assert.Equal(t, 1, len(resp.Matches["eval"])) assert.Equal(t, eval1.ID, resp.Matches["eval"][0]) assert.Equal(t, resp.Truncations["job"], false) + + assert.NotEqual(t, 0, resp.Index) } func TestResourcesEndpoint_List_Allocation(t *testing.T) { @@ -148,6 +152,8 @@ func TestResourcesEndpoint_List_Allocation(t *testing.T) { assert.Equal(t, 1, len(resp.Matches["alloc"])) assert.Equal(t, alloc.ID, resp.Matches["alloc"][0]) assert.Equal(t, resp.Truncations["alloc"], false) + + assert.NotEqual(t, 0, resp.Index) } func TestResourcesEndpoint_List_Node(t *testing.T) { @@ -202,6 +208,8 @@ func TestResourcesEndpoint_List_InvalidContext(t *testing.T) { var resp structs.ResourcesResponse err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp) assert.Equal(t, err.Error(), "invalid context") + + assert.NotEqual(t, 0, resp.Index) } func TestResourcesEndpoint_List_NoContext(t *testing.T) { @@ -244,4 +252,64 @@ func TestResourcesEndpoint_List_NoContext(t *testing.T) { assert.Equal(t, node.ID, resp.Matches["node"][0]) assert.Equal(t, eval1.ID, resp.Matches["eval"][0]) + + assert.NotEqual(t, 0, resp.Index) +} + +// Tests that the top 20 matches are returned when no prefix is set +func TestResourcesEndpoint_List_NoPrefix(t *testing.T) { + prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" + + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + jobID := registerAndVerifyJob(s, t, prefix, 0) + + req := &structs.ResourcesRequest{ + Prefix: "", + Context: "job", + } + + var resp structs.ResourcesResponse + if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + assert.Equal(t, 1, len(resp.Matches["job"])) + assert.Equal(t, jobID, resp.Matches["job"][0]) + assert.NotEqual(t, 0, resp.Index) +} + +// Tests that the zero matches are returned when a prefix has no matching +// results +func TestResourcesEndpoint_List_NoMatches(t *testing.T) { + prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" + + t.Parallel() + s := testServer(t, func(c *Config) { + c.NumSchedulers = 0 + }) + + defer s.Shutdown() + codec := rpcClient(t, s) + testutil.WaitForLeader(t, s.RPC) + + req := &structs.ResourcesRequest{ + Prefix: prefix, + Context: "job", + } + + var resp structs.ResourcesResponse + if err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp); err != nil { + t.Fatalf("err: %v", err) + } + + assert.Equal(t, 0, len(resp.Matches["job"])) + assert.Equal(t, uint64(0), resp.Index) } From ca4ed646a9083fedfd1d2cc7f57f0b8d93fe5ff9 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 4 Aug 2017 14:18:46 +0000 Subject: [PATCH 19/27] resources are expected by state store to be plural --- command/agent/resources_endpoint.go | 2 +- command/agent/resources_endpoint_test.go | 43 +++++++------ nomad/resources_endpoint.go | 15 ++--- nomad/resources_endpoint_test.go | 78 +++++++++++++----------- 4 files changed, 76 insertions(+), 62 deletions(-) diff --git a/command/agent/resources_endpoint.go b/command/agent/resources_endpoint.go index 40106d3c541..36e0054fe49 100644 --- a/command/agent/resources_endpoint.go +++ b/command/agent/resources_endpoint.go @@ -6,7 +6,7 @@ import ( ) // ResourcesRequest accepts a prefix and context and returns a list of matching -// prefixes for that context. +// IDs for that context. func (s *HTTPServer) ResourcesRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) { if req.Method == "POST" || req.Method == "PUT" { return s.resourcesRequest(resp, req) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index 60662f9ca6e..f658cf9364c 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -43,7 +43,7 @@ func TestHTTP_Resources_POST(t *testing.T) { httpTest(t, nil, func(s *TestAgent) { createJobForTest(testJob, s, t) - data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "job"} + data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) if err != nil { @@ -60,11 +60,13 @@ func TestHTTP_Resources_POST(t *testing.T) { assert.Equal(t, 1, len(res.Matches)) - j := res.Matches["job"] + j := res.Matches["jobs"] assert.Equal(t, 1, len(j)) assert.Equal(t, j[0], testJob) + assert.Equal(t, res.Truncations["job"], false) + assert.NotEqual(t, "0", respW.HeaderMap.Get("X-Nomad-Index")) }) } @@ -75,7 +77,7 @@ func TestHTTP_Resources_PUT(t *testing.T) { httpTest(t, nil, func(s *TestAgent) { createJobForTest(testJob, s, t) - data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "job"} + data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("PUT", "/v1/resources", encodeReq(data)) if err != nil { @@ -92,11 +94,13 @@ func TestHTTP_Resources_PUT(t *testing.T) { assert.Equal(t, 1, len(res.Matches)) - j := res.Matches["job"] + j := res.Matches["jobs"] assert.Equal(t, 1, len(j)) assert.Equal(t, j[0], testJob) + assert.Equal(t, res.Truncations["job"], false) + assert.NotEqual(t, "0", respW.HeaderMap.Get("X-Nomad-Index")) }) } @@ -113,7 +117,7 @@ func TestHTTP_Resources_MultipleJobs(t *testing.T) { createJobForTest(testJobB, s, t) createJobForTest(testJobC, s, t) - data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "job"} + data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) if err != nil { @@ -130,12 +134,15 @@ func TestHTTP_Resources_MultipleJobs(t *testing.T) { assert.Equal(t, 1, len(res.Matches)) - j := res.Matches["job"] + j := res.Matches["jobs"] assert.Equal(t, 2, len(j)) assert.Contains(t, j, testJobA) assert.Contains(t, j, testJobB) assert.NotContains(t, j, testJobC) + + assert.Equal(t, res.Truncations["job"], false) + assert.NotEqual(t, "0", respW.HeaderMap.Get("X-Nomad-Index")) }) } @@ -145,15 +152,14 @@ func TestHTTP_ResoucesList_Evaluation(t *testing.T) { state := s.Agent.server.State() eval1 := mock.Eval() eval2 := mock.Eval() - err := state.UpsertEvals(1000, + err := state.UpsertEvals(9000, []*structs.Evaluation{eval1, eval2}) if err != nil { t.Fatalf("err: %v", err) } - // Make the HTTP request - evalPrefix := eval1.ID[:len(eval1.ID)-2] - data := structs.ResourcesRequest{Prefix: evalPrefix, Context: "eval"} + prefix := eval1.ID[:len(eval1.ID)-2] + data := structs.ResourcesRequest{Prefix: prefix, Context: "evals"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) if err != nil { t.Fatalf("err: %v", err) @@ -169,19 +175,20 @@ func TestHTTP_ResoucesList_Evaluation(t *testing.T) { assert.Equal(t, 1, len(res.Matches)) - j := res.Matches["eval"] - + j := res.Matches["evals"] assert.Equal(t, 1, len(j)) assert.Contains(t, j, eval1.ID) assert.NotContains(t, j, eval2.ID) - assert.Equal(t, res.Truncations["eval"], false) + + assert.Equal(t, res.Truncations["evals"], false) + assert.Equal(t, "9000", respW.HeaderMap.Get("X-Nomad-Index")) }) } func TestHTTP_Resources_NoJob(t *testing.T) { t.Parallel() httpTest(t, nil, func(s *TestAgent) { - data := structs.ResourcesRequest{Prefix: "12345", Context: "job"} + data := structs.ResourcesRequest{Prefix: "12345", Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) if err != nil { @@ -198,6 +205,8 @@ func TestHTTP_Resources_NoJob(t *testing.T) { assert.Equal(t, 1, len(res.Matches)) assert.Equal(t, 0, len(res.Matches["jobs"])) + + assert.Equal(t, "0", respW.HeaderMap.Get("X-Nomad-Index")) }) } @@ -211,7 +220,7 @@ func TestHTTP_Resources_NoContext(t *testing.T) { state := s.Agent.server.State() eval1 := mock.Eval() eval1.ID = testJobID - err := state.UpsertEvals(1000, + err := state.UpsertEvals(9000, []*structs.Evaluation{eval1}) if err != nil { t.Fatalf("err: %v", err) @@ -232,8 +241,8 @@ func TestHTTP_Resources_NoContext(t *testing.T) { res := resp.(structs.ResourcesResponse) - matchedJobs := res.Matches["job"] - matchedEvals := res.Matches["eval"] + matchedJobs := res.Matches["jobs"] + matchedEvals := res.Matches["evals"] assert.Equal(t, 1, len(matchedJobs)) assert.Equal(t, 1, len(matchedEvals)) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index f0c165d8736..cdf7f375779 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -39,7 +39,7 @@ func getMatches(iter memdb.ResultIterator) ([]string, bool) { case *structs.Node: return i.(*structs.Node).ID, nil default: - return "", fmt.Errorf("invalid context") + return "", fmt.Errorf("invalid type") } } @@ -62,13 +62,13 @@ func getMatches(iter memdb.ResultIterator) ([]string, bool) { // that context func getResourceIter(context, prefix string, ws memdb.WatchSet, state *state.StateStore) (memdb.ResultIterator, error) { switch context { - case "job": + case "jobs": return state.JobsByIDPrefix(ws, prefix) - case "eval": + case "evals": return state.EvalsByIDPrefix(ws, prefix) - case "alloc": + case "allocs": return state.AllocsByIDPrefix(ws, prefix) - case "node": + case "nodes": return state.NodesByIDPrefix(ws, prefix) default: return nil, fmt.Errorf("invalid context") @@ -97,7 +97,7 @@ func (r *Resources) List(args *structs.ResourcesRequest, } iters[args.Context] = iter } else { - for _, e := range []string{"alloc", "node", "job", "eval"} { + for _, e := range []string{"allocs", "nodes", "jobs", "evals"} { iter, err := getResourceIter(e, args.Prefix, ws, state) if err != nil { return err @@ -106,7 +106,7 @@ func (r *Resources) List(args *structs.ResourcesRequest, } } - // Return jobs matching given prefix + // Return matches for the given prefix for k, v := range iters { res, isTrunc := getMatches(v) reply.Matches[k] = res @@ -123,6 +123,7 @@ func (r *Resources) List(args *structs.ResourcesRequest, for k, v := range reply.Matches { if len(v) != 0 { index, err = state.Index(k) + break } } } diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 4b1e75343a0..a6774379be1 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -10,12 +10,14 @@ import ( "testing" ) +const jobIndex = 1000 + func registerAndVerifyJob(s *Server, t *testing.T, prefix string, counter int) string { job := mock.Job() job.ID = prefix + strconv.Itoa(counter) state := s.fsm.State() - err := state.UpsertJob(1000, job) + err := state.UpsertJob(jobIndex, job) if err != nil { t.Fatalf("err: %v", err) } @@ -39,7 +41,7 @@ func TestResourcesEndpoint_List(t *testing.T) { req := &structs.ResourcesRequest{ Prefix: prefix, - Context: "job", + Context: "jobs", } var resp structs.ResourcesResponse @@ -47,9 +49,9 @@ func TestResourcesEndpoint_List(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["job"])) - assert.Equal(t, jobID, resp.Matches["job"][0]) - assert.NotEqual(t, 0, resp.Index) + assert.Equal(t, 1, len(resp.Matches["jobs"])) + assert.Equal(t, jobID, resp.Matches["jobs"][0]) + assert.Equal(t, uint64(jobIndex), resp.Index) } // truncate should limit results to 20 @@ -71,7 +73,7 @@ func TestResourcesEndpoint_List_Truncate(t *testing.T) { req := &structs.ResourcesRequest{ Prefix: prefix, - Context: "job", + Context: "jobs", } var resp structs.ResourcesResponse @@ -79,9 +81,9 @@ func TestResourcesEndpoint_List_Truncate(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 20, len(resp.Matches["job"])) - assert.Equal(t, resp.Truncations["job"], true) - assert.NotEqual(t, 0, resp.Index) + assert.Equal(t, 20, len(resp.Matches["jobs"])) + assert.Equal(t, resp.Truncations["jobs"], true) + assert.Equal(t, uint64(jobIndex), resp.Index) } func TestResourcesEndpoint_List_Evals(t *testing.T) { @@ -95,13 +97,13 @@ func TestResourcesEndpoint_List_Evals(t *testing.T) { testutil.WaitForLeader(t, s.RPC) eval1 := mock.Eval() - s.fsm.State().UpsertEvals(1000, []*structs.Evaluation{eval1}) + s.fsm.State().UpsertEvals(2000, []*structs.Evaluation{eval1}) prefix := eval1.ID[:len(eval1.ID)-2] req := &structs.ResourcesRequest{ Prefix: prefix, - Context: "eval", + Context: "evals", } var resp structs.ResourcesResponse @@ -109,11 +111,11 @@ func TestResourcesEndpoint_List_Evals(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["eval"])) - assert.Equal(t, eval1.ID, resp.Matches["eval"][0]) - assert.Equal(t, resp.Truncations["job"], false) + assert.Equal(t, 1, len(resp.Matches["evals"])) + assert.Equal(t, eval1.ID, resp.Matches["evals"][0]) + assert.Equal(t, resp.Truncations["evals"], false) - assert.NotEqual(t, 0, resp.Index) + assert.Equal(t, uint64(2000), resp.Index) } func TestResourcesEndpoint_List_Allocation(t *testing.T) { @@ -133,7 +135,7 @@ func TestResourcesEndpoint_List_Allocation(t *testing.T) { if err := state.UpsertJobSummary(999, summary); err != nil { t.Fatalf("err: %v", err) } - if err := state.UpsertAllocs(1000, []*structs.Allocation{alloc}); err != nil { + if err := state.UpsertAllocs(90, []*structs.Allocation{alloc}); err != nil { t.Fatalf("err: %v", err) } @@ -141,7 +143,7 @@ func TestResourcesEndpoint_List_Allocation(t *testing.T) { req := &structs.ResourcesRequest{ Prefix: prefix, - Context: "alloc", + Context: "allocs", } var resp structs.ResourcesResponse @@ -149,11 +151,11 @@ func TestResourcesEndpoint_List_Allocation(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["alloc"])) - assert.Equal(t, alloc.ID, resp.Matches["alloc"][0]) - assert.Equal(t, resp.Truncations["alloc"], false) + assert.Equal(t, 1, len(resp.Matches["allocs"])) + assert.Equal(t, alloc.ID, resp.Matches["allocs"][0]) + assert.Equal(t, resp.Truncations["allocs"], false) - assert.NotEqual(t, 0, resp.Index) + assert.Equal(t, uint64(90), resp.Index) } func TestResourcesEndpoint_List_Node(t *testing.T) { @@ -177,7 +179,7 @@ func TestResourcesEndpoint_List_Node(t *testing.T) { req := &structs.ResourcesRequest{ Prefix: prefix, - Context: "node", + Context: "nodes", } var resp structs.ResourcesResponse @@ -185,9 +187,11 @@ func TestResourcesEndpoint_List_Node(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["node"])) - assert.Equal(t, node.ID, resp.Matches["node"][0]) - assert.Equal(t, resp.Truncations["node"], false) + assert.Equal(t, 1, len(resp.Matches["nodes"])) + assert.Equal(t, node.ID, resp.Matches["nodes"][0]) + assert.Equal(t, false, resp.Truncations["nodes"]) + + assert.Equal(t, uint64(100), resp.Index) } func TestResourcesEndpoint_List_InvalidContext(t *testing.T) { @@ -209,7 +213,7 @@ func TestResourcesEndpoint_List_InvalidContext(t *testing.T) { err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp) assert.Equal(t, err.Error(), "invalid context") - assert.NotEqual(t, 0, resp.Index) + assert.Equal(t, uint64(0), resp.Index) } func TestResourcesEndpoint_List_NoContext(t *testing.T) { @@ -247,13 +251,13 @@ func TestResourcesEndpoint_List_NoContext(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["node"])) - assert.Equal(t, 1, len(resp.Matches["eval"])) + assert.Equal(t, 1, len(resp.Matches["nodes"])) + assert.Equal(t, 1, len(resp.Matches["evals"])) - assert.Equal(t, node.ID, resp.Matches["node"][0]) - assert.Equal(t, eval1.ID, resp.Matches["eval"][0]) + assert.Equal(t, node.ID, resp.Matches["nodes"][0]) + assert.Equal(t, eval1.ID, resp.Matches["evals"][0]) - assert.NotEqual(t, 0, resp.Index) + assert.NotEqual(t, uint64(0), resp.Index) } // Tests that the top 20 matches are returned when no prefix is set @@ -273,7 +277,7 @@ func TestResourcesEndpoint_List_NoPrefix(t *testing.T) { req := &structs.ResourcesRequest{ Prefix: "", - Context: "job", + Context: "jobs", } var resp structs.ResourcesResponse @@ -281,9 +285,9 @@ func TestResourcesEndpoint_List_NoPrefix(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["job"])) - assert.Equal(t, jobID, resp.Matches["job"][0]) - assert.NotEqual(t, 0, resp.Index) + assert.Equal(t, 1, len(resp.Matches["jobs"])) + assert.Equal(t, jobID, resp.Matches["jobs"][0]) + assert.Equal(t, uint64(jobIndex), resp.Index) } // Tests that the zero matches are returned when a prefix has no matching @@ -302,7 +306,7 @@ func TestResourcesEndpoint_List_NoMatches(t *testing.T) { req := &structs.ResourcesRequest{ Prefix: prefix, - Context: "job", + Context: "jobs", } var resp structs.ResourcesResponse @@ -310,6 +314,6 @@ func TestResourcesEndpoint_List_NoMatches(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 0, len(resp.Matches["job"])) + assert.Equal(t, 0, len(resp.Matches["jobs"])) assert.Equal(t, uint64(0), resp.Index) } From 96a17234797517bf3d7c7ca4e09fabbe22be16db Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 4 Aug 2017 15:08:12 +0000 Subject: [PATCH 20/27] further refactoring --- nomad/resources_endpoint.go | 54 +++++++++++++++---------------------- nomad/structs/structs.go | 2 +- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index cdf7f375779..5b4b1c07547 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -28,23 +28,17 @@ func getMatches(iter memdb.ResultIterator) ([]string, bool) { break } - getID := func(i interface{}) (string, error) { - switch i.(type) { - case *structs.Job: - return i.(*structs.Job).ID, nil - case *structs.Evaluation: - return i.(*structs.Evaluation).ID, nil - case *structs.Allocation: - return i.(*structs.Allocation).ID, nil - case *structs.Node: - return i.(*structs.Node).ID, nil - default: - return "", fmt.Errorf("invalid type") - } - } - - id, err := getID(raw) - if err != nil { + var id string + switch raw.(type) { + case *structs.Job: + id = raw.(*structs.Job).ID + case *structs.Evaluation: + id = raw.(*structs.Evaluation).ID + case *structs.Allocation: + id = raw.(*structs.Allocation).ID + case *structs.Node: + id = raw.(*structs.Node).ID + default: continue } @@ -113,24 +107,20 @@ func (r *Resources) List(args *structs.ResourcesRequest, reply.Truncations[k] = isTrunc } - // Set the index of the context if it is specified. Otherwise, set the - // index of the first non-empty match set. - var index uint64 - var err error - if args.Context != "" { - index, err = state.Index(args.Context) - } else { - for k, v := range reply.Matches { - if len(v) != 0 { - index, err = state.Index(k) - break + // Set the index for the context. If the context has been specified, it + // is the only non-empty match set, and the index is set for it. + // If the context was not specified, we set the index of the first + // non-empty match set. + for k, v := range reply.Matches { + if len(v) != 0 { + index, err := state.Index(k) + if err != nil { + return err } + reply.Index = index + break } } - if err != nil { - return err - } - reply.Index = index r.srv.setQueryMeta(&reply.QueryMeta) return nil diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 774cccf5317..24a11298c0f 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -232,7 +232,7 @@ type NodeSpecificRequest struct { } // ResourcesResponse is used to return matches and information about whether -// the match list is truncated, specific to each type of context. +// the match list is truncated specific to each type of context. type ResourcesResponse struct { Matches map[string][]string Truncations map[string]bool From 971183d6ec3b82a41d854e808c97df3c06948cd1 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 4 Aug 2017 19:28:12 +0000 Subject: [PATCH 21/27] fix up tests to intantiate assertion test helper add http tests for remaining contexts --- command/agent/resources_endpoint_test.go | 210 ++++++++++++++--------- nomad/resources_endpoint_test.go | 81 +++++---- 2 files changed, 176 insertions(+), 115 deletions(-) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index f658cf9364c..ab85b56e38c 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -7,36 +7,37 @@ import ( "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" - "github.com/stretchr/testify/assert" + a "github.com/stretchr/testify/assert" ) func TestHTTP_ResourcesWithIllegalMethod(t *testing.T) { + assert := a.New(t) t.Parallel() httpTest(t, nil, func(s *TestAgent) { req, err := http.NewRequest("DELETE", "/v1/resources", nil) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) respW := httptest.NewRecorder() _, err = s.Server.ResourcesRequest(respW, req) - assert.NotNil(t, err, "HTTP DELETE should not be accepted for this endpoint") + assert.NotNil(err, "HTTP DELETE should not be accepted for this endpoint") }) } func createJobForTest(jobID string, s *TestAgent, t *testing.T) { + assert := a.New(t) + job := mock.Job() job.ID = jobID job.TaskGroups[0].Count = 1 state := s.Agent.server.State() err := state.UpsertJob(1000, job) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) } func TestHTTP_Resources_POST(t *testing.T) { + assert := a.New(t) + testJob := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" testJobPrefix := "aaaaaaaa-e8f7-fd38" t.Parallel() @@ -45,32 +46,30 @@ func TestHTTP_Resources_POST(t *testing.T) { data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + assert.Nil(err) - if err != nil { - t.Fatalf("err: %v", err) - } respW := httptest.NewRecorder() resp, err := s.Server.ResourcesRequest(respW, req) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) res := resp.(structs.ResourcesResponse) - assert.Equal(t, 1, len(res.Matches)) + assert.Equal(1, len(res.Matches)) j := res.Matches["jobs"] - assert.Equal(t, 1, len(j)) - assert.Equal(t, j[0], testJob) + assert.Equal(1, len(j)) + assert.Equal(j[0], testJob) - assert.Equal(t, res.Truncations["job"], false) - assert.NotEqual(t, "0", respW.HeaderMap.Get("X-Nomad-Index")) + assert.Equal(res.Truncations["job"], false) + assert.NotEqual("0", respW.HeaderMap.Get("X-Nomad-Index")) }) } func TestHTTP_Resources_PUT(t *testing.T) { + assert := a.New(t) + testJob := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" testJobPrefix := "aaaaaaaa-e8f7-fd38" t.Parallel() @@ -79,32 +78,30 @@ func TestHTTP_Resources_PUT(t *testing.T) { data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("PUT", "/v1/resources", encodeReq(data)) + assert.Nil(err) - if err != nil { - t.Fatalf("err: %v", err) - } respW := httptest.NewRecorder() resp, err := s.Server.ResourcesRequest(respW, req) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) res := resp.(structs.ResourcesResponse) - assert.Equal(t, 1, len(res.Matches)) + assert.Equal(1, len(res.Matches)) j := res.Matches["jobs"] - assert.Equal(t, 1, len(j)) - assert.Equal(t, j[0], testJob) + assert.Equal(1, len(j)) + assert.Equal(j[0], testJob) - assert.Equal(t, res.Truncations["job"], false) - assert.NotEqual(t, "0", respW.HeaderMap.Get("X-Nomad-Index")) + assert.Equal(res.Truncations["job"], false) + assert.NotEqual("0", respW.HeaderMap.Get("X-Nomad-Index")) }) } func TestHTTP_Resources_MultipleJobs(t *testing.T) { + assert := a.New(t) + testJobA := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" testJobB := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89707" testJobC := "bbbbbbbb-e8f7-fd38-c855-ab94ceb89707" @@ -119,34 +116,32 @@ func TestHTTP_Resources_MultipleJobs(t *testing.T) { data := structs.ResourcesRequest{Prefix: testJobPrefix, Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + assert.Nil(err) - if err != nil { - t.Fatalf("err: %v", err) - } respW := httptest.NewRecorder() resp, err := s.Server.ResourcesRequest(respW, req) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) res := resp.(structs.ResourcesResponse) - assert.Equal(t, 1, len(res.Matches)) + assert.Equal(1, len(res.Matches)) j := res.Matches["jobs"] - assert.Equal(t, 2, len(j)) - assert.Contains(t, j, testJobA) - assert.Contains(t, j, testJobB) - assert.NotContains(t, j, testJobC) + assert.Equal(2, len(j)) + assert.Contains(j, testJobA) + assert.Contains(j, testJobB) + assert.NotContains(j, testJobC) - assert.Equal(t, res.Truncations["job"], false) - assert.NotEqual(t, "0", respW.HeaderMap.Get("X-Nomad-Index")) + assert.Equal(res.Truncations["job"], false) + assert.NotEqual("0", respW.HeaderMap.Get("X-Nomad-Index")) }) } func TestHTTP_ResoucesList_Evaluation(t *testing.T) { + assert := a.New(t) + t.Parallel() httpTest(t, nil, func(s *TestAgent) { state := s.Agent.server.State() @@ -154,63 +149,124 @@ func TestHTTP_ResoucesList_Evaluation(t *testing.T) { eval2 := mock.Eval() err := state.UpsertEvals(9000, []*structs.Evaluation{eval1, eval2}) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) prefix := eval1.ID[:len(eval1.ID)-2] data := structs.ResourcesRequest{Prefix: prefix, Context: "evals"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) + respW := httptest.NewRecorder() resp, err := s.Server.ResourcesRequest(respW, req) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) res := resp.(structs.ResourcesResponse) - assert.Equal(t, 1, len(res.Matches)) + assert.Equal(1, len(res.Matches)) j := res.Matches["evals"] - assert.Equal(t, 1, len(j)) - assert.Contains(t, j, eval1.ID) - assert.NotContains(t, j, eval2.ID) + assert.Equal(1, len(j)) + assert.Contains(j, eval1.ID) + assert.NotContains(j, eval2.ID) + + assert.Equal(res.Truncations["evals"], false) + assert.Equal("9000", respW.HeaderMap.Get("X-Nomad-Index")) + }) +} + +func TestHTTP_ResoucesList_Allocations(t *testing.T) { + assert := a.New(t) + + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + state := s.Agent.server.State() + alloc := mock.Alloc() + err := state.UpsertAllocs(7000, []*structs.Allocation{alloc}) + assert.Nil(err) + + prefix := alloc.ID[:len(alloc.ID)-2] + data := structs.ResourcesRequest{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) + assert.Nil(err) + + res := resp.(structs.ResourcesResponse) - assert.Equal(t, res.Truncations["evals"], false) - assert.Equal(t, "9000", respW.HeaderMap.Get("X-Nomad-Index")) + assert.Equal(1, len(res.Matches)) + + a := res.Matches["allocs"] + assert.Equal(1, len(a)) + assert.Contains(a, alloc.ID) + + assert.Equal(res.Truncations["allocs"], false) + assert.Equal("7000", respW.HeaderMap.Get("X-Nomad-Index")) + }) +} + +func TestHTTP_ResoucesList_Nodes(t *testing.T) { + assert := a.New(t) + + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + state := s.Agent.server.State() + node := mock.Node() + err := state.UpsertNode(6000, node) + assert.Nil(err) + + prefix := node.ID[:len(node.ID)-2] + data := structs.ResourcesRequest{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) + assert.Nil(err) + + res := resp.(structs.ResourcesResponse) + + assert.Equal(1, len(res.Matches)) + + n := res.Matches["nodes"] + assert.Equal(1, len(n)) + assert.Contains(n, node.ID) + + assert.Equal(res.Truncations["nodes"], false) + assert.Equal("6000", respW.HeaderMap.Get("X-Nomad-Index")) }) } func TestHTTP_Resources_NoJob(t *testing.T) { + assert := a.New(t) + t.Parallel() httpTest(t, nil, func(s *TestAgent) { data := structs.ResourcesRequest{Prefix: "12345", Context: "jobs"} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + assert.Nil(err) - if err != nil { - t.Fatalf("err: %v", err) - } respW := httptest.NewRecorder() resp, err := s.Server.ResourcesRequest(respW, req) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) res := resp.(structs.ResourcesResponse) - assert.Equal(t, 1, len(res.Matches)) - assert.Equal(t, 0, len(res.Matches["jobs"])) + assert.Equal(1, len(res.Matches)) + assert.Equal(0, len(res.Matches["jobs"])) - assert.Equal(t, "0", respW.HeaderMap.Get("X-Nomad-Index")) + assert.Equal("0", respW.HeaderMap.Get("X-Nomad-Index")) }) } func TestHTTP_Resources_NoContext(t *testing.T) { + assert := a.New(t) + testJobID := "aaaaaaaa-e8f7-fd38-c855-ab94ceb89706" testJobPrefix := "aaaaaaaa-e8f7-fd38" t.Parallel() @@ -222,32 +278,26 @@ func TestHTTP_Resources_NoContext(t *testing.T) { eval1.ID = testJobID err := state.UpsertEvals(9000, []*structs.Evaluation{eval1}) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) data := structs.ResourcesRequest{Prefix: testJobPrefix} req, err := http.NewRequest("POST", "/v1/resources", encodeReq(data)) + assert.Nil(err) - if err != nil { - t.Fatalf("err: %v", err) - } respW := httptest.NewRecorder() resp, err := s.Server.ResourcesRequest(respW, req) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(err) res := resp.(structs.ResourcesResponse) matchedJobs := res.Matches["jobs"] matchedEvals := res.Matches["evals"] - assert.Equal(t, 1, len(matchedJobs)) - assert.Equal(t, 1, len(matchedEvals)) + assert.Equal(1, len(matchedJobs)) + assert.Equal(1, len(matchedEvals)) - assert.Equal(t, matchedJobs[0], testJobID) - assert.Equal(t, matchedEvals[0], eval1.ID) + assert.Equal(matchedJobs[0], testJobID) + assert.Equal(matchedEvals[0], eval1.ID) }) } diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index a6774379be1..4f031d8906e 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -17,8 +17,7 @@ func registerAndVerifyJob(s *Server, t *testing.T, prefix string, counter int) s job.ID = prefix + strconv.Itoa(counter) state := s.fsm.State() - err := state.UpsertJob(jobIndex, job) - if err != nil { + if err := state.UpsertJob(jobIndex, job); err != nil { t.Fatalf("err: %v", err) } @@ -26,6 +25,7 @@ func registerAndVerifyJob(s *Server, t *testing.T, prefix string, counter int) s } func TestResourcesEndpoint_List(t *testing.T) { + assert := assert.New(t) prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" t.Parallel() @@ -49,13 +49,14 @@ func TestResourcesEndpoint_List(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["jobs"])) - assert.Equal(t, jobID, resp.Matches["jobs"][0]) - assert.Equal(t, uint64(jobIndex), resp.Index) + assert.Equal(1, len(resp.Matches["jobs"])) + assert.Equal(jobID, resp.Matches["jobs"][0]) + assert.Equal(uint64(jobIndex), resp.Index) } // truncate should limit results to 20 func TestResourcesEndpoint_List_Truncate(t *testing.T) { + assert := assert.New(t) prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" t.Parallel() @@ -81,12 +82,13 @@ func TestResourcesEndpoint_List_Truncate(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 20, len(resp.Matches["jobs"])) - assert.Equal(t, resp.Truncations["jobs"], true) - assert.Equal(t, uint64(jobIndex), resp.Index) + assert.Equal(20, len(resp.Matches["jobs"])) + assert.Equal(resp.Truncations["jobs"], true) + assert.Equal(uint64(jobIndex), resp.Index) } func TestResourcesEndpoint_List_Evals(t *testing.T) { + assert := assert.New(t) t.Parallel() s := testServer(t, func(c *Config) { c.NumSchedulers = 0 @@ -111,14 +113,15 @@ func TestResourcesEndpoint_List_Evals(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["evals"])) - assert.Equal(t, eval1.ID, resp.Matches["evals"][0]) - assert.Equal(t, resp.Truncations["evals"], false) + assert.Equal(1, len(resp.Matches["evals"])) + assert.Equal(eval1.ID, resp.Matches["evals"][0]) + assert.Equal(resp.Truncations["evals"], false) - assert.Equal(t, uint64(2000), resp.Index) + assert.Equal(uint64(2000), resp.Index) } func TestResourcesEndpoint_List_Allocation(t *testing.T) { + assert := assert.New(t) t.Parallel() s := testServer(t, func(c *Config) { c.NumSchedulers = 0 @@ -151,14 +154,15 @@ func TestResourcesEndpoint_List_Allocation(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["allocs"])) - assert.Equal(t, alloc.ID, resp.Matches["allocs"][0]) - assert.Equal(t, resp.Truncations["allocs"], false) + assert.Equal(1, len(resp.Matches["allocs"])) + assert.Equal(alloc.ID, resp.Matches["allocs"][0]) + assert.Equal(resp.Truncations["allocs"], false) - assert.Equal(t, uint64(90), resp.Index) + assert.Equal(uint64(90), resp.Index) } func TestResourcesEndpoint_List_Node(t *testing.T) { + assert := assert.New(t) t.Parallel() s := testServer(t, func(c *Config) { c.NumSchedulers = 0 @@ -187,14 +191,16 @@ func TestResourcesEndpoint_List_Node(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["nodes"])) - assert.Equal(t, node.ID, resp.Matches["nodes"][0]) - assert.Equal(t, false, resp.Truncations["nodes"]) + assert.Equal(1, len(resp.Matches["nodes"])) + assert.Equal(node.ID, resp.Matches["nodes"][0]) + assert.Equal(false, resp.Truncations["nodes"]) - assert.Equal(t, uint64(100), resp.Index) + assert.Equal(uint64(100), resp.Index) } func TestResourcesEndpoint_List_InvalidContext(t *testing.T) { + assert := assert.New(t) + t.Parallel() s := testServer(t, func(c *Config) { c.NumSchedulers = 0 @@ -211,12 +217,13 @@ func TestResourcesEndpoint_List_InvalidContext(t *testing.T) { var resp structs.ResourcesResponse err := msgpackrpc.CallWithCodec(codec, "Resources.List", req, &resp) - assert.Equal(t, err.Error(), "invalid context") + assert.Equal(err.Error(), "invalid context") - assert.Equal(t, uint64(0), resp.Index) + assert.Equal(uint64(0), resp.Index) } func TestResourcesEndpoint_List_NoContext(t *testing.T) { + assert := assert.New(t) t.Parallel() s := testServer(t, func(c *Config) { c.NumSchedulers = 0 @@ -251,17 +258,19 @@ func TestResourcesEndpoint_List_NoContext(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["nodes"])) - assert.Equal(t, 1, len(resp.Matches["evals"])) + assert.Equal(1, len(resp.Matches["nodes"])) + assert.Equal(1, len(resp.Matches["evals"])) - assert.Equal(t, node.ID, resp.Matches["nodes"][0]) - assert.Equal(t, eval1.ID, resp.Matches["evals"][0]) + assert.Equal(node.ID, resp.Matches["nodes"][0]) + assert.Equal(eval1.ID, resp.Matches["evals"][0]) - assert.NotEqual(t, uint64(0), resp.Index) + assert.NotEqual(uint64(0), resp.Index) } -// Tests that the top 20 matches are returned when no prefix is set +//// Tests that the top 20 matches are returned when no prefix is set func TestResourcesEndpoint_List_NoPrefix(t *testing.T) { + assert := assert.New(t) + prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" t.Parallel() @@ -285,14 +294,16 @@ func TestResourcesEndpoint_List_NoPrefix(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 1, len(resp.Matches["jobs"])) - assert.Equal(t, jobID, resp.Matches["jobs"][0]) - assert.Equal(t, uint64(jobIndex), resp.Index) + assert.Equal(1, len(resp.Matches["jobs"])) + assert.Equal(jobID, resp.Matches["jobs"][0]) + assert.Equal(uint64(jobIndex), resp.Index) } -// Tests that the zero matches are returned when a prefix has no matching -// results +//// Tests that the zero matches are returned when a prefix has no matching +//// results func TestResourcesEndpoint_List_NoMatches(t *testing.T) { + assert := assert.New(t) + prefix := "aaaaaaaa-e8f7-fd38-c855-ab94ceb8970" t.Parallel() @@ -314,6 +325,6 @@ func TestResourcesEndpoint_List_NoMatches(t *testing.T) { t.Fatalf("err: %v", err) } - assert.Equal(t, 0, len(resp.Matches["jobs"])) - assert.Equal(t, uint64(0), resp.Index) + assert.Equal(0, len(resp.Matches["jobs"])) + assert.Equal(uint64(0), resp.Index) } From 74f35dd1af3872f7eee5fa111ff2fe46c3af7422 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 4 Aug 2017 20:14:41 +0000 Subject: [PATCH 22/27] if no context is specified, set maximum index for available contexts --- command/agent/resources_endpoint_test.go | 5 +++-- nomad/resources_endpoint.go | 12 +++++++----- nomad/resources_endpoint_test.go | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/command/agent/resources_endpoint_test.go b/command/agent/resources_endpoint_test.go index ab85b56e38c..375afb8a479 100644 --- a/command/agent/resources_endpoint_test.go +++ b/command/agent/resources_endpoint_test.go @@ -276,8 +276,7 @@ func TestHTTP_Resources_NoContext(t *testing.T) { state := s.Agent.server.State() eval1 := mock.Eval() eval1.ID = testJobID - err := state.UpsertEvals(9000, - []*structs.Evaluation{eval1}) + err := state.UpsertEvals(8000, []*structs.Evaluation{eval1}) assert.Nil(err) data := structs.ResourcesRequest{Prefix: testJobPrefix} @@ -299,5 +298,7 @@ func TestHTTP_Resources_NoContext(t *testing.T) { assert.Equal(matchedJobs[0], testJobID) assert.Equal(matchedEvals[0], eval1.ID) + + assert.Equal("8000", respW.HeaderMap.Get("X-Nomad-Index")) }) } diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 5b4b1c07547..5681ef8b928 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -109,16 +109,18 @@ func (r *Resources) List(args *structs.ResourcesRequest, // Set the index for the context. If the context has been specified, it // is the only non-empty match set, and the index is set for it. - // If the context was not specified, we set the index of the first - // non-empty match set. + // If the context was not specified, we set the index to be the max index + // from available contexts + reply.Index = 0 for k, v := range reply.Matches { - if len(v) != 0 { + if len(v) != 0 { // make sure matches exist for this context index, err := state.Index(k) if err != nil { return err } - reply.Index = index - break + if index > reply.Index { + reply.Index = index + } } } diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index 4f031d8906e..2f67b228024 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -264,7 +264,7 @@ func TestResourcesEndpoint_List_NoContext(t *testing.T) { assert.Equal(node.ID, resp.Matches["nodes"][0]) assert.Equal(eval1.ID, resp.Matches["evals"][0]) - assert.NotEqual(uint64(0), resp.Index) + assert.Equal(uint64(1000), resp.Index) } //// Tests that the top 20 matches are returned when no prefix is set From b2df34cf1889f50da0beff587ef0b29388c9423f Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 4 Aug 2017 22:18:49 +0000 Subject: [PATCH 23/27] 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 be570a9b866..aa6880a5835 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 36e0054fe49..fadc98145e6 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 375afb8a479..c40b39bdb76 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 5681ef8b928..79c46072270 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 2f67b228024..c9abbf241e4 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 24a11298c0f..946f65a6722 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 } From 753b15727bfc2c034b7f4b765b1c94e08b5ff8b1 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Mon, 7 Aug 2017 14:16:24 +0000 Subject: [PATCH 24/27] syntax fixups and logging --- nomad/resources_endpoint.go | 11 +++++------ nomad/resources_endpoint_test.go | 2 +- nomad/structs/structs.go | 7 ++++++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 79c46072270..2f8421905fa 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -26,7 +26,7 @@ type Resources struct { // getMatches extracts matches for an iterator, and returns a list of ids for // these matches. -func getMatches(iter memdb.ResultIterator) ([]string, bool) { +func (r *Resources) getMatches(iter memdb.ResultIterator) ([]string, bool) { var matches []string isTruncated := false @@ -37,7 +37,7 @@ func getMatches(iter memdb.ResultIterator) ([]string, bool) { } var id string - switch raw.(type) { + switch t := raw.(type) { case *structs.Job: id = raw.(*structs.Job).ID case *structs.Evaluation: @@ -47,8 +47,7 @@ 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 + r.srv.logger.Printf("[ERR] nomad.resources: unexpected type for resources context; %T \n", t) continue } @@ -109,7 +108,7 @@ func (r *Resources) List(args *structs.ResourceListRequest, // Return matches for the given prefix for k, v := range iters { - res, isTrunc := getMatches(v) + res, isTrunc := r.getMatches(v) reply.Matches[k] = res reply.Truncations[k] = isTrunc } @@ -117,7 +116,7 @@ func (r *Resources) List(args *structs.ResourceListRequest, // Set the index for the context. If the context has been specified, it // is the only non-empty match set, and the index is set for it. // If the context was not specified, we set the index to be the max index - // from available contexts + // from available matches. reply.Index = 0 for k, v := range reply.Matches { if len(v) != 0 { // make sure matches exist for this context diff --git a/nomad/resources_endpoint_test.go b/nomad/resources_endpoint_test.go index c9abbf241e4..758b08cfa1a 100644 --- a/nomad/resources_endpoint_test.go +++ b/nomad/resources_endpoint_test.go @@ -267,7 +267,7 @@ func TestResourcesEndpoint_List_NoContext(t *testing.T) { assert.Equal(uint64(1000), resp.Index) } -//// Tests that the top 20 matches are returned when no prefix is set +// Tests that the top 20 matches are returned when no prefix is set func TestResourcesEndpoint_List_NoPrefix(t *testing.T) { assert := assert.New(t) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 946f65a6722..7f40183c5d2 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -235,8 +235,12 @@ type NodeSpecificRequest struct { // the match list is truncated specific to each type of context. type ResourceListResponse struct { // Map of context types to resource ids which match a specified prefix - Matches map[string][]string + Matches map[string][]string + + // Truncations indicates whether the matches for a particular context have + // been truncated Truncations map[string]bool + QueryMeta } @@ -247,6 +251,7 @@ 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) From e5e4fc4ad538951c4cd5cc02524e139eb06c0310 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Mon, 7 Aug 2017 15:07:18 +0000 Subject: [PATCH 25/27] max index for any resource, if context is unspecified --- nomad/resources_endpoint.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 2f8421905fa..21688827d42 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -114,19 +114,16 @@ func (r *Resources) List(args *structs.ResourceListRequest, } // Set the index for the context. If the context has been specified, it - // is the only non-empty match set, and the index is set for it. - // If the context was not specified, we set the index to be the max index - // from available matches. + // will be used as the index of the response. Otherwise, the + // maximum index from all resources will be used. reply.Index = 0 - for k, v := range reply.Matches { - if len(v) != 0 { // make sure matches exist for this context - index, err := state.Index(k) - if err != nil { - return err - } - if index > reply.Index { - reply.Index = index - } + for _, e := range contexts { + index, err := state.Index(e) + if err != nil { + return err + } + if index > reply.Index { + reply.Index = index } } From a1b59254fd88f6c651f90b6277f2d20af8eabf88 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Mon, 7 Aug 2017 17:23:32 +0000 Subject: [PATCH 26/27] code simplifications and logging --- nomad/resources_endpoint.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/nomad/resources_endpoint.go b/nomad/resources_endpoint.go index 21688827d42..35e576b7908 100644 --- a/nomad/resources_endpoint.go +++ b/nomad/resources_endpoint.go @@ -28,7 +28,6 @@ type Resources struct { // these matches. func (r *Resources) getMatches(iter memdb.ResultIterator) ([]string, bool) { var matches []string - isTruncated := false for i := 0; i < truncateLimit; i++ { raw := iter.Next() @@ -47,18 +46,14 @@ func (r *Resources) getMatches(iter memdb.ResultIterator) ([]string, bool) { case *structs.Node: id = raw.(*structs.Node).ID default: - r.srv.logger.Printf("[ERR] nomad.resources: unexpected type for resources context; %T \n", t) + r.srv.logger.Printf("[ERR] nomad.resources: unexpected type for resources context: %T", t) continue } matches = append(matches, id) } - if iter.Next() != nil { - isTruncated = true - } - - return matches, isTruncated + return matches, iter.Next() != nil } // getResourceIter takes a context and returns a memdb iterator specific to @@ -116,7 +111,6 @@ func (r *Resources) List(args *structs.ResourceListRequest, // Set the index for the context. If the context has been specified, it // will be used as the index of the response. Otherwise, the // maximum index from all resources will be used. - reply.Index = 0 for _, e := range contexts { index, err := state.Index(e) if err != nil { From 440fea755a54f55209622623be653f1de432d19f Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Mon, 7 Aug 2017 19:21:50 +0000 Subject: [PATCH 27/27] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b620183940..99aeb0f7c23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ ## 0.6.1 (Unreleased) IMPROVEMENTS: + * core: Add autocomplete functionality for resources: allocations, + evaluations, jobs, and nodes [GH-2964] * core: `distinct_property` constraint can set the number of allocations that are allowed to share a property value [GH-2942] * driver/rkt: support read-only volume mounts [GH-2883]