diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 750464faab..4e2e7bdd42 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -300,6 +300,10 @@ func dropDisabledFields( podSpec.PriorityClassName = "" } + if !utilfeature.DefaultFeatureGate.Enabled(features.PodReadinessGates) && !podReadinessGatesInUse(oldPodSpec) { + podSpec.ReadinessGates = nil + } + if !utilfeature.DefaultFeatureGate.Enabled(features.LocalStorageCapacityIsolation) && !emptyDirSizeLimitInUse(oldPodSpec) { for i := range podSpec.Volumes { if podSpec.Volumes[i].EmptyDir != nil { @@ -465,6 +469,17 @@ func podPriorityInUse(podSpec *api.PodSpec) bool { return false } +// podReadinessGatesInUse returns true if the pod spec is non-nil and has ReadinessGates +func podReadinessGatesInUse(podSpec *api.PodSpec) bool { + if podSpec == nil { + return false + } + if podSpec.ReadinessGates != nil { + return true + } + return false +} + // emptyDirSizeLimitInUse returns true if any pod's EptyDir volumes use SizeLimit. func emptyDirSizeLimitInUse(podSpec *api.PodSpec) bool { if podSpec == nil { diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 52d8805e23..c4240497ac 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -996,6 +996,101 @@ func TestDropAppArmor(t *testing.T) { } } +func TestDropReadinessGates(t *testing.T) { + podWithoutReadinessGates := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + ReadinessGates: nil, + }, + } + } + podWithReadinessGates := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + ReadinessGates: []api.PodReadinessGate{ + { + ConditionType: api.PodConditionType("example.com/condition1"), + }, + { + ConditionType: api.PodConditionType("example.com/condition2"), + }, + }, + }, + } + } + + podInfo := []struct { + description string + hasPodReadinessGates bool + pod func() *api.Pod + }{ + { + description: "has ReadinessGates", + hasPodReadinessGates: true, + pod: podWithReadinessGates, + }, + { + description: "does not have ReadinessGates", + hasPodReadinessGates: false, + pod: podWithoutReadinessGates, + }, + { + description: "is nil", + hasPodReadinessGates: false, + pod: func() *api.Pod { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldPodInfo := range podInfo { + for _, newPodInfo := range podInfo { + oldPodHasReadinessGates, oldPod := oldPodInfo.hasPodReadinessGates, oldPodInfo.pod() + newPodHasReadinessGates, newPod := newPodInfo.hasPodReadinessGates, newPodInfo.pod() + if newPod == nil { + continue + } + + t.Run(fmt.Sprintf("featue enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodReadinessGates, enabled)() + + var oldPodSpec *api.PodSpec + if oldPod != nil { + oldPodSpec = &oldPod.Spec + } + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) + + // old pod should never be changed + if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { + t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod())) + } + + switch { + case enabled || oldPodHasReadinessGates: + // new pod should not be changed if the feature is enabled, or if the old pod had ReadinessGates + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) + } + case newPodHasReadinessGates: + // new pod should be changed + if reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod was not changed") + } + // new pod should not have ReadinessGates + if !reflect.DeepEqual(newPod, podWithoutReadinessGates()) { + t.Errorf("new pod had ReadinessGates: %v", diff.ObjectReflectDiff(newPod, podWithoutReadinessGates())) + } + default: + // new pod should not need to be changed + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) + } + } + }) + } + } + } +} + func TestDropRunAsGroup(t *testing.T) { group := func() *int64 { testGroup := int64(1000) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 2aed2236a4..b961d35ccc 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2669,9 +2669,6 @@ const ( func validateReadinessGates(readinessGates []core.PodReadinessGate, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if !utilfeature.DefaultFeatureGate.Enabled(features.PodReadinessGates) && len(readinessGates) > 0 { - return append(allErrs, field.Forbidden(fldPath, "PodReadinessGates is disabled by feature gate")) - } for i, value := range readinessGates { for _, msg := range validation.IsQualifiedName(string(value.ConditionType)) { allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("conditionType"), string(value.ConditionType), msg)) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index f807a31625..615b2ad0d3 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -5739,8 +5739,6 @@ func TestValidatePodDNSConfig(t *testing.T) { } func TestValidatePodReadinessGates(t *testing.T) { - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodReadinessGates, true)() - successCases := []struct { desc string readinessGates []core.PodReadinessGate