diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index e8815c3842d..66118aa7b3c 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/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 diff --git a/pkg/apis/batch/validation/validation_test.go b/pkg/apis/batch/validation/validation_test.go index af5828d284e..9ed87d6d201 100644 --- a/pkg/apis/batch/validation/validation_test.go +++ b/pkg/apis/batch/validation/validation_test.go @@ -223,17 +223,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, @@ -243,17 +239,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, @@ -263,17 +255,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{ @@ -979,23 +967,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{ diff --git a/pkg/registry/batch/job/strategy.go b/pkg/registry/batch/job/strategy.go index 6d12407d51c..6e7056bc481 100644 --- a/pkg/registry/batch/job/strategy.go +++ b/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 diff --git a/pkg/registry/batch/job/strategy_test.go b/pkg/registry/batch/job/strategy_test.go index 1278ecd6d08..bb794122086 100644 --- a/pkg/registry/batch/job/strategy_test.go +++ b/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{