Fix bug where scoring metadata would be overridden during an inplace upgrade.

scheduler/generic_sched_test.go

@@ -4029,6 +4029,18 @@ func TestBatchSched_ScaleDown_SameName(t *testing.T) {
 	job.TaskGroups[0].Count = 1
 	noErr(t, h.State.UpsertJob(h.NextIndex(), job))
 
+	scoreMetric := &structs.AllocMetric{
+		NodesEvaluated: 10,
+		NodesFiltered:  3,
+		ScoreMetaData: []*structs.NodeScoreMeta{
+			{
+				NodeID: node.ID,
+				Scores: map[string]float64{
+					"bin-packing": 0.5435,
+				},
+			},
+		},
+	}
 	// Create a few running alloc
 	var allocs []*structs.Allocation
 	for i := 0; i < 5; i++ {
@@ -4038,10 +4050,16 @@ func TestBatchSched_ScaleDown_SameName(t *testing.T) {
 		alloc.NodeID = node.ID
 		alloc.Name = "my-job.web[0]"
 		alloc.ClientStatus = structs.AllocClientStatusRunning
+		alloc.Metrics = scoreMetric
 		allocs = append(allocs, alloc)
 	}
 	noErr(t, h.State.UpsertAllocs(h.NextIndex(), allocs))
 
+	// Update the job's modify index to force an inplace upgrade
+	updatedJob := job.Copy()
+	updatedJob.JobModifyIndex = job.JobModifyIndex + 1
+	noErr(t, h.State.UpsertJob(h.NextIndex(), updatedJob))
+
 	// Create a mock evaluation to register the job
 	eval := &structs.Evaluation{
 		Namespace:   structs.DefaultNamespace,
@@ -4067,11 +4085,16 @@ func TestBatchSched_ScaleDown_SameName(t *testing.T) {
 
 	plan := h.Plans[0]
 
+	require := require.New(t)
 	// Ensure the plan evicted 4 of the 5
-	if len(plan.NodeUpdate[node.ID]) != 4 {
-		t.Fatalf("bad: %#v", plan)
-	}
+	require.Equal(4, len(plan.NodeUpdate[node.ID]))
 
+	// Ensure that the scheduler did not overwrite the original score metrics for the i
+	for _, inPlaceAllocs := range plan.NodeAllocation {
+		for _, alloc := range inPlaceAllocs {
+			require.Equal(scoreMetric, alloc.Metrics)
+		}
+	}
 	h.AssertEvalStatus(t, structs.EvalStatusComplete)
 }
 

scheduler/util.go

@@ -829,7 +829,12 @@ func genericAllocUpdateFn(ctx Context, stack Stack, evalID string) allocUpdateTy
 				DiskMB: int64(newTG.EphemeralDisk.SizeMB),
 			},
 		}
-		newAlloc.Metrics = ctx.Metrics()
+		// Use metrics from existing alloc for in place upgrade
+		// This is because if the inplace upgrade succeeded, any scoring metadata from
+		// when it first went through the scheduler should still be preserved. Using scoring
+		// metadata from the context would incorrectly replace it with metadata only from a single node that the
+		// allocation is already on.
+		newAlloc.Metrics = existing.Metrics.Copy()
 		return false, false, newAlloc
 	}
 }