scheduler: preserve allocations enriched during placement as 'informational'

scheduler/reconcile.go

@@ -462,8 +462,10 @@ func (a *allocReconciler) computeGroup(groupName string, all allocSet) bool {
 	untainted, migrate, lost, disconnecting, reconnecting, ignore, expiring := all.filterByTainted(a.taintedNodes, a.supportsDisconnectedClients, a.now)
 	desiredChanges.Ignore += uint64(len(ignore))
 
-	// Determine what set of terminal allocations need to be rescheduled
-	untainted, rescheduleNow, rescheduleLater := untainted.filterByRescheduleable(a.batch, false, a.now, a.evalID, a.deployment)
+	// Determine what set of terminal allocations need to be rescheduled and
+	// see that we don't discard allocations that carry important information
+	// for future reschedules or deployments
+	untainted, rescheduleNow, rescheduleLater, informational := untainted.filterByRescheduleable(a.batch, false, a.now, a.evalID, a.deployment)
 
 	// If there are allocations reconnecting we need to reconcile them and
 	// their replacements first because there is specific logic when deciding
@@ -509,7 +511,7 @@ func (a *allocReconciler) computeGroup(groupName string, all allocSet) bool {
 		// the reschedule policy won't be enabled and the lost allocations
 		// wont be rescheduled, and PreventRescheduleOnLost is ignored.
 		if tg.GetDisconnectLostTimeout() != 0 {
-			untaintedDisconnecting, rescheduleDisconnecting, laterDisconnecting := disconnecting.filterByRescheduleable(a.batch, true, a.now, a.evalID, a.deployment)
+			untaintedDisconnecting, rescheduleDisconnecting, laterDisconnecting, _ := disconnecting.filterByRescheduleable(a.batch, true, a.now, a.evalID, a.deployment)
 
 			rescheduleNow = rescheduleNow.union(rescheduleDisconnecting)
 			untainted = untainted.union(untaintedDisconnecting)
@@ -591,7 +593,7 @@ func (a *allocReconciler) computeGroup(groupName string, all allocSet) bool {
 	// * An alloc was lost
 	var place []allocPlaceResult
 	if len(lostLater) == 0 {
-		place = a.computePlacements(tg, nameIndex, untainted, migrate, rescheduleNow, lost, isCanarying)
+		place = a.computePlacements(tg, nameIndex, untainted, migrate, rescheduleNow, lost, isCanarying, informational)
 		if !existingDeployment {
 			dstate.DesiredTotal += len(place)
 		}
@@ -798,7 +800,7 @@ func (a *allocReconciler) computeUnderProvisionedBy(group *structs.TaskGroup, un
 // Placements will meet or exceed group count.
 func (a *allocReconciler) computePlacements(group *structs.TaskGroup,
 	nameIndex *allocNameIndex, untainted, migrate, reschedule, lost allocSet,
-	isCanarying bool) []allocPlaceResult {
+	isCanarying bool, informational []*structs.Allocation) []allocPlaceResult {
 
 	// Add rescheduled placement results
 	var place []allocPlaceResult
@@ -843,9 +845,18 @@ func (a *allocReconciler) computePlacements(group *structs.TaskGroup,
 	// Add remaining placement results
 	if existing < group.Count {
 		for _, name := range nameIndex.Next(uint(group.Count - existing)) {
+
+			// if there are any informational allocs, pop and add them as previousAlloc to
+			// our new placement
+			var a *structs.Allocation
+			if len(informational) > 0 {
+				a, informational = informational[len(informational)-1], informational[:len(informational)-1]
+			}
+
 			place = append(place, allocPlaceResult{
 				name:               name,
 				taskGroup:          group,
+				previousAlloc:      a,
 				downgradeNonCanary: isCanarying,
 			})
 		}

scheduler/reconcile_util.go

@@ -218,8 +218,8 @@ func (a allocSet) fromKeys(keys ...[]string) allocSet {
 	return from
 }
 
-// filterByTainted takes a set of tainted nodes and filters the allocation set
-// into the following groups:
+// filterByTainted takes a set of tainted nodes and partitions the allocation
+// set into the following groups:
 // 1. Those that exist on untainted nodes
 // 2. Those exist on nodes that are draining
 // 3. Those that exist on lost nodes or have expired
@@ -385,14 +385,25 @@ func (a allocSet) filterByTainted(taintedNodes map[string]*structs.Node, serverS
 	return
 }
 
-// filterByRescheduleable filters the allocation set to return the set of allocations that are either
-// untainted or a set of allocations that must be rescheduled now. Allocations that can be rescheduled
-// at a future time are also returned so that we can create follow up evaluations for them. Allocs are
-// skipped or considered untainted according to logic defined in shouldFilter method.
-func (a allocSet) filterByRescheduleable(isBatch, isDisconnecting bool, now time.Time, evalID string, deployment *structs.Deployment) (allocSet, allocSet, []*delayedRescheduleInfo) {
+// filterByRescheduleable filters the allocation set to return the set of
+// allocations that are either untainted or a set of allocations that must be
+// rescheduled now. Allocations that can be rescheduled at a future time are
+// also returned so that we can create follow up evaluations for them. Allocs
+// are skipped or considered untainted according to logic defined in
+// shouldFilter method.
+//
+// filterByRescheduleable returns an extra slice of allocations as its last
+// output: these allocs are "informational." They will not be rescheduled now
+// or later, but they carry important information for future allocations that
+// might get rescheduled. An example of such allocations are stateful
+// deployments: allocs that require particular host volume IDs.
+func (a allocSet) filterByRescheduleable(
+	isBatch, isDisconnecting bool, now time.Time, evalID string, deployment *structs.Deployment,
+) (allocSet, allocSet, []*delayedRescheduleInfo, []*structs.Allocation) {
 	untainted := make(map[string]*structs.Allocation)
 	rescheduleNow := make(map[string]*structs.Allocation)
 	rescheduleLater := []*delayedRescheduleInfo{}
+	informational := []*structs.Allocation{}
 
 	for _, alloc := range a {
 		// Ignore disconnecting allocs that are already unknown. This can happen
@@ -411,6 +422,12 @@ func (a allocSet) filterByRescheduleable(isBatch, isDisconnecting bool, now time
 			continue
 		}
 
+		// Allocations with host volume IDs can be ignored, but we must keep
+		// the information they carry for future migrated allocs
+		if len(alloc.HostVolumeIDs) > 0 {
+			informational = append(informational, alloc)
+		}
+
 		isUntainted, ignore := shouldFilter(alloc, isBatch)
 		if isUntainted && !isDisconnecting {
 			untainted[alloc.ID] = alloc
@@ -436,7 +453,7 @@ func (a allocSet) filterByRescheduleable(isBatch, isDisconnecting bool, now time
 		}
 
 	}
-	return untainted, rescheduleNow, rescheduleLater
+	return untainted, rescheduleNow, rescheduleLater, informational
 }
 
 // shouldFilter returns whether the alloc should be ignored or considered untainted.

scheduler/reconcile_util_test.go

@@ -1875,6 +1875,8 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 	}
 	testJob.TaskGroups[0] = rescheduleTG
 
+	stickyDeploymentJob := mock.Job()
+
 	now := time.Now()
 
 	rt := &structs.RescheduleTracker{
@@ -1895,9 +1897,10 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 		deployment                  *structs.Deployment
 
 		// expected results
-		untainted allocSet
-		resNow    allocSet
-		resLater  []*delayedRescheduleInfo
+		untainted     allocSet
+		resNow        allocSet
+		resLater      []*delayedRescheduleInfo
+		informational []*structs.Allocation
 	}
 
 	testCases := []testCase{
@@ -1921,8 +1924,9 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					TaskGroup:    "noRescheduleTG",
 				},
 			},
-			resNow:   allocSet{},
-			resLater: []*delayedRescheduleInfo{},
+			resNow:        allocSet{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 		{
 			name:            "batch ignore unknown disconnecting allocs",
@@ -1935,9 +1939,10 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					Job:          testJob,
 				},
 			},
-			untainted: allocSet{},
-			resNow:    allocSet{},
-			resLater:  []*delayedRescheduleInfo{},
+			untainted:     allocSet{},
+			resNow:        allocSet{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 		{
 			name:            "batch disconnecting allocation reschedule",
@@ -1962,7 +1967,8 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					RescheduleTracker: rt,
 				},
 			},
-			resLater: []*delayedRescheduleInfo{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 
 		{
@@ -1991,8 +1997,9 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 						"task": {State: structs.TaskStateDead, Failed: false}},
 				},
 			},
-			resNow:   allocSet{},
-			resLater: []*delayedRescheduleInfo{},
+			resNow:        allocSet{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 		{
 			name:            "service disconnecting allocation no reschedule",
@@ -2014,8 +2021,9 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					TaskGroup:    "noRescheduleTG",
 				},
 			},
-			resNow:   allocSet{},
-			resLater: []*delayedRescheduleInfo{},
+			resNow:        allocSet{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 		{
 			name:            "service disconnecting allocation reschedule",
@@ -2040,7 +2048,8 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					RescheduleTracker: rt,
 				},
 			},
-			resLater: []*delayedRescheduleInfo{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 		{
 			name:            "service ignore unknown disconnecting allocs",
@@ -2053,9 +2062,10 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					Job:          testJob,
 				},
 			},
-			untainted: allocSet{},
-			resNow:    allocSet{},
-			resLater:  []*delayedRescheduleInfo{},
+			untainted:     allocSet{},
+			resNow:        allocSet{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 		{
 			name:            "service previously rescheduled alloc should not reschedule",
@@ -2070,9 +2080,10 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					TaskGroup:      "rescheduleTG",
 				},
 			},
-			untainted: allocSet{},
-			resNow:    allocSet{},
-			resLater:  []*delayedRescheduleInfo{},
+			untainted:     allocSet{},
+			resNow:        allocSet{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 		{
 			name:            "service complete should be ignored",
@@ -2087,9 +2098,10 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					TaskGroup:     "rescheduleTG",
 				},
 			},
-			untainted: allocSet{},
-			resNow:    allocSet{},
-			resLater:  []*delayedRescheduleInfo{},
+			untainted:     allocSet{},
+			resNow:        allocSet{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
 		},
 		{
 			name:            "service running allocation no reschedule",
@@ -2111,18 +2123,44 @@ func TestAllocSet_filterByRescheduleable(t *testing.T) {
 					TaskGroup:    "noRescheduleTG",
 				},
 			},
-			resNow:   allocSet{},
-			resLater: []*delayedRescheduleInfo{},
+			resNow:        allocSet{},
+			resLater:      []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{},
+		},
+		{
+			name:            "allocation with sticky volumes",
+			isDisconnecting: false,
+			isBatch:         false,
+			all: allocSet{
+				"sticky": {
+					ID:            "sticky",
+					DesiredStatus: structs.AllocDesiredStatusStop,
+					ClientStatus:  structs.AllocClientStatusComplete,
+					Job:           stickyDeploymentJob,
+					HostVolumeIDs: []string{"hostVolumeID1"},
+				},
+			},
+			untainted: allocSet{},
+			resNow:    allocSet{},
+			resLater:  []*delayedRescheduleInfo{},
+			informational: []*structs.Allocation{{
+				ID:            "sticky",
+				DesiredStatus: structs.AllocDesiredStatusStop,
+				ClientStatus:  structs.AllocClientStatusComplete,
+				Job:           stickyDeploymentJob,
+				HostVolumeIDs: []string{"hostVolumeID1"},
+			}},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			untainted, resNow, resLater := tc.all.filterByRescheduleable(tc.isBatch,
+			untainted, resNow, resLater, informational := tc.all.filterByRescheduleable(tc.isBatch,
 				tc.isDisconnecting, now, "evailID", tc.deployment)
 			must.Eq(t, tc.untainted, untainted, must.Sprintf("with-nodes: untainted"))
 			must.Eq(t, tc.resNow, resNow, must.Sprintf("with-nodes: reschedule-now"))
 			must.Eq(t, tc.resLater, resLater, must.Sprintf("with-nodes: rescheduleLater"))
+			must.Eq(t, tc.informational, informational, must.Sprintf("with-nodes: informational"))
 		})
 	}
 }