Addressing some PR comments

api/jobs.go

@@ -266,9 +266,34 @@ func (j *Jobs) ScaleStatus(jobID string, q *QueryOptions) (*JobScaleStatusRespon
 
 // Versions is used to retrieve all versions of a particular job given its
 // unique ID.
-func (j *Jobs) Versions(jobID string, diffs bool, diffTag string, diffVersion string, q *QueryOptions) ([]*Job, []*JobDiff, *QueryMeta, error) {
+func (j *Jobs) Versions(jobID string, diffs bool, q *QueryOptions) ([]*Job, []*JobDiff, *QueryMeta, error) {
+	opts := &VersionsOptions{
+		Diffs: diffs,
+	}
+	return j.VersionsOpts(jobID, opts, q)
+}
+
+type VersionsOptions struct {
+	Diffs       bool
+	DiffTag     string
+	DiffVersion *uint64
+}
+
+func (j *Jobs) VersionsOpts(jobID string, opts *VersionsOptions, q *QueryOptions) ([]*Job, []*JobDiff, *QueryMeta, error) {
 	var resp JobVersionsResponse
-	qm, err := j.client.query(fmt.Sprintf("/v1/job/%s/versions?diffs=%t&diff_tag=%s&diff_version=%s", url.PathEscape(jobID), diffs, diffTag, diffVersion), &resp, q)
+
+	qp := url.Values{}
+	if opts != nil {
+		qp.Add("diffs", strconv.FormatBool(opts.Diffs))
+		if opts.DiffTag != "" {
+			qp.Add("diff_tag", opts.DiffTag)
+		}
+		if opts.DiffVersion != nil {
+			qp.Add("diff_version", strconv.FormatUint(*opts.DiffVersion, 10))
+		}
+	}
+
+	qm, err := j.client.query(fmt.Sprintf("/v1/job/%s/versions?%s", url.PathEscape(jobID), qp.Encode()), &resp, q)
 	if err != nil {
 		return nil, nil, nil, err
 	}
@@ -449,7 +474,7 @@ func (j *Jobs) PeriodicForce(jobID string, q *WriteOptions) (string, *WriteMeta,
 type PlanOptions struct {
 	Diff           bool
 	PolicyOverride bool
-	DiffVersion    string
+	DiffVersion    *uint64
 	DiffTagName    string
 }
 
@@ -473,7 +498,10 @@ func (j *Jobs) PlanOpts(job *Job, opts *PlanOptions, q *WriteOptions) (*JobPlanR
 	if opts != nil {
 		req.Diff = opts.Diff
 		req.PolicyOverride = opts.PolicyOverride
-		req.DiffVersion = opts.DiffVersion
+		if opts.DiffVersion != nil {
+			req.DiffVersion = opts.DiffVersion
+		}
+
 		req.DiffTagName = opts.DiffTagName
 	}
 
@@ -1483,7 +1511,7 @@ type JobPlanRequest struct {
 	Job            *Job
 	Diff           bool
 	PolicyOverride bool
-	DiffVersion    string
+	DiffVersion    *uint64
 	DiffTagName    string
 	WriteRequest
 }

api/jobs_test.go

@@ -1460,7 +1460,7 @@ func TestJobs_Versions(t *testing.T) {
 	jobs := c.Jobs()
 
 	// Trying to retrieve a job by ID before it exists returns an error
-	_, _, _, err := jobs.Versions("job1", false, "", "", nil)
+	_, _, _, err := jobs.Versions("job1", false, nil)
 	must.ErrorContains(t, err, "not found")
 
 	// Register the job
@@ -1470,7 +1470,7 @@ func TestJobs_Versions(t *testing.T) {
 	assertWriteMeta(t, wm)
 
 	// Query the job again and ensure it exists
-	result, _, qm, err := jobs.Versions("job1", false, "", "", nil)
+	result, _, qm, err := jobs.Versions("job1", false, nil)
 	must.NoError(t, err)
 	assertQueryMeta(t, qm)
 

command/agent/job_endpoint.go

@@ -183,6 +183,7 @@ func (s *HTTPServer) jobPlan(resp http.ResponseWriter, req *http.Request,
 	}
 
 	sJob, writeReq := s.apiJobAndRequestToStructs(args.Job, req, args.WriteRequest)
+
 	planReq := structs.JobPlanRequest{
 		Job:            sJob,
 		Diff:           args.Diff,
@@ -780,10 +781,20 @@ func (s *HTTPServer) jobVersions(resp http.ResponseWriter, req *http.Request, jo
 		}
 	}
 
+	var diffVersionInt *uint64
+
+	if diffVersion != "" {
+		parsedDiffVersion, err := strconv.ParseUint(diffVersion, 10, 64)
+		if err != nil {
+			return nil, fmt.Errorf("Failed to parse value of %q (%v) as a uint64: %v", "diff_version", diffVersion, err)
+		}
+		diffVersionInt = &parsedDiffVersion
+	}
+
 	args := structs.JobVersionsRequest{
 		JobID:       jobID,
 		Diffs:       diffsBool,
-		DiffVersion: diffVersion,
+		DiffVersion: diffVersionInt,
 		DiffTagName: diffTagName,
 	}
 	if s.parse(resp, req, &args.Region, &args.QueryOptions) {

command/job_history.go

@@ -41,16 +41,17 @@ History Options:
 
   -p
     Display the difference between each version of the job and a reference
-		version. The reference version can be specified using the -diff-tag or
-		-diff-version flags. If neither flag is set, the preceding version is used.
+    version. The reference version can be specified using the -diff-tag or
+    -diff-version flags. If neither flag is set, the most recent version is used.
 
-	-diff-tag
-		Specifies the version of the job to compare against, referenced by
-		tag name (defaults to latest). This tag can be set using the
-		"nomad job tag" command.
+  -diff-tag
+    Specifies the version of the job to compare against, referenced by
+    tag name (defaults to latest). Mutually exclusive with -diff-version.
+    This tag can be set using the "nomad job tag" command.
 
-	-diff-version
-		Specifies the version number of the job to compare against.
+  -diff-version
+    Specifies the version number of the job to compare against.
+    Mutually exclusive with -diff-tag.
 
   -full
     Display the full job definition for each version.
@@ -103,7 +104,8 @@ func (c *JobHistoryCommand) Name() string { return "job history" }
 
 func (c *JobHistoryCommand) Run(args []string) int {
 	var json, diff, full bool
-	var tmpl, versionStr, diffTag, diffVersion string
+	var tmpl, versionStr, diffTag, diffVersionFlag string
+	var diffVersion *uint64
 
 	flags := c.Meta.FlagSet(c.Name(), FlagSetClient)
 	flags.Usage = func() { c.Ui.Output(c.Help()) }
@@ -113,7 +115,7 @@ func (c *JobHistoryCommand) Run(args []string) int {
 	flags.StringVar(&versionStr, "version", "", "")
 	flags.StringVar(&tmpl, "t", "", "")
 	flags.StringVar(&diffTag, "diff-tag", "", "")
-	flags.StringVar(&diffVersion, "diff-version", "", "")
+	flags.StringVar(&diffVersionFlag, "diff-version", "", "")
 
 	if err := flags.Parse(args); err != nil {
 		return 1
@@ -132,16 +134,25 @@ func (c *JobHistoryCommand) Run(args []string) int {
 		return 1
 	}
 
-	if (diffTag != "" && !diff) || (diffVersion != "" && !diff) {
+	if (diffTag != "" && !diff) || (diffVersionFlag != "" && !diff) {
 		c.Ui.Error("-diff-tag and -diff-version can only be used with -p")
 		return 1
 	}
 
-	if diffTag != "" && diffVersion != "" {
+	if diffTag != "" && diffVersionFlag != "" {
 		c.Ui.Error("-diff-tag and -diff-version are mutually exclusive")
 		return 1
 	}
 
+	if diffVersionFlag != "" {
+		parsedDiffVersion, err := strconv.ParseUint(diffVersionFlag, 10, 64)
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Error parsing diff version: %s", err))
+			return 1
+		}
+		diffVersion = &parsedDiffVersion
+	}
+
 	// Get the HTTP client
 	client, err := c.Meta.Client()
 	if err != nil {
@@ -160,7 +171,13 @@ func (c *JobHistoryCommand) Run(args []string) int {
 	q := &api.QueryOptions{Namespace: namespace}
 
 	// Prefix lookup matched a single job
-	versions, diffs, _, err := client.Jobs().Versions(jobID, diff, diffTag, diffVersion, q)
+	versionOptions := &api.VersionsOptions{
+		Diffs:       diff,
+		DiffTag:     diffTag,
+		DiffVersion: diffVersion,
+	}
+	c.Ui.Info(fmt.Sprintf("VersionOptions", versionOptions))
+	versions, diffs, _, err := client.Jobs().VersionsOpts(jobID, versionOptions, q)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Error retrieving job versions: %s", err))
 		return 1

command/job_inspect.go

@@ -227,7 +227,7 @@ func getJob(client *api.Client, namespace, jobID string, version *uint64) (*api.
 		return job, err
 	}
 
-	versions, _, _, err := client.Jobs().Versions(jobID, false, "", "", q)
+	versions, _, _, err := client.Jobs().Versions(jobID, false, q)
 	if err != nil {
 		return nil, err
 	}

command/job_plan.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
@@ -84,14 +85,14 @@ Plan Options:
     Determines whether the diff between the remote job and planned job is shown.
     Defaults to true.
 
-	-diff-tag
-		Specifies the version of the remote job to compare against, referenced by
-		tag name (defaults to latest). This tag can be set using the
-		"nomad job tag" command.
+  -diff-tag
+    Specifies the version of the remote job to compare against, referenced by
+    tag name (defaults to latest). This tag can be set using the
+    "nomad job tag" command.
 
-	-diff-version
-		Specifies the version number of the remote job to compare against
-		(defaults to latest).
+  -diff-version
+    Specifies the version number of the remote job to compare against
+    (defaults to latest).
 
   -json
     Parses the job file as JSON. If the outer object has a Job field, such as
@@ -170,13 +171,14 @@ func (c *JobPlanCommand) AutocompleteArgs() complete.Predictor {
 func (c *JobPlanCommand) Name() string { return "job plan" }
 func (c *JobPlanCommand) Run(args []string) int {
 	var diff, policyOverride, verbose bool
-	var vaultToken, vaultNamespace, diffTag, diffVersion string
+	var vaultToken, vaultNamespace, diffTag, diffVersionFlag string
+	var diffVersion *uint64
 
 	flagSet := c.Meta.FlagSet(c.Name(), FlagSetClient)
 	flagSet.Usage = func() { c.Ui.Output(c.Help()) }
 	flagSet.BoolVar(&diff, "diff", true, "")
 	flagSet.StringVar(&diffTag, "diff-tag", "", "")
-	flagSet.StringVar(&diffVersion, "diff-version", "", "")
+	flagSet.StringVar(&diffVersionFlag, "diff-version", "", "")
 	flagSet.BoolVar(&policyOverride, "policy-override", false, "")
 	flagSet.BoolVar(&verbose, "verbose", false, "")
 	flagSet.BoolVar(&c.JobGetter.JSON, "json", false, "")
@@ -266,13 +268,23 @@ func (c *JobPlanCommand) Run(args []string) int {
 		return c.multiregionPlan(client, job, opts, diff, verbose)
 	}
 
-	opts.DiffTagName = diffTag
-	opts.DiffVersion = diffVersion
-	if diffTag != "" && diffVersion != "" {
+	if diffTag != "" && diffVersionFlag != "" {
 		c.Ui.Error("Cannot specify both -diff-tag and -diff-version")
 		return 255
 	}
 
+	if diffVersionFlag != "" {
+		parsedDiffVersion, err := strconv.ParseUint(diffVersionFlag, 10, 64)
+		if err != nil {
+			c.Ui.Error(fmt.Sprintf("Error parsing diff version: %s", err))
+			return 1
+		}
+		diffVersion = &parsedDiffVersion
+	}
+
+	opts.DiffTagName = diffTag
+	opts.DiffVersion = diffVersion
+
 	// Submit the job
 	resp, _, err := client.Jobs().PlanOpts(job, opts, nil)
 	if err != nil {

command/job_restart.go

@@ -305,7 +305,7 @@ func (c *JobRestartCommand) Run(args []string) int {
 
 	// Retrieve the job history so we can properly determine if a group or task
 	// exists in the specific allocation job version.
-	jobVersions, _, _, err := c.client.Jobs().Versions(c.jobID, false, "", "", nil)
+	jobVersions, _, _, err := c.client.Jobs().Versions(c.jobID, false, nil)
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Error retrieving versions of job %q: %s", c.jobID, err))
 		return 1