diff --git a/scheduler/reconcile.go b/scheduler/reconcile.go index 06f224e5a..f44d09b8f 100644 --- a/scheduler/reconcile.go +++ b/scheduler/reconcile.go @@ -113,7 +113,7 @@ func NewAllocReconciler(logger *log.Logger, allocUpdateFn allocUpdateType, batch jobID string, job *structs.Job, deployment *structs.Deployment, existingAllocs []*structs.Allocation, taintedNodes map[string]*structs.Node) *allocReconciler { - a := &allocReconciler{ + return &allocReconciler{ logger: logger, allocUpdateFn: allocUpdateFn, batch: batch, @@ -126,14 +126,6 @@ func NewAllocReconciler(logger *log.Logger, allocUpdateFn allocUpdateType, batch desiredTGUpdates: make(map[string]*structs.DesiredUpdates), }, } - - // Detect if the deployment is paused - if deployment != nil { - a.deploymentPaused = deployment.Status == structs.DeploymentStatusPaused - a.deploymentFailed = deployment.Status == structs.DeploymentStatusFailed - } - - return a } // Compute reconciles the existing cluster state and returns the set of changes @@ -152,6 +144,12 @@ func (a *allocReconciler) Compute() *reconcileResults { return a.result } + // Detect if the deployment is paused + if a.deployment != nil { + a.deploymentPaused = a.deployment.Status == structs.DeploymentStatusPaused + a.deploymentFailed = a.deployment.Status == structs.DeploymentStatusFailed + } + // Reconcile each group complete := true for group, as := range m { @@ -184,8 +182,6 @@ func (a *allocReconciler) cancelDeployments() { return } - // TODO it doesn't like operating on a failed deployment - // Write a test d := a.deployment if d == nil { return diff --git a/scheduler/reconcile_test.go b/scheduler/reconcile_test.go index 9f3194c28..e787987b4 100644 --- a/scheduler/reconcile_test.go +++ b/scheduler/reconcile_test.go @@ -2675,4 +2675,74 @@ func TestReconciler_FailedDeployment_CancelCanaries(t *testing.T) { assertNamesHaveIndexes(t, intRange(0, 1), stopResultsToNames(r.stop)) } -// TODO Test that a failed deployment and updated job works +// Test that a failed deployment and updated job works +func TestReconciler_FailedDeployment_NewJob(t *testing.T) { + job := mock.Job() + job.TaskGroups[0].Update = noCanaryUpdate + + // Create an existing failed deployment that has some placed allocs + d := structs.NewDeployment(job) + d.Status = structs.DeploymentStatusFailed + d.TaskGroups[job.TaskGroups[0].Name] = &structs.DeploymentState{ + Promoted: true, + DesiredTotal: 10, + PlacedAllocs: 4, + } + + // Create 6 allocations from the old job + var allocs []*structs.Allocation + for i := 4; i < 10; i++ { + alloc := mock.Alloc() + alloc.Job = job + alloc.JobID = job.ID + alloc.NodeID = structs.GenerateUUID() + alloc.Name = structs.AllocName(job.ID, job.TaskGroups[0].Name, uint(i)) + alloc.TaskGroup = job.TaskGroups[0].Name + allocs = append(allocs, alloc) + } + + // Create the healthy replacements + for i := 0; i < 4; i++ { + new := mock.Alloc() + new.Job = job + new.JobID = job.ID + new.NodeID = structs.GenerateUUID() + new.Name = structs.AllocName(job.ID, job.TaskGroups[0].Name, uint(i)) + new.TaskGroup = job.TaskGroups[0].Name + new.DeploymentID = d.ID + new.DeploymentStatus = &structs.AllocDeploymentStatus{ + Healthy: helper.BoolToPtr(true), + } + allocs = append(allocs, new) + } + + // Up the job version + jobNew := job.Copy() + jobNew.JobModifyIndex += 100 + + reconciler := NewAllocReconciler(testLogger(), allocUpdateFnDestructive, false, job.ID, jobNew, d, allocs, nil) + r := reconciler.Compute() + + dnew := structs.NewDeployment(job) + dnew.TaskGroups[job.TaskGroups[0].Name] = &structs.DeploymentState{ + DesiredTotal: 10, + } + + // Assert the correct results + assertResults(t, r, &resultExpectation{ + createDeployment: dnew, + deploymentUpdates: nil, + place: 4, + inplace: 0, + stop: 4, + desiredTGUpdates: map[string]*structs.DesiredUpdates{ + job.TaskGroups[0].Name: { + DestructiveUpdate: 4, + Ignore: 6, + }, + }, + }) + + assertNamesHaveIndexes(t, intRange(0, 3), stopResultsToNames(r.stop)) + assertNamesHaveIndexes(t, intRange(0, 3), placeResultsToNames(r.place)) +}