remove unnecessary check and other fixes from code review

scheduler/reconcile.go

@@ -17,10 +17,10 @@ const (
 	// to batch up failed allocations before creating an eval
 	batchedFailedAllocWindowSize = 5 * time.Second
 
-	// rescheduleTimeLapseWindowSize is the window size relative to
+	// rescheduleWindowSize is the window size relative to
 	// current time within which reschedulable allocations are placed.
 	// This helps protect against small clock drifts between servers
-	rescheduleTimeLapseWindowSize = 1 * time.Second
+	rescheduleWindowSize = 1 * time.Second
 )
 
 // allocUpdateType takes an existing allocation and a new job definition and
@@ -75,8 +75,8 @@ type allocReconciler struct {
 	// evalID is the ID of the evaluation that triggered the reconciler
 	evalID string
 
-	// now is used to override current time used when determining rescheduling eligibility
-	// used in unit tests
+	// now is the time used when determining rescheduling eligibility
+	// defaults to time.Now, and overidden in unit tests
 	now time.Time
 
 	// result is the results of the reconcile. During computation it can be
@@ -158,7 +158,6 @@ func (r *reconcileResults) Changes() int {
 func NewAllocReconciler(logger *log.Logger, allocUpdateFn allocUpdateType, batch bool,
 	jobID string, job *structs.Job, deployment *structs.Deployment,
 	existingAllocs []*structs.Allocation, taintedNodes map[string]*structs.Node, evalID string) *allocReconciler {
-
 	return &allocReconciler{
 		logger:         logger,
 		allocUpdateFn:  allocUpdateFn,
@@ -169,6 +168,7 @@ func NewAllocReconciler(logger *log.Logger, allocUpdateFn allocUpdateType, batch
 		existingAllocs: existingAllocs,
 		taintedNodes:   taintedNodes,
 		evalID:         evalID,
+		now:            time.Now(),
 		result: &reconcileResults{
 			desiredTGUpdates:     make(map[string]*structs.DesiredUpdates),
 			desiredFollowupEvals: make(map[string][]*structs.Evaluation),
@@ -353,10 +353,6 @@ func (a *allocReconciler) computeGroup(group string, all allocSet) bool {
 	// Determine what set of allocations are on tainted nodes
 	untainted, migrate, lost := all.filterByTainted(a.taintedNodes)
 
-	if a.now.IsZero() {
-		a.now = time.Now()
-	}
-
 	// Determine what set of terminal allocations need to be rescheduled
 	untainted, rescheduleNow, rescheduleLater := untainted.filterByRescheduleable(a.batch, a.now, a.evalID)
 

scheduler/reconcile_test.go

@@ -1248,7 +1248,7 @@ func TestReconciler_RescheduleLater_Batch(t *testing.T) {
 	// Mark one as complete
 	allocs[5].ClientStatus = structs.AllocClientStatusComplete
 
-	reconciler := NewAllocReconciler(testLogger(), allocUpdateFnIgnore, true, job.ID, job, nil, allocs, nil, "")
+	reconciler := NewAllocReconciler(testLogger(), allocUpdateFnIgnore, true, job.ID, job, nil, allocs, nil, uuid.Generate())
 	r := reconciler.Compute()
 
 	// Two reschedule attempts were already made, one more can be made at a future time
@@ -1327,7 +1327,7 @@ func TestReconciler_RescheduleLaterWithBatchedEvals_Batch(t *testing.T) {
 			FinishedAt: now.Add(10 * time.Second)}}
 	}
 
-	reconciler := NewAllocReconciler(testLogger(), allocUpdateFnIgnore, true, job.ID, job, nil, allocs, nil, "")
+	reconciler := NewAllocReconciler(testLogger(), allocUpdateFnIgnore, true, job.ID, job, nil, allocs, nil, uuid.Generate())
 	r := reconciler.Compute()
 
 	// Verify that two follow up evals were created
@@ -1494,7 +1494,7 @@ func TestReconciler_RescheduleLater_Service(t *testing.T) {
 	// Mark one as desired state stop
 	allocs[4].DesiredStatus = structs.AllocDesiredStatusStop
 
-	reconciler := NewAllocReconciler(testLogger(), allocUpdateFnIgnore, false, job.ID, job, nil, allocs, nil, "")
+	reconciler := NewAllocReconciler(testLogger(), allocUpdateFnIgnore, false, job.ID, job, nil, allocs, nil, uuid.Generate())
 	r := reconciler.Compute()
 
 	// Should place a new placement and create a follow up eval for the delayed reschedule

scheduler/reconcile_util.go

@@ -323,7 +323,7 @@ func shouldFilter(alloc *structs.Allocation, isBatch bool) (untainted, ignore bo
 func updateByReschedulable(alloc *structs.Allocation, now time.Time, evalID string) (rescheduleNow, rescheduleLater bool, rescheduleTime time.Time) {
 	rescheduleTime, eligible := alloc.NextRescheduleTime()
 	// Reschedule if the eval ID matches the alloc's followup evalID or if its close to its reschedule time
-	if eligible && ((alloc.FollowupEvalID != "" && alloc.FollowupEvalID == evalID) || rescheduleTime.Sub(now) <= rescheduleTimeLapseWindowSize) {
+	if eligible && (alloc.FollowupEvalID == evalID || rescheduleTime.Sub(now) <= rescheduleWindowSize) {
 		rescheduleNow = true
 		return
 	}