PDB MaxUnavailable: API changes

pkg/apis/policy/types.go

@@ -28,12 +28,19 @@ type PodDisruptionBudgetSpec struct {
 	// absence of the evicted pod.  So for example you can prevent all voluntary
 	// evictions by specifying "100%".
 	// +optional
-	MinAvailable intstr.IntOrString
+	MinAvailable *intstr.IntOrString
 
 	// Label query over pods whose evictions are managed by the disruption
 	// budget.
 	// +optional
 	Selector *metav1.LabelSelector
+
+	// An eviction is allowed if at most "maxUnavailable" pods selected by
+	// "selector" are unavailable after the eviction, i.e. even in absence of
+	// the evicted pod. For example, one can prevent all voluntary evictions
+	// by specifying 0. This is a mutually exclusive setting with "minAvailable".
+	// +optional
+	MaxUnavailable *intstr.IntOrString
 }
 
 // PodDisruptionBudgetStatus represents information about the status of a

pkg/apis/policy/v1beta1/types.go

@@ -27,11 +27,17 @@ type PodDisruptionBudgetSpec struct {
 	// "selector" will still be available after the eviction, i.e. even in the
 	// absence of the evicted pod.  So for example you can prevent all voluntary
 	// evictions by specifying "100%".
-	MinAvailable intstr.IntOrString `json:"minAvailable,omitempty" protobuf:"bytes,1,opt,name=minAvailable"`
+	MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty" protobuf:"bytes,1,opt,name=minAvailable"`
 
 	// Label query over pods whose evictions are managed by the disruption
 	// budget.
 	Selector *metav1.LabelSelector `json:"selector,omitempty" protobuf:"bytes,2,opt,name=selector"`
+
+	// An eviction is allowed if at most "maxUnavailable" pods selected by
+	// "selector" are unavailable after the eviction, i.e. even in absence of
+	// the evicted pod. For example, one can prevent all voluntary evictions
+	// by specifying 0. This is a mutually exclusive setting with "minAvailable".
+	MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty" protobuf:"bytes,3,opt,name=maxUnavailable"`
 }
 
 // PodDisruptionBudgetStatus represents information about the status of a

pkg/apis/policy/validation/validation.go

