TimeZone support for CronJobs

This commit is contained in:
Ross Peoples
2022-03-21 11:27:10 -05:00
parent f25c0e5f09
commit 98837de446
9 changed files with 352 additions and 13 deletions

View File

@@ -376,6 +376,12 @@ type CronJobSpec struct {
// The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
Schedule string
// The time zone for the given schedule, see https://en.wikipedia.org/wiki/List_of_tz_database_time_zones.
// If not specified, this will rely on the time zone of the kube-controller-manager process.
// ALPHA: This field is in alpha and must be enabled via the `CronJobTimeZone` feature gate.
// +optional
TimeZone *string
// Optional deadline in seconds for starting the job if it misses scheduled
// time for any reason. Missed jobs executions will be counted as failed ones.
// +optional

View File

@@ -18,6 +18,8 @@ package validation
import (
"fmt"
"strings"
"time"
"github.com/robfig/cron/v3"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -308,11 +310,14 @@ func ValidateCronJobSpec(spec *batch.CronJobSpec, fldPath *field.Path, opts apiv
if len(spec.Schedule) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("schedule"), ""))
} else {
allErrs = append(allErrs, validateScheduleFormat(spec.Schedule, fldPath.Child("schedule"))...)
allErrs = append(allErrs, validateScheduleFormat(spec.Schedule, spec.TimeZone, fldPath.Child("schedule"))...)
}
if spec.StartingDeadlineSeconds != nil {
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.StartingDeadlineSeconds), fldPath.Child("startingDeadlineSeconds"))...)
}
allErrs = append(allErrs, validateTimeZone(spec.TimeZone, fldPath.Child("timeZone"))...)
allErrs = append(allErrs, validateConcurrencyPolicy(&spec.ConcurrencyPolicy, fldPath.Child("concurrencyPolicy"))...)
allErrs = append(allErrs, ValidateJobTemplateSpec(&spec.JobTemplate, fldPath.Child("jobTemplate"), opts)...)
@@ -343,11 +348,36 @@ func validateConcurrencyPolicy(concurrencyPolicy *batch.ConcurrencyPolicy, fldPa
return allErrs
}
func validateScheduleFormat(schedule string, fldPath *field.Path) field.ErrorList {
func validateScheduleFormat(schedule string, timeZone *string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if _, err := cron.ParseStandard(schedule); err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, schedule, err.Error()))
}
if strings.Contains(schedule, "TZ") && timeZone != nil {
allErrs = append(allErrs, field.Invalid(fldPath, schedule, "cannot use both timeZone field and TZ or CRON_TZ in schedule"))
}
return allErrs
}
func validateTimeZone(timeZone *string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if timeZone == nil {
return allErrs
}
if len(*timeZone) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath, timeZone, "timeZone must be nil or non-empty string"))
return allErrs
}
if strings.EqualFold(*timeZone, "Local") {
allErrs = append(allErrs, field.Invalid(fldPath, timeZone, "timeZone must be an explicit time zone as defined in https://www.iana.org/time-zones"))
}
if _, err := time.LoadLocation(*timeZone); err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, timeZone, err.Error()))
}
return allErrs
}

View File

