Merge pull request kubernetes#123428 from atiratree/UnhealthyPodEvict…

…ionPolicy-GA promote PDBUnhealthyPodEvictionPolicy to GA
openshift · Jun 26, 2024 · fb0195d · fb0195d
2 parents c6fd466 + b779fb8
commit fb0195d
Show file tree

Hide file tree

Showing 7 changed files with 156 additions and 308 deletions.
diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go
@@ -526,6 +526,7 @@ const (
 	// kep: http://kep.k8s.io/3018
 	// alpha: v1.26
 	// beta: v1.27
+	// GA: v1.31
 	//
 	// Enables PDBUnhealthyPodEvictionPolicy for PodDisruptionBudgets
 	PDBUnhealthyPodEvictionPolicy featuregate.Feature = "PDBUnhealthyPodEvictionPolicy"
@@ -1095,7 +1096,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	NodeSwap: {Default: true, PreRelease: featuregate.Beta},
 
-	PDBUnhealthyPodEvictionPolicy: {Default: true, PreRelease: featuregate.Beta},
+	PDBUnhealthyPodEvictionPolicy: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.33
 
 	PersistentVolumeLastPhaseTransitionTime: {Default: true, PreRelease: featuregate.Beta},
 

diff --git a/pkg/registry/core/pod/storage/eviction.go b/pkg/registry/core/pod/storage/eviction.go
@@ -230,12 +230,10 @@ func (r *EvictionREST) Create(ctx context.Context, name string, obj runtime.Obje
 		// IsPodReady is the current implementation of IsHealthy
 		// If the pod is healthy, it should be guarded by the PDB.
 		if !podutil.IsPodReady(pod) {
-			if feature.DefaultFeatureGate.Enabled(features.PDBUnhealthyPodEvictionPolicy) {
-				if pdb.Spec.UnhealthyPodEvictionPolicy != nil && *pdb.Spec.UnhealthyPodEvictionPolicy == policyv1.AlwaysAllow {
-					// Delete the unhealthy pod, it doesn't count towards currentHealthy and desiredHealthy and we should not decrement disruptionsAllowed.
-					updateDeletionOptions = true
-					return nil
-				}
+			if pdb.Spec.UnhealthyPodEvictionPolicy != nil && *pdb.Spec.UnhealthyPodEvictionPolicy == policyv1.AlwaysAllow {
+				// Delete the unhealthy pod, it doesn't count towards currentHealthy and desiredHealthy and we should not decrement disruptionsAllowed.
+				updateDeletionOptions = true
+				return nil
 			}
 			// default nil and IfHealthyBudget policy
 			if pdb.Status.CurrentHealthy >= pdb.Status.DesiredHealthy && pdb.Status.DesiredHealthy > 0 {

diff --git a/pkg/registry/core/pod/storage/eviction_test.go b/pkg/registry/core/pod/storage/eviction_test.go
@@ -165,8 +165,6 @@ func TestEviction(t *testing.T) {
 				continue
 			}
 			t.Run(fmt.Sprintf("%v with %v policy", tc.name, unhealthyPolicyStr(unhealthyPodEvictionPolicy)), func(t *testing.T) {
-				featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, true)
-
 				// same test runs multiple times, make copy of objects to have unique ones
 				evictionCopy := tc.eviction.DeepCopy()
 				var pdbsCopy []runtime.Object
@@ -530,8 +528,6 @@ func TestEvictionIgnorePDB(t *testing.T) {
 				continue
 			}
 			t.Run(fmt.Sprintf("%v with %v policy", tc.name, unhealthyPolicyStr(unhealthyPodEvictionPolicy)), func(t *testing.T) {
-				featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, true)
-
 				// same test runs 3 times, make copy of objects to have unique ones
 				evictionCopy := tc.eviction.DeepCopy()
 				prcCopy := tc.prc.DeepCopy()

diff --git a/pkg/registry/policy/poddisruptionbudget/strategy.go b/pkg/registry/policy/poddisruptionbudget/strategy.go
@@ -26,11 +26,9 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/storage/names"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/apis/policy"
 	"k8s.io/kubernetes/pkg/apis/policy/validation"
-	"k8s.io/kubernetes/pkg/features"
 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
 )
 
@@ -70,8 +68,6 @@ func (podDisruptionBudgetStrategy) PrepareForCreate(ctx context.Context, obj run
 	podDisruptionBudget.Status = policy.PodDisruptionBudgetStatus{}
 
 	podDisruptionBudget.Generation = 1
-
-	dropDisabledFields(&podDisruptionBudget.Spec, nil)
 }
 
 // PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@@ -87,8 +83,6 @@ func (podDisruptionBudgetStrategy) PrepareForUpdate(ctx context.Context, obj, ol
 	if !apiequality.Semantic.DeepEqual(oldPodDisruptionBudget.Spec, newPodDisruptionBudget.Spec) {
 		newPodDisruptionBudget.Generation = oldPodDisruptionBudget.Generation + 1
 	}
-
-	dropDisabledFields(&newPodDisruptionBudget.Spec, &oldPodDisruptionBudget.Spec)
 }
 
 // Validate validates a new PodDisruptionBudget.
@@ -188,23 +182,3 @@ func hasInvalidLabelValueInLabelSelector(pdb *policy.PodDisruptionBudget) bool {
 	}
 	return false
 }
-
-// dropDisabledFields removes disabled fields from the pod disruption budget spec.
-// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pod disruption budget spec.
-func dropDisabledFields(pdbSpec, oldPDBSpec *policy.PodDisruptionBudgetSpec) {
-	if !utilfeature.DefaultFeatureGate.Enabled(features.PDBUnhealthyPodEvictionPolicy) {
-		if !unhealthyPodEvictionPolicyInUse(oldPDBSpec) {
-			pdbSpec.UnhealthyPodEvictionPolicy = nil
-		}
-	}
-}
-
-func unhealthyPodEvictionPolicyInUse(oldPDBSpec *policy.PodDisruptionBudgetSpec) bool {
-	if oldPDBSpec == nil {
-		return false
-	}
-	if oldPDBSpec.UnhealthyPodEvictionPolicy != nil {
-		return true
-	}
-	return false
-}