Merge pull request #109250 from d-honeybadger/fix-cronjob-scheduling-every-syntax

Fix requeueing of cronjobs with every-style schedule
This commit is contained in:
Kubernetes Prow Robot 2022-06-28 04:37:57 -07:00 committed by GitHub
commit 6269784cd0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 420 additions and 59 deletions

View File

@ -396,7 +396,7 @@ func (jm *ControllerV2) updateCronJob(old interface{}, curr interface{}) {
return return
} }
now := jm.now() now := jm.now()
t := nextScheduledTimeDuration(sched, now) t := nextScheduledTimeDuration(*newCJ, sched, now)
jm.enqueueControllerAfter(curr, *t) jm.enqueueControllerAfter(curr, *t)
return return
@ -529,7 +529,7 @@ func (jm *ControllerV2) syncCronJob(
// Otherwise, the queue is always suppose to trigger sync function at the time of // Otherwise, the queue is always suppose to trigger sync function at the time of
// the scheduled time, that will give atleast 1 unmet time schedule // the scheduled time, that will give atleast 1 unmet time schedule
klog.V(4).InfoS("No unmet start times", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName())) klog.V(4).InfoS("No unmet start times", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName()))
t := nextScheduledTimeDuration(sched, now) t := nextScheduledTimeDuration(*cronJob, sched, now)
return cronJob, t, updateStatus, nil return cronJob, t, updateStatus, nil
} }
@ -548,7 +548,7 @@ func (jm *ControllerV2) syncCronJob(
// Status.LastScheduleTime, Status.LastMissedTime), and then so we won't generate // Status.LastScheduleTime, Status.LastMissedTime), and then so we won't generate
// and event the next time we process it, and also so the user looking at the status // and event the next time we process it, and also so the user looking at the status
// can see easily that there was a missed execution. // can see easily that there was a missed execution.
t := nextScheduledTimeDuration(sched, now) t := nextScheduledTimeDuration(*cronJob, sched, now)
return cronJob, t, updateStatus, nil return cronJob, t, updateStatus, nil
} }
if isJobInActiveList(&batchv1.Job{ if isJobInActiveList(&batchv1.Job{
@ -557,7 +557,7 @@ func (jm *ControllerV2) syncCronJob(
Namespace: cronJob.Namespace, Namespace: cronJob.Namespace,
}}, cronJob.Status.Active) || cronJob.Status.LastScheduleTime.Equal(&metav1.Time{Time: *scheduledTime}) { }}, cronJob.Status.Active) || cronJob.Status.LastScheduleTime.Equal(&metav1.Time{Time: *scheduledTime}) {
klog.V(4).InfoS("Not starting job because the scheduled time is already processed", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName()), "schedule", scheduledTime) klog.V(4).InfoS("Not starting job because the scheduled time is already processed", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName()), "schedule", scheduledTime)
t := nextScheduledTimeDuration(sched, now) t := nextScheduledTimeDuration(*cronJob, sched, now)
return cronJob, t, updateStatus, nil return cronJob, t, updateStatus, nil
} }
if cronJob.Spec.ConcurrencyPolicy == batchv1.ForbidConcurrent && len(cronJob.Status.Active) > 0 { if cronJob.Spec.ConcurrencyPolicy == batchv1.ForbidConcurrent && len(cronJob.Status.Active) > 0 {
@ -572,7 +572,7 @@ func (jm *ControllerV2) syncCronJob(
// But that would mean that you could not inspect prior successes or failures of Forbid jobs. // But that would mean that you could not inspect prior successes or failures of Forbid jobs.
klog.V(4).InfoS("Not starting job because prior execution is still running and concurrency policy is Forbid", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName())) klog.V(4).InfoS("Not starting job because prior execution is still running and concurrency policy is Forbid", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName()))
jm.recorder.Eventf(cronJob, corev1.EventTypeNormal, "JobAlreadyActive", "Not starting job because prior execution is running and concurrency policy is Forbid") jm.recorder.Eventf(cronJob, corev1.EventTypeNormal, "JobAlreadyActive", "Not starting job because prior execution is running and concurrency policy is Forbid")
t := nextScheduledTimeDuration(sched, now) t := nextScheduledTimeDuration(*cronJob, sched, now)
return cronJob, t, updateStatus, nil return cronJob, t, updateStatus, nil
} }
if cronJob.Spec.ConcurrencyPolicy == batchv1.ReplaceConcurrent { if cronJob.Spec.ConcurrencyPolicy == batchv1.ReplaceConcurrent {
@ -633,7 +633,7 @@ func (jm *ControllerV2) syncCronJob(
cronJob.Status.LastScheduleTime = &metav1.Time{Time: *scheduledTime} cronJob.Status.LastScheduleTime = &metav1.Time{Time: *scheduledTime}
updateStatus = true updateStatus = true
t := nextScheduledTimeDuration(sched, now) t := nextScheduledTimeDuration(*cronJob, sched, now)
return cronJob, t, updateStatus, nil return cronJob, t, updateStatus, nil
} }
@ -642,12 +642,26 @@ func getJobName(cj *batchv1.CronJob, scheduledTime time.Time) string {
} }
// nextScheduledTimeDuration returns the time duration to requeue based on // nextScheduledTimeDuration returns the time duration to requeue based on
// the schedule and current time. It adds a 100ms padding to the next requeue to account // the schedule and last schedule time. It adds a 100ms padding to the next requeue to account
// for Network Time Protocol(NTP) time skews. If the time drifts are adjusted which in most // for Network Time Protocol(NTP) time skews. If the time drifts are adjusted which in most
// realistic cases would be around 100s, scheduled cron will still be executed without missing // realistic cases would be around 100s, scheduled cron will still be executed without missing
// the schedule. // the schedule.
func nextScheduledTimeDuration(sched cron.Schedule, now time.Time) *time.Duration { func nextScheduledTimeDuration(cj batchv1.CronJob, sched cron.Schedule, now time.Time) *time.Duration {
t := sched.Next(now).Add(nextScheduleDelta).Sub(now) earliestTime := cj.ObjectMeta.CreationTimestamp.Time
if cj.Status.LastScheduleTime != nil {
earliestTime = cj.Status.LastScheduleTime.Time
}
mostRecentTime, _, err := getMostRecentScheduleTime(earliestTime, now, sched)
if err != nil {
// we still have to requeue at some point, so aim for the next scheduling slot from now
mostRecentTime = &now
} else if mostRecentTime == nil {
// no missed schedules since earliestTime
mostRecentTime = &earliestTime
}
t := sched.Next(*mostRecentTime).Add(nextScheduleDelta).Sub(now)
return &t return &t
} }

