scottk/kubernetes
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/kubernetes.git synced 2025-11-03 19:58:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

cronjob_controllerv2: gracefully handle 0 seconds between schedules

Browse Source
This commit is contained in:
Alay Patel
2021-02-22 21:40:05 -05:00
parent 4d7d14d249
commit 6290a23ceb
3 changed files with 32 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
pkg/controller/cronjob/cronjob_controllerv2.go
View File

@@ -477,7 +477,14 @@ func (jm *ControllerV2) syncCronJob(
jm.recorder.Eventf(cj, corev1.EventTypeWarning, "UnparseableSchedule", "unparseable schedule: %s : %s", cj.Spec.Schedule, err) jm.recorder.Eventf(cj, corev1.EventTypeWarning, "UnparseableSchedule", "unparseable schedule: %s : %s", cj.Spec.Schedule, err)
return cj, nil, nil return cj, nil, nil
} }
scheduledTime := getNextScheduleTime(*cj, now, sched, jm.recorder) scheduledTime, err := getNextScheduleTime(*cj, now, sched, jm.recorder)
if err != nil {
// this is likely a user error in defining the spec value
// we should log the error and not reconcile this cronjob until an update to spec
klog.V(2).InfoS("invalid schedule", "cronjob", klog.KRef(cj.GetNamespace(), cj.GetName()), "schedule", cj.Spec.Schedule, "err", err)
jm.recorder.Eventf(cj, corev1.EventTypeWarning, "InvalidSchedule", "invalid schedule schedule: %s : %s", cj.Spec.Schedule, err)
return cj, nil, nil
}
if scheduledTime == nil { if scheduledTime == nil {
// no unmet start time, return cj,. // no unmet start time, return cj,.
// The only time this should happen is if queue is filled after restart. // The only time this should happen is if queue is filled after restart.

24
pkg/controller/cronjob/utils.go
View File

@@ -154,7 +154,7 @@ func getRecentUnmetScheduleTimes(cj batchv1beta1.CronJob, now time.Time) ([]time
// it returns nil if no unmet schedule times. // it returns nil if no unmet schedule times.
// If there are too many (>100) unstarted times, it will raise a warning and but still return // If there are too many (>100) unstarted times, it will raise a warning and but still return
// the list of missed times. // the list of missed times.
func getNextScheduleTime(cj batchv1beta1.CronJob, now time.Time, schedule cron.Schedule, recorder record.EventRecorder) *time.Time { func getNextScheduleTime(cj batchv1beta1.CronJob, now time.Time, schedule cron.Schedule, recorder record.EventRecorder) (*time.Time, error) {
starts := []time.Time{} starts := []time.Time{}
var earliestTime time.Time var earliestTime time.Time
@@ -178,10 +178,10 @@ func getNextScheduleTime(cj batchv1beta1.CronJob, now time.Time, schedule cron.S
} }
} }
if earliestTime.After(now) { if earliestTime.After(now) {
return nil return nil, nil
} }
t, numberOfMissedSchedules := getMostRecentScheduleTime(earliestTime, now, schedule) t, numberOfMissedSchedules, err := getMostRecentScheduleTime(earliestTime, now, schedule)
if numberOfMissedSchedules > 100 { if numberOfMissedSchedules > 100 {
// An object might miss several starts. For example, if // An object might miss several starts. For example, if
@@ -204,27 +204,33 @@ func getNextScheduleTime(cj batchv1beta1.CronJob, now time.Time, schedule cron.S
recorder.Eventf(&cj, corev1.EventTypeWarning, "TooManyMissedTimes", "too many missed start times: %d. Set or decrease .spec.startingDeadlineSeconds or check clock skew", len(starts)) recorder.Eventf(&cj, corev1.EventTypeWarning, "TooManyMissedTimes", "too many missed start times: %d. Set or decrease .spec.startingDeadlineSeconds or check clock skew", len(starts))
klog.InfoS("too many missed times", "cronjob", klog.KRef(cj.GetNamespace(), cj.GetName()), "missed times", len(starts)) klog.InfoS("too many missed times", "cronjob", klog.KRef(cj.GetNamespace(), cj.GetName()), "missed times", len(starts))
} }
return t return t, err
} }
// getMostRecentScheduleTime returns the latest schedule time between earliestTime and the count of number of // getMostRecentScheduleTime returns the latest schedule time between earliestTime and the count of number of
// schedules in between them // schedules in between them
func getMostRecentScheduleTime(earliestTime time.Time, now time.Time, schedule cron.Schedule) (*time.Time, int64) { func getMostRecentScheduleTime(earliestTime time.Time, now time.Time, schedule cron.Schedule) (*time.Time, int64, error) {
t1 := schedule.Next(earliestTime) t1 := schedule.Next(earliestTime)
t2 := schedule.Next(t1) t2 := schedule.Next(t1)
if now.Before(t1) { if now.Before(t1) {
return nil, 0 return nil, 0, nil
} }
if now.Before(t2) { if now.Before(t2) {
return &t1, 1 return &t1, 1, nil
} }
timeBetweenTwoSchedules := int64(t2.Sub(t1).Seconds()) // It is possible for cron.ParseStandard("59 23 31 2 *") to return an invalid schedule
// seconds - 59, minute - 23, hour - 31 (?!) dom - 2, and dow is optional, clearly 31 is invalid
// In this case the timeBetweenTwoSchedules will be 0, and we error out the invalid schedule
timeBetweenTwoSchedules := int64(t2.Sub(t1).Round(time.Second).Seconds())
if timeBetweenTwoSchedules < 1 {
return nil, 0, fmt.Errorf("time difference between two schedules less than 1 second")
}
timeElapsed := int64(now.Sub(t1).Seconds()) timeElapsed := int64(now.Sub(t1).Seconds())
numberOfMissedSchedules := (timeElapsed / timeBetweenTwoSchedules) + 1 numberOfMissedSchedules := (timeElapsed / timeBetweenTwoSchedules) + 1
t := time.Unix(t1.Unix()+((numberOfMissedSchedules-1)*timeBetweenTwoSchedules), 0).UTC() t := time.Unix(t1.Unix()+((numberOfMissedSchedules-1)*timeBetweenTwoSchedules), 0).UTC()
return &t, numberOfMissedSchedules return &t, numberOfMissedSchedules, nil
} }
// getJobFromTemplate makes a Job from a CronJob // getJobFromTemplate makes a Job from a CronJob

18
pkg/controller/cronjob/utils_test.go
View File

@@ -344,7 +344,7 @@ func TestGetNextScheduleTime(t *testing.T) {
cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-10 * time.Minute)} cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-10 * time.Minute)}
// Current time is more than creation time, but less than T1. // Current time is more than creation time, but less than T1.
now := T1.Add(-7 * time.Minute) now := T1.Add(-7 * time.Minute)
schedule := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder) schedule, _ := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder)
if schedule != nil { if schedule != nil {
t.Errorf("expected no start time, got: %v", schedule) t.Errorf("expected no start time, got: %v", schedule)
} }
@@ -355,7 +355,7 @@ func TestGetNextScheduleTime(t *testing.T) {
cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-10 * time.Minute)} cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-10 * time.Minute)}
// Current time is after T1 // Current time is after T1
now := T1.Add(2 * time.Second) now := T1.Add(2 * time.Second)
schedule := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder) schedule, _ := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder)
if schedule == nil { if schedule == nil {
t.Errorf("expected 1 start time, got nil") t.Errorf("expected 1 start time, got nil")
} else if !schedule.Equal(T1) { } else if !schedule.Equal(T1) {
@@ -370,7 +370,7 @@ func TestGetNextScheduleTime(t *testing.T) {
cj.Status.LastScheduleTime = &metav1.Time{Time: T1} cj.Status.LastScheduleTime = &metav1.Time{Time: T1}
// Current time is after T1 // Current time is after T1
now := T1.Add(2 * time.Minute) now := T1.Add(2 * time.Minute)
schedule := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder) schedule, _ := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder)
if schedule != nil { if schedule != nil {
t.Errorf("expected 0 start times, got: %v", schedule) t.Errorf("expected 0 start times, got: %v", schedule)
} }
@@ -383,7 +383,7 @@ func TestGetNextScheduleTime(t *testing.T) {
cj.Status.LastScheduleTime = &metav1.Time{Time: T1} cj.Status.LastScheduleTime = &metav1.Time{Time: T1}
// Current time is after T1 and after T2 // Current time is after T1 and after T2
now := T2.Add(5 * time.Minute) now := T2.Add(5 * time.Minute)
schedule := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder) schedule, _ := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder)
if schedule == nil { if schedule == nil {
t.Errorf("expected 1 start times, got nil") t.Errorf("expected 1 start times, got nil")
} else if !schedule.Equal(T2) { } else if !schedule.Equal(T2) {
@@ -396,7 +396,7 @@ func TestGetNextScheduleTime(t *testing.T) {
cj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)} cj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)}
// Current time is after T1 and after T2 // Current time is after T1 and after T2
now := T2.Add(5 * time.Minute) now := T2.Add(5 * time.Minute)
schedule := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder) schedule, _ := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder)
if schedule == nil { if schedule == nil {
t.Errorf("expected 1 start times, got nil") t.Errorf("expected 1 start times, got nil")
} else if !schedule.Equal(T2) { } else if !schedule.Equal(T2) {
@@ -408,7 +408,7 @@ func TestGetNextScheduleTime(t *testing.T) {
cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-2 * time.Hour)} cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: T1.Add(-2 * time.Hour)}
cj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)} cj.Status.LastScheduleTime = &metav1.Time{Time: T1.Add(-1 * time.Hour)}
now := T2.Add(10 * 24 * time.Hour) now := T2.Add(10 * 24 * time.Hour)
schedule := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder) schedule, _ := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder)
if schedule == nil { if schedule == nil {
t.Errorf("expected more than 0 missed times") t.Errorf("expected more than 0 missed times")
} }
@@ -421,7 +421,7 @@ func TestGetNextScheduleTime(t *testing.T) {
// Deadline is short // Deadline is short
deadline := int64(2 * 60 * 60) deadline := int64(2 * 60 * 60)
cj.Spec.StartingDeadlineSeconds = &deadline cj.Spec.StartingDeadlineSeconds = &deadline
schedule := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder) schedule, _ := getNextScheduleTime(cj, now, PraseSchedule(cj.Spec.Schedule), recorder)
if schedule == nil { if schedule == nil {
t.Errorf("expected more than 0 missed times") t.Errorf("expected more than 0 missed times")
} }
@@ -673,9 +673,9 @@ func TestGetMostRecentScheduleTime(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
sched, err := cron.Parse(tt.args.schedule) sched, err := cron.Parse(tt.args.schedule)
if err != nil { if err != nil {
t.Errorf("error setting up the test") t.Errorf("error setting up the test, %s", err)
} }
gotTime, gotNumberOfMisses := getMostRecentScheduleTime(*tt.args.earliestTime, tt.args.now, sched) gotTime, gotNumberOfMisses, _ := getMostRecentScheduleTime(*tt.args.earliestTime, tt.args.now, sched)
if gotTime == nil && tt.expectedTime != nil { if gotTime == nil && tt.expectedTime != nil {
t.Errorf("getMostRecentScheduleTime() got nil, want %v", tt.expectedTime) t.Errorf("getMostRecentScheduleTime() got nil, want %v", tt.expectedTime)
} }