Merge pull request #3372 from hashicorp/b-versions

Fix sorting of job versions

CHANGELOG.md

@@ -21,6 +21,8 @@ IMPROVEMENTS:
 BUG FIXES:
  * core: Fix restoration of stopped periodic jobs [GH-3201]
  * core: Run deployment garbage collector on an interval [GH-3267]
+ * core: Fix issue in which job versions above a threshold potentially wouldn't
+   be stored [GH-3372]
  * core: Fix issue where node-drain with complete batch allocation would create
    replacement [GH-3217]
  * core: Fix issue in which batch allocations from previous job versions may not

nomad/state/state_store.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"sort"
 
 	"github.com/hashicorp/go-memdb"
 	multierror "github.com/hashicorp/go-multierror"
@@ -1017,10 +1018,10 @@ func (s *StateStore) jobVersionByID(txn *memdb.Txn, ws *memdb.WatchSet, namespac
 		all = append(all, j)
 	}
 
-	// Reverse so that highest versions first
-	for i, j := 0, len(all)-1; i < j; i, j = i+1, j-1 {
-		all[i], all[j] = all[j], all[i]
-	}
+	// Sort in reverse order so that the highest version is first
+	sort.Slice(all, func(i, j int) bool {
+		return all[i].Version > all[j].Version
+	})
 
 	return all, nil
 }

nomad/state/state_store_test.go

@@ -1226,7 +1226,7 @@ func TestStateStore_UpdateUpsertJob_JobVersion(t *testing.T) {
 	// Create a job and mark it as stable
 	job := mock.Job()
 	job.Stable = true
-	job.Priority = 0
+	job.Name = "0"
 
 	// Create a watchset so we can test that upsert fires the watch
 	ws := memdb.NewWatchSet()
@@ -1244,10 +1244,10 @@ func TestStateStore_UpdateUpsertJob_JobVersion(t *testing.T) {
 	}
 
 	var finalJob *structs.Job
-	for i := 1; i < 20; i++ {
+	for i := 1; i < 300; i++ {
 		finalJob = mock.Job()
 		finalJob.ID = job.ID
-		finalJob.Priority = i
+		finalJob.Name = fmt.Sprintf("%d", i)
 		err = state.UpsertJob(uint64(1000+i), finalJob)
 		if err != nil {
 			t.Fatalf("err: %v", err)
@@ -1267,18 +1267,18 @@ func TestStateStore_UpdateUpsertJob_JobVersion(t *testing.T) {
 	if out.CreateIndex != 1000 {
 		t.Fatalf("bad: %#v", out)
 	}
-	if out.ModifyIndex != 1019 {
+	if out.ModifyIndex != 1299 {
 		t.Fatalf("bad: %#v", out)
 	}
-	if out.Version != 19 {
+	if out.Version != 299 {
 		t.Fatalf("bad: %#v", out)
 	}
 
 	index, err := state.Index("job_version")
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	if index != 1019 {
+	if index != 1299 {
 		t.Fatalf("bad: %d", index)
 	}
 
@@ -1288,19 +1288,19 @@ func TestStateStore_UpdateUpsertJob_JobVersion(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 	if len(allVersions) != structs.JobTrackedVersions {
-		t.Fatalf("got %d; want 1", len(allVersions))
+		t.Fatalf("got %d; want %d", len(allVersions), structs.JobTrackedVersions)
 	}
 
-	if a := allVersions[0]; a.ID != job.ID || a.Version != 19 || a.Priority != 19 {
+	if a := allVersions[0]; a.ID != job.ID || a.Version != 299 || a.Name != "299" {
 		t.Fatalf("bad: %+v", a)
 	}
-	if a := allVersions[1]; a.ID != job.ID || a.Version != 18 || a.Priority != 18 {
+	if a := allVersions[1]; a.ID != job.ID || a.Version != 298 || a.Name != "298" {
 		t.Fatalf("bad: %+v", a)
 	}
 
 	// Ensure we didn't delete the stable job
 	if a := allVersions[structs.JobTrackedVersions-1]; a.ID != job.ID ||
-		a.Version != 0 || a.Priority != 0 || !a.Stable {
+		a.Version != 0 || a.Name != "0" || !a.Stable {
 		t.Fatalf("bad: %+v", a)
 	}