api: set the job submission during job reversion (#17097)

* api: set the job submission during job reversion

This PR fixes a bug where the job submission would always be nil when
a job goes through a reversion to a previous version. Basically we need
to detect when this happens, lookup the submission of the job version
being reverted to, and set that as the submission of the new job being
created.

* e2e: add e2e test for job submissions during reversion

This e2e test ensures a reverted job inherits the job submission
associated with the version of the job being reverted to.

e2e/jobsubmissions/jobsubapi_test.go

@@ -1,6 +1,7 @@
 package jobsubmissions
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -23,6 +24,7 @@ func TestJobSubmissionAPI(t *testing.T) {
 	t.Run("testRunCLIVarFlags", testRunCLIVarFlags)
 	t.Run("testSubmissionACL", testSubmissionACL)
 	t.Run("testMaxSize", testMaxSize)
+	t.Run("testReversion", testReversion)
 }
 
 func testParseAPI(t *testing.T) {
@@ -275,3 +277,43 @@ func testMaxSize(t *testing.T) {
 	must.ErrorContains(t, err, "job source not found")
 	must.Nil(t, sub)
 }
+
+func testReversion(t *testing.T) {
+	nomad := e2eutil.NomadClient(t)
+
+	jobID := "job-sub-reversion-" + uuid.Short()
+	jobIDs := []string{jobID}
+	t.Cleanup(e2eutil.MaybeCleanupJobsAndGC(&jobIDs))
+
+	// register job 3 times
+	for i := 0; i < 3; i++ {
+		yVar := fmt.Sprintf("-var=Y=%d", i)
+
+		// register the job
+		err := e2eutil.RegisterWithArgs(jobID, "input/xyz.hcl", "-var=X=hello", yVar, "-var=Z=false")
+		must.NoError(t, err)
+
+		// find our alloc id
+		allocID := e2eutil.SingleAllocID(t, jobID, "", i)
+
+		// wait for alloc to complete
+		_ = e2eutil.WaitForAllocStopped(t, nomad, allocID)
+	}
+
+	// revert the job back to version 1
+
+	err := e2eutil.Revert(jobID, "input/xyz.hcl", 1)
+	must.NoError(t, err)
+
+	// there should be a submission for version 3, and it should
+	// contain Y=1 as did the version 1 of the job
+	expectY := []string{"0", "1", "2", "1"}
+	for version := 0; version < 4; version++ {
+		sub, _, err := nomad.Jobs().Submission(jobID, version, &api.QueryOptions{
+			Region:    "global",
+			Namespace: "default",
+		})
+		must.NoError(t, err)
+		must.Eq(t, expectY[version], sub.VariableFlags["Y"])
+	}
+}

nomad/state/state_store.go

@@ -1681,6 +1681,11 @@ func (s *StateStore) upsertJobImpl(index uint64, sub *structs.JobSubmission, job
 		if !keepVersion {
 			job.JobModifyIndex = index
 			if job.Version <= existingJob.Version {
+				if sub == nil {
+					// in the reversion case we must set the submission to be
+					// that of the job version we are reverting to
+					sub, _ = s.jobSubmission(nil, job.Namespace, job.ID, job.Version, txn)
+				}
 				job.Version = existingJob.Version + 1
 			}
 		}