From 2fe38f93e53201b0c9e58aa6e3d37b8a61d2ca23 Mon Sep 17 00:00:00 2001 From: Heba Elayoty <31887807+helayoty@users.noreply.github.com> Date: Thu, 6 Jul 2023 14:39:04 -0700 Subject: [PATCH] feat: Append job creation timestamp to cronjob annotations (#118137) * Append job name to job annotations Signed-off-by: Heba Elayoty * Update annotation description, remove timezone, and fix time Signed-off-by: Heba Elayoty * Remove unused ctx Signed-off-by: Heba Elayoty * code review comments Signed-off-by: Heba Elayoty * code review comments Signed-off-by: Heba Elayoty * Add timezone back Signed-off-by: Heba Elayoty --------- Signed-off-by: Heba Elayoty --- pkg/controller/cronjob/utils.go | 13 ++++ pkg/controller/cronjob/utils_test.go | 95 ++++++++++++++++++++---- pkg/features/kube_features.go | 7 ++ staging/src/k8s.io/api/batch/v1/types.go | 5 ++ 4 files changed, 104 insertions(+), 16 deletions(-) diff --git a/pkg/controller/cronjob/utils.go b/pkg/controller/cronjob/utils.go index 7325b327be7..1ca6dd819a2 100644 --- a/pkg/controller/cronjob/utils.go +++ b/pkg/controller/cronjob/utils.go @@ -21,14 +21,17 @@ import ( "time" "github.com/robfig/cron/v3" + "k8s.io/utils/pointer" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/features" ) // Utilities for dealing with Jobs and CronJobs and time. @@ -213,6 +216,16 @@ func getJobFromTemplate2(cj *batchv1.CronJob, scheduledTime time.Time) (*batchv1 // We want job names for a given nominal start time to have a deterministic name to avoid the same job being created twice name := getJobName(cj, scheduledTime) + if utilfeature.DefaultFeatureGate.Enabled(features.CronJobsScheduledAnnotation) { + + timeZoneLocation, err := time.LoadLocation(pointer.StringDeref(cj.Spec.TimeZone, "")) + if err != nil { + return nil, err + } + // Append job creation timestamp to the cronJob annotations. The time will be in RFC3339 form. + annotations[batchv1.CronJobScheduledTimestampAnnotation] = scheduledTime.In(timeZoneLocation).Format(time.RFC3339) + } + job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, diff --git a/pkg/controller/cronjob/utils_test.go b/pkg/controller/cronjob/utils_test.go index f04b37a4558..e0012e986d5 100644 --- a/pkg/controller/cronjob/utils_test.go +++ b/pkg/controller/cronjob/utils_test.go @@ -28,16 +28,26 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/record" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/klog/v2/ktesting" + "k8s.io/kubernetes/pkg/features" + "k8s.io/utils/pointer" ) func TestGetJobFromTemplate2(t *testing.T) { // getJobFromTemplate2() needs to take the job template and copy the labels and annotations // and other fields, and add a created-by reference. + var ( + one int64 = 1 + no bool + timeZoneUTC = "UTC" + timeZoneCorrect = "Europe/Rome" + scheduledTime = *topOfTheHour() + ) - var one int64 = 1 - var no bool + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CronJobsScheduledAnnotation, true)() cj := batchv1.CronJob{ ObjectMeta: metav1.ObjectMeta{ @@ -50,8 +60,8 @@ func TestGetJobFromTemplate2(t *testing.T) { ConcurrencyPolicy: batchv1.AllowConcurrent, JobTemplate: batchv1.JobTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"a": "b"}, - Annotations: map[string]string{"x": "y"}, + CreationTimestamp: metav1.Time{Time: scheduledTime}, + Labels: map[string]string{"a": "b"}, }, Spec: batchv1.JobSpec{ ActiveDeadlineSeconds: &one, @@ -73,19 +83,72 @@ func TestGetJobFromTemplate2(t *testing.T) { }, } - var job *batchv1.Job - job, err := getJobFromTemplate2(&cj, time.Time{}) - if err != nil { - t.Errorf("Did not expect error: %s", err) + testCases := []struct { + name string + timeZone *string + inputAnnotations map[string]string + expectedScheduledTime func() time.Time + expectedNumberOfAnnotations int + }{ + { + name: "UTC timezone and one annotation", + timeZone: &timeZoneUTC, + inputAnnotations: map[string]string{"x": "y"}, + expectedScheduledTime: func() time.Time { + return scheduledTime + }, + expectedNumberOfAnnotations: 2, + }, + { + name: "nil timezone and one annotation", + timeZone: nil, + inputAnnotations: map[string]string{"x": "y"}, + expectedScheduledTime: func() time.Time { + return scheduledTime + }, + expectedNumberOfAnnotations: 2, + }, + { + name: "correct timezone and multiple annotation", + timeZone: &timeZoneCorrect, + inputAnnotations: map[string]string{"x": "y", "z": "x"}, + expectedScheduledTime: func() time.Time { + location, _ := time.LoadLocation(timeZoneCorrect) + return scheduledTime.In(location) + }, + expectedNumberOfAnnotations: 3, + }, } - if !strings.HasPrefix(job.ObjectMeta.Name, "mycronjob-") { - t.Errorf("Wrong Name") - } - if len(job.ObjectMeta.Labels) != 1 { - t.Errorf("Wrong number of labels") - } - if len(job.ObjectMeta.Annotations) != 1 { - t.Errorf("Wrong number of annotations") + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + cj.Spec.JobTemplate.Annotations = tt.inputAnnotations + cj.Spec.TimeZone = tt.timeZone + + var job *batchv1.Job + job, err := getJobFromTemplate2(&cj, scheduledTime) + if err != nil { + t.Errorf("Did not expect error: %s", err) + } + if !strings.HasPrefix(job.ObjectMeta.Name, "mycronjob-") { + t.Errorf("Wrong Name") + } + if len(job.ObjectMeta.Labels) != 1 { + t.Errorf("Wrong number of labels") + } + if len(job.ObjectMeta.Annotations) != tt.expectedNumberOfAnnotations { + t.Errorf("Wrong number of annotations") + } + + scheduledAnnotation := job.ObjectMeta.Annotations[batchv1.CronJobScheduledTimestampAnnotation] + timeZoneLocation, err := time.LoadLocation(pointer.StringDeref(tt.timeZone, "")) + if err != nil { + t.Errorf("Wrong timezone location") + } + if len(job.ObjectMeta.Annotations) != 0 && scheduledAnnotation != tt.expectedScheduledTime().Format(time.RFC3339) { + t.Errorf("Wrong cronJob scheduled timestamp annotation, expexted %s, got %s.", tt.expectedScheduledTime().In(timeZoneLocation).Format(time.RFC3339), scheduledAnnotation) + } + }) } } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 4b250991ea7..deca96c84c4 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -187,6 +187,11 @@ const ( // Normalize HttpGet URL and Header passing for lifecycle handlers with probers. ConsistentHTTPGetHandlers featuregate.Feature = "ConsistentHTTPGetHandlers" + // owner: @helayoty + // beta: v1.28 + // Set the scheduled time as an annotation in the job. + CronJobsScheduledAnnotation featuregate.Feature = "CronJobsScheduledAnnotation" + // owner: @deejross, @soltysh // kep: https://kep.k8s.io/3140 // alpha: v1.24 @@ -892,6 +897,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS ConsistentHTTPGetHandlers: {Default: true, PreRelease: featuregate.GA}, + CronJobsScheduledAnnotation: {Default: true, PreRelease: featuregate.Beta}, + CronJobTimeZone: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29 DefaultHostNetworkHostPortsInPodTemplates: {Default: false, PreRelease: featuregate.Deprecated}, diff --git a/staging/src/k8s.io/api/batch/v1/types.go b/staging/src/k8s.io/api/batch/v1/types.go index 22cf9ee9cb6..6e616390627 100644 --- a/staging/src/k8s.io/api/batch/v1/types.go +++ b/staging/src/k8s.io/api/batch/v1/types.go @@ -27,6 +27,11 @@ const ( // More info: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#label-selector-and-annotation-conventions labelPrefix = "batch.kubernetes.io/" + // CronJobScheduledTimestampAnnotation is the scheduled timestamp annotation for the Job. + // It records the original/expected scheduled timestamp for the running job, represented in RFC3339. + // The CronJob controller adds this annotation if the CronJobsScheduledAnnotation feature gate (beta in 1.28) is enabled. + CronJobScheduledTimestampAnnotation = labelPrefix + "cronjob-scheduled-timestamp" + JobCompletionIndexAnnotation = labelPrefix + "job-completion-index" // JobTrackingFinalizer is a finalizer for Job's pods. It prevents them from // being deleted before being accounted in the Job status.