From fcb15e7e0604c78c1890f1890d44efdb27df1a29 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 8 Oct 2020 22:21:41 -0700 Subject: [PATCH] api: add ?resources=true 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. --- 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 | 9 +++ command/agent/csi_endpoint.go | 4 +- command/agent/http.go | 13 +++ command/agent/http_test.go | 45 +++++++++++ command/agent/node_endpoint.go | 9 +++ nomad/alloc_endpoint.go | 2 +- 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/state/state_store.go | 2 +- nomad/structs/structs.go | 45 +++++++++-- scheduler/generic_sched.go | 2 +- scheduler/system_sched.go | 2 +- .../hashicorp/nomad/api/allocations.go | 1 + .../github.com/hashicorp/nomad/api/nodes.go | 2 + 22 files changed, 227 insertions(+), 43 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..6cebb0820d8e 100644 --- a/command/agent/alloc_endpoint.go +++ b/command/agent/alloc_endpoint.go @@ -33,6 +33,15 @@ func (s *HTTPServer) AllocsRequest(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.AllocStubFields{ + Resources: true, + } + } + 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..ec3e84546253 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -653,6 +653,19 @@ 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 +} + // 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..3e9fe3e4e243 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -515,6 +515,51 @@ 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) + } + }) + } +} + // 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/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 09deab68640d..2ac5618fa0c0 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/state/state_store.go b/nomad/state/state_store.go index 2e217c5dd0c8..0f5c5f36951d 100644 --- a/nomad/state/state_store.go +++ b/nomad/state/state_store.go @@ -2422,7 +2422,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 ab076d3df823..1160584c0b5b 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 { @@ -9221,8 +9242,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, @@ -9249,6 +9270,14 @@ func (a *Allocation) Stub() *AllocListStub { CreateTime: a.CreateTime, ModifyTime: a.ModifyTime, } + + if fields != nil { + if fields.Resources { + s.AllocatedResources = a.AllocatedResources + } + } + + return s } // AllocationDiff converts an Allocation type to an AllocationDiff type @@ -9276,6 +9305,7 @@ type AllocListStub struct { JobType string JobVersion uint64 TaskGroup string + AllocatedResources *AllocatedResources `json:",omitempty"` DesiredStatus string DesiredDescription string ClientStatus string @@ -9310,6 +9340,11 @@ func setDisplayMsg(taskStates map[string]*TaskState) { } } +// AllocStubFields defines which fields are included in the AllocListStub. +type AllocStubFields struct { + Resources bool +} + // 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 }