diff --git a/CHANGELOG.md b/CHANGELOG.md index 638252a84d4..8a3bd4e5709 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go index 9056841d67d..076b34104c0 100644 --- a/nomad/state/state_store.go +++ b/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 } diff --git a/nomad/state/state_store_test.go b/nomad/state/state_store_test.go index 0d62ff78b7c..dde4b2a9684 100644 --- a/nomad/state/state_store_test.go +++ b/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,10 +1267,10 @@ 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) } @@ -1278,7 +1278,7 @@ func TestStateStore_UpdateUpsertJob_JobVersion(t *testing.T) { 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) }