Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

api: add field filters to /v1/{allocations,nodes} #9055

Merged
merged 3 commits into from
Oct 14, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
1 change: 1 addition & 0 deletions api/allocations.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ type AllocationListStub struct {
JobType string
JobVersion uint64
TaskGroup string
AllocatedResources *AllocatedResources `json:",omitempty"`
DesiredStatus string
DesiredDescription string
ClientStatus string
Expand Down
80 changes: 54 additions & 26 deletions api/allocations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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) {
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 2 additions & 0 deletions api/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
39 changes: 39 additions & 0 deletions api/nodes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion api/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
10 changes: 10 additions & 0 deletions command/agent/alloc_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions command/agent/csi_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
}

Expand Down
26 changes: 26 additions & 0 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Copy link
Member

Choose a reason for hiding this comment

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

golf: seems like these two could be combined into something like parseBool(req *http.Request, param string)

Copy link
Member Author

Choose a reason for hiding this comment

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

Pushed. Mind taking another peek? My old approach let a bug sneak in that should be fixed now.

Copy link
Member

Choose a reason for hiding this comment

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

The bug being parseTaskStates would return false by default if the field was absent?
(LGTM)

In the past I've used a [closed] library for extracting url parameters by providing a schema, providing the name, type, and default value into a call to lib.Parse. It helped cleanup a lot of this url parameter boilerplate - might be worth trying to recreate it sometime.


// 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 != "" {
Expand Down
90 changes: 90 additions & 0 deletions command/agent/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 9 additions & 0 deletions command/agent/node_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion nomad/alloc_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading