From 8dff1f758a145db3c12441d9affe0372a6c94e6d Mon Sep 17 00:00:00 2001
From: Seth Hoenig <shoenig@duck.com>
Date: Mon, 8 May 2023 14:18:34 -0500
Subject: [PATCH] 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 | 42 ++++++++++++++++++++++++++++
 nomad/state/state_store.go           |  5 ++++
 2 files changed, 47 insertions(+)

diff --git a/e2e/jobsubmissions/jobsubapi_test.go b/e2e/jobsubmissions/jobsubapi_test.go
index f2db72878..86498d97f 100644
--- a/e2e/jobsubmissions/jobsubapi_test.go
+++ b/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"])
+	}
+}
diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go
index cfae8e47a..c993be517 100644
--- a/nomad/state/state_store.go
+++ b/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
 			}
 		}