api: add UnhealthyPodEvictionPolicy for PDBs

pkg/apis/policy/types.go

@@ -42,8 +42,56 @@ type PodDisruptionBudgetSpec struct {
 	// by specifying 0. This is a mutually exclusive setting with "minAvailable".
 	// +optional
 	MaxUnavailable *intstr.IntOrString
+
+	// UnhealthyPodEvictionPolicy defines the criteria for when unhealthy pods
+	// should be considered for eviction. Current implementation considers healthy pods,
+	// as pods that have status.conditions item with type="Ready",status="True".
+	//
+	// Valid policies are IfHealthyBudget and AlwaysAllow.
+	// If no policy is specified, the default behavior will be used,
+	// which corresponds to the IfHealthyBudget policy.
+	//
+	// IfHealthyBudget policy means that running pods (status.phase="Running"),
+	// but not yet healthy can be evicted only if the guarded application is not
+	// disrupted (status.currentHealthy is at least equal to status.desiredHealthy).
+	// Healthy pods will be subject to the PDB for eviction.
+	//
+	// AlwaysAllow policy means that all running pods (status.phase="Running"),
+	// but not yet healthy are considered disrupted and can be evicted regardless
+	// of whether the criteria in a PDB is met. This means perspective running
+	// pods of a disrupted application might not get a chance to become healthy.
+	// Healthy pods will be subject to the PDB for eviction.
+	//
+	// Additional policies may be added in the future.
+	// Clients making eviction decisions should disallow eviction of unhealthy pods
+	// if they encounter an unrecognized policy in this field.
+	//
+	// This field is alpha-level. The eviction API uses this field when
+	// the feature gate PDBUnhealthyPodEvictionPolicy is enabled (disabled by default).
+	// +optional
+	UnhealthyPodEvictionPolicy *UnhealthyPodEvictionPolicyType
 }
 
+// UnhealthyPodEvictionPolicyType defines the criteria for when unhealthy pods
+// should be considered for eviction.
+// +enum
+type UnhealthyPodEvictionPolicyType string
+
+const (
+	// IfHealthyBudget policy means that running pods (status.phase="Running"),
+	// but not yet healthy can be evicted only if the guarded application is not
+	// disrupted (status.currentHealthy is at least equal to status.desiredHealthy).
+	// Healthy pods will be subject to the PDB for eviction.
+	IfHealthyBudget UnhealthyPodEvictionPolicyType = "IfHealthyBudget"
+
+	// AlwaysAllow policy means that all running pods (status.phase="Running"),
+	// but not yet healthy are considered disrupted and can be evicted regardless
+	// of whether the criteria in a PDB is met. This means perspective running
+	// pods of a disrupted application might not get a chance to become healthy.
+	// Healthy pods will be subject to the PDB for eviction.
+	AlwaysAllow UnhealthyPodEvictionPolicyType = "AlwaysAllow"
+)
+
 // PodDisruptionBudgetStatus represents information about the status of a
 // PodDisruptionBudget. Status may trail the actual state of a system.
 type PodDisruptionBudgetStatus struct {

pkg/apis/policy/validation/validation.go

@@ -44,6 +44,11 @@ const (
 	seccompAllowedProfilesAnnotationKey = "seccomp.security.alpha.kubernetes.io/allowedProfileNames"
 )
 
+var supportedUnhealthyPodEvictionPolicies = sets.NewString(
+	string(policy.IfHealthyBudget),
+	string(policy.AlwaysAllow),
+)
+
 type PodDisruptionBudgetValidationOptions struct {
 	AllowInvalidLabelValueInSelector bool
 }
@@ -78,6 +83,10 @@ func ValidatePodDisruptionBudgetSpec(spec policy.PodDisruptionBudgetSpec, opts P
 
 	allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, labelSelectorValidationOptions, fldPath.Child("selector"))...)
 
+	if spec.UnhealthyPodEvictionPolicy != nil && !supportedUnhealthyPodEvictionPolicies.Has(string(*spec.UnhealthyPodEvictionPolicy)) {
+		allErrs = append(allErrs, field.NotSupported(fldPath.Child("unhealthyPodEvictionPolicy"), *spec.UnhealthyPodEvictionPolicy, supportedUnhealthyPodEvictionPolicies.List()))
+	}
+
 	return allErrs
 }
 

pkg/apis/policy/validation/validation_test.go

@@ -98,6 +98,63 @@ func TestValidateMinAvailablePodAndMaxUnavailableDisruptionBudgetSpec(t *testing
 	}
 }
 
