diff --git a/pkg/apis/policy/types.go b/pkg/apis/policy/types.go index ef2ff713ad2..4d49da17471 100644 --- a/pkg/apis/policy/types.go +++ b/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 diff --git a/pkg/apis/policy/v1beta1/types.go b/pkg/apis/policy/v1beta1/types.go index ca5ea37306a..6cc56256a6c 100644 --- a/pkg/apis/policy/v1beta1/types.go +++ b/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 diff --git a/pkg/apis/policy/validation/validation.go b/pkg/apis/policy/validation/validation.go index 88471b416ec..528a9bc5218 100644 --- a/pkg/apis/policy/validation/validation.go +++ b/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 diff --git a/pkg/apis/policy/validation/validation_test.go b/pkg/apis/policy/validation/validation_test.go index 7dfc403ad9a..a11454374c3 100644 --- a/pkg/apis/policy/validation/validation_test.go +++ b/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}, diff --git a/pkg/registry/policy/poddisruptionbudget/storage/storage_test.go b/pkg/registry/policy/poddisruptionbudget/storage/storage_test.go index aec19efa0d1..ab8e8851e49 100644 --- a/pkg/registry/policy/poddisruptionbudget/storage/storage_test.go +++ b/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, diff --git a/pkg/registry/policy/poddisruptionbudget/strategy_test.go b/pkg/registry/policy/poddisruptionbudget/strategy_test.go index 33cf6f54257..579e28c9690 100644 --- a/pkg/registry/policy/poddisruptionbudget/strategy_test.go +++ b/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,