View File

@ -23,8 +23,6 @@ import (
"testing" "testing"
"time" "time"
"github.com/robfig/cron/v3"
batchv1 "k8s.io/api/batch/v1" batchv1 "k8s.io/api/batch/v1"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/errors"
@ -53,6 +51,7 @@ var (
errorSchedule = "obvious error schedule" errorSchedule = "obvious error schedule"
// schedule is hourly on the hour // schedule is hourly on the hour
onTheHour = "0 * * * ?" onTheHour = "0 * * * ?"
everyHour = "@every 1h"
errorTimeZone = "bad timezone" errorTimeZone = "bad timezone"
newYork = "America/New_York" newYork = "America/New_York"
@ -154,6 +153,14 @@ func justBeforeTheHour() time.Time {
return T1 return T1
} }
func justBeforeTheNextHour() time.Time {
T1, err := time.Parse(time.RFC3339, "2016-05-19T10:59:00Z")
if err != nil {
panic("test setup error")
}
return T1
}
func weekAfterTheHour() time.Time { func weekAfterTheHour() time.Time {
T1, err := time.Parse(time.RFC3339, "2016-05-26T10:00:00Z") T1, err := time.Parse(time.RFC3339, "2016-05-26T10:00:00Z")
if err != nil { if err != nil {
@ -189,7 +196,9 @@ func TestControllerV2SyncCronJob(t *testing.T) {
stillActive bool stillActive bool
// environment // environment
cronjobCreationTime time.Time
jobCreationTime time.Time jobCreationTime time.Time
lastScheduleTime time.Time
now time.Time now time.Time
jobCreateError error jobCreateError error
jobGetErr error jobGetErr error
@ -202,6 +211,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectedWarnings int expectedWarnings int
expectErr bool expectErr bool
expectRequeueAfter bool expectRequeueAfter bool
expectedRequeueDuration time.Duration
expectUpdateStatus bool expectUpdateStatus bool
jobStillNotFoundInLister bool jobStillNotFoundInLister bool
jobPresentInCJActiveStatus bool jobPresentInCJActiveStatus bool
@ -251,6 +261,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
jobCreationTime: justAfterThePriorHour(), jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true}, jobPresentInCJActiveStatus: true},
"never ran, not time, F": { "never ran, not time, F": {
concurrencyPolicy: "Forbid", concurrencyPolicy: "Forbid",
@ -259,6 +270,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
jobCreationTime: justAfterThePriorHour(), jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"never ran, not time, R": { "never ran, not time, R": {
@ -268,6 +280,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
jobCreationTime: justAfterThePriorHour(), jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"never ran, not time in zone": { "never ran, not time in zone": {
@ -279,6 +292,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justBeforeTheHour(), now: justBeforeTheHour(),
enableTimeZone: true, enableTimeZone: true,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"never ran, is time, A": { "never ran, is time, A": {
@ -290,6 +304,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -302,6 +317,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -314,6 +330,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -328,6 +345,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -342,6 +360,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -356,6 +375,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectedWarnings: 1, expectedWarnings: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -375,6 +395,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
jobCreationTime: justAfterThePriorHour(), jobCreationTime: justAfterThePriorHour(),
now: justAfterTheHour().Add(time.Minute * time.Duration(shortDead+1)), now: justAfterTheHour().Add(time.Minute * time.Duration(shortDead+1)),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute - time.Minute*time.Duration(shortDead+1) + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"never ran, is time, not past deadline": { "never ran, is time, not past deadline": {
@ -386,6 +407,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -398,6 +420,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
jobCreationTime: justAfterThePriorHour(), jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -409,6 +432,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
jobCreationTime: justAfterThePriorHour(), jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -420,6 +444,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
jobCreationTime: justAfterThePriorHour(), jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -433,6 +458,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -458,6 +484,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -471,6 +498,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -493,6 +521,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
jobCreationTime: justAfterThePriorHour(), jobCreationTime: justAfterThePriorHour(),
now: *justAfterTheHour(), now: *justAfterTheHour(),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -506,6 +535,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -520,6 +550,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"still active, not time, F": { "still active, not time, F": {
@ -532,6 +563,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"still active, not time, R": { "still active, not time, R": {
@ -544,6 +576,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justBeforeTheHour(), now: justBeforeTheHour(),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"still active, is time, A": { "still active, is time, A": {
@ -557,6 +590,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 2, expectActive: 2,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -570,6 +604,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: *justAfterTheHour(), now: *justAfterTheHour(),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"still active, is time, R": { "still active, is time, R": {
@ -584,6 +619,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectDelete: true, expectDelete: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -622,6 +658,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: *justAfterTheHour(), now: *justAfterTheHour(),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
"still active, is time, not past deadline": { "still active, is time, not past deadline": {
@ -635,6 +672,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 2, expectActive: 2,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -652,6 +690,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectActive: 1, expectActive: 1,
expectedWarnings: 1, expectedWarnings: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -666,6 +705,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectActive: 1, expectActive: 1,
expectedWarnings: 1, expectedWarnings: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -680,6 +720,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectActive: 1, expectActive: 1,
expectedWarnings: 1, expectedWarnings: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -694,6 +735,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectActive: 1, expectActive: 1,
expectedWarnings: 1, expectedWarnings: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -708,6 +750,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectActive: 1, expectActive: 1,
expectedWarnings: 1, expectedWarnings: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -722,6 +765,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectActive: 1, expectActive: 1,
expectedWarnings: 1, expectedWarnings: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -736,6 +780,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -749,6 +794,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -763,6 +809,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -776,6 +823,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -790,6 +838,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -803,6 +852,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectCreate: true, expectCreate: true,
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -818,6 +868,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justBeforeTheHour(), now: justBeforeTheHour(),
jobCreateError: errors.NewAlreadyExists(schema.GroupResource{Resource: "jobs", Group: "batch"}, ""), jobCreateError: errors.NewAlreadyExists(schema.GroupResource{Resource: "jobs", Group: "batch"}, ""),
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true, expectUpdateStatus: true,
}, },
@ -832,6 +883,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justAfterTheHour().Add(time.Millisecond * 100), now: justAfterTheHour().Add(time.Millisecond * 100),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute - time.Millisecond*100 + nextScheduleDelta,
jobStillNotFoundInLister: true, jobStillNotFoundInLister: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -845,6 +897,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justAfterTheHour().Add(time.Millisecond * 100), now: justAfterTheHour().Add(time.Millisecond * 100),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute - time.Millisecond*100 + nextScheduleDelta,
jobStillNotFoundInLister: true, jobStillNotFoundInLister: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -858,6 +911,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justAfterTheHour().Add(time.Millisecond * 100), now: justAfterTheHour().Add(time.Millisecond * 100),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute - time.Millisecond*100 + nextScheduleDelta,
jobStillNotFoundInLister: true, jobStillNotFoundInLister: true,
jobPresentInCJActiveStatus: true, jobPresentInCJActiveStatus: true,
}, },
@ -873,6 +927,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justAfterTheHour().Add(time.Millisecond * 100), now: justAfterTheHour().Add(time.Millisecond * 100),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute - time.Millisecond*100 + nextScheduleDelta,
}, },
"this started but is not present in cronjob active list, not past deadline, f": { "this started but is not present in cronjob active list, not past deadline, f": {
concurrencyPolicy: "Forbid", concurrencyPolicy: "Forbid",
@ -884,6 +939,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justAfterTheHour().Add(time.Millisecond * 100), now: justAfterTheHour().Add(time.Millisecond * 100),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute - time.Millisecond*100 + nextScheduleDelta,
}, },
"this started but is not present in cronjob active list, not past deadline, R": { "this started but is not present in cronjob active list, not past deadline, R": {
concurrencyPolicy: "Replace", concurrencyPolicy: "Replace",
@ -895,6 +951,219 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now: justAfterTheHour().Add(time.Millisecond * 100), now: justAfterTheHour().Add(time.Millisecond * 100),
expectActive: 1, expectActive: 1,
expectRequeueAfter: true, expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute - time.Millisecond*100 + nextScheduleDelta,
},
// Tests for @every-style schedule
"with @every schedule, never ran, not time": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: noDead,
cronjobCreationTime: justBeforeTheHour(),
jobCreationTime: justBeforeTheHour(),
now: *topOfTheHour(),
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, never ran, is time": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: noDead,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeThePriorHour(),
now: justBeforeTheHour(),
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour + nextScheduleDelta,
jobPresentInCJActiveStatus: true,
expectCreate: true,
expectActive: 1,
expectUpdateStatus: true,
},
"with @every schedule, never ran, is time, past deadline": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: shortDead,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeThePriorHour(),
now: justBeforeTheHour().Add(time.Second * time.Duration(shortDead+1)),
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - time.Second*time.Duration(shortDead+1) + nextScheduleDelta,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, never ran, is time, not past deadline": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: longDead,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeThePriorHour(),
now: justBeforeTheHour().Add(time.Second * time.Duration(shortDead-1)),
expectCreate: true,
expectActive: 1,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - time.Second*time.Duration(shortDead-1) + nextScheduleDelta,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, prev ran but done, not time": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: noDead,
ranPreviously: true,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeThePriorHour(),
lastScheduleTime: justBeforeTheHour(),
now: *topOfTheHour(),
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, prev ran but done, is time": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: noDead,
ranPreviously: true,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeThePriorHour(),
lastScheduleTime: justBeforeTheHour(),
now: topOfTheHour().Add(1 * time.Hour),
expectCreate: true,
expectActive: 1,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, prev ran but done, is time, past deadline": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: shortDead,
ranPreviously: true,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeThePriorHour(),
lastScheduleTime: justBeforeTheHour(),
now: justBeforeTheNextHour().Add(time.Second * time.Duration(shortDead+1)),
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - time.Second*time.Duration(shortDead+1) + nextScheduleDelta,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
// This test will fail: the logic around StartingDeadlineSecond in getNextScheduleTime messes up
// the time that calculating schedule.Next(earliestTime) is based on. While this works perfectly
// well for classic cron scheduled, with @every X, schedule.Next(earliestTime) just returns the time
// offset by X relative to the earliestTime.
// "with @every schedule, prev ran but done, is time, not past deadline": {
// concurrencyPolicy: "Allow",
// schedule: everyHour,
// deadline: shortDead,
// ranPreviously: true,
// cronjobCreationTime: justBeforeThePriorHour(),
// jobCreationTime: justBeforeThePriorHour(),
// lastScheduleTime: justBeforeTheHour(),
// now: justBeforeTheNextHour().Add(time.Second * time.Duration(shortDead-1)),
// expectCreate: true,
// expectActive: 1,
// expectRequeueAfter: true,
// expectedRequeueDuration: 1*time.Hour - time.Second*time.Duration(shortDead-1) + nextScheduleDelta,
// expectUpdateStatus: true,
// jobPresentInCJActiveStatus: true,
// },
"with @every schedule, still active, not time": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: noDead,
ranPreviously: true,
stillActive: true,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeTheHour(),
lastScheduleTime: justBeforeTheHour(),
now: *topOfTheHour(),
expectActive: 1,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, still active, is time": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: noDead,
ranPreviously: true,
stillActive: true,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeThePriorHour(),
lastScheduleTime: justBeforeThePriorHour(),
now: *justAfterTheHour(),
expectCreate: true,
expectActive: 2,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - 2*time.Minute + nextScheduleDelta,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, still active, is time, past deadline": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: shortDead,
ranPreviously: true,
stillActive: true,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeTheHour(),
lastScheduleTime: justBeforeTheHour(),
now: justBeforeTheNextHour().Add(time.Second * time.Duration(shortDead+1)),
expectActive: 1,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - time.Second*time.Duration(shortDead+1) + nextScheduleDelta,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, still active, is time, not past deadline": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: longDead,
ranPreviously: true,
stillActive: true,
cronjobCreationTime: justBeforeThePriorHour(),
jobCreationTime: justBeforeTheHour(),
lastScheduleTime: justBeforeTheHour(),
now: justBeforeTheNextHour().Add(time.Second * time.Duration(shortDead-1)),
expectCreate: true,
expectActive: 2,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - time.Second*time.Duration(shortDead-1) + nextScheduleDelta,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, prev ran but done, long overdue, no deadline": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: noDead,
ranPreviously: true,
cronjobCreationTime: justAfterThePriorHour(),
lastScheduleTime: *justAfterTheHour(),
jobCreationTime: justAfterThePriorHour(),
now: weekAfterTheHour(),
expectCreate: true,
expectActive: 1,
expectedWarnings: 1,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"with @every schedule, prev ran but done, long overdue, past deadline": {
concurrencyPolicy: "Allow",
schedule: everyHour,
deadline: shortDead,
ranPreviously: true,
cronjobCreationTime: justAfterThePriorHour(),
lastScheduleTime: *justAfterTheHour(),
jobCreationTime: justAfterThePriorHour(),
now: weekAfterTheHour().Add(1 * time.Minute).Add(time.Second * time.Duration(shortDead+1)),
expectActive: 1,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Hour - time.Second*time.Duration(shortDead+1) + nextScheduleDelta,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
}, },
} }
for name, tc := range testCases { for name, tc := range testCases {
@ -921,7 +1190,13 @@ func TestControllerV2SyncCronJob(t *testing.T) {
realCJ := cj.DeepCopy() realCJ := cj.DeepCopy()
if tc.ranPreviously { if tc.ranPreviously {
cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: justBeforeThePriorHour()} cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: justBeforeThePriorHour()}
if !tc.cronjobCreationTime.IsZero() {
cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: tc.cronjobCreationTime}
}
cj.Status.LastScheduleTime = &metav1.Time{Time: justAfterThePriorHour()} cj.Status.LastScheduleTime = &metav1.Time{Time: justAfterThePriorHour()}
if !tc.lastScheduleTime.IsZero() {
cj.Status.LastScheduleTime = &metav1.Time{Time: tc.lastScheduleTime}
}
job, err = getJobFromTemplate2(&cj, tc.jobCreationTime) job, err = getJobFromTemplate2(&cj, tc.jobCreationTime)
if err != nil { if err != nil {
t.Fatalf("%s: unexpected error creating a job from template: %v", name, err) t.Fatalf("%s: unexpected error creating a job from template: %v", name, err)
@ -952,6 +1227,9 @@ func TestControllerV2SyncCronJob(t *testing.T) {
} }
} else { } else {
cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: justBeforeTheHour()} cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: justBeforeTheHour()}
if !tc.cronjobCreationTime.IsZero() {
cj.ObjectMeta.CreationTimestamp = metav1.Time{Time: tc.cronjobCreationTime}
}
if tc.stillActive { if tc.stillActive {
t.Errorf("%s: test setup error: this case makes no sense", name) t.Errorf("%s: test setup error: this case makes no sense", name)
} }
@ -974,13 +1252,8 @@ func TestControllerV2SyncCronJob(t *testing.T) {
t.Errorf("%s: expected error got none with requeueAfter time: %#v", name, requeueAfter) t.Errorf("%s: expected error got none with requeueAfter time: %#v", name, requeueAfter)
} }
if tc.expectRequeueAfter { if tc.expectRequeueAfter {
sched, err := cron.ParseStandard(tc.schedule) if !reflect.DeepEqual(requeueAfter, &tc.expectedRequeueDuration) {
if err != nil { t.Errorf("%s: expected requeueAfter: %+v, got requeueAfter time: %+v", name, tc.expectedRequeueDuration, requeueAfter)
t.Errorf("%s: test setup error: the schedule %s is unparseable: %#v", name, tc.schedule, err)
}
expectedRequeueAfter := nextScheduledTimeDuration(sched, tc.now)
if !reflect.DeepEqual(requeueAfter, expectedRequeueAfter) {
t.Errorf("%s: expected requeueAfter: %+v, got requeueAfter time: %+v", name, expectedRequeueAfter, requeueAfter)
} }
} }
if updateStatus != tc.expectUpdateStatus { if updateStatus != tc.expectUpdateStatus {
@ -1132,6 +1405,9 @@ func TestControllerV2UpdateCronJob(t *testing.T) {
Spec: jobSpec(), Spec: jobSpec(),
}, },
}, },
Status: batchv1.CronJobStatus{
LastScheduleTime: &metav1.Time{Time: justBeforeTheHour()},
},
}, },
newCronJob: &batchv1.CronJob{ newCronJob: &batchv1.CronJob{
Spec: batchv1.CronJobSpec{ Spec: batchv1.CronJobSpec{
@ -1144,6 +1420,77 @@ func TestControllerV2UpdateCronJob(t *testing.T) {
Spec: jobSpec(), Spec: jobSpec(),
}, },
}, },
Status: batchv1.CronJobStatus{
LastScheduleTime: &metav1.Time{Time: justBeforeTheHour()},
},
},
expectedDelay: 1*time.Second + nextScheduleDelta,
},
{
name: "spec.schedule with @every changed - cadence decrease",
oldCronJob: &batchv1.CronJob{
Spec: batchv1.CronJobSpec{
Schedule: "@every 1m",
JobTemplate: batchv1.JobTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{"a": "b"},
Annotations: map[string]string{"x": "y"},
},
Spec: jobSpec(),
},
},
Status: batchv1.CronJobStatus{
LastScheduleTime: &metav1.Time{Time: justBeforeTheHour()},
},
},
newCronJob: &batchv1.CronJob{
Spec: batchv1.CronJobSpec{
Schedule: "@every 3m",
JobTemplate: batchv1.JobTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{"a": "foo"},
Annotations: map[string]string{"x": "y"},
},
Spec: jobSpec(),
},
},
Status: batchv1.CronJobStatus{
LastScheduleTime: &metav1.Time{Time: justBeforeTheHour()},
},
},
expectedDelay: 2*time.Minute + 1*time.Second + nextScheduleDelta,
},
{
name: "spec.schedule with @every changed - cadence increase",
oldCronJob: &batchv1.CronJob{
Spec: batchv1.CronJobSpec{
Schedule: "@every 3m",
JobTemplate: batchv1.JobTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{"a": "b"},
Annotations: map[string]string{"x": "y"},
},
Spec: jobSpec(),
},
},
Status: batchv1.CronJobStatus{
LastScheduleTime: &metav1.Time{Time: justBeforeTheHour()},
},
},
newCronJob: &batchv1.CronJob{
Spec: batchv1.CronJobSpec{
Schedule: "@every 1m",
JobTemplate: batchv1.JobTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{"a": "foo"},
Annotations: map[string]string{"x": "y"},
},
Spec: jobSpec(),
},
},
Status: batchv1.CronJobStatus{
LastScheduleTime: &metav1.Time{Time: justBeforeTheHour()},
},
}, },
expectedDelay: 1*time.Second + nextScheduleDelta, expectedDelay: 1*time.Second + nextScheduleDelta,
}, },