Merge pull request #26953 from hashicorp/f-system-deployments-canaries-evict-refactor

scheduler: perform feasibility checks for system canaries before computing placements

Author: pkazmierczak

e2e/scheduler_system/systemsched_test.go

@@ -218,16 +218,6 @@ func testCanaryDeploymentToAllEligibleNodes(t *testing.T) {
 	)
 	t.Cleanup(cleanup2)
 
-	// how many eligible nodes do we have?
-	nodesApi := job2.NodesApi()
-	nodesList, _, err := nodesApi.List(nil)
-	must.Nil(t, err)
-	must.SliceNotEmpty(t, nodesList)
-
-	// Get updated allocations
-	allocs := job2.Allocs()
-	must.SliceNotEmpty(t, allocs)
-
 	deploymentsApi := job2.DeploymentsApi()
 	deploymentsList, _, err := deploymentsApi.List(nil)
 	must.NoError(t, err)
@@ -253,6 +243,10 @@ func testCanaryDeploymentToAllEligibleNodes(t *testing.T) {
 		return false
 	})
 
+	// Get updated allocations
+	allocs := job2.Allocs()
+	must.SliceNotEmpty(t, allocs)
+
 	// find allocations from v1 version of the job, they should all be canaries
 	count := 0
 	for _, a := range allocs {
@@ -263,7 +257,10 @@ func testCanaryDeploymentToAllEligibleNodes(t *testing.T) {
 	}
 	must.Eq(t, len(initialAllocs), count, must.Sprint("expected canaries to be placed on all eligible nodes"))
 
+	updatedDeployment, _, err := deploymentsApi.Info(deployment.ID, nil)
+	must.NoError(t, err)
+
 	// deployment must not be terminal and needs to have the right status
 	// description set
-	must.Eq(t, structs.DeploymentStatusDescriptionRunningNeedsPromotion, deployment.StatusDescription)
+	must.Eq(t, structs.DeploymentStatusDescriptionRunningNeedsPromotion, updatedDeployment.StatusDescription)
 }

scheduler/feasible/context.go

@@ -252,6 +252,13 @@ func NewEvalEligibility() *EvalEligibility {
 	}
 }
 
+// Reset clears the contents of the eval eligibility
+func (e *EvalEligibility) Reset() {
+	e.job = make(map[string]ComputedClassFeasibility)
+	e.taskGroups = make(map[string]map[string]ComputedClassFeasibility)
+	e.tgEscapedConstraints = make(map[string]bool)
+}
+
 // SetJob takes the job being evaluated and calculates the escaped constraints
 // at the job and task group level.
 func (e *EvalEligibility) SetJob(job *structs.Job) {

scheduler/feasible/stack.go

@@ -352,6 +352,11 @@ func (s *SystemStack) Select(tg *structs.TaskGroup, options *SelectOptions) *Ran
 	// Reset the binpack selector and context
 	s.scoreNorm.Reset()
 	s.ctx.Reset()
+
+	// Since the system stack is always evaluating a single
+	// node, previous eligibility information is not applicable
+	// so reset it
+	s.ctx.Eligibility().Reset()
 	start := time.Now()
 
 	// Get the task groups constraints.

scheduler/reconciler/deployments.go

@@ -277,7 +277,7 @@ func (a *AllocReconciler) Compute() *ReconcileResults {
 	// Create the allocation matrix
 	m := newAllocMatrix(a.jobState.Job, a.jobState.ExistingAllocs)
 
-	a.jobState.DeploymentOld, a.jobState.DeploymentCurrent, result.DeploymentUpdates = cancelUnneededDeployments(a.jobState.Job, a.jobState.DeploymentCurrent)
+	a.jobState.DeploymentOld, a.jobState.DeploymentCurrent, result.DeploymentUpdates = cancelUnneededServiceDeployments(a.jobState.Job, a.jobState.DeploymentCurrent)
 
 	// If we are just stopping a job we do not need to do anything more than
 	// stopping all running allocs
@@ -569,6 +569,57 @@ func (a *AllocReconciler) computeGroup(group string, all allocSet) (*ReconcileRe
 	return result, deploymentComplete
 }
 
+// cancelUnneededServiceDeployments cancels any deployment that is not needed.
+// A deployment update will be staged for jobs that should stop or have the
+// wrong version. Unneeded deployments include:
+// 1. Jobs that are marked for stop, but there is a non-terminal deployment.
+// 2. Deployments that are active, but referencing a different job version.
+// 3. Deployments that are already successful.
+//
+// returns: old deployment, current deployment and a slice of deployment status
+// updates.
+func cancelUnneededServiceDeployments(j *structs.Job, d *structs.Deployment) (*structs.Deployment, *structs.Deployment, []*structs.DeploymentStatusUpdate) {
+	var updates []*structs.DeploymentStatusUpdate
+
+	// If the job is stopped and there is a non-terminal deployment, cancel it
+	if j.Stopped() {
+		if d != nil && d.Active() {
+			updates = append(updates, &structs.DeploymentStatusUpdate{
+				DeploymentID:      d.ID,
+				Status:            structs.DeploymentStatusCancelled,
+				StatusDescription: structs.DeploymentStatusDescriptionStoppedJob,
+			})
+		}
+
+		// Nothing else to do
+		return d, nil, updates
+	}
+
+	if d == nil {
+		return nil, nil, nil
+	}
+
+	// Check if the deployment is active and referencing an older job and cancel it
+	if d.JobCreateIndex != j.CreateIndex || d.JobVersion != j.Version {
+		if d.Active() {
+			updates = append(updates, &structs.DeploymentStatusUpdate{
+				DeploymentID:      d.ID,
+				Status:            structs.DeploymentStatusCancelled,
+				StatusDescription: structs.DeploymentStatusDescriptionNewerJob,
+			})
+		}
+
+		return d, nil, updates
+	}
+
+	// Clear it as the current deployment if it is successful
+	if d.Status == structs.DeploymentStatusSuccessful {
+		return d, nil, updates
+	}
+
+	return nil, d, updates
+}
+
 // setDeploymentStatusAndUpdates sets status for a.deployment if necessary and
 // returns an array of DeploymentStatusUpdates.
 func (a *AllocReconciler) setDeploymentStatusAndUpdates(deploymentComplete bool, createdDeployment *structs.Deployment) []*structs.DeploymentStatusUpdate {