From a55f46e9babfa92862c96776d5bdb9f811725660 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 8 Oct 2020 22:21:41 -0700 Subject: [PATCH 1/3] api: add field filters to /v1/{allocations,nodes} Fixes #9017 The ?resources=true query parameter includes resources in the object stub listings. Specifically: - For `/v1/nodes?resources=true` both the `NodeResources` and `ReservedResources` field are included. - For `/v1/allocations?resources=true` the `AllocatedResources` field is included. The ?task_states=false query parameter removes TaskStates from /v1/allocations responses. (By default TaskStates are included.) --- api/allocations.go | 1 + api/allocations_test.go | 80 +++++++++---- api/nodes.go | 2 + api/nodes_test.go | 39 ++++++ api/util_test.go | 2 +- command/agent/alloc_endpoint.go | 10 ++ command/agent/csi_endpoint.go | 4 +- command/agent/http.go | 26 ++++ command/agent/http_test.go | 90 ++++++++++++++ command/agent/node_endpoint.go | 9 ++ nomad/alloc_endpoint.go | 2 +- nomad/alloc_endpoint_test.go | 112 ++++++++++++++++-- nomad/deployment_endpoint.go | 2 +- nomad/deploymentwatcher/deployment_watcher.go | 2 +- nomad/eval_endpoint.go | 2 +- nomad/job_endpoint.go | 2 +- nomad/node_endpoint.go | 2 +- nomad/node_endpoint_test.go | 41 +++++++ nomad/state/state_store.go | 2 +- nomad/structs/structs.go | 61 +++++++++- scheduler/generic_sched.go | 2 +- scheduler/system_sched.go | 2 +- .../hashicorp/nomad/api/allocations.go | 1 + .../github.com/hashicorp/nomad/api/nodes.go | 2 + 24 files changed, 445 insertions(+), 53 deletions(-) diff --git a/api/allocations.go b/api/allocations.go index 737c0f8e9fcb..50f8cce938cf 100644 --- a/api/allocations.go +++ b/api/allocations.go @@ -432,6 +432,7 @@ type AllocationListStub struct { JobType string JobVersion uint64 TaskGroup string + AllocatedResources *AllocatedResources `json:",omitempty"` DesiredStatus string DesiredDescription string ClientStatus string diff --git a/api/allocations_test.go b/api/allocations_test.go index d9543ccf29e4..0e514a01e5f2 100644 --- a/api/allocations_test.go +++ b/api/allocations_test.go @@ -7,15 +7,17 @@ import ( "reflect" "sort" "testing" - "time" + "github.com/hashicorp/nomad/api/internal/testutil" "github.com/stretchr/testify/require" ) func TestAllocations_List(t *testing.T) { t.Parallel() - c, s := makeClient(t, nil, nil) + c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) { + c.DevMode = true + }) defer s.Stop() a := c.Allocations() @@ -31,33 +33,28 @@ func TestAllocations_List(t *testing.T) { t.Fatalf("expected 0 allocs, got: %d", n) } - // TODO: do something that causes an allocation to actually happen - // so we can query for them. - return + // Create a job and attempt to register it + job := testJob() + resp, wm, err := c.Jobs().Register(job, nil) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotEmpty(t, resp.EvalID) + assertWriteMeta(t, wm) - //job := &Job{ - //ID: stringToPtr("job1"), - //Name: stringToPtr("Job #1"), - //Type: stringToPtr(JobTypeService), - //} - //eval, _, err := c.Jobs().Register(job, nil) - //if err != nil { - //t.Fatalf("err: %s", err) - //} + // List the allocations again + qo := &QueryOptions{ + WaitIndex: wm.LastIndex, + } + allocs, qm, err = a.List(qo) + require.NoError(t, err) + require.NotZero(t, qm.LastIndex) - //// List the allocations again - //allocs, qm, err = a.List(nil) - //if err != nil { - //t.Fatalf("err: %s", err) - //} - //if qm.LastIndex == 0 { - //t.Fatalf("bad index: %d", qm.LastIndex) - //} + // Check that we got the allocation back + require.Len(t, allocs, 1) + require.Equal(t, resp.EvalID, allocs[0].EvalID) - //// Check that we got the allocation back - //if len(allocs) == 0 || allocs[0].EvalID != eval { - //t.Fatalf("bad: %#v", allocs) - //} + // Resources should be unset by default + require.Nil(t, allocs[0].AllocatedResources) } func TestAllocations_PrefixList(t *testing.T) { @@ -108,6 +105,37 @@ func TestAllocations_PrefixList(t *testing.T) { //} } +func TestAllocations_List_Resources(t *testing.T) { + t.Parallel() + c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) { + c.DevMode = true + }) + defer s.Stop() + a := c.Allocations() + + // Create a job and register it + job := testJob() + resp, wm, err := c.Jobs().Register(job, nil) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotEmpty(t, resp.EvalID) + assertWriteMeta(t, wm) + + // List the allocations + qo := &QueryOptions{ + Params: map[string]string{"resources": "true"}, + WaitIndex: wm.LastIndex, + } + allocs, qm, err := a.List(qo) + require.NoError(t, err) + require.NotZero(t, qm.LastIndex) + + // Check that we got the allocation back with resources + require.Len(t, allocs, 1) + require.Equal(t, resp.EvalID, allocs[0].EvalID) + require.NotNil(t, allocs[0].AllocatedResources) +} + func TestAllocations_CreateIndexSort(t *testing.T) { t.Parallel() allocs := []*AllocationListStub{ diff --git a/api/nodes.go b/api/nodes.go index 3a6aa3b3b59d..4dc399e3fc67 100644 --- a/api/nodes.go +++ b/api/nodes.go @@ -786,6 +786,8 @@ type NodeListStub struct { Status string StatusDescription string Drivers map[string]*DriverInfo + NodeResources *NodeResources `json:",omitempty"` + ReservedResources *NodeReservedResources `json:",omitempty"` CreateIndex uint64 ModifyIndex uint64 } diff --git a/api/nodes_test.go b/api/nodes_test.go index 49b9c1e956f3..a1d41cda2d11 100644 --- a/api/nodes_test.go +++ b/api/nodes_test.go @@ -83,6 +83,45 @@ func TestNodes_PrefixList(t *testing.T) { assertQueryMeta(t, qm) } +// TestNodes_List_Resources asserts that ?resources=true includes allocated and +// reserved resources in the response. +func TestNodes_List_Resources(t *testing.T) { + t.Parallel() + c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) { + c.DevMode = true + }) + defer s.Stop() + nodes := c.Nodes() + + var out []*NodeListStub + var err error + + testutil.WaitForResult(func() (bool, error) { + out, _, err = nodes.List(nil) + if err != nil { + return false, err + } + if n := len(out); n != 1 { + return false, fmt.Errorf("expected 1 node, got: %d", n) + } + return true, nil + }, func(err error) { + t.Fatalf("err: %s", err) + }) + + // By default resources should *not* be included + require.Nil(t, out[0].NodeResources) + require.Nil(t, out[0].ReservedResources) + + qo := &QueryOptions{ + Params: map[string]string{"resources": "true"}, + } + out, _, err = nodes.List(qo) + require.NoError(t, err) + require.NotNil(t, out[0].NodeResources) + require.NotNil(t, out[0].ReservedResources) +} + func TestNodes_Info(t *testing.T) { t.Parallel() startTime := time.Now().Unix() diff --git a/api/util_test.go b/api/util_test.go index 1be4f60aad09..758855750d21 100644 --- a/api/util_test.go +++ b/api/util_test.go @@ -24,7 +24,7 @@ func assertWriteMeta(t *testing.T, wm *WriteMeta) { } func testJob() *Job { - task := NewTask("task1", "exec"). + task := NewTask("task1", "raw_exec"). SetConfig("command", "/bin/sleep"). Require(&Resources{ CPU: intToPtr(100), diff --git a/command/agent/alloc_endpoint.go b/command/agent/alloc_endpoint.go index 5988ae69d3ec..ef2ae58658d3 100644 --- a/command/agent/alloc_endpoint.go +++ b/command/agent/alloc_endpoint.go @@ -33,6 +33,16 @@ func (s *HTTPServer) AllocsRequest(resp http.ResponseWriter, req *http.Request) return nil, nil } + // Parse resources and task_states field selection + var err error + args.Fields = structs.NewAllocStubFields() + if args.Fields.Resources, err = parseResources(req); err != nil { + return nil, err + } + if args.Fields.TaskStates, err = parseTaskStates(req); err != nil { + return nil, err + } + var out structs.AllocListResponse if err := s.agent.RPC("Alloc.List", &args, &out); err != nil { return nil, err diff --git a/command/agent/csi_endpoint.go b/command/agent/csi_endpoint.go index d25528e7b2e4..d3f6a2c4231a 100644 --- a/command/agent/csi_endpoint.go +++ b/command/agent/csi_endpoint.go @@ -344,13 +344,13 @@ func structsCSIVolumeToApi(vol *structs.CSIVolume) *api.CSIVolume { for _, a := range vol.WriteAllocs { if a != nil { - out.Allocations = append(out.Allocations, structsAllocListStubToApi(a.Stub())) + out.Allocations = append(out.Allocations, structsAllocListStubToApi(a.Stub(nil))) } } for _, a := range vol.ReadAllocs { if a != nil { - out.Allocations = append(out.Allocations, structsAllocListStubToApi(a.Stub())) + out.Allocations = append(out.Allocations, structsAllocListStubToApi(a.Stub(nil))) } } diff --git a/command/agent/http.go b/command/agent/http.go index 87b51c0aca89..23c67032cf6d 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -653,6 +653,32 @@ func parseNamespace(req *http.Request, n *string) { } } +// parseResources is used to parse the ?resources parameter +func parseResources(req *http.Request) (bool, error) { + if resourcesStr := req.URL.Query().Get("resources"); resourcesStr != "" { + resources, err := strconv.ParseBool(resourcesStr) + if err != nil { + return false, fmt.Errorf("Failed to parse value of %q (%v) as a bool: %v", "resources", resourcesStr, err) + } + return resources, nil + } + + return false, nil +} + +// parseTaskStates is used to parse the ?task_states parameter +func parseTaskStates(req *http.Request) (bool, error) { + if str := req.URL.Query().Get("task_states"); str != "" { + param, err := strconv.ParseBool(str) + if err != nil { + return false, fmt.Errorf("Failed to parse value of %q (%v) as a bool: %v", "task_states", str, err) + } + return param, nil + } + + return false, nil +} + // parseToken is used to parse the X-Nomad-Token param func (s *HTTPServer) parseToken(req *http.Request, token *string) { if other := req.Header.Get("X-Nomad-Token"); other != "" { diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 30e4047e6fbb..d2f1548b559b 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -515,6 +515,96 @@ func TestParseToken(t *testing.T) { } } +func TestParseResources(t *testing.T) { + t.Parallel() + + cases := []struct { + Value string + Resources bool + Err bool // true if an error should be expected + }{ + { + Value: "", + Resources: false, + }, + { + Value: "true", + Resources: true, + }, + { + Value: "false", + Resources: false, + }, + { + Value: "1234", + Err: true, + }, + } + + for i := range cases { + tc := cases[i] + t.Run("Value-"+tc.Value, func(t *testing.T) { + testURL, err := url.Parse("http://localhost/foo?resources=" + tc.Value) + require.NoError(t, err) + req := &http.Request{ + URL: testURL, + } + + result, err := parseResources(req) + if tc.Err { + require.Error(t, err) + } else { + require.Equal(t, tc.Resources, result) + } + }) + } +} + +func TestParseTaskStates(t *testing.T) { + t.Parallel() + + cases := []struct { + Value string + TaskStates bool + Err bool // true if an error should be expected + }{ + { + Value: "", + TaskStates: false, + }, + { + Value: "true", + TaskStates: true, + }, + { + Value: "false", + TaskStates: false, + }, + { + Value: "1234", + Err: true, + }, + } + + for i := range cases { + tc := cases[i] + t.Run("Value-"+tc.Value, func(t *testing.T) { + testURL, err := url.Parse("http://localhost/foo?task_states=" + tc.Value) + require.NoError(t, err) + req := &http.Request{ + URL: testURL, + } + + result, err := parseTaskStates(req) + if tc.Err { + require.Error(t, err) + } else { + require.Equal(t, tc.TaskStates, result) + } + }) + } +} + // TestHTTP_VerifyHTTPSClient asserts that a client certificate signed by the // appropriate CA is required when VerifyHTTPSClient=true. func TestHTTP_VerifyHTTPSClient(t *testing.T) { diff --git a/command/agent/node_endpoint.go b/command/agent/node_endpoint.go index 3b10eb282b93..b2f53a9d71c8 100644 --- a/command/agent/node_endpoint.go +++ b/command/agent/node_endpoint.go @@ -20,6 +20,15 @@ func (s *HTTPServer) NodesRequest(resp http.ResponseWriter, req *http.Request) ( return nil, nil } + // Parse resources field selection + if resources, err := parseResources(req); err != nil { + return nil, err + } else if resources { + args.Fields = &structs.NodeStubFields{ + Resources: true, + } + } + var out structs.NodeListResponse if err := s.agent.RPC("Node.List", &args, &out); err != nil { return nil, err diff --git a/nomad/alloc_endpoint.go b/nomad/alloc_endpoint.go index 0909c3907a63..7d518c5688fd 100644 --- a/nomad/alloc_endpoint.go +++ b/nomad/alloc_endpoint.go @@ -72,7 +72,7 @@ func (a *Alloc) List(args *structs.AllocListRequest, reply *structs.AllocListRes break } alloc := raw.(*structs.Allocation) - allocs = append(allocs, alloc.Stub()) + allocs = append(allocs, alloc.Stub(args.Fields)) } reply.Allocations = allocs diff --git a/nomad/alloc_endpoint_test.go b/nomad/alloc_endpoint_test.go index d9123b03f56c..a9791f80b1a1 100644 --- a/nomad/alloc_endpoint_test.go +++ b/nomad/alloc_endpoint_test.go @@ -69,19 +69,111 @@ func TestAllocEndpoint_List(t *testing.T) { } var resp2 structs.AllocListResponse - if err := msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp2); err != nil { - t.Fatalf("err: %v", err) - } - if resp2.Index != 1000 { - t.Fatalf("Bad index: %d %d", resp2.Index, 1000) + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp2)) + require.Equal(t, uint64(1000), resp2.Index) + require.Len(t, resp2.Allocations, 1) + require.Equal(t, alloc.ID, resp2.Allocations[0].ID) +} + +func TestAllocEndpoint_List_Fields(t *testing.T) { + t.Parallel() + + s1, cleanupS1 := TestServer(t, nil) + defer cleanupS1() + + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + + // Create a running alloc + alloc := mock.Alloc() + alloc.ClientStatus = structs.AllocClientStatusRunning + alloc.TaskStates = map[string]*structs.TaskState{ + "web": { + State: structs.TaskStateRunning, + StartedAt: time.Now(), + }, } + summary := mock.JobSummary(alloc.JobID) + state := s1.fsm.State() - if len(resp2.Allocations) != 1 { - t.Fatalf("bad: %#v", resp2.Allocations) + require.NoError(t, state.UpsertJobSummary(999, summary)) + require.NoError(t, state.UpsertAllocs(1000, []*structs.Allocation{alloc})) + + cases := []struct { + Name string + Fields *structs.AllocStubFields + Assert func(t *testing.T, allocs []*structs.AllocListStub) + }{ + { + Name: "None", + Fields: nil, + Assert: func(t *testing.T, allocs []*structs.AllocListStub) { + require.Nil(t, allocs[0].AllocatedResources) + require.Len(t, allocs[0].TaskStates, 1) + }, + }, + { + Name: "Default", + Fields: structs.NewAllocStubFields(), + Assert: func(t *testing.T, allocs []*structs.AllocListStub) { + require.Nil(t, allocs[0].AllocatedResources) + require.Len(t, allocs[0].TaskStates, 1) + }, + }, + { + Name: "Resources", + Fields: &structs.AllocStubFields{ + Resources: true, + TaskStates: false, + }, + Assert: func(t *testing.T, allocs []*structs.AllocListStub) { + require.NotNil(t, allocs[0].AllocatedResources) + require.Len(t, allocs[0].TaskStates, 0) + }, + }, + { + Name: "NoTaskStates", + Fields: &structs.AllocStubFields{ + Resources: false, + TaskStates: false, + }, + Assert: func(t *testing.T, allocs []*structs.AllocListStub) { + require.Nil(t, allocs[0].AllocatedResources) + require.Len(t, allocs[0].TaskStates, 0) + }, + }, + { + Name: "Both", + Fields: &structs.AllocStubFields{ + Resources: true, + TaskStates: true, + }, + Assert: func(t *testing.T, allocs []*structs.AllocListStub) { + require.NotNil(t, allocs[0].AllocatedResources) + require.Len(t, allocs[0].TaskStates, 1) + }, + }, } - if resp2.Allocations[0].ID != alloc.ID { - t.Fatalf("bad: %#v", resp2.Allocations[0]) + + for i := range cases { + tc := cases[i] + t.Run(tc.Name, func(t *testing.T) { + get := &structs.AllocListRequest{ + QueryOptions: structs.QueryOptions{ + Region: "global", + Namespace: structs.DefaultNamespace, + }, + Fields: tc.Fields, + } + var resp structs.AllocListResponse + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp)) + require.Equal(t, uint64(1000), resp.Index) + require.Len(t, resp.Allocations, 1) + require.Equal(t, alloc.ID, resp.Allocations[0].ID) + tc.Assert(t, resp.Allocations) + }) } + } func TestAllocEndpoint_List_ACL(t *testing.T) { @@ -102,7 +194,7 @@ func TestAllocEndpoint_List_ACL(t *testing.T) { assert.Nil(state.UpsertJobSummary(999, summary), "UpsertJobSummary") assert.Nil(state.UpsertAllocs(1000, allocs), "UpsertAllocs") - stubAllocs := []*structs.AllocListStub{alloc.Stub()} + stubAllocs := []*structs.AllocListStub{alloc.Stub(nil)} stubAllocs[0].CreateIndex = 1000 stubAllocs[0].ModifyIndex = 1000 diff --git a/nomad/deployment_endpoint.go b/nomad/deployment_endpoint.go index 03a9341eeddc..b7073eb8f881 100644 --- a/nomad/deployment_endpoint.go +++ b/nomad/deployment_endpoint.go @@ -478,7 +478,7 @@ func (d *Deployment) Allocations(args *structs.DeploymentSpecificRequest, reply stubs := make([]*structs.AllocListStub, 0, len(allocs)) for _, alloc := range allocs { - stubs = append(stubs, alloc.Stub()) + stubs = append(stubs, alloc.Stub(nil)) } reply.Allocations = stubs diff --git a/nomad/deploymentwatcher/deployment_watcher.go b/nomad/deploymentwatcher/deployment_watcher.go index 24f4c38403b6..955fc9346a85 100644 --- a/nomad/deploymentwatcher/deployment_watcher.go +++ b/nomad/deploymentwatcher/deployment_watcher.go @@ -899,7 +899,7 @@ func (w *deploymentWatcher) getAllocsImpl(ws memdb.WatchSet, state *state.StateS maxIndex := uint64(0) stubs := make([]*structs.AllocListStub, 0, len(allocs)) for _, alloc := range allocs { - stubs = append(stubs, alloc.Stub()) + stubs = append(stubs, alloc.Stub(nil)) if maxIndex < alloc.ModifyIndex { maxIndex = alloc.ModifyIndex diff --git a/nomad/eval_endpoint.go b/nomad/eval_endpoint.go index b5f649bb0215..51a2f6cda2bd 100644 --- a/nomad/eval_endpoint.go +++ b/nomad/eval_endpoint.go @@ -426,7 +426,7 @@ func (e *Eval) Allocations(args *structs.EvalSpecificRequest, reply.Allocations = make([]*structs.AllocListStub, 0, len(allocs)) for _, alloc := range allocs { - reply.Allocations = append(reply.Allocations, alloc.Stub()) + reply.Allocations = append(reply.Allocations, alloc.Stub(nil)) } } diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index a75e9eddc2e4..e9f55e10dc40 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -1471,7 +1471,7 @@ func (j *Job) Allocations(args *structs.JobSpecificRequest, if len(allocs) > 0 { reply.Allocations = make([]*structs.AllocListStub, 0, len(allocs)) for _, alloc := range allocs { - reply.Allocations = append(reply.Allocations, alloc.Stub()) + reply.Allocations = append(reply.Allocations, alloc.Stub(nil)) } } diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 97af9aafc7b5..f795d1bd2c3a 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -1295,7 +1295,7 @@ func (n *Node) List(args *structs.NodeListRequest, break } node := raw.(*structs.Node) - nodes = append(nodes, node.Stub()) + nodes = append(nodes, node.Stub(args.Fields)) } reply.Nodes = nodes diff --git a/nomad/node_endpoint_test.go b/nomad/node_endpoint_test.go index 1be5728c598b..f59a7ad62f58 100644 --- a/nomad/node_endpoint_test.go +++ b/nomad/node_endpoint_test.go @@ -2591,6 +2591,10 @@ func TestClientEndpoint_ListNodes(t *testing.T) { // #7344 - Assert HostVolumes are included in stub require.Equal(t, node.HostVolumes, resp2.Nodes[0].HostVolumes) + // #9055 - Assert Resources are *not* included by default + require.Nil(t, resp2.Nodes[0].NodeResources) + require.Nil(t, resp2.Nodes[0].ReservedResources) + // Lookup the node with prefix get = &structs.NodeListRequest{ QueryOptions: structs.QueryOptions{Region: "global", Prefix: node.ID[:4]}, @@ -2611,6 +2615,43 @@ func TestClientEndpoint_ListNodes(t *testing.T) { } } +func TestClientEndpoint_ListNodes_Fields(t *testing.T) { + t.Parallel() + + s1, cleanupS1 := TestServer(t, nil) + defer cleanupS1() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + + // Create the register request + node := mock.Node() + reg := &structs.NodeRegisterRequest{ + Node: node, + WriteRequest: structs.WriteRequest{Region: "global"}, + } + + // Fetch the response + var resp structs.GenericResponse + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Node.Register", reg, &resp)) + node.CreateIndex = resp.Index + node.ModifyIndex = resp.Index + + // Lookup the node with fields + get := &structs.NodeListRequest{ + QueryOptions: structs.QueryOptions{Region: "global"}, + Fields: &structs.NodeStubFields{ + Resources: true, + }, + } + var resp2 structs.NodeListResponse + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Node.List", get, &resp2)) + require.Equal(t, resp.Index, resp2.Index) + require.Len(t, resp2.Nodes, 1) + require.Equal(t, node.ID, resp2.Nodes[0].ID) + require.NotNil(t, resp2.Nodes[0].NodeResources) + require.NotNil(t, resp2.Nodes[0].ReservedResources) +} + func TestClientEndpoint_ListNodes_ACL(t *testing.T) { t.Parallel() diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go index 6bb71382d88e..0e7e6382dd95 100644 --- a/nomad/state/state_store.go +++ b/nomad/state/state_store.go @@ -2421,7 +2421,7 @@ func (s *StateStore) CSIPluginDenormalize(ws memdb.WatchSet, plug *structs.CSIPl if alloc == nil { continue } - plug.Allocations = append(plug.Allocations, alloc.Stub()) + plug.Allocations = append(plug.Allocations, alloc.Stub(nil)) } return plug, nil diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 59d5ae9b4978..2f0a83e75257 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -742,6 +742,8 @@ type JobStabilityResponse struct { // NodeListRequest is used to parameterize a list request type NodeListRequest struct { QueryOptions + + Fields *NodeStubFields } // EvalUpdateRequest is used for upserting evaluations. @@ -887,6 +889,8 @@ type AllocStopResponse struct { // AllocListRequest is used to request a list of allocations type AllocListRequest struct { QueryOptions + + Fields *AllocStubFields } // AllocSpecificRequest is used to query a specific allocation @@ -1756,7 +1760,7 @@ type Node struct { // Resources is the available resources on the client. // For example 'cpu=2' 'memory=2048' - // COMPAT(0.10): Remove in 0.10 + // COMPAT(0.10): Remove after 0.10 Resources *Resources // Reserved is the set of resources that are reserved, @@ -1764,6 +1768,7 @@ type Node struct { // the purposes of scheduling. This may be provide certain // high-watermark tolerances or because of external schedulers // consuming resources. + // COMPAT(0.10): Remove after 0.10 Reserved *Resources // Links are used to 'link' this client to external @@ -2025,11 +2030,11 @@ func (n *Node) ComparableResources() *ComparableResources { } // Stub returns a summarized version of the node -func (n *Node) Stub() *NodeListStub { +func (n *Node) Stub(fields *NodeStubFields) *NodeListStub { addr, _, _ := net.SplitHostPort(n.HTTPAddr) - return &NodeListStub{ + s := &NodeListStub{ Address: addr, ID: n.ID, Datacenter: n.Datacenter, @@ -2045,6 +2050,15 @@ func (n *Node) Stub() *NodeListStub { CreateIndex: n.CreateIndex, ModifyIndex: n.ModifyIndex, } + + if fields != nil { + if fields.Resources { + s.NodeResources = n.NodeResources + s.ReservedResources = n.ReservedResources + } + } + + return s } // NodeListStub is used to return a subset of job information @@ -2062,10 +2076,17 @@ type NodeListStub struct { StatusDescription string Drivers map[string]*DriverInfo HostVolumes map[string]*ClientHostVolumeConfig + NodeResources *NodeResources `json:",omitempty"` + ReservedResources *NodeReservedResources `json:",omitempty"` CreateIndex uint64 ModifyIndex uint64 } +// NodeStubFields defines which fields are included in the NodeListStub. +type NodeStubFields struct { + Resources bool +} + // Resources is used to define the resources available // on a client type Resources struct { @@ -9242,8 +9263,8 @@ func (a *Allocation) LookupTask(name string) *Task { } // Stub returns a list stub for the allocation -func (a *Allocation) Stub() *AllocListStub { - return &AllocListStub{ +func (a *Allocation) Stub(fields *AllocStubFields) *AllocListStub { + s := &AllocListStub{ ID: a.ID, EvalID: a.EvalID, Name: a.Name, @@ -9270,6 +9291,17 @@ func (a *Allocation) Stub() *AllocListStub { CreateTime: a.CreateTime, ModifyTime: a.ModifyTime, } + + if fields != nil { + if fields.Resources { + s.AllocatedResources = a.AllocatedResources + } + if !fields.TaskStates { + s.TaskStates = nil + } + } + + return s } // AllocationDiff converts an Allocation type to an AllocationDiff type @@ -9297,6 +9329,7 @@ type AllocListStub struct { JobType string JobVersion uint64 TaskGroup string + AllocatedResources *AllocatedResources `json:",omitempty"` DesiredStatus string DesiredDescription string ClientStatus string @@ -9331,6 +9364,24 @@ func setDisplayMsg(taskStates map[string]*TaskState) { } } +// AllocStubFields defines which fields are included in the AllocListStub. +type AllocStubFields struct { + // Resources includes resource-related fields if true. + Resources bool + + // TaskStates removes the TaskStates field if false (default is to + // include TaskStates). + TaskStates bool +} + +func NewAllocStubFields() *AllocStubFields { + return &AllocStubFields{ + // Maintain backward compatibility by retaining task states by + // default. + TaskStates: true, + } +} + // AllocMetric is used to track various metrics while attempting // to make an allocation. These are used to debug a job, or to better // understand the pressure within the system. diff --git a/scheduler/generic_sched.go b/scheduler/generic_sched.go index 61957a90ea01..c67eafad870a 100644 --- a/scheduler/generic_sched.go +++ b/scheduler/generic_sched.go @@ -751,7 +751,7 @@ func (s *GenericScheduler) handlePreemptions(option *RankedNode, alloc *structs. preemptedAllocIDs = append(preemptedAllocIDs, stop.ID) if s.eval.AnnotatePlan && s.plan.Annotations != nil { - s.plan.Annotations.PreemptedAllocs = append(s.plan.Annotations.PreemptedAllocs, stop.Stub()) + s.plan.Annotations.PreemptedAllocs = append(s.plan.Annotations.PreemptedAllocs, stop.Stub(nil)) if s.plan.Annotations.DesiredTGUpdates != nil { desired := s.plan.Annotations.DesiredTGUpdates[missing.TaskGroup().Name] desired.Preemptions += 1 diff --git a/scheduler/system_sched.go b/scheduler/system_sched.go index 038a74188f80..4b1e5c8cbfaa 100644 --- a/scheduler/system_sched.go +++ b/scheduler/system_sched.go @@ -389,7 +389,7 @@ func (s *SystemScheduler) computePlacements(place []allocTuple) error { preemptedAllocIDs = append(preemptedAllocIDs, stop.ID) if s.eval.AnnotatePlan && s.plan.Annotations != nil { - s.plan.Annotations.PreemptedAllocs = append(s.plan.Annotations.PreemptedAllocs, stop.Stub()) + s.plan.Annotations.PreemptedAllocs = append(s.plan.Annotations.PreemptedAllocs, stop.Stub(nil)) if s.plan.Annotations.DesiredTGUpdates != nil { desired := s.plan.Annotations.DesiredTGUpdates[missing.TaskGroup.Name] desired.Preemptions += 1 diff --git a/vendor/github.com/hashicorp/nomad/api/allocations.go b/vendor/github.com/hashicorp/nomad/api/allocations.go index 737c0f8e9fcb..50f8cce938cf 100644 --- a/vendor/github.com/hashicorp/nomad/api/allocations.go +++ b/vendor/github.com/hashicorp/nomad/api/allocations.go @@ -432,6 +432,7 @@ type AllocationListStub struct { JobType string JobVersion uint64 TaskGroup string + AllocatedResources *AllocatedResources `json:",omitempty"` DesiredStatus string DesiredDescription string ClientStatus string diff --git a/vendor/github.com/hashicorp/nomad/api/nodes.go b/vendor/github.com/hashicorp/nomad/api/nodes.go index 3a6aa3b3b59d..4dc399e3fc67 100644 --- a/vendor/github.com/hashicorp/nomad/api/nodes.go +++ b/vendor/github.com/hashicorp/nomad/api/nodes.go @@ -786,6 +786,8 @@ type NodeListStub struct { Status string StatusDescription string Drivers map[string]*DriverInfo + NodeResources *NodeResources `json:",omitempty"` + ReservedResources *NodeReservedResources `json:",omitempty"` CreateIndex uint64 ModifyIndex uint64 } From 276be8604a84bd8680f84da343c5855ec910d563 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 14 Oct 2020 10:41:32 -0700 Subject: [PATCH 2/3] docs: document #9055 --- CHANGELOG.md | 2 ++ website/pages/api-docs/allocations.mdx | 9 +++++++++ website/pages/api-docs/nodes.mdx | 3 +++ 3 files changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d31e3ffb353..42a821877aed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ IMPROVEMENTS: * core: Improved job deregistration error logging. [[GH-8745](https://github.com/hashicorp/nomad/issues/8745)] * api: Added support for cancellation contexts to HTTP API. [[GH-8836](https://github.com/hashicorp/nomad/issues/8836)] * api: Job Register API now permits non-zero initial Version to accommodate multi-region deployments. [[GH-9071](https://github.com/hashicorp/nomad/issues/9071)] + * api: Added ?resources=true query parameter to /v1/nodes and /v1/allocations to include resource allocations in listings. [[GH-9055](https://github.com/hashicorp/nomad/issues/9055)] + * api: Added ?task_states=false query parameter to /v1/allocations to remove TaskStates from listings. Defaults to being included as before. [[GH-9055](https://github.com/hashicorp/nomad/issues/9055)] * cli: Added `scale` and `scaling-events` subcommands to the `job` command. [[GH-9023](https://github.com/hashicorp/nomad/pull/9023)] * cli: Added `scaling` command for interaction with the scaling API endpoint. [[GH-9025](https://github.com/hashicorp/nomad/pull/9025)] * client: Use ec2 CPU perf data from AWS API [[GH-7830](https://github.com/hashicorp/nomad/issues/7830)] diff --git a/website/pages/api-docs/allocations.mdx b/website/pages/api-docs/allocations.mdx index b2567b51c03f..98d143958aae 100644 --- a/website/pages/api-docs/allocations.mdx +++ b/website/pages/api-docs/allocations.mdx @@ -35,6 +35,15 @@ The table below shows this endpoint's support for - `namespace` `(string: "default")` - Specifies the namespace to search. Specifying `*` would return all allocations across all the authorized namespaces. +- `resources` `(bool: false)` - Specifies whether or not to include the + `AllocatedResources` field in the response. + +- `task_states` `(bool: true)` - Specifies whether or not to include the + `TaskStates` field in the response. TaskStates are included by default but + can represent a large percentage of the overall response size. Clusters with + a large number of allocations may set `task_states=false` to significantly + reduce the size of the response. + ### Sample Request ```shell-session diff --git a/website/pages/api-docs/nodes.mdx b/website/pages/api-docs/nodes.mdx index a3308b2a4e07..ba4c810c9b4b 100644 --- a/website/pages/api-docs/nodes.mdx +++ b/website/pages/api-docs/nodes.mdx @@ -32,6 +32,9 @@ The table below shows this endpoint's support for number of hexadecimal characters (0-9a-f). This is specified as a query string parameter. +- `resources` `(bool: false)` - Specifies whether or not to include the + `NodeResources` and `ReservedResources` fields in the response. + ### Sample Request ```shell-session From 0695801256b473d9370b49737659283a0a88c318 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 14 Oct 2020 12:23:25 -0700 Subject: [PATCH 3/3] unify boolean parameter parsing --- command/agent/alloc_endpoint.go | 18 ++++++-- command/agent/http.go | 26 +++-------- command/agent/http_test.go | 77 ++++++++------------------------- command/agent/node_endpoint.go | 8 ++-- 4 files changed, 43 insertions(+), 86 deletions(-) diff --git a/command/agent/alloc_endpoint.go b/command/agent/alloc_endpoint.go index ef2ae58658d3..1f55007e1d9e 100644 --- a/command/agent/alloc_endpoint.go +++ b/command/agent/alloc_endpoint.go @@ -34,15 +34,25 @@ func (s *HTTPServer) AllocsRequest(resp http.ResponseWriter, req *http.Request) } // Parse resources and task_states field selection - var err error - args.Fields = structs.NewAllocStubFields() - if args.Fields.Resources, err = parseResources(req); err != nil { + resources, err := parseBool(req, "resources") + if err != nil { return nil, err } - if args.Fields.TaskStates, err = parseTaskStates(req); err != nil { + taskStates, err := parseBool(req, "task_states") + if err != nil { return nil, err } + if resources != nil || taskStates != nil { + args.Fields = structs.NewAllocStubFields() + if resources != nil { + args.Fields.Resources = *resources + } + if taskStates != nil { + args.Fields.TaskStates = *taskStates + } + } + var out structs.AllocListResponse if err := s.agent.RPC("Alloc.List", &args, &out); err != nil { return nil, err diff --git a/command/agent/http.go b/command/agent/http.go index 23c67032cf6d..62bb972e455a 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -653,30 +653,18 @@ func parseNamespace(req *http.Request, n *string) { } } -// parseResources is used to parse the ?resources parameter -func parseResources(req *http.Request) (bool, error) { - if resourcesStr := req.URL.Query().Get("resources"); resourcesStr != "" { - resources, err := strconv.ParseBool(resourcesStr) - if err != nil { - return false, fmt.Errorf("Failed to parse value of %q (%v) as a bool: %v", "resources", resourcesStr, err) - } - return resources, nil - } - - return false, nil -} - -// parseTaskStates is used to parse the ?task_states parameter -func parseTaskStates(req *http.Request) (bool, error) { - if str := req.URL.Query().Get("task_states"); str != "" { +// parseBool parses a query parameter to a boolean or returns (nil, nil) if the +// parameter is not present. +func parseBool(req *http.Request, field string) (*bool, error) { + if str := req.URL.Query().Get(field); str != "" { param, err := strconv.ParseBool(str) if err != nil { - return false, fmt.Errorf("Failed to parse value of %q (%v) as a bool: %v", "task_states", str, err) + return nil, fmt.Errorf("Failed to parse value of %q (%v) as a bool: %v", field, str, err) } - return param, nil + return ¶m, nil } - return false, nil + return nil, nil } // parseToken is used to parse the X-Nomad-Token param diff --git a/command/agent/http_test.go b/command/agent/http_test.go index d2f1548b559b..4fd6b39b3061 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -515,91 +515,48 @@ func TestParseToken(t *testing.T) { } } -func TestParseResources(t *testing.T) { +func TestParseBool(t *testing.T) { t.Parallel() cases := []struct { - Value string - Resources bool - Err bool // true if an error should be expected + Input string + Expected *bool + Err bool // true if an error should be expected }{ { - Value: "", - Resources: false, + Input: "", + Expected: nil, }, { - Value: "true", - Resources: true, + Input: "true", + Expected: helper.BoolToPtr(true), }, { - Value: "false", - Resources: false, + Input: "false", + Expected: helper.BoolToPtr(false), }, { - Value: "1234", + Input: "1234", Err: true, }, } for i := range cases { tc := cases[i] - t.Run("Value-"+tc.Value, func(t *testing.T) { - testURL, err := url.Parse("http://localhost/foo?resources=" + tc.Value) + t.Run("Input-"+tc.Input, func(t *testing.T) { + testURL, err := url.Parse("http://localhost/foo?resources=" + tc.Input) require.NoError(t, err) req := &http.Request{ URL: testURL, } - result, err := parseResources(req) + result, err := parseBool(req, "resources") if tc.Err { require.Error(t, err) + require.Nil(t, result) } else { - require.Equal(t, tc.Resources, result) - } - }) - } -} - -func TestParseTaskStates(t *testing.T) { - t.Parallel() - - cases := []struct { - Value string - TaskStates bool - Err bool // true if an error should be expected - }{ - { - Value: "", - TaskStates: false, - }, - { - Value: "true", - TaskStates: true, - }, - { - Value: "false", - TaskStates: false, - }, - { - Value: "1234", - Err: true, - }, - } - - for i := range cases { - tc := cases[i] - t.Run("Value-"+tc.Value, func(t *testing.T) { - testURL, err := url.Parse("http://localhost/foo?task_states=" + tc.Value) - require.NoError(t, err) - req := &http.Request{ - URL: testURL, - } - - result, err := parseTaskStates(req) - if tc.Err { - require.Error(t, err) - } else { - require.Equal(t, tc.TaskStates, result) + require.NoError(t, err) + require.Equal(t, tc.Expected, result) } }) } diff --git a/command/agent/node_endpoint.go b/command/agent/node_endpoint.go index b2f53a9d71c8..95044db83385 100644 --- a/command/agent/node_endpoint.go +++ b/command/agent/node_endpoint.go @@ -21,11 +21,13 @@ func (s *HTTPServer) NodesRequest(resp http.ResponseWriter, req *http.Request) ( } // Parse resources field selection - if resources, err := parseResources(req); err != nil { + resources, err := parseBool(req, "resources") + if err != nil { return nil, err - } else if resources { + } + if resources != nil { args.Fields = &structs.NodeStubFields{ - Resources: true, + Resources: *resources, } }