promote PDBUnhealthyPodEvictionPolicy to GA
This commit is contained in:
		| @@ -520,6 +520,7 @@ const ( | |||||||
| 	// kep: http://kep.k8s.io/3018 | 	// kep: http://kep.k8s.io/3018 | ||||||
| 	// alpha: v1.26 | 	// alpha: v1.26 | ||||||
| 	// beta: v1.27 | 	// beta: v1.27 | ||||||
|  | 	// GA: v1.31 | ||||||
| 	// | 	// | ||||||
| 	// Enables PDBUnhealthyPodEvictionPolicy for PodDisruptionBudgets | 	// Enables PDBUnhealthyPodEvictionPolicy for PodDisruptionBudgets | ||||||
| 	PDBUnhealthyPodEvictionPolicy featuregate.Feature = "PDBUnhealthyPodEvictionPolicy" | 	PDBUnhealthyPodEvictionPolicy featuregate.Feature = "PDBUnhealthyPodEvictionPolicy" | ||||||
| @@ -1086,7 +1087,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS | |||||||
|  |  | ||||||
| 	NodeSwap: {Default: true, PreRelease: featuregate.Beta}, | 	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}, | 	PersistentVolumeLastPhaseTransitionTime: {Default: true, PreRelease: featuregate.Beta}, | ||||||
|  |  | ||||||
|   | |||||||
| @@ -230,12 +230,10 @@ func (r *EvictionREST) Create(ctx context.Context, name string, obj runtime.Obje | |||||||
| 		// IsPodReady is the current implementation of IsHealthy | 		// IsPodReady is the current implementation of IsHealthy | ||||||
| 		// If the pod is healthy, it should be guarded by the PDB. | 		// If the pod is healthy, it should be guarded by the PDB. | ||||||
| 		if !podutil.IsPodReady(pod) { | 		if !podutil.IsPodReady(pod) { | ||||||
| 			if feature.DefaultFeatureGate.Enabled(features.PDBUnhealthyPodEvictionPolicy) { | 			if pdb.Spec.UnhealthyPodEvictionPolicy != nil && *pdb.Spec.UnhealthyPodEvictionPolicy == policyv1.AlwaysAllow { | ||||||
| 				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. | ||||||
| 					// Delete the unhealthy pod, it doesn't count towards currentHealthy and desiredHealthy and we should not decrement disruptionsAllowed. | 				updateDeletionOptions = true | ||||||
| 					updateDeletionOptions = true | 				return nil | ||||||
| 					return nil |  | ||||||
| 				} |  | ||||||
| 			} | 			} | ||||||
| 			// default nil and IfHealthyBudget policy | 			// default nil and IfHealthyBudget policy | ||||||
| 			if pdb.Status.CurrentHealthy >= pdb.Status.DesiredHealthy && pdb.Status.DesiredHealthy > 0 { | 			if pdb.Status.CurrentHealthy >= pdb.Status.DesiredHealthy && pdb.Status.DesiredHealthy > 0 { | ||||||
|   | |||||||
| @@ -165,8 +165,6 @@ func TestEviction(t *testing.T) { | |||||||
| 				continue | 				continue | ||||||
| 			} | 			} | ||||||
| 			t.Run(fmt.Sprintf("%v with %v policy", tc.name, unhealthyPolicyStr(unhealthyPodEvictionPolicy)), func(t *testing.T) { | 			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 | 				// same test runs multiple times, make copy of objects to have unique ones | ||||||
| 				evictionCopy := tc.eviction.DeepCopy() | 				evictionCopy := tc.eviction.DeepCopy() | ||||||
| 				var pdbsCopy []runtime.Object | 				var pdbsCopy []runtime.Object | ||||||
| @@ -530,8 +528,6 @@ func TestEvictionIgnorePDB(t *testing.T) { | |||||||
| 				continue | 				continue | ||||||
| 			} | 			} | ||||||
| 			t.Run(fmt.Sprintf("%v with %v policy", tc.name, unhealthyPolicyStr(unhealthyPodEvictionPolicy)), func(t *testing.T) { | 			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 | 				// same test runs 3 times, make copy of objects to have unique ones | ||||||
| 				evictionCopy := tc.eviction.DeepCopy() | 				evictionCopy := tc.eviction.DeepCopy() | ||||||
| 				prcCopy := tc.prc.DeepCopy() | 				prcCopy := tc.prc.DeepCopy() | ||||||
|   | |||||||
| @@ -26,11 +26,9 @@ import ( | |||||||
| 	"k8s.io/apimachinery/pkg/util/validation/field" | 	"k8s.io/apimachinery/pkg/util/validation/field" | ||||||
| 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request" | 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request" | ||||||
| 	"k8s.io/apiserver/pkg/storage/names" | 	"k8s.io/apiserver/pkg/storage/names" | ||||||
| 	utilfeature "k8s.io/apiserver/pkg/util/feature" |  | ||||||
| 	"k8s.io/kubernetes/pkg/api/legacyscheme" | 	"k8s.io/kubernetes/pkg/api/legacyscheme" | ||||||
| 	"k8s.io/kubernetes/pkg/apis/policy" | 	"k8s.io/kubernetes/pkg/apis/policy" | ||||||
| 	"k8s.io/kubernetes/pkg/apis/policy/validation" | 	"k8s.io/kubernetes/pkg/apis/policy/validation" | ||||||
| 	"k8s.io/kubernetes/pkg/features" |  | ||||||
| 	"sigs.k8s.io/structured-merge-diff/v4/fieldpath" | 	"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.Status = policy.PodDisruptionBudgetStatus{} | ||||||
|  |  | ||||||
| 	podDisruptionBudget.Generation = 1 | 	podDisruptionBudget.Generation = 1 | ||||||
|  |  | ||||||
| 	dropDisabledFields(&podDisruptionBudget.Spec, nil) |  | ||||||
| } | } | ||||||
|  |  | ||||||
| // PrepareForUpdate clears fields that are not allowed to be set by end users on update. | // 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) { | 	if !apiequality.Semantic.DeepEqual(oldPodDisruptionBudget.Spec, newPodDisruptionBudget.Spec) { | ||||||
| 		newPodDisruptionBudget.Generation = oldPodDisruptionBudget.Generation + 1 | 		newPodDisruptionBudget.Generation = oldPodDisruptionBudget.Generation + 1 | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	dropDisabledFields(&newPodDisruptionBudget.Spec, &oldPodDisruptionBudget.Spec) |  | ||||||
| } | } | ||||||
|  |  | ||||||
| // Validate validates a new PodDisruptionBudget. | // Validate validates a new PodDisruptionBudget. | ||||||
| @@ -188,23 +182,3 @@ func hasInvalidLabelValueInLabelSelector(pdb *policy.PodDisruptionBudget) bool { | |||||||
| 	} | 	} | ||||||
| 	return false | 	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 |  | ||||||
| } |  | ||||||
|   | |||||||
| @@ -17,112 +17,16 @@ limitations under the License. | |||||||
| package poddisruptionbudget | package poddisruptionbudget | ||||||
|  |  | ||||||
| import ( | import ( | ||||||
| 	"reflect" |  | ||||||
| 	"testing" | 	"testing" | ||||||
|  |  | ||||||
| 	"github.com/google/go-cmp/cmp" |  | ||||||
|  |  | ||||||
| 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||
| 	"k8s.io/apimachinery/pkg/util/intstr" | 	"k8s.io/apimachinery/pkg/util/intstr" | ||||||
| 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request" | 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request" | ||||||
| 	utilfeature "k8s.io/apiserver/pkg/util/feature" |  | ||||||
| 	featuregatetesting "k8s.io/component-base/featuregate/testing" |  | ||||||
| 	"k8s.io/kubernetes/pkg/apis/policy" | 	"k8s.io/kubernetes/pkg/apis/policy" | ||||||
| 	"k8s.io/kubernetes/pkg/features" | 	"k8s.io/utils/ptr" | ||||||
| ) | ) | ||||||
|  |  | ||||||
| type unhealthyPodEvictionPolicyStrategyTestCase struct { |  | ||||||
| 	name                                                       string |  | ||||||
| 	enableUnhealthyPodEvictionPolicy                           bool |  | ||||||
| 	disablePDBUnhealthyPodEvictionPolicyFeatureGateAfterCreate bool |  | ||||||
| 	unhealthyPodEvictionPolicy                                 *policy.UnhealthyPodEvictionPolicyType |  | ||||||
| 	expectedUnhealthyPodEvictionPolicy                         *policy.UnhealthyPodEvictionPolicyType |  | ||||||
| 	expectedValidationErr                                      bool |  | ||||||
| 	updateUnhealthyPodEvictionPolicy                           *policy.UnhealthyPodEvictionPolicyType |  | ||||||
| 	expectedUpdateUnhealthyPodEvictionPolicy                   *policy.UnhealthyPodEvictionPolicyType |  | ||||||
| 	expectedValidationUpdateErr                                bool |  | ||||||
| } |  | ||||||
|  |  | ||||||
| func TestPodDisruptionBudgetStrategy(t *testing.T) { | func TestPodDisruptionBudgetStrategy(t *testing.T) { | ||||||
| 	tests := map[string]bool{ |  | ||||||
| 		"PodDisruptionBudget strategy with PDBUnhealthyPodEvictionPolicy feature gate disabled": false, |  | ||||||
| 		"PodDisruptionBudget strategy with PDBUnhealthyPodEvictionPolicy feature gate enabled":  true, |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	for name, enableUnhealthyPodEvictionPolicy := range tests { |  | ||||||
| 		t.Run(name, func(t *testing.T) { |  | ||||||
| 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, enableUnhealthyPodEvictionPolicy) |  | ||||||
| 			testPodDisruptionBudgetStrategy(t) |  | ||||||
| 		}) |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	healthyPolicyTests := []unhealthyPodEvictionPolicyStrategyTestCase{ |  | ||||||
| 		{ |  | ||||||
| 			name:                             "PodDisruptionBudget strategy with FeatureGate disabled should remove unhealthyPodEvictionPolicy", |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: false, |  | ||||||
| 			unhealthyPodEvictionPolicy:       unhealthyPolicyPtr(policy.IfHealthyBudget), |  | ||||||
| 			updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.IfHealthyBudget), |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			name:                             "PodDisruptionBudget strategy with FeatureGate disabled should remove invalid unhealthyPodEvictionPolicy", |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: false, |  | ||||||
| 			unhealthyPodEvictionPolicy:       unhealthyPolicyPtr("Invalid"), |  | ||||||
| 			updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr("Invalid"), |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			name:                             "PodDisruptionBudget strategy with FeatureGate enabled", |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			name:                                     "PodDisruptionBudget strategy with FeatureGate enabled should respect unhealthyPodEvictionPolicy", |  | ||||||
| 			enableUnhealthyPodEvictionPolicy:         true, |  | ||||||
| 			unhealthyPodEvictionPolicy:               unhealthyPolicyPtr(policy.AlwaysAllow), |  | ||||||
| 			expectedUnhealthyPodEvictionPolicy:       unhealthyPolicyPtr(policy.AlwaysAllow), |  | ||||||
| 			updateUnhealthyPodEvictionPolicy:         unhealthyPolicyPtr(policy.IfHealthyBudget), |  | ||||||
| 			expectedUpdateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.IfHealthyBudget), |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			name:                             "PodDisruptionBudget strategy with FeatureGate enabled should fail invalid unhealthyPodEvictionPolicy", |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 			unhealthyPodEvictionPolicy:       unhealthyPolicyPtr("Invalid"), |  | ||||||
| 			expectedValidationErr:            true, |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			name:                             "PodDisruptionBudget strategy with FeatureGate enabled should fail invalid unhealthyPodEvictionPolicy when updated", |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 			updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr("Invalid"), |  | ||||||
| 			expectedValidationUpdateErr:      true, |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			name:                             "PodDisruptionBudget strategy with unhealthyPodEvictionPolicy should be updated when feature gate is disabled", |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 			disablePDBUnhealthyPodEvictionPolicyFeatureGateAfterCreate: true, |  | ||||||
| 			unhealthyPodEvictionPolicy:                                 unhealthyPolicyPtr(policy.AlwaysAllow), |  | ||||||
| 			expectedUnhealthyPodEvictionPolicy:                         unhealthyPolicyPtr(policy.AlwaysAllow), |  | ||||||
| 			updateUnhealthyPodEvictionPolicy:                           unhealthyPolicyPtr(policy.IfHealthyBudget), |  | ||||||
| 			expectedUpdateUnhealthyPodEvictionPolicy:                   unhealthyPolicyPtr(policy.IfHealthyBudget), |  | ||||||
| 		}, |  | ||||||
| 		{ |  | ||||||
| 			name:                             "PodDisruptionBudget strategy with unhealthyPodEvictionPolicy should not be updated to invalid when feature gate is disabled", |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 			disablePDBUnhealthyPodEvictionPolicyFeatureGateAfterCreate: true, |  | ||||||
| 			unhealthyPodEvictionPolicy:                                 unhealthyPolicyPtr(policy.AlwaysAllow), |  | ||||||
| 			expectedUnhealthyPodEvictionPolicy:                         unhealthyPolicyPtr(policy.AlwaysAllow), |  | ||||||
| 			updateUnhealthyPodEvictionPolicy:                           unhealthyPolicyPtr("Invalid"), |  | ||||||
| 			expectedValidationUpdateErr:                                true, |  | ||||||
| 			expectedUpdateUnhealthyPodEvictionPolicy:                   unhealthyPolicyPtr(policy.AlwaysAllow), |  | ||||||
| 		}, |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	for _, tc := range healthyPolicyTests { |  | ||||||
| 		t.Run(tc.name, func(t *testing.T) { |  | ||||||
| 			testPodDisruptionBudgetStrategyWithUnhealthyPodEvictionPolicy(t, tc) |  | ||||||
| 		}) |  | ||||||
| 	} |  | ||||||
| } |  | ||||||
|  |  | ||||||
| func testPodDisruptionBudgetStrategyWithUnhealthyPodEvictionPolicy(t *testing.T, tc unhealthyPodEvictionPolicyStrategyTestCase) { |  | ||||||
| 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, tc.enableUnhealthyPodEvictionPolicy) |  | ||||||
| 	ctx := genericapirequest.NewDefaultContext() | 	ctx := genericapirequest.NewDefaultContext() | ||||||
| 	if !Strategy.NamespaceScoped() { | 	if !Strategy.NamespaceScoped() { | ||||||
| 		t.Errorf("PodDisruptionBudget must be namespace scoped") | 		t.Errorf("PodDisruptionBudget must be namespace scoped") | ||||||
| @@ -138,70 +42,7 @@ func testPodDisruptionBudgetStrategyWithUnhealthyPodEvictionPolicy(t *testing.T, | |||||||
| 		Spec: policy.PodDisruptionBudgetSpec{ | 		Spec: policy.PodDisruptionBudgetSpec{ | ||||||
| 			MinAvailable:               &minAvailable, | 			MinAvailable:               &minAvailable, | ||||||
| 			Selector:                   &metav1.LabelSelector{MatchLabels: validSelector}, | 			Selector:                   &metav1.LabelSelector{MatchLabels: validSelector}, | ||||||
| 			UnhealthyPodEvictionPolicy: tc.unhealthyPodEvictionPolicy, | 			UnhealthyPodEvictionPolicy: ptr.To(policy.AlwaysAllow), | ||||||
| 		}, |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	Strategy.PrepareForCreate(ctx, pdb) |  | ||||||
| 	errs := Strategy.Validate(ctx, pdb) |  | ||||||
| 	if len(errs) != 0 { |  | ||||||
| 		if !tc.expectedValidationErr { |  | ||||||
| 			t.Errorf("Unexpected error validating %v", errs) |  | ||||||
| 		} |  | ||||||
| 		return // no point going further when we have invalid PDB |  | ||||||
| 	} |  | ||||||
| 	if len(errs) == 0 && tc.expectedValidationErr { |  | ||||||
| 		t.Errorf("Expected error validating") |  | ||||||
| 	} |  | ||||||
| 	if !reflect.DeepEqual(pdb.Spec.UnhealthyPodEvictionPolicy, tc.expectedUnhealthyPodEvictionPolicy) { |  | ||||||
| 		t.Errorf("Unexpected UnhealthyPodEvictionPolicy set: expected %v, got %v", tc.expectedUnhealthyPodEvictionPolicy, pdb.Spec.UnhealthyPodEvictionPolicy) |  | ||||||
| 	} |  | ||||||
| 	if tc.disablePDBUnhealthyPodEvictionPolicyFeatureGateAfterCreate { |  | ||||||
| 		featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, false) |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	newPdb := &policy.PodDisruptionBudget{ |  | ||||||
| 		ObjectMeta: metav1.ObjectMeta{Name: pdb.Name, Namespace: pdb.Namespace}, |  | ||||||
| 		Spec:       pdb.Spec, |  | ||||||
| 	} |  | ||||||
| 	if tc.updateUnhealthyPodEvictionPolicy != nil { |  | ||||||
| 		newPdb.Spec.UnhealthyPodEvictionPolicy = tc.updateUnhealthyPodEvictionPolicy |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	// Nothing in Spec changes: OK |  | ||||||
| 	Strategy.PrepareForUpdate(ctx, newPdb, pdb) |  | ||||||
| 	errs = Strategy.ValidateUpdate(ctx, newPdb, pdb) |  | ||||||
|  |  | ||||||
| 	if len(errs) != 0 { |  | ||||||
| 		if !tc.expectedValidationUpdateErr { |  | ||||||
| 			t.Errorf("Unexpected error updating PodDisruptionBudget %v", errs) |  | ||||||
| 		} |  | ||||||
| 		return // no point going further when we have invalid PDB |  | ||||||
| 	} |  | ||||||
| 	if len(errs) == 0 && tc.expectedValidationUpdateErr { |  | ||||||
| 		t.Errorf("Expected error updating PodDisruptionBudget") |  | ||||||
| 	} |  | ||||||
| 	if !reflect.DeepEqual(newPdb.Spec.UnhealthyPodEvictionPolicy, tc.expectedUpdateUnhealthyPodEvictionPolicy) { |  | ||||||
| 		t.Errorf("Unexpected UnhealthyPodEvictionPolicy set: expected %v, got %v", tc.expectedUpdateUnhealthyPodEvictionPolicy, newPdb.Spec.UnhealthyPodEvictionPolicy) |  | ||||||
| 	} |  | ||||||
| } |  | ||||||
|  |  | ||||||
| func testPodDisruptionBudgetStrategy(t *testing.T) { |  | ||||||
| 	ctx := genericapirequest.NewDefaultContext() |  | ||||||
| 	if !Strategy.NamespaceScoped() { |  | ||||||
| 		t.Errorf("PodDisruptionBudget must be namespace scoped") |  | ||||||
| 	} |  | ||||||
| 	if Strategy.AllowCreateOnUpdate() { |  | ||||||
| 		t.Errorf("PodDisruptionBudget should not allow create on update") |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	validSelector := map[string]string{"a": "b"} |  | ||||||
| 	minAvailable := intstr.FromInt32(3) |  | ||||||
| 	pdb := &policy.PodDisruptionBudget{ |  | ||||||
| 		ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault}, |  | ||||||
| 		Spec: policy.PodDisruptionBudgetSpec{ |  | ||||||
| 			MinAvailable: &minAvailable, |  | ||||||
| 			Selector:     &metav1.LabelSelector{MatchLabels: validSelector}, |  | ||||||
| 		}, | 		}, | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| @@ -256,6 +97,25 @@ func testPodDisruptionBudgetStrategy(t *testing.T) { | |||||||
| 	if len(errs) != 0 { | 	if len(errs) != 0 { | ||||||
| 		t.Errorf("Expected no error updating replacing MinAvailable with MaxUnavailable on poddisruptionbudgets.") | 		t.Errorf("Expected no error updating replacing MinAvailable with MaxUnavailable on poddisruptionbudgets.") | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | 	// Changing UnhealthyPodEvictionPolicy? OK | ||||||
|  | 	newPdb.Spec.UnhealthyPodEvictionPolicy = ptr.To(policy.IfHealthyBudget) | ||||||
|  | 	Strategy.PrepareForUpdate(ctx, newPdb, pdb) | ||||||
|  | 	errs = Strategy.ValidateUpdate(ctx, newPdb, pdb) | ||||||
|  | 	if len(errs) != 0 { | ||||||
|  | 		t.Errorf("Expected no error on changing UnhealthyPodEvictionPolicy on poddisruptionbudgets.") | ||||||
|  | 	} | ||||||
|  | 	if *newPdb.Spec.UnhealthyPodEvictionPolicy != policy.IfHealthyBudget { | ||||||
|  | 		t.Errorf("Unexpected UnhealthyPodEvictionPolicy: expected %v, got %v", *newPdb.Spec.UnhealthyPodEvictionPolicy, policy.IfHealthyBudget) | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	// Changing to invalid UnhealthyPodEvictionPolicy. | ||||||
|  | 	newPdb.Spec.UnhealthyPodEvictionPolicy = ptr.To(policy.UnhealthyPodEvictionPolicyType("invalid")) | ||||||
|  | 	Strategy.PrepareForUpdate(ctx, newPdb, pdb) | ||||||
|  | 	errs = Strategy.ValidateUpdate(ctx, newPdb, pdb) | ||||||
|  | 	if len(errs) == 0 { | ||||||
|  | 		t.Errorf("Expected error on changing to invalid UnhealthyPodEvictionPolicy on poddisruptionbudgets.") | ||||||
|  | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
| func TestPodDisruptionBudgetStatusStrategy(t *testing.T) { | func TestPodDisruptionBudgetStatusStrategy(t *testing.T) { | ||||||
| @@ -371,86 +231,3 @@ func TestPodDisruptionBudgetStatusValidationByApiVersion(t *testing.T) { | |||||||
| 		}) | 		}) | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
| func TestDropDisabledFields(t *testing.T) { |  | ||||||
| 	tests := map[string]struct { |  | ||||||
| 		oldSpec                          *policy.PodDisruptionBudgetSpec |  | ||||||
| 		newSpec                          *policy.PodDisruptionBudgetSpec |  | ||||||
| 		expectNewSpec                    *policy.PodDisruptionBudgetSpec |  | ||||||
| 		enableUnhealthyPodEvictionPolicy bool |  | ||||||
| 	}{ |  | ||||||
| 		"disabled clears unhealthyPodEvictionPolicy": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: false, |  | ||||||
| 			oldSpec:                          nil, |  | ||||||
| 			newSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			expectNewSpec:                    specWithUnhealthyPodEvictionPolicy(nil), |  | ||||||
| 		}, |  | ||||||
| 		"disabled does not allow updating unhealthyPodEvictionPolicy": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: false, |  | ||||||
| 			oldSpec:                          specWithUnhealthyPodEvictionPolicy(nil), |  | ||||||
| 			newSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			expectNewSpec:                    specWithUnhealthyPodEvictionPolicy(nil), |  | ||||||
| 		}, |  | ||||||
| 		"disabled preserves old unhealthyPodEvictionPolicy when both old and new have it": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: false, |  | ||||||
| 			oldSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			newSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			expectNewSpec:                    specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 		}, |  | ||||||
| 		"disabled allows updating unhealthyPodEvictionPolicy": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: false, |  | ||||||
| 			oldSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			newSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.AlwaysAllow)), |  | ||||||
| 			expectNewSpec:                    specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.AlwaysAllow)), |  | ||||||
| 		}, |  | ||||||
| 		"enabled preserve unhealthyPodEvictionPolicy": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 			oldSpec:                          nil, |  | ||||||
| 			newSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			expectNewSpec:                    specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 		}, |  | ||||||
| 		"enabled allows updating unhealthyPodEvictionPolicy": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 			oldSpec:                          specWithUnhealthyPodEvictionPolicy(nil), |  | ||||||
| 			newSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			expectNewSpec:                    specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 		}, |  | ||||||
| 		"enabled preserve unhealthyPodEvictionPolicy when both old and new have it": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 			oldSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			newSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			expectNewSpec:                    specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 		}, |  | ||||||
| 		"enabled updates unhealthyPodEvictionPolicy": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, |  | ||||||
| 			oldSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)), |  | ||||||
| 			newSpec:                          specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.AlwaysAllow)), |  | ||||||
| 			expectNewSpec:                    specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.AlwaysAllow)), |  | ||||||
| 		}, |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	for name, tc := range tests { |  | ||||||
| 		t.Run(name, func(t *testing.T) { |  | ||||||
| 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, tc.enableUnhealthyPodEvictionPolicy) |  | ||||||
|  |  | ||||||
| 			oldSpecBefore := tc.oldSpec.DeepCopy() |  | ||||||
| 			dropDisabledFields(tc.newSpec, tc.oldSpec) |  | ||||||
| 			if !reflect.DeepEqual(tc.newSpec, tc.expectNewSpec) { |  | ||||||
| 				t.Error(cmp.Diff(tc.newSpec, tc.expectNewSpec)) |  | ||||||
| 			} |  | ||||||
| 			if !reflect.DeepEqual(tc.oldSpec, oldSpecBefore) { |  | ||||||
| 				t.Error(cmp.Diff(tc.oldSpec, oldSpecBefore)) |  | ||||||
| 			} |  | ||||||
| 		}) |  | ||||||
| 	} |  | ||||||
| } |  | ||||||
|  |  | ||||||
| func unhealthyPolicyPtr(unhealthyPodEvictionPolicy policy.UnhealthyPodEvictionPolicyType) *policy.UnhealthyPodEvictionPolicyType { |  | ||||||
| 	return &unhealthyPodEvictionPolicy |  | ||||||
| } |  | ||||||
|  |  | ||||||
| func specWithUnhealthyPodEvictionPolicy(unhealthyPodEvictionPolicy *policy.UnhealthyPodEvictionPolicyType) *policy.PodDisruptionBudgetSpec { |  | ||||||
| 	return &policy.PodDisruptionBudgetSpec{ |  | ||||||
| 		UnhealthyPodEvictionPolicy: unhealthyPodEvictionPolicy, |  | ||||||
| 	} |  | ||||||
| } |  | ||||||
|   | |||||||
| @@ -434,40 +434,29 @@ func TestEvictionWithFinalizers(t *testing.T) { | |||||||
| // TestEvictionWithUnhealthyPodEvictionPolicy tests eviction with a PDB that has a UnhealthyPodEvictionPolicy | // TestEvictionWithUnhealthyPodEvictionPolicy tests eviction with a PDB that has a UnhealthyPodEvictionPolicy | ||||||
| func TestEvictionWithUnhealthyPodEvictionPolicy(t *testing.T) { | func TestEvictionWithUnhealthyPodEvictionPolicy(t *testing.T) { | ||||||
| 	cases := map[string]struct { | 	cases := map[string]struct { | ||||||
| 		enableUnhealthyPodEvictionPolicy bool | 		unhealthyPodEvictionPolicy *policyv1.UnhealthyPodEvictionPolicyType | ||||||
| 		unhealthyPodEvictionPolicy       *policyv1.UnhealthyPodEvictionPolicyType | 		isPodReady                 bool | ||||||
| 		isPodReady                       bool |  | ||||||
| 	}{ | 	}{ | ||||||
| 		"UnhealthyPodEvictionPolicy disabled and policy not set": { |  | ||||||
| 			enableUnhealthyPodEvictionPolicy: false, |  | ||||||
| 			unhealthyPodEvictionPolicy:       nil, |  | ||||||
| 			isPodReady:                       true, |  | ||||||
| 		}, |  | ||||||
| 		"UnhealthyPodEvictionPolicy enabled but policy not set": { | 		"UnhealthyPodEvictionPolicy enabled but policy not set": { | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, | 			unhealthyPodEvictionPolicy: nil, | ||||||
| 			unhealthyPodEvictionPolicy:       nil, | 			isPodReady:                 true, | ||||||
| 			isPodReady:                       true, |  | ||||||
| 		}, | 		}, | ||||||
| 		"UnhealthyPodEvictionPolicy enabled but policy set to IfHealthyBudget with ready pod": { | 		"UnhealthyPodEvictionPolicy enabled but policy set to IfHealthyBudget with ready pod": { | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, | 			unhealthyPodEvictionPolicy: unhealthyPolicyPtr(policyv1.IfHealthyBudget), | ||||||
| 			unhealthyPodEvictionPolicy:       unhealthyPolicyPtr(policyv1.IfHealthyBudget), | 			isPodReady:                 true, | ||||||
| 			isPodReady:                       true, |  | ||||||
| 		}, | 		}, | ||||||
| 		"UnhealthyPodEvictionPolicy enabled but policy set to AlwaysAllow with ready pod": { | 		"UnhealthyPodEvictionPolicy enabled but policy set to AlwaysAllow with ready pod": { | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, | 			unhealthyPodEvictionPolicy: unhealthyPolicyPtr(policyv1.AlwaysAllow), | ||||||
| 			unhealthyPodEvictionPolicy:       unhealthyPolicyPtr(policyv1.AlwaysAllow), | 			isPodReady:                 true, | ||||||
| 			isPodReady:                       true, |  | ||||||
| 		}, | 		}, | ||||||
| 		"UnhealthyPodEvictionPolicy enabled but policy set to AlwaysAllow with unready pod": { | 		"UnhealthyPodEvictionPolicy enabled but policy set to AlwaysAllow with unready pod": { | ||||||
| 			enableUnhealthyPodEvictionPolicy: true, | 			unhealthyPodEvictionPolicy: unhealthyPolicyPtr(policyv1.AlwaysAllow), | ||||||
| 			unhealthyPodEvictionPolicy:       unhealthyPolicyPtr(policyv1.AlwaysAllow), | 			isPodReady:                 false, | ||||||
| 			isPodReady:                       false, |  | ||||||
| 		}, | 		}, | ||||||
| 	} | 	} | ||||||
| 	for name, tc := range cases { | 	for name, tc := range cases { | ||||||
| 		t.Run(name, func(t *testing.T) { | 		t.Run(name, func(t *testing.T) { | ||||||
| 			tCtx := ktesting.Init(t) | 			tCtx := ktesting.Init(t) | ||||||
| 			featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, tc.enableUnhealthyPodEvictionPolicy) |  | ||||||
| 			closeFn, rm, informers, _, clientSet := rmSetup(tCtx, t) | 			closeFn, rm, informers, _, clientSet := rmSetup(tCtx, t) | ||||||
| 			defer closeFn() | 			defer closeFn() | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Filip Křepinský
					Filip Křepinský