Merge pull request #117633 from kannon92/remove-job-tracking-finalizers

remove tracking annotation from validation and webhooks

pkg/apis/batch/validation/validation.go

@@ -130,9 +130,6 @@ func ValidateJob(job *batch.Job, opts JobValidationOptions) field.ErrorList {
 	allErrs := apivalidation.ValidateObjectMeta(&job.ObjectMeta, true, apivalidation.ValidateReplicationControllerName, field.NewPath("metadata"))
 	allErrs = append(allErrs, validateGeneratedSelector(job, opts.RequirePrefixedLabels)...)
 	allErrs = append(allErrs, ValidateJobSpec(&job.Spec, field.NewPath("spec"), opts.PodValidationOptions)...)
-	if !opts.AllowTrackingAnnotation && hasJobTrackingAnnotation(job) {
-		allErrs = append(allErrs, field.Forbidden(field.NewPath("metadata").Child("annotations").Key(batch.JobTrackingFinalizer), "cannot add this annotation"))
-	}
 	if job.Spec.CompletionMode != nil && *job.Spec.CompletionMode == batch.IndexedCompletion && job.Spec.Completions != nil && *job.Spec.Completions > 0 {
 		// For indexed job, the job controller appends a suffix (`-$INDEX`)
 		// to the pod hostname when indexed job create pods.
@@ -146,14 +143,6 @@ func ValidateJob(job *batch.Job, opts JobValidationOptions) field.ErrorList {
 	return allErrs
 }
 
-func hasJobTrackingAnnotation(job *batch.Job) bool {
-	if job.Annotations == nil {
-		return false
-	}
-	_, ok := job.Annotations[batch.JobTrackingFinalizer]
-	return ok
-}
-
 // ValidateJobSpec validates a JobSpec and returns an ErrorList with any errors.
 func ValidateJobSpec(spec *batch.JobSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
 	allErrs := validateJobSpec(spec, fldPath, opts)
@@ -598,8 +587,6 @@ func validateCompletions(spec, oldSpec batch.JobSpec, fldPath *field.Path, opts
 
 type JobValidationOptions struct {
 	apivalidation.PodValidationOptions
-	// Allow Job to have the annotation batch.kubernetes.io/job-tracking
-	AllowTrackingAnnotation bool
 	// Allow mutable node affinity, selector and tolerations of the template
 	AllowMutableSchedulingDirectives bool
 	// Allow elastic indexed jobs

pkg/apis/batch/validation/validation_test.go

@@ -213,17 +213,13 @@ func TestValidateJob(t *testing.T) {
 		},
 		"valid job tracking annotation": {
 			opts: JobValidationOptions{
-				AllowTrackingAnnotation: true,
+				RequirePrefixedLabels: true,
-				RequirePrefixedLabels:   true,
 			},
 			job: batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "myjob",
 					Namespace: metav1.NamespaceDefault,
 					UID:       types.UID("1a2b3c"),
-					Annotations: map[string]string{
-						batch.JobTrackingFinalizer: "",
-					},
 				},
 				Spec: batch.JobSpec{
 					Selector: validGeneratedSelector,
@@ -233,17 +229,13 @@ func TestValidateJob(t *testing.T) {
 		},
 		"valid batch labels": {
 			opts: JobValidationOptions{
-				AllowTrackingAnnotation: true,
+				RequirePrefixedLabels: true,
-				RequirePrefixedLabels:   true,
 			},
 			job: batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "myjob",
 					Namespace: metav1.NamespaceDefault,
 					UID:       types.UID("1a2b3c"),
-					Annotations: map[string]string{
-						batch.JobTrackingFinalizer: "",
-					},
 				},
 				Spec: batch.JobSpec{
 					Selector: validGeneratedSelector,
@@ -253,17 +245,13 @@ func TestValidateJob(t *testing.T) {
 		},
 		"do not allow new batch labels": {
 			opts: JobValidationOptions{
-				AllowTrackingAnnotation: true,
+				RequirePrefixedLabels: false,
-				RequirePrefixedLabels:   false,
 			},
 			job: batch.Job{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "myjob",
 					Namespace: metav1.NamespaceDefault,
 					UID:       types.UID("1a2b3c"),
-					Annotations: map[string]string{
-						batch.JobTrackingFinalizer: "",
-					},
 				},
 				Spec: batch.JobSpec{
 					Selector: &metav1.LabelSelector{
@@ -922,23 +910,6 @@ func TestValidateJob(t *testing.T) {
 			},
 			opts: JobValidationOptions{RequirePrefixedLabels: true},
 		},
-		"metadata.annotations[batch.kubernetes.io/job-tracking]: cannot add this annotation": {
-			job: batch.Job{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "myjob",
-					Namespace: metav1.NamespaceDefault,
-					UID:       types.UID("1a2b3c"),
-					Annotations: map[string]string{
-						batch.JobTrackingFinalizer: "",
-					},
-				},
-				Spec: batch.JobSpec{
-					Selector: validGeneratedSelector,
-					Template: validPodTemplateSpecForGenerated,
-				},
-			},
-			opts: JobValidationOptions{RequirePrefixedLabels: true},
-		},
 		"spec.template.metadata.labels[controller-uid]: Required value: must be '1a2b3c'": {
 			job: batch.Job{
 				ObjectMeta: metav1.ObjectMeta{

pkg/registry/batch/job/strategy.go

@@ -95,10 +95,6 @@ func (jobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 
 	job.Generation = 1
 
-	// While legacy tracking is supported, we use an annotation to mark whether
-	// jobs are tracked with finalizers.
-	addJobTrackingAnnotation(job)
-
 	if !utilfeature.DefaultFeatureGate.Enabled(features.JobPodFailurePolicy) {
 		job.Spec.PodFailurePolicy = nil
 	}
@@ -106,21 +102,6 @@ func (jobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	pod.DropDisabledTemplateFields(&job.Spec.Template, nil)
 }
 
-func addJobTrackingAnnotation(job *batch.Job) {
-	if job.Annotations == nil {
-		job.Annotations = map[string]string{}
-	}
-	job.Annotations[batchv1.JobTrackingFinalizer] = ""
-}
-
-func hasJobTrackingAnnotation(job *batch.Job) bool {
-	if job.Annotations == nil {
-		return false
-	}
-	_, ok := job.Annotations[batchv1.JobTrackingFinalizer]
-	return ok
-}
-
 // PrepareForUpdate clears fields that are not allowed to be set by end users on update.
 func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	newJob := obj.(*batch.Job)
@@ -139,12 +120,6 @@ func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object
 		newJob.Generation = oldJob.Generation + 1
 	}
 
-	// While legacy tracking is supported, we use an annotation to mark whether
-	// jobs are tracked with finalizers. This annotation cannot be removed by
-	// users.
-	if hasJobTrackingAnnotation(oldJob) {
-		addJobTrackingAnnotation(newJob)
-	}
 }
 
 // Validate validates a new job.
@@ -168,18 +143,12 @@ func validationOptionsForJob(newJob, oldJob *batch.Job) batchvalidation.JobValid
 	}
 	opts := batchvalidation.JobValidationOptions{
 		PodValidationOptions:    pod.GetValidationOptionsFromPodTemplate(newPodTemplate, oldPodTemplate),
-		AllowTrackingAnnotation: true,
 		AllowElasticIndexedJobs: utilfeature.DefaultFeatureGate.Enabled(features.ElasticIndexedJob),
 		RequirePrefixedLabels:   true,
 	}
 	if oldJob != nil {
 		opts.AllowInvalidLabelValueInSelector = opts.AllowInvalidLabelValueInSelector || metav1validation.LabelSelectorHasInvalidLabelValue(oldJob.Spec.Selector)
 
-		// Because we don't support the tracking with finalizers for already
-		// existing jobs, we allow the annotation only if the Job already had it,
-		// regardless of the feature gate.
-		opts.AllowTrackingAnnotation = hasJobTrackingAnnotation(oldJob)
-
 		// Updating node affinity, node selector and tolerations is allowed
 		// only for suspended jobs that never started before.
 		suspended := oldJob.Spec.Suspend != nil && *oldJob.Spec.Suspend

pkg/registry/batch/job/strategy_test.go

@@ -21,7 +21,6 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
-	batchv1 "k8s.io/api/batch/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
@@ -186,7 +185,7 @@ func TestJobStrategy_PrepareForUpdate(t *testing.T) {
 		},
 		"add tracking annotation back": {
 			job: batch.Job{
-				ObjectMeta: getValidObjectMetaWithAnnotations(0, map[string]string{batchv1.JobTrackingFinalizer: ""}),
+				ObjectMeta: getValidObjectMeta(0),
 				Spec: batch.JobSpec{
 					Selector:         validSelector,
 					Template:         validPodTemplateSpec,
@@ -201,7 +200,7 @@ func TestJobStrategy_PrepareForUpdate(t *testing.T) {
 				},
 			},
 			wantJob: batch.Job{
-				ObjectMeta: getValidObjectMetaWithAnnotations(1, map[string]string{batchv1.JobTrackingFinalizer: ""}),
+				ObjectMeta: getValidObjectMeta(1),
 				Spec: batch.JobSpec{
 					Selector: validSelector,
 					Template: validPodTemplateSpec,
@@ -373,7 +372,7 @@ func TestJobStrategy_PrepareForCreate(t *testing.T) {
 				},
 			},
 			wantJob: batch.Job{
-				ObjectMeta: getValidObjectMetaWithAnnotations(1, map[string]string{batchv1.JobTrackingFinalizer: ""}),
+				ObjectMeta: getValidObjectMeta(1),
 				Spec: batch.JobSpec{
 					Selector:         validSelector,
 					Template:         validPodTemplateSpec,
@@ -392,7 +391,7 @@ func TestJobStrategy_PrepareForCreate(t *testing.T) {
 				},
 			},
 			wantJob: batch.Job{
-				ObjectMeta: getValidObjectMetaWithAnnotations(1, map[string]string{batchv1.JobTrackingFinalizer: ""}),
+				ObjectMeta: getValidObjectMeta(1),
 				Spec: batch.JobSpec{
 					Selector:         validSelector,
 					Template:         validPodTemplateSpec,
@@ -412,7 +411,7 @@ func TestJobStrategy_PrepareForCreate(t *testing.T) {
 				},
 			},
 			wantJob: batch.Job{
-				ObjectMeta: getValidObjectMetaWithAnnotations(1, map[string]string{batchv1.JobTrackingFinalizer: ""}),
+				ObjectMeta: getValidObjectMeta(1),
 				Spec: batch.JobSpec{
 					Selector: validSelector,
 					Template: validPodTemplateSpec,
@@ -516,28 +515,6 @@ func TestJobStrategy_ValidateUpdate(t *testing.T) {
 				{Type: field.ErrorTypeInvalid, Field: "spec.completions"},
 			},
 		},
-		"adding tracking annotation disallowed": {
-			job: &batch.Job{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:            "myjob",
-					Namespace:       metav1.NamespaceDefault,
-					ResourceVersion: "0",
-					Annotations:     map[string]string{"foo": "bar"},
-				},
-				Spec: batch.JobSpec{
-					Selector:       validSelector,
-					Template:       validPodTemplateSpec,
-					ManualSelector: pointer.BoolPtr(true),
-					Parallelism:    pointer.Int32Ptr(1),
-				},
-			},
-			update: func(job *batch.Job) {
-				job.Annotations[batch.JobTrackingFinalizer] = ""
-			},
-			wantErrs: field.ErrorList{
-				{Type: field.ErrorTypeForbidden, Field: "metadata.annotations[batch.kubernetes.io/job-tracking]"},
-			},
-		},
 		"preserving tracking annotation": {
 			job: &batch.Job{
 				ObjectMeta: metav1.ObjectMeta{