Do not list CronJob unmet starting times beyond deadline

pkg/controller/cronjob/cronjob_controller_test.go

@@ -56,6 +56,14 @@ func justAfterTheHour() time.Time {
 	return T1
 }
 
+func weekAfterTheHour() time.Time {
+	T1, err := time.Parse(time.RFC3339, "2016-05-26T10:00:00Z")
+	if err != nil {
+		panic("test setup error")
+	}
+	return T1
+}
+
 func justBeforeThePriorHour() time.Time {
 	T1, err := time.Parse(time.RFC3339, "2016-05-19T08:59:00Z")
 	if err != nil {
@@ -129,17 +137,31 @@ func newJob(UID string) batch.Job {
 }
 
 var (
-	shortDead int64                   = 10
-	longDead  int64                   = 1000000
-	noDead    int64                   = -12345
-	A         batch.ConcurrencyPolicy = batch.AllowConcurrent
-	f         batch.ConcurrencyPolicy = batch.ForbidConcurrent
-	R         batch.ConcurrencyPolicy = batch.ReplaceConcurrent
-	T         bool                    = true
-	F         bool                    = false
+	shortDead  int64                   = 10
+	mediumDead int64                   = 2 * 60 * 60
+	longDead   int64                   = 1000000
+	noDead     int64                   = -12345
+	A          batch.ConcurrencyPolicy = batch.AllowConcurrent
+	f          batch.ConcurrencyPolicy = batch.ForbidConcurrent
+	R          batch.ConcurrencyPolicy = batch.ReplaceConcurrent
+	T          bool                    = true
+	F          bool                    = false
 )
 
 func TestSyncOne_RunOrNot(t *testing.T) {
+	// Check expectations on deadline parameters
+	if shortDead/60/60 >= 1 {
+		t.Errorf("shortDead should be less than one hour")
+	}
+
+	if mediumDead/60/60 < 1 || mediumDead/60/60 >= 24 {
+		t.Errorf("mediumDead should be between one hour and one day")
+	}
+
+	if longDead/60/60/24 < 10 {
+		t.Errorf("longDead should be at least ten days")
+	}
+
 	testCases := map[string]struct {
 		// sj spec
 		concurrencyPolicy batch.ConcurrencyPolicy
@@ -188,6 +210,24 @@ func TestSyncOne_RunOrNot(t *testing.T) {
 		"still active, is time, suspended":         {A, T, onTheHour, noDead, T, T, justAfterTheHour(), F, F, 1},
 		"still active, is time, past deadline":     {A, F, onTheHour, shortDead, T, T, justAfterTheHour(), F, F, 1},
 		"still active, is time, not past deadline": {A, F, onTheHour, longDead, T, T, justAfterTheHour(), T, F, 2},
+
+		// Controller should fail to schedule these, as there are too many missed starting times
+		// and either no deadline or a too long deadline.
+		"prev ran but done, long overdue, not past deadline, A": {A, F, onTheHour, longDead, T, F, weekAfterTheHour(), F, F, 0},
+		"prev ran but done, long overdue, not past deadline, R": {R, F, onTheHour, longDead, T, F, weekAfterTheHour(), F, F, 0},
+		"prev ran but done, long overdue, not past deadline, F": {f, F, onTheHour, longDead, T, F, weekAfterTheHour(), F, F, 0},
+		"prev ran but done, long overdue, no deadline, A":       {A, F, onTheHour, noDead, T, F, weekAfterTheHour(), F, F, 0},
+		"prev ran but done, long overdue, no deadline, R":       {R, F, onTheHour, noDead, T, F, weekAfterTheHour(), F, F, 0},
+		"prev ran but done, long overdue, no deadline, F":       {f, F, onTheHour, noDead, T, F, weekAfterTheHour(), F, F, 0},
+
+		"prev ran but done, long overdue, past medium deadline, A": {A, F, onTheHour, mediumDead, T, F, weekAfterTheHour(), T, F, 1},
+		"prev ran but done, long overdue, past short deadline, A":  {A, F, onTheHour, shortDead, T, F, weekAfterTheHour(), T, F, 1},
+
+		"prev ran but done, long overdue, past medium deadline, R": {R, F, onTheHour, mediumDead, T, F, weekAfterTheHour(), T, F, 1},
+		"prev ran but done, long overdue, past short deadline, R":  {R, F, onTheHour, shortDead, T, F, weekAfterTheHour(), T, F, 1},
+
+		"prev ran but done, long overdue, past medium deadline, F": {f, F, onTheHour, mediumDead, T, F, weekAfterTheHour(), T, F, 1},
+		"prev ran but done, long overdue, past short deadline, F":  {f, F, onTheHour, shortDead, T, F, weekAfterTheHour(), T, F, 1},
 	}
 	for name, tc := range testCases {
 		sj := cronJob()

pkg/controller/cronjob/utils.go

@@ -125,6 +125,7 @@ func getRecentUnmetScheduleTimes(sj batch.CronJob, now time.Time) ([]time.Time,
 	if err != nil {
 		return starts, fmt.Errorf("Unparseable schedule: %s : %s", sj.Spec.Schedule, err)
 	}
+
 	var earliestTime time.Time
 	if sj.Status.LastScheduleTime != nil {
 		earliestTime = sj.Status.LastScheduleTime.Time
@@ -137,7 +138,14 @@ func getRecentUnmetScheduleTimes(sj batch.CronJob, now time.Time) ([]time.Time,
 		// CronJob as last known start time.
 		earliestTime = sj.ObjectMeta.CreationTimestamp.Time
 	}
+	if sj.Spec.StartingDeadlineSeconds != nil {
+		// Controller is not going to schedule anything below this point
+		schedulingDeadline := now.Add(-time.Second * time.Duration(*sj.Spec.StartingDeadlineSeconds))
 
+		if schedulingDeadline.After(earliestTime) {
+			earliestTime = schedulingDeadline
+		}
+	}
 	if earliestTime.After(now) {
 		return []time.Time{}, nil
 	}
@@ -163,7 +171,7 @@ func getRecentUnmetScheduleTimes(sj batch.CronJob, now time.Time) ([]time.Time,
 		// but less than "lots".
 		if len(starts) > 100 {
 			// We can't get the most recent times so just return an empty slice
-			return []time.Time{}, fmt.Errorf("Too many missed start times to list")
+			return []time.Time{}, fmt.Errorf("Too many missed start time (> 100). Set or decrease .spec.startingDeadlineSeconds or check clock skew.")
 		}
 	}
 	return starts, nil

pkg/controller/cronjob/utils_test.go

@@ -367,7 +367,7 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) {
 		}
 	}
 	{
-		// Case 6: now is way way ahead of last start time.
+		// Case 6: now is way way ahead of last start time, and there is no deadline.
 		sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-2 * time.Hour)}
 		sj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)}
 		now := T2.Add(10 * 24 * time.Hour)
@@ -376,5 +376,18 @@ func TestGetRecentUnmetScheduleTimes(t *testing.T) {
 			t.Errorf("unexpected lack of error")
 		}
 	}
+	{
+		// Case 7: now is way way ahead of last start time, but there is a short deadline.
+		sj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-2 * time.Hour)}
+		sj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)}
+		now := T2.Add(10 * 24 * time.Hour)
+		// Deadline is short
+		deadline := int64(2 * 60 * 60)
+		sj.Spec.StartingDeadlineSeconds = &deadline
+		_, err := getRecentUnmetScheduleTimes(sj, now)
+		if err != nil {
+			t.Errorf("unexpected error")
+		}
+	}
 
 }