@@ -31,6 +31,18 @@ import (
"k8s.io/utils/pointer"
)
var (
timeZoneEmpty = ""
timeZoneLocal = "LOCAL"
timeZoneUTC = "UTC"
timeZoneCorrectCasing = "America/New_York"
timeZoneBadCasing = "AMERICA/new_york"
timeZoneBadPrefix = " America/New_York"
timeZoneBadSuffix = "America/New_York "
timeZoneBadName = "America/New York"
timeZoneEmptySpace = " "
)
var ignoreErrValueDetail = cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail")
func getValidManualSelector() *metav1.LabelSelector {
@@ -902,6 +914,23 @@ func TestValidateCronJob(t *testing.T) {
},
},
},
"correct timeZone value casing": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "0 * * * *",
TimeZone: &timeZoneCorrectCasing,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
}
for k, v := range successCases {
if errs := ValidateCronJob(&v, corevalidation.PodValidationOptions{}); len(errs) != 0 {
@@ -953,6 +982,142 @@ func TestValidateCronJob(t *testing.T) {
},
},
},
"spec.schedule: cannot use both timeZone field and TZ or CRON_TZ in schedule": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "TZ=UTC 0 * * * *",
TimeZone: &timeZoneUTC,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
"spec.timeZone: timeZone must be nil or non-empty string": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "0 * * * *",
TimeZone: &timeZoneEmpty,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
"spec.timeZone: timeZone must be an explicit time zone as defined in https://www.iana.org/time-zones": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "0 * * * *",
TimeZone: &timeZoneLocal,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
"spec.timeZone: Invalid value: \"AMERICA/new_york\": unknown time zone AMERICA/new_york": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "0 * * * *",
TimeZone: &timeZoneBadCasing,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
"spec.timeZone: Invalid value: \" America/New_York\": unknown time zone America/New_York": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "0 * * * *",
TimeZone: &timeZoneBadPrefix,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
"spec.timeZone: Invalid value: \"America/New_York \": unknown time zone America/New_York ": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "0 * * * *",
TimeZone: &timeZoneBadSuffix,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
"spec.timeZone: Invalid value: \"America/New York\": unknown time zone America/New York": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "0 * * * *",
TimeZone: &timeZoneBadName,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
"spec.timeZone: Invalid value: \" \": unknown time zone ": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",
Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"),
},
Spec: batch.CronJobSpec{
Schedule: "0 * * * *",
TimeZone: &timeZoneEmptySpace,
ConcurrencyPolicy: batch.AllowConcurrent,
JobTemplate: batch.JobTemplateSpec{
Spec: batch.JobSpec{
Template: validPodTemplateSpec,
},
},
},
},
"spec.startingDeadlineSeconds:must be greater than or equal to 0": {
ObjectMeta: metav1.ObjectMeta{
Name: "mycronjob",

View File

@@ -35,6 +35,8 @@ import (
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apiserver/pkg/features"
utilfeature "k8s.io/apiserver/pkg/util/feature"
batchv1informers "k8s.io/client-go/informers/batch/v1"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme"
@@ -48,6 +50,7 @@ import (
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/controller/cronjob/metrics"
"k8s.io/utils/pointer"
)
var (
@@ -371,6 +374,7 @@ func (jm *ControllerV2) enqueueControllerAfter(obj interface{}, t time.Duration)
// updateCronJob re-queues the CronJob for next scheduled time if there is a
// change in spec.schedule otherwise it re-queues it now
func (jm *ControllerV2) updateCronJob(old interface{}, curr interface{}) {
timeZoneEnabled := utilfeature.DefaultFeatureGate.Enabled(features.CronJobTimeZone)
oldCJ, okOld := old.(*batchv1.CronJob)
newCJ, okNew := curr.(*batchv1.CronJob)
@@ -381,9 +385,9 @@ func (jm *ControllerV2) updateCronJob(old interface{}, curr interface{}) {
// if the change in schedule results in next requeue having to be sooner than it already was,
// it will be handled here by the queue. If the next requeue is further than previous schedule,
// the sync loop will essentially be a no-op for the already queued key with old schedule.
if oldCJ.Spec.Schedule != newCJ.Spec.Schedule {
if oldCJ.Spec.Schedule != newCJ.Spec.Schedule || (timeZoneEnabled && !pointer.StringEqual(oldCJ.Spec.TimeZone, newCJ.Spec.TimeZone)) {
// schedule changed, change the requeue time
sched, err := cron.ParseStandard(newCJ.Spec.Schedule)
sched, err := cron.ParseStandard(formatSchedule(timeZoneEnabled, newCJ, 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
@@ -420,6 +424,7 @@ func (jm *ControllerV2) syncCronJob(
cronJob = cronJob.DeepCopy()
now := jm.now()
updateStatus := false
timeZoneEnabled := utilfeature.DefaultFeatureGate.Enabled(features.CronJobTimeZone)
childrenJobs := make(map[types.UID]bool)
for _, j := range jobs {
@@ -487,12 +492,21 @@ func (jm *ControllerV2) syncCronJob(
return cronJob, nil, updateStatus, nil
}
if timeZoneEnabled && cronJob.Spec.TimeZone != nil {
if _, err := time.LoadLocation(*cronJob.Spec.TimeZone); err != nil {
timeZone := pointer.StringDeref(cronJob.Spec.TimeZone, "")
klog.V(4).InfoS("Not starting job because timeZone is invalid", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName()), "timeZone", timeZone, "err", err)
jm.recorder.Eventf(cronJob, corev1.EventTypeWarning, "UnknownTimeZone", "invalid timeZone: %q: %s", timeZone, err)
return cronJob, nil, updateStatus, nil
}
}
if cronJob.Spec.Suspend != nil && *cronJob.Spec.Suspend {
klog.V(4).InfoS("Not starting job because the cron is suspended", "cronjob", klog.KRef(cronJob.GetNamespace(), cronJob.GetName()))
return cronJob, nil, updateStatus, nil
}
sched, err := cron.ParseStandard(cronJob.Spec.Schedule)
sched, err := cron.ParseStandard(formatSchedule(timeZoneEnabled, cronJob, 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
@@ -501,10 +515,6 @@ func (jm *ControllerV2) syncCronJob(
return cronJob, nil, updateStatus, nil
}
if strings.Contains(cronJob.Spec.Schedule, "TZ") {
jm.recorder.Eventf(cronJob, corev1.EventTypeWarning, "UnsupportedSchedule", "CRON_TZ or TZ used in schedule %q is not officially supported, see https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/ for more details", cronJob.Spec.Schedule)
}
scheduledTime, err := getNextScheduleTime(*cronJob, now, sched, jm.recorder)
if err != nil {
// this is likely a user error in defining the spec value
@@ -739,3 +749,20 @@ func deleteJob(cj *batchv1.CronJob, job *batchv1.Job, jc jobControlInterface, re
func getRef(object runtime.Object) (*corev1.ObjectReference, error) {
return ref.GetReference(scheme.Scheme, object)
}
func formatSchedule(timeZoneEnabled bool, cj *batchv1.CronJob, recorder record.EventRecorder) string {
if strings.Contains(cj.Spec.Schedule, "TZ") {
recorder.Eventf(cj, corev1.EventTypeWarning, "UnsupportedSchedule", "CRON_TZ or TZ used in schedule %q is not officially supported, see https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/ for more details", cj.Spec.Schedule)
return cj.Spec.Schedule
}
if timeZoneEnabled && cj.Spec.TimeZone != nil {
if _, err := time.LoadLocation(*cj.Spec.TimeZone); err != nil {
return cj.Spec.Schedule
}
return fmt.Sprintf("TZ=%s %s", *cj.Spec.TimeZone, cj.Spec.Schedule)
}
return cj.Spec.Schedule
}

View File

@@ -32,10 +32,13 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/features"
"k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/workqueue"
featuregatetesting "k8s.io/component-base/featuregate/testing"
_ "k8s.io/kubernetes/pkg/apis/batch/install"
_ "k8s.io/kubernetes/pkg/apis/core/install"
"k8s.io/kubernetes/pkg/controller"
@@ -50,6 +53,9 @@ var (
errorSchedule = "obvious error schedule"
// schedule is hourly on the hour
onTheHour = "0 * * * ?"
errorTimeZone = "bad timezone"
newYork = "America/New_York"
)
// returns a cronJob with some fields filled in.
@@ -127,6 +133,19 @@ func justAfterTheHour() *time.Time {
return &T1
}
func justAfterTheHourInZone(tz string) time.Time {
location, err := time.LoadLocation(tz)
if err != nil {
panic("tz error: " + err.Error())
}
T1, err := time.ParseInLocation(time.RFC3339, "2016-05-19T10:01:00Z", location)
if err != nil {
panic("test setup error: " + err.Error())
}
return T1
}
func justBeforeTheHour() time.Time {
T1, err := time.Parse(time.RFC3339, "2016-05-19T09:59:00Z")
if err != nil {
@@ -162,6 +181,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
concurrencyPolicy batchv1.ConcurrencyPolicy
suspend bool
schedule string
timeZone *string
deadline int64
// cj status
@@ -173,6 +193,7 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now time.Time
jobCreateError error
jobGetErr error
enableTimeZone bool
// expectations
expectCreate bool
@@ -212,6 +233,17 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectedWarnings: 1,
jobPresentInCJActiveStatus: true,
},
"never ran, not valid time zone": {
concurrencyPolicy: "Allow",
schedule: onTheHour,
timeZone: &errorTimeZone,
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(),
enableTimeZone: true,
expectedWarnings: 1,
jobPresentInCJActiveStatus: true,
},
"never ran, not time, A": {
concurrencyPolicy: "Allow",
schedule: onTheHour,
@@ -238,6 +270,17 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectRequeueAfter: true,
jobPresentInCJActiveStatus: true,
},
"never ran, not time in zone": {
concurrencyPolicy: "Allow",
schedule: onTheHour,
timeZone: &newYork,
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(),
enableTimeZone: true,
expectRequeueAfter: true,
jobPresentInCJActiveStatus: true,
},
"never ran, is time, A": {
concurrencyPolicy: "Allow",
schedule: onTheHour,
@@ -274,6 +317,48 @@ func TestControllerV2SyncCronJob(t *testing.T) {
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"never ran, is time in zone, but time zone disabled": {
concurrencyPolicy: "Allow",
schedule: onTheHour,
timeZone: &newYork,
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justAfterTheHourInZone(newYork),
enableTimeZone: false,
expectCreate: true,
expectActive: 1,
expectRequeueAfter: true,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"never ran, is time in zone": {
concurrencyPolicy: "Allow",
schedule: onTheHour,
timeZone: &newYork,
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justAfterTheHourInZone(newYork),
enableTimeZone: true,
expectCreate: true,
expectActive: 1,
expectRequeueAfter: true,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"never ran, is time in zone, but TZ is also set in schedule": {
concurrencyPolicy: "Allow",
schedule: "TZ=UTC " + onTheHour,
timeZone: &newYork,
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justAfterTheHourInZone(newYork),
enableTimeZone: true,
expectCreate: true,
expectedWarnings: 1,
expectRequeueAfter: true,
expectUpdateStatus: true,
jobPresentInCJActiveStatus: true,
},
"never ran, is time, suspended": {
concurrencyPolicy: "Allow",
suspend: true,
@@ -820,10 +905,15 @@ func TestControllerV2SyncCronJob(t *testing.T) {
cj.Spec.ConcurrencyPolicy = tc.concurrencyPolicy
cj.Spec.Suspend = &tc.suspend
cj.Spec.Schedule = tc.schedule
cj.Spec.TimeZone = tc.timeZone
if tc.deadline != noDead {
cj.Spec.StartingDeadlineSeconds = &tc.deadline
}
if tc.enableTimeZone {
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.CronJobTimeZone, true)
}
var (
job *batchv1.Job
err error