Merge pull request #72695 from rajathagasthya/podreadiness-72651

Move PodReadinessGates feature gate out of validation

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 {

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)

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))

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