remove tracking annotation from validation and webhooks

This commit is contained in:
kannon92 2022-12-21 19:26:43 +00:00
parent afeb78fd8f
commit 6a4cf352b8
4 changed files with 8 additions and 104 deletions

View File

@ -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 := apivalidation.ValidateObjectMeta(&job.ObjectMeta, true, apivalidation.ValidateReplicationControllerName, field.NewPath("metadata"))
allErrs = append(allErrs, validateGeneratedSelector(job, opts.RequirePrefixedLabels)...) allErrs = append(allErrs, validateGeneratedSelector(job, opts.RequirePrefixedLabels)...)
allErrs = append(allErrs, ValidateJobSpec(&job.Spec, field.NewPath("spec"), opts.PodValidationOptions)...) 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 { 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`) // For indexed job, the job controller appends a suffix (`-$INDEX`)
// to the pod hostname when indexed job create pods. // to the pod hostname when indexed job create pods.
@ -146,14 +143,6 @@ func ValidateJob(job *batch.Job, opts JobValidationOptions) field.ErrorList {
return allErrs 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. // ValidateJobSpec validates a JobSpec and returns an ErrorList with any errors.
func ValidateJobSpec(spec *batch.JobSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList { func ValidateJobSpec(spec *batch.JobSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
allErrs := validateJobSpec(spec, fldPath, opts) allErrs := validateJobSpec(spec, fldPath, opts)
@ -598,8 +587,6 @@ func validateCompletions(spec, oldSpec batch.JobSpec, fldPath *field.Path, opts
type JobValidationOptions struct { type JobValidationOptions struct {
apivalidation.PodValidationOptions 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 // Allow mutable node affinity, selector and tolerations of the template
AllowMutableSchedulingDirectives bool AllowMutableSchedulingDirectives bool
// Allow elastic indexed jobs // Allow elastic indexed jobs

View File

@ -223,7 +223,6 @@ func TestValidateJob(t *testing.T) {
}, },
"valid job tracking annotation": { "valid job tracking annotation": {
opts: JobValidationOptions{ opts: JobValidationOptions{
AllowTrackingAnnotation: true,
RequirePrefixedLabels: true, RequirePrefixedLabels: true,
}, },
job: batch.Job{ job: batch.Job{
@ -231,9 +230,6 @@ func TestValidateJob(t *testing.T) {
Name: "myjob", Name: "myjob",
Namespace: metav1.NamespaceDefault, Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"), UID: types.UID("1a2b3c"),
Annotations: map[string]string{
batch.JobTrackingFinalizer: "",
},
}, },
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: validGeneratedSelector, Selector: validGeneratedSelector,
@ -243,7 +239,6 @@ func TestValidateJob(t *testing.T) {
}, },
"valid batch labels": { "valid batch labels": {
opts: JobValidationOptions{ opts: JobValidationOptions{
AllowTrackingAnnotation: true,
RequirePrefixedLabels: true, RequirePrefixedLabels: true,
}, },
job: batch.Job{ job: batch.Job{
@ -251,9 +246,6 @@ func TestValidateJob(t *testing.T) {
Name: "myjob", Name: "myjob",
Namespace: metav1.NamespaceDefault, Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"), UID: types.UID("1a2b3c"),
Annotations: map[string]string{
batch.JobTrackingFinalizer: "",
},
}, },
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: validGeneratedSelector, Selector: validGeneratedSelector,
@ -263,7 +255,6 @@ func TestValidateJob(t *testing.T) {
}, },
"do not allow new batch labels": { "do not allow new batch labels": {
opts: JobValidationOptions{ opts: JobValidationOptions{
AllowTrackingAnnotation: true,
RequirePrefixedLabels: false, RequirePrefixedLabels: false,
}, },
job: batch.Job{ job: batch.Job{
@ -271,9 +262,6 @@ func TestValidateJob(t *testing.T) {
Name: "myjob", Name: "myjob",
Namespace: metav1.NamespaceDefault, Namespace: metav1.NamespaceDefault,
UID: types.UID("1a2b3c"), UID: types.UID("1a2b3c"),
Annotations: map[string]string{
batch.JobTrackingFinalizer: "",
},
}, },
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: &metav1.LabelSelector{ Selector: &metav1.LabelSelector{
@ -979,23 +967,6 @@ func TestValidateJob(t *testing.T) {
}, },
opts: JobValidationOptions{RequirePrefixedLabels: true}, 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'": { "spec.template.metadata.labels[controller-uid]: Required value: must be '1a2b3c'": {
job: batch.Job{ job: batch.Job{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{

View File

@ -95,10 +95,6 @@ func (jobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
job.Generation = 1 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) { if !utilfeature.DefaultFeatureGate.Enabled(features.JobPodFailurePolicy) {
job.Spec.PodFailurePolicy = nil job.Spec.PodFailurePolicy = nil
} }
@ -106,21 +102,6 @@ func (jobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
pod.DropDisabledTemplateFields(&job.Spec.Template, nil) 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. // 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) { func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
newJob := obj.(*batch.Job) newJob := obj.(*batch.Job)
@ -139,12 +120,6 @@ func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object
newJob.Generation = oldJob.Generation + 1 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. // Validate validates a new job.
@ -168,18 +143,12 @@ func validationOptionsForJob(newJob, oldJob *batch.Job) batchvalidation.JobValid
} }
opts := batchvalidation.JobValidationOptions{ opts := batchvalidation.JobValidationOptions{
PodValidationOptions: pod.GetValidationOptionsFromPodTemplate(newPodTemplate, oldPodTemplate), PodValidationOptions: pod.GetValidationOptionsFromPodTemplate(newPodTemplate, oldPodTemplate),
AllowTrackingAnnotation: true,
AllowElasticIndexedJobs: utilfeature.DefaultFeatureGate.Enabled(features.ElasticIndexedJob), AllowElasticIndexedJobs: utilfeature.DefaultFeatureGate.Enabled(features.ElasticIndexedJob),
RequirePrefixedLabels: true, RequirePrefixedLabels: true,
} }
if oldJob != nil { if oldJob != nil {
opts.AllowInvalidLabelValueInSelector = opts.AllowInvalidLabelValueInSelector || metav1validation.LabelSelectorHasInvalidLabelValue(oldJob.Spec.Selector) 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 // Updating node affinity, node selector and tolerations is allowed
// only for suspended jobs that never started before. // only for suspended jobs that never started before.
suspended := oldJob.Spec.Suspend != nil && *oldJob.Spec.Suspend suspended := oldJob.Spec.Suspend != nil && *oldJob.Spec.Suspend

View File

@ -21,7 +21,6 @@ import (
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts" "github.com/google/go-cmp/cmp/cmpopts"
batchv1 "k8s.io/api/batch/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
@ -186,7 +185,7 @@ func TestJobStrategy_PrepareForUpdate(t *testing.T) {
}, },
"add tracking annotation back": { "add tracking annotation back": {
job: batch.Job{ job: batch.Job{
ObjectMeta: getValidObjectMetaWithAnnotations(0, map[string]string{batchv1.JobTrackingFinalizer: ""}), ObjectMeta: getValidObjectMeta(0),
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: validSelector, Selector: validSelector,
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
@ -201,7 +200,7 @@ func TestJobStrategy_PrepareForUpdate(t *testing.T) {
}, },
}, },
wantJob: batch.Job{ wantJob: batch.Job{
ObjectMeta: getValidObjectMetaWithAnnotations(1, map[string]string{batchv1.JobTrackingFinalizer: ""}), ObjectMeta: getValidObjectMeta(1),
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: validSelector, Selector: validSelector,
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
@ -373,7 +372,7 @@ func TestJobStrategy_PrepareForCreate(t *testing.T) {
}, },
}, },
wantJob: batch.Job{ wantJob: batch.Job{
ObjectMeta: getValidObjectMetaWithAnnotations(1, map[string]string{batchv1.JobTrackingFinalizer: ""}), ObjectMeta: getValidObjectMeta(1),
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: validSelector, Selector: validSelector,
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
@ -392,7 +391,7 @@ func TestJobStrategy_PrepareForCreate(t *testing.T) {
}, },
}, },
wantJob: batch.Job{ wantJob: batch.Job{
ObjectMeta: getValidObjectMetaWithAnnotations(1, map[string]string{batchv1.JobTrackingFinalizer: ""}), ObjectMeta: getValidObjectMeta(1),
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: validSelector, Selector: validSelector,
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
@ -412,7 +411,7 @@ func TestJobStrategy_PrepareForCreate(t *testing.T) {
}, },
}, },
wantJob: batch.Job{ wantJob: batch.Job{
ObjectMeta: getValidObjectMetaWithAnnotations(1, map[string]string{batchv1.JobTrackingFinalizer: ""}), ObjectMeta: getValidObjectMeta(1),
Spec: batch.JobSpec{ Spec: batch.JobSpec{
Selector: validSelector, Selector: validSelector,
Template: validPodTemplateSpec, Template: validPodTemplateSpec,
@ -516,28 +515,6 @@ func TestJobStrategy_ValidateUpdate(t *testing.T) {
{Type: field.ErrorTypeInvalid, Field: "spec.completions"}, {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": { "preserving tracking annotation": {
job: &batch.Job{ job: &batch.Job{
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{