feat: Append job creation timestamp to cronjob annotations (#118137)

* Append job name to job annotations Signed-off-by: Heba Elayoty <hebaelayoty@gmail.com> * Update annotation description, remove timezone, and fix time Signed-off-by: Heba Elayoty <hebaelayoty@gmail.com> * Remove unused ctx Signed-off-by: Heba Elayoty <hebaelayoty@gmail.com> * code review comments Signed-off-by: Heba Elayoty <hebaelayoty@gmail.com> * code review comments Signed-off-by: Heba Elayoty <hebaelayoty@gmail.com> * Add timezone back Signed-off-by: Heba Elayoty <hebaelayoty@gmail.com> --------- Signed-off-by: Heba Elayoty <hebaelayoty@gmail.com>
2023-07-06 14:39:04 -07:00
parent aeed7da616
commit 2fe38f93e5
4 changed files with 104 additions and 16 deletions
--- 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,
--- 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)
+			}
+		})
 	}
 }

--- 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},
--- 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.