Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Promote CronJob TZ to GA #115904

Merged
merged 2 commits into from Mar 7, 2023
Merged

Promote CronJob TZ to GA #115904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b825c1
Promote CronJob TZ to GA
soltysh Feb 20, 2023
e047c85
Update generated
soltysh Feb 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/openapi-spec/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api/openapi-spec/v3/apis__batch__v1_openapi.json
View file
Open in desktop
Expand Up @@ -135,7 +135,7 @@
"type": "boolean"
},
"timeZone": {
"description": "The time zone name for the given schedule, see https://en.wikipedia.org/wiki/List_of_tz_database_time_zones. If not specified, this will default to the time zone of the kube-controller-manager process. The set of valid time zone names and the time zone offset is loaded from the system-wide time zone database by the API server during CronJob validation and the controller manager during execution. If no system-wide time zone database can be found a bundled version of the database is used instead. If the time zone name becomes invalid during the lifetime of a CronJob or due to a change in host configuration, the controller will stop creating new new Jobs and will create a system event with the reason UnknownTimeZone. More information can be found in https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#time-zones This is beta field and must be enabled via the `CronJobTimeZone` feature gate.",
"description": "The time zone name for the given schedule, see https://en.wikipedia.org/wiki/List_of_tz_database_time_zones. If not specified, this will default to the time zone of the kube-controller-manager process. The set of valid time zone names and the time zone offset is loaded from the system-wide time zone database by the API server during CronJob validation and the controller manager during execution. If no system-wide time zone database can be found a bundled version of the database is used instead. If the time zone name becomes invalid during the lifetime of a CronJob or due to a change in host configuration, the controller will stop creating new new Jobs and will create a system event with the reason UnknownTimeZone. More information can be found in https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#time-zones",
"type": "string"
}
},
Expand Down
1 change: 0 additions & 1 deletion pkg/apis/batch/types.go
View file
Open in desktop
Expand Up @@ -501,7 +501,6 @@ type CronJobSpec struct {
// configuration, the controller will stop creating new new Jobs and will create a system event with the
// reason UnknownTimeZone.
// More information can be found in https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#time-zones
// This is beta field and must be enabled via the `CronJobTimeZone` feature gate.
// +optional
TimeZone *string

Expand Down
20 changes: 8 additions & 12 deletions pkg/controller/cronjob/cronjob_controllerv2.go
View file
Open in desktop
Expand Up @@ -35,7 +35,6 @@ import (
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
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"
Expand All @@ -48,7 +47,6 @@ import (
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/controller/cronjob/metrics"
"k8s.io/kubernetes/pkg/features"
"k8s.io/utils/pointer"
)

Expand Down Expand Up @@ -380,7 +378,6 @@ 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(logger klog.Logger, old interface{}, curr interface{}) {
timeZoneEnabled := utilfeature.DefaultFeatureGate.Enabled(features.CronJobTimeZone)
oldCJ, okOld := old.(*batchv1.CronJob)
newCJ, okNew := curr.(*batchv1.CronJob)

Expand All @@ -391,9 +388,9 @@ func (jm *ControllerV2) updateCronJob(logger klog.Logger, old interface{}, curr
// 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 || (timeZoneEnabled && !pointer.StringEqual(oldCJ.Spec.TimeZone, newCJ.Spec.TimeZone)) {
if oldCJ.Spec.Schedule != newCJ.Spec.Schedule || !pointer.StringEqual(oldCJ.Spec.TimeZone, newCJ.Spec.TimeZone) {
// schedule changed, change the requeue time, pass nil recorder so that syncCronJob will output any warnings
sched, err := cron.ParseStandard(formatSchedule(timeZoneEnabled, newCJ, nil))
sched, err := cron.ParseStandard(formatSchedule(newCJ, nil))
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
Expand Down Expand Up @@ -430,7 +427,6 @@ 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 {
Expand Down Expand Up @@ -499,9 +495,9 @@ func (jm *ControllerV2) syncCronJob(
}

logger := klog.FromContext(ctx)
if timeZoneEnabled && cronJob.Spec.TimeZone != nil {
if _, err := time.LoadLocation(*cronJob.Spec.TimeZone); err != nil {
timeZone := pointer.StringDeref(cronJob.Spec.TimeZone, "")
if cronJob.Spec.TimeZone != nil {
timeZone := pointer.StringDeref(cronJob.Spec.TimeZone, "")
if _, err := time.LoadLocation(timeZone); err != nil {
logger.V(4).Info("Not starting job because timeZone is invalid", "cronjob", klog.KObj(cronJob), "timeZone", timeZone, "err", err)
jm.recorder.Eventf(cronJob, corev1.EventTypeWarning, "UnknownTimeZone", "invalid timeZone: %q: %s", timeZone, err)
return cronJob, nil, updateStatus, nil
Expand All @@ -513,7 +509,7 @@ func (jm *ControllerV2) syncCronJob(
return cronJob, nil, updateStatus, nil
}

sched, err := cron.ParseStandard(formatSchedule(timeZoneEnabled, cronJob, jm.recorder))
sched, err := cron.ParseStandard(formatSchedule(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
Expand Down Expand Up @@ -733,7 +729,7 @@ 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 {
func formatSchedule(cj *batchv1.CronJob, recorder record.EventRecorder) string {
if strings.Contains(cj.Spec.Schedule, "TZ") {
if recorder != nil {
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)
Expand All @@ -742,7 +738,7 @@ func formatSchedule(timeZoneEnabled bool, cj *batchv1.CronJob, recorder record.E
return cj.Spec.Schedule
}

if timeZoneEnabled && cj.Spec.TimeZone != nil {
if cj.Spec.TimeZone != nil {
if _, err := time.LoadLocation(*cj.Spec.TimeZone); err != nil {
return cj.Spec.Schedule
}
Expand Down
11 changes: 0 additions & 11 deletions pkg/controller/cronjob/cronjob_controllerv2_test.go
View file
Open in desktop
Expand Up @@ -30,17 +30,14 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"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/klog/v2/ktesting"
_ "k8s.io/kubernetes/pkg/apis/batch/install"
_ "k8s.io/kubernetes/pkg/apis/core/install"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/features"
)

var (
Expand Down Expand Up @@ -203,7 +200,6 @@ func TestControllerV2SyncCronJob(t *testing.T) {
now time.Time
jobCreateError error
jobGetErr error
enableTimeZone bool

// expectations
expectCreate bool
Expand Down Expand Up @@ -251,7 +247,6 @@ func TestControllerV2SyncCronJob(t *testing.T) {
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(),
enableTimeZone: true,
expectedWarnings: 1,
jobPresentInCJActiveStatus: true,
},
Expand Down Expand Up @@ -291,7 +286,6 @@ func TestControllerV2SyncCronJob(t *testing.T) {
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justBeforeTheHour(),
enableTimeZone: true,
expectRequeueAfter: true,
expectedRequeueDuration: 1*time.Minute + nextScheduleDelta,
jobPresentInCJActiveStatus: true,
Expand Down Expand Up @@ -342,7 +336,6 @@ func TestControllerV2SyncCronJob(t *testing.T) {
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justAfterTheHourInZone(newYork),
enableTimeZone: false,
expectCreate: true,
expectActive: 1,
expectRequeueAfter: true,
Expand All @@ -357,7 +350,6 @@ func TestControllerV2SyncCronJob(t *testing.T) {
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justAfterTheHourInZone(newYork),
enableTimeZone: true,
expectCreate: true,
expectActive: 1,
expectRequeueAfter: true,
Expand All @@ -372,7 +364,6 @@ func TestControllerV2SyncCronJob(t *testing.T) {
deadline: noDead,
jobCreationTime: justAfterThePriorHour(),
now: justAfterTheHourInZone(newYork),
enableTimeZone: true,
expectCreate: true,
expectedWarnings: 1,
expectRequeueAfter: true,
Expand Down Expand Up @@ -1172,8 +1163,6 @@ func TestControllerV2SyncCronJob(t *testing.T) {
tc := tc

t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.CronJobTimeZone, tc.enableTimeZone)()

cj := cronJob()
cj.Spec.ConcurrencyPolicy = tc.concurrencyPolicy
cj.Spec.Suspend = &tc.suspend
Expand Down
3 changes: 2 additions & 1 deletion pkg/features/kube_features.go
View file
Open in desktop
Expand Up @@ -188,6 +188,7 @@ const (
// kep: https://kep.k8s.io/3140
// alpha: v1.24
// beta: v1.25
// GA: 1.27
//
// Enables support for time zones in CronJobs.
CronJobTimeZone featuregate.Feature = "CronJobTimeZone"
Expand Down Expand Up @@ -900,7 +901,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS

ConsistentHTTPGetHandlers: {Default: true, PreRelease: featuregate.GA},

CronJobTimeZone: {Default: true, PreRelease: featuregate.Beta},
CronJobTimeZone: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29

DelegateFSGroupToCSIDriver: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28

Expand Down
4 changes: 2 additions & 2 deletions pkg/generated/openapi/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 0 additions & 10 deletions pkg/registry/batch/cronjob/strategy.go
View file
Open in desktop
Expand Up @@ -30,12 +30,10 @@ import (
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/apis/batch"
batchvalidation "k8s.io/kubernetes/pkg/apis/batch/validation"
"k8s.io/kubernetes/pkg/features"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
)

Expand Down Expand Up @@ -91,10 +89,6 @@ func (cronJobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object)

cronJob.Generation = 1

if !utilfeature.DefaultFeatureGate.Enabled(features.CronJobTimeZone) {
cronJob.Spec.TimeZone = nil
}

pod.DropDisabledTemplateFields(&cronJob.Spec.JobTemplate.Spec.Template, nil)
}

Expand All @@ -104,10 +98,6 @@ func (cronJobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob
oldCronJob := old.(*batch.CronJob)
newCronJob.Status = oldCronJob.Status

if !utilfeature.DefaultFeatureGate.Enabled(features.CronJobTimeZone) && oldCronJob.Spec.TimeZone == nil {
newCronJob.Spec.TimeZone = nil
}

pod.DropDisabledTemplateFields(&newCronJob.Spec.JobTemplate.Spec.Template, &oldCronJob.Spec.JobTemplate.Spec.Template)

// Any changes to the spec increment the generation number.
Expand Down
1 change: 0 additions & 1 deletion staging/src/k8s.io/api/batch/v1/generated.proto
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion staging/src/k8s.io/api/batch/v1/types.go
View file
Open in desktop
Expand Up @@ -517,7 +517,6 @@ type CronJobSpec struct {
// configuration, the controller will stop creating new new Jobs and will create a system event with the
// reason UnknownTimeZone.
// More information can be found in https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#time-zones
// This is beta field and must be enabled via the `CronJobTimeZone` feature gate.
// +optional
TimeZone *string `json:"timeZone,omitempty" protobuf:"bytes,8,opt,name=timeZone"`

Expand Down