diff --git a/scheduler/generic_sched_test.go b/scheduler/generic_sched_test.go index c6f2aeea996..aefe8695af4 100644 --- a/scheduler/generic_sched_test.go +++ b/scheduler/generic_sched_test.go @@ -1596,7 +1596,7 @@ func TestServiceSched_JobModify_Rolling_FullNode(t *testing.T) { job2 := job.Copy() job2.TaskGroups[0].Count = 5 job2.TaskGroups[0].Update = &structs.UpdateStrategy{ - MaxParallel: 1, + MaxParallel: 5, HealthCheck: structs.UpdateStrategyHealthCheck_Checks, MinHealthyTime: 10 * time.Second, HealthyDeadline: 10 * time.Minute, @@ -1607,7 +1607,6 @@ func TestServiceSched_JobModify_Rolling_FullNode(t *testing.T) { job2.TaskGroups[0].Tasks[0].Config["command"] = "/bin/other" noErr(t, h.State.UpsertJob(h.NextIndex(), job2)) - // Create a mock evaluation to deal with drain eval := &structs.Evaluation{ ID: structs.GenerateUUID(), Priority: 50, diff --git a/scheduler/reconcile.go b/scheduler/reconcile.go index dae2bb715f1..d4468615e1f 100644 --- a/scheduler/reconcile.go +++ b/scheduler/reconcile.go @@ -374,6 +374,9 @@ func (a *allocReconciler) computeGroup(group string, all allocSet) bool { for _, p := range place { a.result.place = append(a.result.place, p) } + + min := helper.IntMin(len(place), limit) + limit -= min } else if !deploymentPlaceReady && len(lost) != 0 { // We are in a situation where we shouldn't be placing more than we need // to but we have lost allocations. It is a very weird user experience diff --git a/scheduler/reconcile_test.go b/scheduler/reconcile_test.go index 282cdfadea9..bcf2d356b0c 100644 --- a/scheduler/reconcile_test.go +++ b/scheduler/reconcile_test.go @@ -69,6 +69,7 @@ Update stanza Tests: √ Finished deployment gets marked as complete √ The stagger is correctly calculated when it is applied across multiple task groups. √ Change job change while scaling up +√ Update the job when all allocations from the previous job haven't been placed yet. */ var ( @@ -2468,9 +2469,9 @@ func TestReconciler_TaintedNode_RollingUpgrade(t *testing.T) { PlacedAllocs: 7, } - // Create 3 allocations from the old job + // Create 2 allocations from the old job var allocs []*structs.Allocation - for i := 7; i < 10; i++ { + for i := 8; i < 10; i++ { alloc := mock.Alloc() alloc.Job = job alloc.JobID = job.ID @@ -2482,7 +2483,7 @@ func TestReconciler_TaintedNode_RollingUpgrade(t *testing.T) { // Create the healthy replacements handled := make(map[string]allocUpdateType) - for i := 0; i < 7; i++ { + for i := 0; i < 8; i++ { new := mock.Alloc() new.Job = job new.JobID = job.ID @@ -2501,7 +2502,7 @@ func TestReconciler_TaintedNode_RollingUpgrade(t *testing.T) { tainted := make(map[string]*structs.Node, 3) for i := 0; i < 3; i++ { n := mock.Node() - n.ID = allocs[3+i].NodeID + n.ID = allocs[2+i].NodeID if i == 0 { n.Status = structs.NodeStatusDown } else { @@ -2519,7 +2520,7 @@ func TestReconciler_TaintedNode_RollingUpgrade(t *testing.T) { createDeployment: nil, deploymentUpdates: nil, place: 2, - destructive: 3, + destructive: 2, stop: 2, followupEvalWait: 31 * time.Second, desiredTGUpdates: map[string]*structs.DesiredUpdates{ @@ -2527,13 +2528,13 @@ func TestReconciler_TaintedNode_RollingUpgrade(t *testing.T) { Place: 1, // Place the lost Stop: 1, // Stop the lost Migrate: 1, // Migrate the tainted - DestructiveUpdate: 3, - Ignore: 5, + DestructiveUpdate: 2, + Ignore: 6, }, }, }) - assertNamesHaveIndexes(t, intRange(7, 9), destructiveResultsToNames(r.destructiveUpdate)) + assertNamesHaveIndexes(t, intRange(8, 9), destructiveResultsToNames(r.destructiveUpdate)) assertNamesHaveIndexes(t, intRange(0, 1), placeResultsToNames(r.place)) assertNamesHaveIndexes(t, intRange(0, 1), stopResultsToNames(r.stop)) } @@ -3012,3 +3013,48 @@ func TestReconciler_JobChange_ScaleUp_SecondEval(t *testing.T) { }, }) } + +// Tests the reconciler doesn't stop allocations when doing a rolling upgrade +// where the count of the old job allocs is < desired count. +func TestReconciler_RollingUpgrade_MissingAllocs(t *testing.T) { + job := mock.Job() + job.TaskGroups[0].Update = noCanaryUpdate + + // Create 7 allocations from the old job + var allocs []*structs.Allocation + for i := 0; i < 7; 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) + } + + reconciler := NewAllocReconciler(testLogger(), allocUpdateFnDestructive, false, job.ID, job, nil, allocs, nil) + r := reconciler.Compute() + + d := structs.NewDeployment(job) + d.TaskGroups[job.TaskGroups[0].Name] = &structs.DeploymentState{ + DesiredTotal: 10, + } + + // Assert the correct results + assertResults(t, r, &resultExpectation{ + createDeployment: d, + deploymentUpdates: nil, + place: 3, + destructive: 1, + desiredTGUpdates: map[string]*structs.DesiredUpdates{ + job.TaskGroups[0].Name: { + Place: 3, + DestructiveUpdate: 1, + Ignore: 6, + }, + }, + }) + + assertNamesHaveIndexes(t, intRange(7, 9), placeResultsToNames(r.place)) + assertNamesHaveIndexes(t, intRange(0, 0), destructiveResultsToNames(r.destructiveUpdate)) +}