+func TestValidateUnhealthyPodEvictionPolicyDisruptionBudgetSpec(t *testing.T) {
+	c1 := intstr.FromString("10%")
+	alwaysAllowPolicy := policy.AlwaysAllow
+	invalidPolicy := policy.UnhealthyPodEvictionPolicyType("Invalid")
+
+	testCases := []struct {
+		name      string
+		pdbSpec   policy.PodDisruptionBudgetSpec
+		expectErr bool
+	}{
+		{
+			name: "valid nil UnhealthyPodEvictionPolicy",
+			pdbSpec: policy.PodDisruptionBudgetSpec{
+				MinAvailable:               &c1,
+				UnhealthyPodEvictionPolicy: nil,
+			},
+			expectErr: false,
+		},
+		{
+			name: "valid UnhealthyPodEvictionPolicy",
+			pdbSpec: policy.PodDisruptionBudgetSpec{
+				MinAvailable:               &c1,
+				UnhealthyPodEvictionPolicy: &alwaysAllowPolicy,
+			},
+			expectErr: false,
+		},
+		{
+			name: "empty UnhealthyPodEvictionPolicy",
+			pdbSpec: policy.PodDisruptionBudgetSpec{
+				MinAvailable:               &c1,
+				UnhealthyPodEvictionPolicy: new(policy.UnhealthyPodEvictionPolicyType),
+			},
+			expectErr: true,
+		},
+		{
+			name: "invalid UnhealthyPodEvictionPolicy",
+			pdbSpec: policy.PodDisruptionBudgetSpec{
+				MinAvailable:               &c1,
+				UnhealthyPodEvictionPolicy: &invalidPolicy,
+			},
+			expectErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			errs := ValidatePodDisruptionBudgetSpec(tc.pdbSpec, PodDisruptionBudgetValidationOptions{true}, field.NewPath("foo"))
+			if len(errs) == 0 && tc.expectErr {
+				t.Errorf("unexpected success for %v", tc.pdbSpec)
+			}
+			if len(errs) != 0 && !tc.expectErr {
+				t.Errorf("unexpected failure for %v", tc.pdbSpec)
+			}
+		})
+	}
+}
+
 func TestValidatePodDisruptionBudgetStatus(t *testing.T) {
 	const expectNoErrors = false
 	const expectErrors = true

pkg/features/kube_features.go

@@ -625,6 +625,13 @@ const (
 	// Permits kubelet to run with swap enabled
 	NodeSwap featuregate.Feature = "NodeSwap"
 
+	// owner: @mortent, @atiratree, @ravig
+	// kep: http://kep.k8s.io/3018
+	// alpha: v1.26
+	//
+	// Enables PDBUnhealthyPodEvictionPolicy for PodDisruptionBudgets
+	PDBUnhealthyPodEvictionPolicy featuregate.Feature = "PDBUnhealthyPodEvictionPolicy"
+
 	// owner: @haircommander
 	// kep: https://kep.k8s.io/2364
 	// alpha: v1.23
@@ -1045,6 +1052,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	NodeSwap: {Default: false, PreRelease: featuregate.Alpha},
 
+	PDBUnhealthyPodEvictionPolicy: {Default: false, PreRelease: featuregate.Alpha},
+
 	PodAndContainerStatsFromCRI: {Default: false, PreRelease: featuregate.Alpha},
 
 	PodDeletionCost: {Default: true, PreRelease: featuregate.Beta},

pkg/registry/core/pod/storage/eviction.go

@@ -226,10 +226,24 @@ func (r *EvictionREST) Create(ctx context.Context, name string, obj runtime.Obje
 		pdb := &pdbs[0]
 		pdbName = pdb.Name
 
-		// If the pod is not ready, it doesn't count towards healthy and we should not decrement
-		if !podutil.IsPodReady(pod) && pdb.Status.CurrentHealthy >= pdb.Status.DesiredHealthy && pdb.Status.DesiredHealthy > 0 {
-			updateDeletionOptions = true
-			return nil
+		// 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
+				}
+			}
+			// default nil and IfHealthyBudget policy
+			if pdb.Status.CurrentHealthy >= pdb.Status.DesiredHealthy && pdb.Status.DesiredHealthy > 0 {
+				// Delete the unhealthy pod, it doesn't count towards currentHealthy and desiredHealthy and we should not decrement disruptionsAllowed.
+				// Application guarded by the PDB is not disrupted at the moment and deleting unhealthy (unready) pod will not disrupt it.
+				updateDeletionOptions = true
+				return nil
+			}
+			// confirm no disruptions allowed in checkAndDecrement
 		}
 
 		refresh := false
@@ -264,12 +278,12 @@ func (r *EvictionREST) Create(ctx context.Context, name string, obj runtime.Obje
 	}
 
 	// At this point there was either no PDB or we succeeded in decrementing or
-	// the pod was unready and we have enough healthy replicas
+	// the pod was unhealthy (unready) and we have enough healthy replicas
 
 	deleteOptions := originalDeleteOptions
 
 	// Set deleteOptions.Preconditions.ResourceVersion to ensure
-	// the pod hasn't been considered ready since we calculated
+	// the pod hasn't been considered healthy (ready) since we calculated
 	if updateDeletionOptions {
 		// Take a copy so we can compare to client-provied Options later.
 		deleteOptions = deleteOptions.DeepCopy()
@@ -351,7 +365,7 @@ func shouldEnforceResourceVersion(pod *api.Pod) bool {
 		return false
 	}
 	// Return true for all other pods to ensure we don't race against a pod becoming
-	// ready and violating PDBs.
+	// healthy (ready) and violating PDBs.
 	return true
 }