@@ -50,8 +50,20 @@ func ValidatePodDisruptionBudgetUpdate(pdb, oldPdb *policy.PodDisruptionBudget)
 func ValidatePodDisruptionBudgetSpec(spec policy.PodDisruptionBudgetSpec, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	allErrs = append(allErrs, extensionsvalidation.ValidatePositiveIntOrPercent(spec.MinAvailable, fldPath.Child("minAvailable"))...)
-	allErrs = append(allErrs, extensionsvalidation.IsNotMoreThan100Percent(spec.MinAvailable, fldPath.Child("minAvailable"))...)
+	if spec.MinAvailable != nil && spec.MaxUnavailable != nil {
+		allErrs = append(allErrs, field.Invalid(fldPath, spec, "minAvailable and maxUnavailable cannot be both set"))
+	}
+
+	if spec.MinAvailable != nil {
+		allErrs = append(allErrs, extensionsvalidation.ValidatePositiveIntOrPercent(*spec.MinAvailable, fldPath.Child("minAvailable"))...)
+		allErrs = append(allErrs, extensionsvalidation.IsNotMoreThan100Percent(*spec.MinAvailable, fldPath.Child("minAvailable"))...)
+	}
+
+	if spec.MaxUnavailable != nil {
+		allErrs = append(allErrs, extensionsvalidation.ValidatePositiveIntOrPercent(*spec.MaxUnavailable, fldPath.Child("maxUnavailable"))...)
+		allErrs = append(allErrs, extensionsvalidation.IsNotMoreThan100Percent(*spec.MaxUnavailable, fldPath.Child("maxUnavailable"))...)
+	}
+
 	allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
 
 	return allErrs

pkg/apis/policy/validation/validation_test.go

@@ -25,6 +25,20 @@ import (
 )
 
 func TestValidatePodDisruptionBudgetSpec(t *testing.T) {
+	minAvailable := intstr.FromString("0%")
+	maxUnavailable := intstr.FromString("10%")
+
+	spec := policy.PodDisruptionBudgetSpec{
+		MinAvailable:   &minAvailable,
+		MaxUnavailable: &maxUnavailable,
+	}
+	errs := ValidatePodDisruptionBudgetSpec(spec, field.NewPath("foo"))
+	if len(errs) == 0 {
+		t.Errorf("unexpected success for %v", spec)
+	}
+}
+
+func TestValidateMinAvailablePodDisruptionBudgetSpec(t *testing.T) {
 	successCases := []intstr.IntOrString{
 		intstr.FromString("0%"),
 		intstr.FromString("1%"),
@@ -35,7 +49,7 @@ func TestValidatePodDisruptionBudgetSpec(t *testing.T) {
 	}
 	for _, c := range successCases {
 		spec := policy.PodDisruptionBudgetSpec{
-			MinAvailable: c,
+			MinAvailable: &c,
 		}
 		errs := ValidatePodDisruptionBudgetSpec(spec, field.NewPath("foo"))
 		if len(errs) != 0 {
@@ -52,7 +66,7 @@ func TestValidatePodDisruptionBudgetSpec(t *testing.T) {
 	}
 	for _, c := range failureCases {
 		spec := policy.PodDisruptionBudgetSpec{
-			MinAvailable: c,
+			MinAvailable: &c,
 		}
 		errs := ValidatePodDisruptionBudgetSpec(spec, field.NewPath("foo"))
 		if len(errs) == 0 {
@@ -61,6 +75,20 @@ func TestValidatePodDisruptionBudgetSpec(t *testing.T) {
 	}
 }
 
+func TestValidateMinAvailablePodAndMaxUnavailableDisruptionBudgetSpec(t *testing.T) {
+	c1 := intstr.FromString("10%")
+	c2 := intstr.FromInt(1)
+
+	spec := policy.PodDisruptionBudgetSpec{
+		MinAvailable:   &c1,
+		MaxUnavailable: &c2,
+	}
+	errs := ValidatePodDisruptionBudgetSpec(spec, field.NewPath("foo"))
+	if len(errs) == 0 {
+		t.Errorf("unexpected success for %v", spec)
+	}
+}
+
 func TestValidatePodDisruptionBudgetStatus(t *testing.T) {
 	successCases := []policy.PodDisruptionBudgetStatus{
 		{PodDisruptionsAllowed: 10},

pkg/registry/policy/poddisruptionbudget/storage/storage_test.go

@@ -51,6 +51,7 @@ func createPodDisruptionBudget(storage *REST, pdb policy.PodDisruptionBudget, t
 }
 
 func validNewPodDisruptionBudget() *policy.PodDisruptionBudget {
+	minAvailable := intstr.FromInt(7)
 	return &policy.PodDisruptionBudget{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "foo",
@@ -59,7 +60,7 @@ func validNewPodDisruptionBudget() *policy.PodDisruptionBudget {
 		},
 		Spec: policy.PodDisruptionBudgetSpec{
 			Selector:     &metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}},
-			MinAvailable: intstr.FromInt(7),
+			MinAvailable: &minAvailable,
 		},
 		Status: policy.PodDisruptionBudgetStatus{},
 	}
@@ -98,10 +99,11 @@ func TestStatusUpdate(t *testing.T) {
 	}
 	obtainedPdb := obj.(*policy.PodDisruptionBudget)
 
+	minAvailable := intstr.FromInt(8)
 	update := policy.PodDisruptionBudget{
 		ObjectMeta: obtainedPdb.ObjectMeta,
 		Spec: policy.PodDisruptionBudgetSpec{
-			MinAvailable: intstr.FromInt(8),
+			MinAvailable: &minAvailable,
 		},
 		Status: policy.PodDisruptionBudgetStatus{
 			ExpectedPods: 8,

pkg/registry/policy/poddisruptionbudget/strategy_test.go

@@ -35,10 +35,11 @@ func TestPodDisruptionBudgetStrategy(t *testing.T) {
 	}
 
 	validSelector := map[string]string{"a": "b"}
+	minAvailable := intstr.FromInt(3)
 	pdb := &policy.PodDisruptionBudget{
 		ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
 		Spec: policy.PodDisruptionBudgetSpec{
-			MinAvailable: intstr.FromInt(3),
+			MinAvailable: &minAvailable,
 			Selector:     &metav1.LabelSelector{MatchLabels: validSelector},
 		},
 	}
@@ -77,7 +78,16 @@ func TestPodDisruptionBudgetStrategy(t *testing.T) {
 	newPdb.Spec.Selector = pdb.Spec.Selector
 
 	// Changing MinAvailable?  Also no.
-	newPdb.Spec.MinAvailable = intstr.FromString("28%")
+	newMinAvailable := intstr.FromString("28%")
+	newPdb.Spec.MinAvailable = &newMinAvailable
+	Strategy.PrepareForUpdate(ctx, newPdb, pdb)
+	errs = Strategy.ValidateUpdate(ctx, newPdb, pdb)
+	if len(errs) == 0 {
+		t.Errorf("Expected a validation error since updates are disallowed on poddisruptionbudgets.")
+	}
+
+	maxUnavailable := intstr.FromString("28%")
+	newPdb.Spec.MaxUnavailable = &maxUnavailable
 	Strategy.PrepareForUpdate(ctx, newPdb, pdb)
 	errs = Strategy.ValidateUpdate(ctx, newPdb, pdb)
 	if len(errs) == 0 {
@@ -93,12 +103,16 @@ func TestPodDisruptionBudgetStatusStrategy(t *testing.T) {
 	if StatusStrategy.AllowCreateOnUpdate() {
 		t.Errorf("PodDisruptionBudgetStatus should not allow create on update")
 	}
+
+	oldMinAvailable := intstr.FromInt(3)
+	newMinAvailable := intstr.FromInt(2)
+
 	validSelector := map[string]string{"a": "b"}
 	oldPdb := &policy.PodDisruptionBudget{
 		ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault, ResourceVersion: "10"},
 		Spec: policy.PodDisruptionBudgetSpec{
 			Selector:     &metav1.LabelSelector{MatchLabels: validSelector},
-			MinAvailable: intstr.FromInt(3),
+			MinAvailable: &oldMinAvailable,
 		},
 		Status: policy.PodDisruptionBudgetStatus{
 			PodDisruptionsAllowed: 1,
@@ -111,7 +125,7 @@ func TestPodDisruptionBudgetStatusStrategy(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault, ResourceVersion: "9"},
 		Spec: policy.PodDisruptionBudgetSpec{
 			Selector:     &metav1.LabelSelector{MatchLabels: validSelector},
-			MinAvailable: intstr.FromInt(2),
+			MinAvailable: &newMinAvailable,
 		},
 		Status: policy.PodDisruptionBudgetStatus{
 			PodDisruptionsAllowed: 0,