diff --git a/pkg/apis/batch/validation/BUILD b/pkg/apis/batch/validation/BUILD index f9a72f4ed17..0d8df02ba1a 100644 --- a/pkg/apis/batch/validation/BUILD +++ b/pkg/apis/batch/validation/BUILD @@ -14,13 +14,11 @@ go_library( "//pkg/apis/batch:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/core/validation:go_default_library", - "//pkg/features:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/github.com/robfig/cron:go_default_library", ], ) diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index 1e04e9779e5..1907ac9079e 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/pkg/apis/batch/validation/validation.go @@ -24,11 +24,9 @@ import ( "k8s.io/apimachinery/pkg/labels" apimachineryvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/apis/batch" api "k8s.io/kubernetes/pkg/apis/core" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" - "k8s.io/kubernetes/pkg/features" ) // TODO: generalize for other controller objects that will follow the same pattern, such as ReplicaSet and DaemonSet, and @@ -119,13 +117,8 @@ func validateJobSpec(spec *batch.JobSpec, fldPath *field.Path) field.ErrorList { if spec.BackoffLimit != nil { allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.BackoffLimit), fldPath.Child("backoffLimit"))...) } - if utilfeature.DefaultFeatureGate.Enabled(features.TTLAfterFinished) { - // normal validation for TTLSecondsAfterFinished - if spec.TTLSecondsAfterFinished != nil { - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.TTLSecondsAfterFinished), fldPath.Child("ttlSecondsAfterFinished"))...) - } - } else if spec.TTLSecondsAfterFinished != nil { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("ttlSecondsAfterFinished"), "disabled by feature-gate")) + if spec.TTLSecondsAfterFinished != nil { + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.TTLSecondsAfterFinished), fldPath.Child("ttlSecondsAfterFinished"))...) } allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(&spec.Template, fldPath.Child("template"))...) diff --git a/pkg/registry/batch/job/BUILD b/pkg/registry/batch/job/BUILD index 98cfab64cc1..d61f5bae717 100644 --- a/pkg/registry/batch/job/BUILD +++ b/pkg/registry/batch/job/BUILD @@ -50,6 +50,7 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library", ], ) diff --git a/pkg/registry/batch/job/strategy.go b/pkg/registry/batch/job/strategy.go index 4f5845d62db..5ce5e51178b 100644 --- a/pkg/registry/batch/job/strategy.go +++ b/pkg/registry/batch/job/strategy.go @@ -89,9 +89,8 @@ func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object oldJob := old.(*batch.Job) newJob.Status = oldJob.Status - if !utilfeature.DefaultFeatureGate.Enabled(features.TTLAfterFinished) { + if !utilfeature.DefaultFeatureGate.Enabled(features.TTLAfterFinished) && oldJob.Spec.TTLSecondsAfterFinished == nil { newJob.Spec.TTLSecondsAfterFinished = nil - oldJob.Spec.TTLSecondsAfterFinished = nil } pod.DropDisabledFields(&newJob.Spec.Template.Spec, &oldJob.Spec.Template.Spec) diff --git a/pkg/registry/batch/job/strategy_test.go b/pkg/registry/batch/job/strategy_test.go index a60af6ab201..ceac03fb675 100644 --- a/pkg/registry/batch/job/strategy_test.go +++ b/pkg/registry/batch/job/strategy_test.go @@ -25,6 +25,7 @@ import ( genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" "k8s.io/kubernetes/pkg/api/testapi" apitesting "k8s.io/kubernetes/pkg/api/testing" "k8s.io/kubernetes/pkg/apis/batch" @@ -121,6 +122,24 @@ func TestJobStrategy(t *testing.T) { t.Errorf("Job should only allow updating .spec.ttlSecondsAfterFinished when %v feature is enabled", features.TTLAfterFinished) } + // set TTLSecondsAfterFinished on both old and new jobs + job.Spec.TTLSecondsAfterFinished = newInt32(1) + updatedJob.Spec.TTLSecondsAfterFinished = newInt32(2) + + // Existing TTLSecondsAfterFinished should be preserved when feature is on + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TTLAfterFinished, true)() + Strategy.PrepareForUpdate(ctx, updatedJob, job) + if job.Spec.TTLSecondsAfterFinished == nil || updatedJob.Spec.TTLSecondsAfterFinished == nil { + t.Errorf("existing TTLSecondsAfterFinished should be preserved") + } + + // Existing TTLSecondsAfterFinished should be preserved when feature is off + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TTLAfterFinished, false)() + Strategy.PrepareForUpdate(ctx, updatedJob, job) + if job.Spec.TTLSecondsAfterFinished == nil || updatedJob.Spec.TTLSecondsAfterFinished == nil { + t.Errorf("existing TTLSecondsAfterFinished should be preserved") + } + // Make sure we correctly implement the interface. // Otherwise a typo could silently change the default. var gcds rest.GarbageCollectionDeleteStrategy = Strategy