Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions .changelog/27680.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
cli: Fixed `job dispatch` and `job periodic force` failing with a paginator error against servers older than the CLI
```
7 changes: 7 additions & 0 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ const (
// by the API which indicates the caller does not have permission to
// perform the action.
PermissionDeniedErrorContent = "Permission denied"

// ResultPaginatorErrorContent is the string content of an error returned by
// the API when it cannot build the paginator for a list query, for example
// when the server is unable to evaluate the requested filter expression.
// This duplicates the message of structs.ErrResultPaginatorCreation, which
// the server emits. The api module cannot import structs, so keep them in sync.
ResultPaginatorErrorContent = "failed to create result paginator"
Comment thread
tgross marked this conversation as resolved.
)

// QueryOptions are used to parametrize a query
Expand Down
2 changes: 1 addition & 1 deletion command/alloc_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func (l *AllocExecCommand) Run(args []string) int {

var allocStub *api.AllocationListStub
if job {
jobID, ns, err := l.JobIDByPrefix(client, args[0], "")
jobID, ns, err := l.JobIDByPrefix(client, args[0])
if err != nil {
l.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/alloc_fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (f *AllocFSCommand) Run(args []string) int {
// If -job is specified, use random allocation, otherwise use provided allocation
allocID := args[0]
if job {
jobID, ns, err := f.JobIDByPrefix(client, args[0], "")
jobID, ns, err := f.JobIDByPrefix(client, args[0])
if err != nil {
f.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/alloc_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func (l *AllocLogsCommand) Run(args []string) int {
// If -job is specified, use random allocation, otherwise use provided allocation
allocID := args[0]
if l.job {
jobID, ns, err := l.JobIDByPrefix(client, args[0], "")
jobID, ns, err := l.JobIDByPrefix(client, args[0])
if err != nil {
l.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func (c *JobActionCommand) Run(args []string) int {
return 1
}

jobID, ns, err := c.JobIDByPrefix(client, job, "")
jobID, ns, err := c.JobIDByPrefix(client, job)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_allocs.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func (c *JobAllocsCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (c *JobDeploymentsCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
5 changes: 3 additions & 2 deletions command/job_dispatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,9 @@ func (c *JobDispatchCommand) Run(args []string) int {
}

jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix,
`ParentID == "" and ParameterizedJob is not nil`)
jobID, namespace, err := c.jobIDByPrefix(client, jobIDPrefix,
`ParentID == "" and ParameterizedJob is not nil`,
func(j *api.JobListStub) bool { return j.ParentID == "" && j.ParameterizedJob })
if err != nil {
var noPrefixErr *NoJobWithPrefixError
if errors.As(err, &noPrefixErr) {
Expand Down
2 changes: 1 addition & 1 deletion command/job_eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (c *JobEvalCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func (c *JobHistoryCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (c *JobInspectCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_inspect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestInspectCommand_HCLOutput(t *testing.T) {

// trigger version #3 retrieve the job struct from client.Meta.JobByPrefix
// to imitate job.Start
stateJob, err := cmd.Meta.JobByPrefix(client, *job.ID, "")
stateJob, err := cmd.Meta.JobByPrefix(client, *job.ID)
must.NoError(t, err)
_, _, err = client.Jobs().Register(stateJob, nil)
must.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion command/job_periodic_force.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ func (c *JobPeriodicForceCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "Periodic is not nil")
jobID, namespace, err := c.jobIDByPrefix(client, jobIDPrefix, "Periodic is not nil",
func(j *api.JobListStub) bool { return j.Periodic })
if err != nil {
var noPrefixErr *NoJobWithPrefixError
if errors.As(err, &noPrefixErr) {
Expand Down
2 changes: 1 addition & 1 deletion command/job_promote.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (c *JobPromoteCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func (c *JobRestartCommand) Run(args []string) int {
}

// Use prefix matching to find job.
job, err := c.JobByPrefix(c.client, c.jobID, "")
job, err := c.JobByPrefix(c.client, c.jobID)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_revert.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (c *JobRevertCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (j *JobScaleCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := j.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := j.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
j.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_scaling_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (j *JobScalingEventsCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := j.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := j.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
j.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (c *JobStartCommand) Run(args []string) int {
length = fullId
}

job, err := c.JobByPrefix(client, jobIDPrefix, "")
job, err := c.JobByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (c *JobStatusCommand) Run(args []string) int {

// Try querying the job
jobIDPrefix := strings.TrimSpace(args[0])
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (c *JobStopCommand) Run(args []string) int {
}

// Check if the job exists
job, err := c.JobByPrefix(client, jobID, "")
job, err := c.JobByPrefix(client, jobID)
if err != nil {
c.Ui.Error(err.Error())
statusCh <- 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_tag_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (c *JobTagApplyCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(job)
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
2 changes: 1 addition & 1 deletion command/job_tag_unset.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (c *JobTagUnsetCommand) Run(args []string) int {

// Check if the job exists
jobIDPrefix := strings.TrimSpace(job)
jobID, _, err := c.JobIDByPrefix(client, jobIDPrefix, "")
jobID, _, err := c.JobIDByPrefix(client, jobIDPrefix)
if err != nil {
c.Ui.Error(err.Error())
return 1
Expand Down
41 changes: 36 additions & 5 deletions command/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,8 @@ func (e *NoJobWithPrefixError) Error() string {
// JobByPrefix returns the job that best matches the given prefix. Returns an
// error if there are no matches or if there are more than one exact match
// across namespaces.
func (m *Meta) JobByPrefix(client *api.Client, prefix string, filter string) (*api.Job, error) {
jobID, namespace, err := m.JobIDByPrefix(client, prefix, filter)
func (m *Meta) JobByPrefix(client *api.Client, prefix string) (*api.Job, error) {
jobID, namespace, err := m.JobIDByPrefix(client, prefix)
if err != nil {
return nil, err
}
Expand All @@ -340,22 +340,53 @@ func (m *Meta) JobByPrefix(client *api.Client, prefix string, filter string) (*a
return job, nil
}

// JobByPrefixFilterFunc filters jobs during a client-side prefix match. It is
// the fallback for servers that cannot evaluate the server-side filter expression.
type JobByPrefixFilterFunc func(*api.JobListStub) bool

// JobIDByPrefix provides best effort match for the given job prefix.
// Returns the prefix itself if job prefix search is not allowed and an error
// if there are no matches or if there are more than one exact match across
// namespaces.
func (m *Meta) JobIDByPrefix(client *api.Client, prefix string, filter string) (string, string, error) {
func (m *Meta) JobIDByPrefix(client *api.Client, prefix string) (string, string, error) {
return m.jobIDByPrefix(client, prefix, "", nil)
}

// jobIDByPrefix backs JobIDByPrefix. When clientFilter is set and the server is
// too old to parse the filter expression (the "is not nil" operator landed in
// 1.11.3), it retries without the server-side filter and applies clientFilter
// to the results instead.
func (m *Meta) jobIDByPrefix(client *api.Client, prefix, filter string, clientFilter JobByPrefixFilterFunc) (string, string, error) {
maxResults := 20 // reduce load for large sets
jobs, _, err := client.Jobs().ListOptions(nil, &api.QueryOptions{
Prefix: prefix,
Filter: filter,
PerPage: int32(maxResults),
})
truncated := len(jobs) >= maxResults
if err != nil {
if strings.Contains(err.Error(), api.PermissionDeniedErrorContent) {
return prefix, "", nil
}
return "", "", fmt.Errorf("Error querying job prefix %q: %s", prefix, err)
// Servers older than 1.11.3 reject the "is not nil" filter operator
// while building the result paginator. Retry without the server-side
// filter and narrow the results on the client.
Comment thread
tgross marked this conversation as resolved.
// COMPAT(2.0): remove this fallback once 1.10LTS is out of support
if clientFilter == nil || !strings.Contains(err.Error(), api.ResultPaginatorErrorContent) {
return "", "", fmt.Errorf("Error querying job prefix %q: %s", prefix, err)
}
jobs, _, err = client.Jobs().PrefixList(prefix)
if err != nil {
return "", "", fmt.Errorf("Error querying job prefix %q: %s", prefix, err)
}
var filtered []*api.JobListStub
for _, j := range jobs {
if clientFilter(j) {
filtered = append(filtered, j)
}
}
jobs = filtered
truncated = false // the unfiltered prefix list is complete
}

if len(jobs) == 0 {
Expand All @@ -365,7 +396,7 @@ func (m *Meta) JobIDByPrefix(client *api.Client, prefix string, filter string) (
exactMatch := prefix == jobs[0].ID
matchInMultipleNamespaces := m.allNamespaces() && jobs[0].ID == jobs[1].ID
truncatedMsg := ""
if len(jobs) >= maxResults {
if truncated {
truncatedMsg = "\n(results may be truncated)"
}
if !exactMatch || matchInMultipleNamespaces {
Expand Down
56 changes: 48 additions & 8 deletions command/meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
package command

import (
"encoding/json"
"flag"
"fmt"
"net/http"
"net/http/httptest"
"os"
"reflect"
"sort"
Expand Down Expand Up @@ -202,7 +205,6 @@ func TestMeta_JobByPrefix(t *testing.T) {
testCases := []struct {
name string
prefix string
filter string
expectedError string
}{
{
Expand All @@ -213,12 +215,6 @@ func TestMeta_JobByPrefix(t *testing.T) {
name: "partial match",
prefix: "exam",
},
{
name: "match with filter",
prefix: "job-",
// Filter out jobs so that only "job-2" matches.
filter: `ID == "job-2"`,
},
{
name: "multiple matches",
prefix: "job-",
Expand All @@ -238,7 +234,7 @@ func TestMeta_JobByPrefix(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
job, err := meta.JobByPrefix(client, tc.prefix, tc.filter)
job, err := meta.JobByPrefix(client, tc.prefix)
if tc.expectedError != "" {
must.Nil(t, job)
must.ErrorContains(t, err, tc.expectedError)
Expand All @@ -251,6 +247,50 @@ func TestMeta_JobByPrefix(t *testing.T) {
}
}

func TestMeta_JobIDByPrefix_OldServerFallback(t *testing.T) {
ci.Parallel(t)

// Simulate a server older than 1.11.3: it cannot parse the "is not nil"
// filter operator and rejects the filtered query while building the result
// paginator, but still serves an unfiltered prefix list.
Comment on lines +253 to +255

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Query().Get("filter") != "" {
w.WriteHeader(http.StatusBadRequest)
fmt.Fprint(w, `failed to create result paginator: unknown operator "is not nil"`)
return
}
json.NewEncoder(w).Encode([]*api.JobListStub{
{ID: "example", JobSummary: &api.JobSummary{Namespace: "default"}},
{ID: "example-batch", ParameterizedJob: true, JobSummary: &api.JobSummary{Namespace: "default"}},
})
}))
defer ts.Close()

conf := api.DefaultConfig()
conf.Address = ts.URL
client, err := api.NewClient(conf)
must.NoError(t, err)

meta := &Meta{Ui: cli.NewMockUi()}
onlyParameterized := func(j *api.JobListStub) bool {
return j.ParentID == "" && j.ParameterizedJob
}

// The fallback must narrow to the parameterized job, not the exact-match
// non-parameterized "example".
jobID, ns, err := meta.jobIDByPrefix(client, "example",
`ParentID == "" and ParameterizedJob is not nil`, onlyParameterized)
must.NoError(t, err)
must.Eq(t, "example-batch", jobID)
must.Eq(t, "default", ns)

// With a filter but no client filter, the old-server error is surfaced
// rather than swallowed.
_, _, err = meta.jobIDByPrefix(client, "example",
`ParentID == "" and ParameterizedJob is not nil`, nil)
must.ErrorContains(t, err, api.ResultPaginatorErrorContent)
}

func TestMeta_ShowUIPath(t *testing.T) {
ci.Parallel(t)

Expand Down
2 changes: 1 addition & 1 deletion nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1465,7 +1465,7 @@ func (j *Job) List(args *structs.JobListRequest, reply *structs.JobListResponse)
stubFn)
if err != nil {
return structs.NewErrRPCCodedf(
http.StatusBadRequest, "failed to create result paginator: %v", err)
http.StatusBadRequest, "%s: %v", structs.ErrResultPaginatorCreation, err)
}

jobs, nextToken, err := pager.Page()
Expand Down
Loading
Loading