From 1d051d834db434a7102e1c1f3ee41b48745d69d8 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Fri, 3 Mar 2023 14:43:20 -0500 Subject: [PATCH] cli: use shared logic for resolving job prefix (#16306) Several `nomad job` subcommands had duplicate or slightly similar logic for resolving a job ID from a CLI argument prefix, while others did not have this functionality at all. This commit pulls the shared logic to the command Meta and updates all `nomad job` subcommands to use it. --- .changelog/16306.txt | 7 +++ command/job_allocs.go | 20 ++----- command/job_allocs_test.go | 4 +- command/job_deployments.go | 20 ++----- command/job_deployments_test.go | 2 +- command/job_dispatch.go | 14 ++++- command/job_dispatch_test.go | 2 +- command/job_eval.go | 16 ++++- command/job_history.go | 21 ++----- command/job_history_test.go | 2 +- command/job_inspect.go | 18 ++---- command/job_inspect_test.go | 4 +- command/job_periodic_force.go | 31 ++++------ command/job_periodic_force_test.go | 2 +- command/job_promote.go | 21 ++----- command/job_promote_test.go | 2 +- command/job_revert.go | 21 ++----- command/job_revert_test.go | 2 +- command/job_scale.go | 17 ++++-- command/job_scaling_events.go | 14 +++-- command/job_status.go | 21 ++----- command/job_stop.go | 27 +-------- command/job_stop_test.go | 4 +- command/meta.go | 72 ++++++++++++++++++++++ command/meta_test.go | 95 ++++++++++++++++++++++++++++++ 25 files changed, 280 insertions(+), 179 deletions(-) create mode 100644 .changelog/16306.txt diff --git a/.changelog/16306.txt b/.changelog/16306.txt new file mode 100644 index 000000000..340bb1e37 --- /dev/null +++ b/.changelog/16306.txt @@ -0,0 +1,7 @@ +```release-note:improvement +cli: Add job prefix match to the `nomad job dispatch`, `nomad job eval`, `nomad job scale`, and `nomad job scaling-events` commands +``` + +```release-note:improvement +cli: Add support for the wildcard namespace `*` to the `nomad job dispatch`, `nomad job eval`, `nomad job scale`, and `nomad job scaling-events` commands +``` diff --git a/command/job_allocs.go b/command/job_allocs.go index a976f789c..587d79e58 100644 --- a/command/job_allocs.go +++ b/command/job_allocs.go @@ -105,27 +105,15 @@ func (c *JobAllocsCommand) Run(args []string) int { return 1 } - jobID := strings.TrimSpace(args[0]) - // Check if the job exists - jobs, _, err := client.Jobs().PrefixList(jobID) + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, nil) if err != nil { - c.Ui.Error(fmt.Sprintf("Error listing jobs: %s", err)) + c.Ui.Error(err.Error()) return 1 } - if len(jobs) == 0 { - c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID)) - return 1 - } - if len(jobs) > 1 { - if (jobID != jobs[0].ID) || (c.allNamespaces() && jobs[0].ID == jobs[1].ID) { - c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs, c.allNamespaces()))) - return 1 - } - } - jobID = jobs[0].ID - q := &api.QueryOptions{Namespace: jobs[0].JobSummary.Namespace} + q := &api.QueryOptions{Namespace: namespace} allocs, _, err := client.Jobs().Allocations(jobID, all, q) if err != nil { diff --git a/command/job_allocs_test.go b/command/job_allocs_test.go index ce2cfea63..9d322d124 100644 --- a/command/job_allocs_test.go +++ b/command/job_allocs_test.go @@ -36,7 +36,7 @@ func TestJobAllocsCommand_Fails(t *testing.T) { code = cmd.Run([]string{"-address=nope", "foo"}) outerr = ui.ErrorWriter.String() require.Equalf(t, 1, code, "expected exit code 1, got: %d", code) - require.Containsf(t, outerr, "Error listing jobs", "expected failed query error, got: %s", outerr) + require.Containsf(t, outerr, "Error querying job prefix", "expected failed query error, got: %s", outerr) ui.ErrorWriter.Reset() @@ -44,7 +44,7 @@ func TestJobAllocsCommand_Fails(t *testing.T) { code = cmd.Run([]string{"-address=" + url, "foo"}) outerr = ui.ErrorWriter.String() require.Equalf(t, 1, code, "expected exit 1, got: %d", code) - require.Containsf(t, outerr, "No job(s) with prefix or id \"foo\" found", "expected no job found, got: %s", outerr) + require.Containsf(t, outerr, "No job(s) with prefix or ID \"foo\" found", "expected no job found, got: %s", outerr) ui.ErrorWriter.Reset() } diff --git a/command/job_deployments.go b/command/job_deployments.go index a6c3abc8c..45c7c0fd8 100644 --- a/command/job_deployments.go +++ b/command/job_deployments.go @@ -110,27 +110,15 @@ func (c *JobDeploymentsCommand) Run(args []string) int { return 1 } - jobID := strings.TrimSpace(args[0]) - // Check if the job exists - jobs, _, err := client.Jobs().PrefixList(jobID) + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, nil) if err != nil { - c.Ui.Error(fmt.Sprintf("Error listing jobs: %s", err)) + c.Ui.Error(err.Error()) return 1 } - if len(jobs) == 0 { - c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID)) - return 1 - } - if len(jobs) > 1 { - if (jobID != jobs[0].ID) || (c.allNamespaces() && jobs[0].ID == jobs[1].ID) { - c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs, c.allNamespaces()))) - return 1 - } - } - jobID = jobs[0].ID - q := &api.QueryOptions{Namespace: jobs[0].JobSummary.Namespace} + q := &api.QueryOptions{Namespace: namespace} // Truncate the id unless full length is requested length := shortId diff --git a/command/job_deployments_test.go b/command/job_deployments_test.go index d727ac2fa..97ee41cec 100644 --- a/command/job_deployments_test.go +++ b/command/job_deployments_test.go @@ -34,7 +34,7 @@ func TestJobDeploymentsCommand_Fails(t *testing.T) { if code := cmd.Run([]string{"-address=nope", "foo"}); code != 1 { t.Fatalf("expected exit code 1, got: %d", code) } - if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error listing jobs") { + if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error querying job prefix") { t.Fatalf("expected failed query error, got: %s", out) } ui.ErrorWriter.Reset() diff --git a/command/job_dispatch.go b/command/job_dispatch.go index d8965d1d4..499dea3c4 100644 --- a/command/job_dispatch.go +++ b/command/job_dispatch.go @@ -138,7 +138,6 @@ func (c *JobDispatchCommand) Run(args []string) int { return 1 } - job := args[0] var payload []byte var readErr error @@ -175,11 +174,22 @@ func (c *JobDispatchCommand) Run(args []string) int { return 1 } + // Check if the job exists + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, func(j *api.JobListStub) bool { + return j.ParameterizedJob + }) + if err != nil { + c.Ui.Error(err.Error()) + return 1 + } + // Dispatch the job w := &api.WriteOptions{ IdempotencyToken: idempotencyToken, + Namespace: namespace, } - resp, _, err := client.Jobs().Dispatch(job, metaMap, payload, idPrefixTemplate, w) + resp, _, err := client.Jobs().Dispatch(jobID, metaMap, payload, idPrefixTemplate, w) if err != nil { c.Ui.Error(fmt.Sprintf("Failed to dispatch job: %s", err)) return 1 diff --git a/command/job_dispatch_test.go b/command/job_dispatch_test.go index ff7410340..d72af9ead 100644 --- a/command/job_dispatch_test.go +++ b/command/job_dispatch_test.go @@ -43,7 +43,7 @@ func TestJobDispatchCommand_Fails(t *testing.T) { if code := cmd.Run([]string{"-address=nope", "foo"}); code != 1 { t.Fatalf("expected exit code 1, got: %d", code) } - if out := ui.ErrorWriter.String(); !strings.Contains(out, "Failed to dispatch") { + if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error querying job prefix") { t.Fatalf("expected failed query error, got: %s", out) } ui.ErrorWriter.Reset() diff --git a/command/job_eval.go b/command/job_eval.go index b2ceafe8c..26cdd0583 100644 --- a/command/job_eval.go +++ b/command/job_eval.go @@ -110,13 +110,23 @@ func (c *JobEvalCommand) Run(args []string) int { if verbose { length = fullId } - // Call eval endpoint - jobID := args[0] + // Check if the job exists + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, nil) + if err != nil { + c.Ui.Error(err.Error()) + return 1 + } + + // Call eval endpoint opts := api.EvalOptions{ ForceReschedule: c.forceRescheduling, } - evalId, _, err := client.Jobs().EvaluateWithOpts(jobID, opts, nil) + w := &api.WriteOptions{ + Namespace: namespace, + } + evalId, _, err := client.Jobs().EvaluateWithOpts(jobID, opts, w) if err != nil { c.Ui.Error(fmt.Sprintf("Error evaluating job: %s", err)) return 1 diff --git a/command/job_history.go b/command/job_history.go index 05e05aa3d..273634f6a 100644 --- a/command/job_history.go +++ b/command/job_history.go @@ -121,29 +121,18 @@ func (c *JobHistoryCommand) Run(args []string) int { return 1 } - jobID := strings.TrimSpace(args[0]) - // Check if the job exists - jobs, _, err := client.Jobs().PrefixList(jobID) + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, nil) if err != nil { - c.Ui.Error(fmt.Sprintf("Error listing jobs: %s", err)) + c.Ui.Error(err.Error()) return 1 } - if len(jobs) == 0 { - c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID)) - return 1 - } - if len(jobs) > 1 { - if (jobID != jobs[0].ID) || (c.allNamespaces() && jobs[0].ID == jobs[1].ID) { - c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs, c.allNamespaces()))) - return 1 - } - } - q := &api.QueryOptions{Namespace: jobs[0].JobSummary.Namespace} + q := &api.QueryOptions{Namespace: namespace} // Prefix lookup matched a single job - versions, diffs, _, err := client.Jobs().Versions(jobs[0].ID, diff, q) + versions, diffs, _, err := client.Jobs().Versions(jobID, diff, q) if err != nil { c.Ui.Error(fmt.Sprintf("Error retrieving job versions: %s", err)) return 1 diff --git a/command/job_history_test.go b/command/job_history_test.go index c85fee2a9..ad0f43753 100644 --- a/command/job_history_test.go +++ b/command/job_history_test.go @@ -34,7 +34,7 @@ func TestJobHistoryCommand_Fails(t *testing.T) { if code := cmd.Run([]string{"-address=nope", "foo"}); code != 1 { t.Fatalf("expected exit code 1, got: %d", code) } - if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error listing jobs") { + if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error querying job prefix") { t.Fatalf("expected failed query error, got: %s", out) } ui.ErrorWriter.Reset() diff --git a/command/job_inspect.go b/command/job_inspect.go index 8cf2f36eb..16a32bcbd 100644 --- a/command/job_inspect.go +++ b/command/job_inspect.go @@ -117,24 +117,14 @@ func (c *JobInspectCommand) Run(args []string) int { c.Ui.Error(commandErrorText(c)) return 1 } - jobID := strings.TrimSpace(args[0]) // Check if the job exists - jobs, _, err := client.Jobs().PrefixList(jobID) + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, nil) if err != nil { - c.Ui.Error(fmt.Sprintf("Error inspecting job: %s", err)) + c.Ui.Error(err.Error()) return 1 } - if len(jobs) == 0 { - c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID)) - return 1 - } - if len(jobs) > 1 { - if (jobID != jobs[0].ID) || (c.allNamespaces() && jobs[0].ID == jobs[1].ID) { - c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs, c.allNamespaces()))) - return 1 - } - } var version *uint64 if versionStr != "" { @@ -148,7 +138,7 @@ func (c *JobInspectCommand) Run(args []string) int { } // Prefix lookup matched a single job - job, err := getJob(client, jobs[0].JobSummary.Namespace, jobs[0].ID, version) + job, err := getJob(client, namespace, jobID, version) if err != nil { c.Ui.Error(fmt.Sprintf("Error inspecting job: %s", err)) return 1 diff --git a/command/job_inspect_test.go b/command/job_inspect_test.go index ac3809778..dbbdcf08c 100644 --- a/command/job_inspect_test.go +++ b/command/job_inspect_test.go @@ -38,7 +38,7 @@ func TestInspectCommand_Fails(t *testing.T) { if code := cmd.Run([]string{"-address=" + url, "nope"}); code != 1 { t.Fatalf("expect exit 1, got: %d", code) } - if out := ui.ErrorWriter.String(); !strings.Contains(out, "No job(s) with prefix or id") { + if out := ui.ErrorWriter.String(); !strings.Contains(out, "No job(s) with prefix or ID") { t.Fatalf("expect not found error, got: %s", out) } ui.ErrorWriter.Reset() @@ -47,7 +47,7 @@ func TestInspectCommand_Fails(t *testing.T) { if code := cmd.Run([]string{"-address=nope", "nope"}); code != 1 { t.Fatalf("expected exit code 1, got: %d", code) } - if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error inspecting job") { + if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error querying job prefix") { t.Fatalf("expected failed query error, got: %s", out) } ui.ErrorWriter.Reset() diff --git a/command/job_periodic_force.go b/command/job_periodic_force.go index 2e074c0f9..720dd83cb 100644 --- a/command/job_periodic_force.go +++ b/command/job_periodic_force.go @@ -1,6 +1,7 @@ package command import ( + "errors" "fmt" "strings" @@ -112,31 +113,19 @@ func (c *JobPeriodicForceCommand) Run(args []string) int { } // Check if the job exists - jobID := args[0] - jobs, _, err := client.Jobs().PrefixList(jobID) + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, func(j *api.JobListStub) bool { + return j.Periodic + }) if err != nil { - c.Ui.Error(fmt.Sprintf("Error forcing periodic job: %s", err)) - return 1 - } - // filter non-periodic jobs - periodicJobs := make([]*api.JobListStub, 0, len(jobs)) - for _, j := range jobs { - if j.Periodic { - periodicJobs = append(periodicJobs, j) + var noPrefixErr *NoJobWithPrefixError + if errors.As(err, &noPrefixErr) { + err = fmt.Errorf("No periodic job(s) with prefix or ID %q found", jobIDPrefix) } - } - if len(periodicJobs) == 0 { - c.Ui.Error(fmt.Sprintf("No periodic job(s) with prefix or id %q found", jobID)) + c.Ui.Error(err.Error()) return 1 } - // preriodicJobs is sorted by job ID - // so if there is a job whose ID is equal to jobID then it must be the first item - if len(periodicJobs) > 1 && periodicJobs[0].ID != jobID { - c.Ui.Error(fmt.Sprintf("Prefix matched multiple periodic jobs\n\n%s", createStatusListOutput(periodicJobs, c.allNamespaces()))) - return 1 - } - jobID = periodicJobs[0].ID - q := &api.WriteOptions{Namespace: periodicJobs[0].JobSummary.Namespace} + q := &api.WriteOptions{Namespace: namespace} // force the evaluation evalID, _, err := client.Jobs().PeriodicForce(jobID, q) diff --git a/command/job_periodic_force_test.go b/command/job_periodic_force_test.go index efc40c8f6..7f892a0b6 100644 --- a/command/job_periodic_force_test.go +++ b/command/job_periodic_force_test.go @@ -35,7 +35,7 @@ func TestJobPeriodicForceCommand_Fails(t *testing.T) { code = cmd.Run([]string{"-address=nope", "12"}) require.Equal(t, code, 1, "expected error") out = ui.ErrorWriter.String() - require.Contains(t, out, "Error forcing periodic job", "expected force error") + require.Contains(t, out, "Error querying job prefix", "expected force error") } func TestJobPeriodicForceCommand_AutocompleteArgs(t *testing.T) { diff --git a/command/job_promote.go b/command/job_promote.go index 5d308b17d..2b63933be 100644 --- a/command/job_promote.go +++ b/command/job_promote.go @@ -117,24 +117,13 @@ func (c *JobPromoteCommand) Run(args []string) int { } // Check if the job exists - jobID := strings.TrimSpace(args[0]) - jobs, _, err := client.Jobs().PrefixList(jobID) + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, nil) if err != nil { - c.Ui.Error(fmt.Sprintf("Error promoting job: %s", err)) + c.Ui.Error(err.Error()) return 1 } - if len(jobs) == 0 { - c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID)) - return 1 - } - if len(jobs) > 1 { - if (jobID != jobs[0].ID) || (c.allNamespaces() && jobs[0].ID == jobs[1].ID) { - c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs, c.allNamespaces()))) - return 1 - } - } - jobID = jobs[0].ID - q := &api.QueryOptions{Namespace: jobs[0].JobSummary.Namespace} + q := &api.QueryOptions{Namespace: namespace} // Do a prefix lookup deploy, _, err := client.Jobs().LatestDeployment(jobID, q) @@ -148,7 +137,7 @@ func (c *JobPromoteCommand) Run(args []string) int { return 1 } - wq := &api.WriteOptions{Namespace: jobs[0].JobSummary.Namespace} + wq := &api.WriteOptions{Namespace: namespace} var u *api.DeploymentUpdateResponse if len(groups) == 0 { u, _, err = client.Deployments().PromoteAll(deploy.ID, wq) diff --git a/command/job_promote_test.go b/command/job_promote_test.go index ec036e9a1..cc69336b2 100644 --- a/command/job_promote_test.go +++ b/command/job_promote_test.go @@ -35,7 +35,7 @@ func TestJobPromoteCommand_Fails(t *testing.T) { if code := cmd.Run([]string{"-address=nope", "12"}); code != 1 { t.Fatalf("expected exit code 1, got: %d", code) } - if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error promoting") { + if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error querying job prefix") { t.Fatalf("expected failed to promote error, got: %s", out) } ui.ErrorWriter.Reset() diff --git a/command/job_revert.go b/command/job_revert.go index 6ffde4736..fa2198b56 100644 --- a/command/job_revert.go +++ b/command/job_revert.go @@ -126,7 +126,7 @@ func (c *JobRevertCommand) Run(args []string) int { vaultToken = os.Getenv("VAULT_TOKEN") } - jobID := strings.TrimSpace(args[0]) + // Parse the job version revertVersion, ok, err := parseVersion(args[1]) if !ok { c.Ui.Error("The job version to revert to must be specified using the -job-version flag") @@ -138,25 +138,16 @@ func (c *JobRevertCommand) Run(args []string) int { } // Check if the job exists - jobs, _, err := client.Jobs().PrefixList(jobID) + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, nil) if err != nil { - c.Ui.Error(fmt.Sprintf("Error listing jobs: %s", err)) + c.Ui.Error(err.Error()) return 1 } - if len(jobs) == 0 { - c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID)) - return 1 - } - if len(jobs) > 1 { - if (jobID != jobs[0].ID) || (c.allNamespaces() && jobs[0].ID == jobs[1].ID) { - c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs, c.allNamespaces()))) - return 1 - } - } // Prefix lookup matched a single job - q := &api.WriteOptions{Namespace: jobs[0].JobSummary.Namespace} - resp, _, err := client.Jobs().Revert(jobs[0].ID, revertVersion, nil, q, consulToken, vaultToken) + q := &api.WriteOptions{Namespace: namespace} + resp, _, err := client.Jobs().Revert(jobID, revertVersion, nil, q, consulToken, vaultToken) if err != nil { c.Ui.Error(fmt.Sprintf("Error retrieving job versions: %s", err)) return 1 diff --git a/command/job_revert_test.go b/command/job_revert_test.go index f7378f2ad..d906707c2 100644 --- a/command/job_revert_test.go +++ b/command/job_revert_test.go @@ -34,7 +34,7 @@ func TestJobRevertCommand_Fails(t *testing.T) { if code := cmd.Run([]string{"-address=nope", "foo", "1"}); code != 1 { t.Fatalf("expected exit code 1, got: %d", code) } - if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error listing jobs") { + if out := ui.ErrorWriter.String(); !strings.Contains(out, "Error querying job prefix") { t.Fatalf("expected failed query error, got: %s", out) } ui.ErrorWriter.Reset() diff --git a/command/job_scale.go b/command/job_scale.go index 97606dace..2dabe11e4 100644 --- a/command/job_scale.go +++ b/command/job_scale.go @@ -80,7 +80,7 @@ func (j *JobScaleCommand) Run(args []string) int { return 1 } - var jobString, countString, groupString string + var countString, groupString string args = flags.Args() // It is possible to specify either 2 or 3 arguments. Check and assign the @@ -94,7 +94,6 @@ func (j *JobScaleCommand) Run(args []string) int { } else { countString = args[1] } - jobString = args[0] // Convert the count string arg to an int as required by the API. count, err := strconv.Atoi(countString) @@ -110,9 +109,18 @@ func (j *JobScaleCommand) Run(args []string) int { return 1 } + // Check if the job exists + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := j.JobIDByPrefix(client, jobIDPrefix, nil) + if err != nil { + j.Ui.Error(err.Error()) + return 1 + } + // Detail the job so we can perform addition checks before submitting the // scaling request. - job, _, err := client.Jobs().ScaleStatus(jobString, nil) + q := &api.QueryOptions{Namespace: namespace} + job, _, err := client.Jobs().ScaleStatus(jobID, q) if err != nil { j.Ui.Error(fmt.Sprintf("Error querying job: %v", err)) return 1 @@ -127,7 +135,8 @@ func (j *JobScaleCommand) Run(args []string) int { msg := "submitted using the Nomad CLI" // Perform the scaling action. - resp, _, err := client.Jobs().Scale(jobString, groupString, &count, msg, false, nil, nil) + w := &api.WriteOptions{Namespace: namespace} + resp, _, err := client.Jobs().Scale(jobID, groupString, &count, msg, false, nil, w) if err != nil { j.Ui.Error(fmt.Sprintf("Error submitting scaling request: %s", err)) return 1 diff --git a/command/job_scaling_events.go b/command/job_scaling_events.go index 2184dd647..b1fad6d31 100644 --- a/command/job_scaling_events.go +++ b/command/job_scaling_events.go @@ -76,9 +76,6 @@ func (j *JobScalingEventsCommand) Run(args []string) int { return 1 } - // Get the job ID. - jobID := args[0] - // Get the HTTP client. client, err := j.Meta.Client() if err != nil { @@ -86,7 +83,16 @@ func (j *JobScalingEventsCommand) Run(args []string) int { return 1 } - events, _, err := client.Jobs().ScaleStatus(jobID, nil) + // Check if the job exists + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := j.JobIDByPrefix(client, jobIDPrefix, nil) + if err != nil { + j.Ui.Error(err.Error()) + return 1 + } + + q := &api.QueryOptions{Namespace: namespace} + events, _, err := client.Jobs().ScaleStatus(jobID, q) if err != nil { j.Ui.Error(fmt.Sprintf("Error listing scaling events: %s", err)) return 1 diff --git a/command/job_status.go b/command/job_status.go index ea464974c..7d842ba4b 100644 --- a/command/job_status.go +++ b/command/job_status.go @@ -145,27 +145,16 @@ func (c *JobStatusCommand) Run(args []string) int { } // Try querying the job - jobID := strings.TrimSpace(args[0]) - - jobs, _, err := client.Jobs().PrefixList(jobID) + jobIDPrefix := strings.TrimSpace(args[0]) + jobID, namespace, err := c.JobIDByPrefix(client, jobIDPrefix, nil) if err != nil { - c.Ui.Error(fmt.Sprintf("Error querying job: %s", err)) + c.Ui.Error(err.Error()) return 1 } - if len(jobs) == 0 { - c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID)) - return 1 - } - if len(jobs) > 1 { - if (jobID != jobs[0].ID) || (allNamespaces && jobs[0].ID == jobs[1].ID) { - c.Ui.Error(fmt.Sprintf("Prefix matched multiple jobs\n\n%s", createStatusListOutput(jobs, allNamespaces))) - return 1 - } - } // Prefix lookup matched a single job - q := &api.QueryOptions{Namespace: jobs[0].JobSummary.Namespace} - job, _, err := client.Jobs().Info(jobs[0].ID, q) + q := &api.QueryOptions{Namespace: namespace} + job, _, err := client.Jobs().Info(jobID, q) if err != nil { c.Ui.Error(fmt.Sprintf("Error querying job: %s", err)) return 1 diff --git a/command/job_stop.go b/command/job_stop.go index 938f7d684..99d1eeecd 100644 --- a/command/job_stop.go +++ b/command/job_stop.go @@ -156,30 +156,9 @@ func (c *JobStopCommand) Run(args []string) int { } // Check if the job exists - jobs, _, err := client.Jobs().PrefixList(jobID) + job, err := c.JobByPrefix(client, jobID, nil) if err != nil { - c.Ui.Error(fmt.Sprintf("Error finding jobs with prefix: %s err: %s", jobID, err)) - statusCh <- 1 - return - } - if len(jobs) == 0 { - c.Ui.Error(fmt.Sprintf("No job(s) with prefix or id %q found", jobID)) - statusCh <- 1 - return - } - if len(jobs) > 1 { - if (jobID != jobs[0].ID) || (c.allNamespaces() && jobs[0].ID == jobs[1].ID) { - c.Ui.Error(fmt.Sprintf("Prefix %q matched multiple jobs\n\n%s", jobID, createStatusListOutput(jobs, c.allNamespaces()))) - statusCh <- 1 - return - } - } - - // Prefix lookup matched a single job - q := &api.QueryOptions{Namespace: jobs[0].JobSummary.Namespace} - job, _, err := client.Jobs().Info(jobs[0].ID, q) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error deregistering job with id %s err: %s", jobID, err)) + c.Ui.Error(err.Error()) statusCh <- 1 return } @@ -233,7 +212,7 @@ func (c *JobStopCommand) Run(args []string) int { // Invoke the stop opts := &api.DeregisterOptions{Purge: purge, Global: global, EvalPriority: evalPriority, NoShutdownDelay: noShutdownDelay} - wq := &api.WriteOptions{Namespace: jobs[0].JobSummary.Namespace} + wq := &api.WriteOptions{Namespace: *job.Namespace} evalID, _, err := client.Jobs().DeregisterOpts(*job.ID, opts, wq) if err != nil { c.Ui.Error(fmt.Sprintf("Error deregistering job with id %s err: %s", jobID, err)) diff --git a/command/job_stop_test.go b/command/job_stop_test.go index 4ab17f5f7..331124697 100644 --- a/command/job_stop_test.go +++ b/command/job_stop_test.go @@ -114,7 +114,7 @@ func TestStopCommand_Fails(t *testing.T) { must.One(t, code) out = ui.ErrorWriter.String() - must.StrContains(t, out, "No job(s) with prefix or id") + must.StrContains(t, out, "No job(s) with prefix or ID") ui.ErrorWriter.Reset() @@ -123,7 +123,7 @@ func TestStopCommand_Fails(t *testing.T) { must.One(t, code) out = ui.ErrorWriter.String() - must.StrContains(t, out, "Error finding jobs with prefix: nope") + must.StrContains(t, out, "Error querying job prefix") } func TestStopCommand_AutocompleteArgs(t *testing.T) { diff --git a/command/meta.go b/command/meta.go index aa2b505cb..a49560fc1 100644 --- a/command/meta.go +++ b/command/meta.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/helper/pointer" colorable "github.com/mattn/go-colorable" "github.com/mitchellh/cli" "github.com/mitchellh/colorstring" @@ -226,6 +227,77 @@ func (m *Meta) FormatWarnings(header string, warnings string) string { )) } +// JobByPrefixFilterFunc is a function used to filter jobs when performing a +// prefix match. Only jobs that return true are included in the prefix match. +type JobByPrefixFilterFunc func(*api.JobListStub) bool + +// NoJobWithPrefixError is the error returned when the job prefix doesn't +// return any matches. +type NoJobWithPrefixError struct { + Prefix string +} + +func (e *NoJobWithPrefixError) Error() string { + return fmt.Sprintf("No job(s) with prefix or ID %q found", e.Prefix) +} + +// 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 JobByPrefixFilterFunc) (*api.Job, error) { + jobID, namespace, err := m.JobIDByPrefix(client, prefix, filter) + if err != nil { + return nil, err + } + + q := &api.QueryOptions{Namespace: namespace} + job, _, err := client.Jobs().Info(jobID, q) + if err != nil { + return nil, fmt.Errorf("Error querying job %q: %s", jobID, err) + } + job.Namespace = pointer.Of(namespace) + + return job, nil +} + +// JobIDByPrefix returns the job that best matches the given prefix and its +// namespace. Returns 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 JobByPrefixFilterFunc) (string, string, error) { + // Search job by prefix. Return an error if there is not an exact match. + jobs, _, err := client.Jobs().PrefixList(prefix) + if err != nil { + return "", "", fmt.Errorf("Error querying job prefix %q: %s", prefix, err) + } + + if filter != nil { + var filtered []*api.JobListStub + for _, j := range jobs { + if filter(j) { + filtered = append(filtered, j) + } + } + jobs = filtered + } + + if len(jobs) == 0 { + return "", "", &NoJobWithPrefixError{Prefix: prefix} + } + if len(jobs) > 1 { + exactMatch := prefix == jobs[0].ID + matchInMultipleNamespaces := m.allNamespaces() && jobs[0].ID == jobs[1].ID + if !exactMatch || matchInMultipleNamespaces { + return "", "", fmt.Errorf( + "Prefix %q matched multiple jobs\n\n%s", + prefix, + createStatusListOutput(jobs, m.allNamespaces()), + ) + } + } + + return jobs[0].ID, jobs[0].JobSummary.Namespace, nil +} + type usageOptsFlags uint8 const ( diff --git a/command/meta_test.go b/command/meta_test.go index 2e347ad8f..99e069619 100644 --- a/command/meta_test.go +++ b/command/meta_test.go @@ -8,8 +8,11 @@ import ( "testing" "github.com/creack/pty" + "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/ci" + "github.com/hashicorp/nomad/helper/pointer" "github.com/mitchellh/cli" + "github.com/shoenig/test/must" "github.com/stretchr/testify/require" ) @@ -153,3 +156,95 @@ func TestMeta_Colorize(t *testing.T) { }) } } + +func TestMeta_JobByPrefix(t *testing.T) { + ci.Parallel(t) + + srv, client, _ := testServer(t, true, nil) + defer srv.Shutdown() + + // Wait for a node to be ready + waitForNodes(t, client) + + ui := cli.NewMockUi() + meta := &Meta{Ui: ui, namespace: api.AllNamespacesNamespace} + client.SetNamespace(api.AllNamespacesNamespace) + + jobs := []struct { + namespace string + id string + }{ + {namespace: "default", id: "example"}, + {namespace: "default", id: "job"}, + {namespace: "default", id: "job-1"}, + {namespace: "default", id: "job-2"}, + {namespace: "prod", id: "job-1"}, + } + for _, j := range jobs { + job := testJob(j.id) + job.Namespace = pointer.Of(j.namespace) + + _, err := client.Namespaces().Register(&api.Namespace{Name: j.namespace}, nil) + must.NoError(t, err) + + w := &api.WriteOptions{Namespace: j.namespace} + resp, _, err := client.Jobs().Register(job, w) + must.NoError(t, err) + + code := waitForSuccess(ui, client, fullId, t, resp.EvalID) + must.Zero(t, code) + } + + testCases := []struct { + name string + prefix string + filterFunc JobByPrefixFilterFunc + expectedError string + }{ + { + name: "exact match", + prefix: "job", + }, + { + name: "partial match", + prefix: "exam", + }, + { + name: "match with filter", + prefix: "job-", + filterFunc: func(j *api.JobListStub) bool { + // Filter out jobs with "job-" so that only "job-2" matches. + return j.ID == "job-2" + }, + }, + { + name: "multiple matches", + prefix: "job-", + expectedError: "matched multiple jobs", + }, + { + name: "no match", + prefix: "not-found", + expectedError: "No job(s) with prefix or ID", + }, + { + name: "multiple matches across namespaces", + prefix: "job-1", + expectedError: "matched multiple jobs", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + job, err := meta.JobByPrefix(client, tc.prefix, tc.filterFunc) + if tc.expectedError != "" { + must.Nil(t, job) + must.ErrorContains(t, err, tc.expectedError) + } else { + must.NoError(t, err) + must.NotNil(t, job) + must.StrContains(t, *job.ID, tc.prefix) + } + }) + } +}