diff --git a/command/agent/deployment_endpoint.go b/command/agent/deployment_endpoint.go index 729f95fd386..58829adce36 100644 --- a/command/agent/deployment_endpoint.go +++ b/command/agent/deployment_endpoint.go @@ -17,9 +17,6 @@ func (s *HTTPServer) DeploymentsRequest(resp http.ResponseWriter, req *http.Requ return nil, nil } - query := req.URL.Query() - args.OrderAscending = query.Get("ascending") == "true" - var out structs.DeploymentListResponse if err := s.agent.RPC("Deployment.List", &args, &out); err != nil { return nil, err diff --git a/command/agent/eval_endpoint.go b/command/agent/eval_endpoint.go index 3494b85085c..a51c9e9407c 100644 --- a/command/agent/eval_endpoint.go +++ b/command/agent/eval_endpoint.go @@ -20,7 +20,6 @@ func (s *HTTPServer) EvalsRequest(resp http.ResponseWriter, req *http.Request) ( query := req.URL.Query() args.FilterEvalStatus = query.Get("status") args.FilterJobID = query.Get("job") - args.OrderAscending = query.Get("ascending") == "true" var out structs.EvalListResponse if err := s.agent.RPC("Eval.List", &args, &out); err != nil { diff --git a/command/agent/http.go b/command/agent/http.go index d2ac5cdf5d3..8568a0b0e9d 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -788,6 +788,7 @@ func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, r *strin parseNamespace(req, &b.Namespace) parsePagination(req, b) parseFilter(req, b) + parseAscending(req, b) return parseWait(resp, req, b) } @@ -813,6 +814,12 @@ func parseFilter(req *http.Request, b *structs.QueryOptions) { } } +// parseAscending parses the ascending query parameter for QueryOptions +func parseAscending(req *http.Request, b *structs.QueryOptions) { + query := req.URL.Query() + b.Ascending = query.Get("ascending") == "true" +} + // parseWriteRequest is a convenience method for endpoints that need to parse a // write request. func (s *HTTPServer) parseWriteRequest(req *http.Request, w *structs.WriteRequest) { diff --git a/nomad/deployment_endpoint.go b/nomad/deployment_endpoint.go index 04ccacf1669..bfd3be7f4c0 100644 --- a/nomad/deployment_endpoint.go +++ b/nomad/deployment_endpoint.go @@ -413,9 +413,9 @@ func (d *Deployment) List(args *structs.DeploymentListRequest, reply *structs.De if prefix := args.QueryOptions.Prefix; prefix != "" { iter, err = store.DeploymentsByIDPrefix(ws, namespace, prefix) } else if namespace != structs.AllNamespacesSentinel { - iter, err = store.DeploymentsByNamespaceOrdered(ws, namespace, args.OrderAscending) + iter, err = store.DeploymentsByNamespaceOrdered(ws, namespace, args.Ascending) } else { - iter, err = store.Deployments(ws, args.OrderAscending) + iter, err = store.Deployments(ws, args.Ascending) } if err != nil { return err @@ -423,9 +423,10 @@ func (d *Deployment) List(args *structs.DeploymentListRequest, reply *structs.De var deploys []*structs.Deployment paginator, err := state.NewPaginator(iter, args.QueryOptions, - func(raw interface{}) { + func(raw interface{}) error { deploy := raw.(*structs.Deployment) deploys = append(deploys, deploy) + return nil }) if err != nil { return structs.NewErrRPCCodedf( diff --git a/nomad/deployment_endpoint_test.go b/nomad/deployment_endpoint_test.go index 42300062355..7170b507b3d 100644 --- a/nomad/deployment_endpoint_test.go +++ b/nomad/deployment_endpoint_test.go @@ -1072,8 +1072,8 @@ func TestDeploymentEndpoint_List_order(t *testing.T) { QueryOptions: structs.QueryOptions{ Region: "global", Namespace: "*", + Ascending: true, }, - OrderAscending: true, } var resp structs.DeploymentListResponse @@ -1099,8 +1099,8 @@ func TestDeploymentEndpoint_List_order(t *testing.T) { QueryOptions: structs.QueryOptions{ Region: "global", Namespace: "*", + Ascending: false, }, - OrderAscending: false, } var resp structs.DeploymentListResponse @@ -1399,7 +1399,6 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { req := &structs.DeploymentListRequest{ - OrderAscending: true, // counting up is easier to think about QueryOptions: structs.QueryOptions{ Region: "global", Namespace: tc.namespace, @@ -1407,6 +1406,7 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) { Filter: tc.filter, PerPage: tc.pageSize, NextToken: tc.nextToken, + Ascending: true, // counting up is easier to think about }, } req.AuthToken = aclToken diff --git a/nomad/eval_endpoint.go b/nomad/eval_endpoint.go index 9094258d1c1..b938ec9280e 100644 --- a/nomad/eval_endpoint.go +++ b/nomad/eval_endpoint.go @@ -418,9 +418,9 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon if prefix := args.QueryOptions.Prefix; prefix != "" { iter, err = store.EvalsByIDPrefix(ws, namespace, prefix) } else if namespace != structs.AllNamespacesSentinel { - iter, err = store.EvalsByNamespaceOrdered(ws, namespace, args.OrderAscending) + iter, err = store.EvalsByNamespaceOrdered(ws, namespace, args.Ascending) } else { - iter, err = store.Evals(ws, args.OrderAscending) + iter, err = store.Evals(ws, args.Ascending) } if err != nil { return err @@ -435,9 +435,10 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon var evals []*structs.Evaluation paginator, err := state.NewPaginator(iter, args.QueryOptions, - func(raw interface{}) { + func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) + return nil }) if err != nil { return structs.NewErrRPCCodedf( diff --git a/nomad/eval_endpoint_test.go b/nomad/eval_endpoint_test.go index ed9c2e59018..3aa327508f1 100644 --- a/nomad/eval_endpoint_test.go +++ b/nomad/eval_endpoint_test.go @@ -757,8 +757,8 @@ func TestEvalEndpoint_List_order(t *testing.T) { QueryOptions: structs.QueryOptions{ Region: "global", Namespace: "*", + Ascending: false, }, - OrderAscending: false, } var resp structs.EvalListResponse @@ -784,8 +784,8 @@ func TestEvalEndpoint_List_order(t *testing.T) { QueryOptions: structs.QueryOptions{ Region: "global", Namespace: "*", + Ascending: true, }, - OrderAscending: true, } var resp structs.EvalListResponse @@ -811,8 +811,8 @@ func TestEvalEndpoint_List_order(t *testing.T) { QueryOptions: structs.QueryOptions{ Region: "global", Namespace: "*", + Ascending: false, }, - OrderAscending: false, } var resp structs.EvalListResponse @@ -1249,7 +1249,6 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { req := &structs.EvalListRequest{ FilterJobID: tc.filterJobID, FilterEvalStatus: tc.filterStatus, - OrderAscending: true, // counting up is easier to think about QueryOptions: structs.QueryOptions{ Region: "global", Namespace: tc.namespace, @@ -1257,6 +1256,7 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) { PerPage: tc.pageSize, NextToken: tc.nextToken, Filter: tc.filter, + Ascending: true, // counting up is easier to think about }, } req.AuthToken = aclToken diff --git a/nomad/state/filter_test.go b/nomad/state/filter_test.go index 73f6754caa3..f0ba14a73b6 100644 --- a/nomad/state/filter_test.go +++ b/nomad/state/filter_test.go @@ -76,9 +76,10 @@ func BenchmarkEvalListFilter(b *testing.B) { for i := 0; i < b.N; i++ { iter, _ := state.EvalsByNamespace(nil, structs.DefaultNamespace) var evals []*structs.Evaluation - paginator, err := NewPaginator(iter, opts, func(raw interface{}) { + paginator, err := NewPaginator(iter, opts, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) + return nil }) if err != nil { b.Fatalf("failed: %v", err) @@ -98,9 +99,10 @@ func BenchmarkEvalListFilter(b *testing.B) { for i := 0; i < b.N; i++ { iter, _ := state.Evals(nil, false) var evals []*structs.Evaluation - paginator, err := NewPaginator(iter, opts, func(raw interface{}) { + paginator, err := NewPaginator(iter, opts, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) + return nil }) if err != nil { b.Fatalf("failed: %v", err) @@ -133,9 +135,10 @@ func BenchmarkEvalListFilter(b *testing.B) { for i := 0; i < b.N; i++ { iter, _ := state.EvalsByNamespace(nil, structs.DefaultNamespace) var evals []*structs.Evaluation - paginator, err := NewPaginator(iter, opts, func(raw interface{}) { + paginator, err := NewPaginator(iter, opts, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) + return nil }) if err != nil { b.Fatalf("failed: %v", err) @@ -169,9 +172,10 @@ func BenchmarkEvalListFilter(b *testing.B) { for i := 0; i < b.N; i++ { iter, _ := state.Evals(nil, false) var evals []*structs.Evaluation - paginator, err := NewPaginator(iter, opts, func(raw interface{}) { + paginator, err := NewPaginator(iter, opts, func(raw interface{}) error { eval := raw.(*structs.Evaluation) evals = append(evals, eval) + return nil }) if err != nil { b.Fatalf("failed: %v", err) diff --git a/nomad/state/paginator.go b/nomad/state/paginator.go index 138398908d1..02f7f6fa8c5 100644 --- a/nomad/state/paginator.go +++ b/nomad/state/paginator.go @@ -32,10 +32,10 @@ type Paginator struct { // appendFunc is the function the caller should use to append raw // entries to the results set. The object is guaranteed to be // non-nil. - appendFunc func(interface{}) + appendFunc func(interface{}) error } -func NewPaginator(iter Iterator, opts structs.QueryOptions, appendFunc func(interface{})) (*Paginator, error) { +func NewPaginator(iter Iterator, opts structs.QueryOptions, appendFunc func(interface{}) error) (*Paginator, error) { var evaluator *bexpr.Evaluator var err error @@ -64,7 +64,11 @@ DONE: raw, andThen := p.next() switch andThen { case paginatorInclude: - p.appendFunc(raw) + err := p.appendFunc(raw) + if err != nil { + p.pageErr = err + break DONE + } case paginatorSkip: continue case paginatorComplete: diff --git a/nomad/state/paginator_test.go b/nomad/state/paginator_test.go index 27d34d8f86d..b0871ddd3fb 100644 --- a/nomad/state/paginator_test.go +++ b/nomad/state/paginator_test.go @@ -1,6 +1,7 @@ package state import ( + "errors" "testing" "github.com/stretchr/testify/require" @@ -19,6 +20,7 @@ func TestPaginator(t *testing.T) { nextToken string expected []string expectedNextToken string + expectedError string }{ { name: "size-3 page-1", @@ -47,6 +49,10 @@ func TestPaginator(t *testing.T) { expected: []string{}, expectedNextToken: "", }, + { + name: "error during append", + expectedError: "failed to append", + }, } for _, tc := range cases { @@ -59,17 +65,27 @@ func TestPaginator(t *testing.T) { structs.QueryOptions{ PerPage: tc.perPage, NextToken: tc.nextToken, }, - func(raw interface{}) { + func(raw interface{}) error { + if tc.expectedError != "" { + return errors.New(tc.expectedError) + } + result := raw.(*mockObject) results = append(results, result.GetID()) + return nil }, ) require.NoError(t, err) nextToken, err := paginator.Page() - require.NoError(t, err) - require.Equal(t, tc.expected, results) - require.Equal(t, tc.expectedNextToken, nextToken) + if tc.expectedError == "" { + require.NoError(t, err) + require.Equal(t, tc.expected, results) + require.Equal(t, tc.expectedNextToken, nextToken) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedError) + } }) } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 9d6331a7528..be2ef8706f6 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -288,6 +288,9 @@ type QueryOptions struct { // previous response. NextToken string + // Ascending is used to have results sorted in ascending chronological order. + Ascending bool + InternalRpcInfo } @@ -863,7 +866,6 @@ type EvalDequeueRequest struct { type EvalListRequest struct { FilterJobID string FilterEvalStatus string - OrderAscending bool QueryOptions } @@ -1098,7 +1100,6 @@ type GenericRequest struct { // DeploymentListRequest is used to list the deployments type DeploymentListRequest struct { - OrderAscending bool QueryOptions }