From 3537eed826c5e3d4b329d73e156e45ec165fe513 Mon Sep 17 00:00:00 2001 From: David McCormick Date: Fri, 26 Apr 2019 10:36:54 +0100 Subject: [PATCH] Remove the generation altering code - validate an update for a PDB by running ValidatePodDisruptionBudget only. --- pkg/apis/policy/validation/validation.go | 11 -- pkg/apis/policy/validation/validation_test.go | 105 ------------------ .../policy/poddisruptionbudget/strategy.go | 4 +- 3 files changed, 1 insertion(+), 119 deletions(-) diff --git a/pkg/apis/policy/validation/validation.go b/pkg/apis/policy/validation/validation.go index 386d809231a..2d880a20968 100644 --- a/pkg/apis/policy/validation/validation.go +++ b/pkg/apis/policy/validation/validation.go @@ -41,17 +41,6 @@ func ValidatePodDisruptionBudget(pdb *policy.PodDisruptionBudget) field.ErrorLis return allErrs } -func ValidatePodDisruptionBudgetUpdate(pdb, oldPdb *policy.PodDisruptionBudget) field.ErrorList { - restoreGeneration := pdb.Generation - pdb.Generation = oldPdb.Generation - - allErrs := ValidatePodDisruptionBudgetSpec(pdb.Spec, field.NewPath("spec")) - allErrs = append(allErrs, ValidatePodDisruptionBudgetStatus(pdb.Status, field.NewPath("status"))...) - - pdb.Generation = restoreGeneration - return allErrs -} - func ValidatePodDisruptionBudgetSpec(spec policy.PodDisruptionBudgetSpec, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apis/policy/validation/validation_test.go b/pkg/apis/policy/validation/validation_test.go index 259f929b275..c390097bcc3 100644 --- a/pkg/apis/policy/validation/validation_test.go +++ b/pkg/apis/policy/validation/validation_test.go @@ -122,111 +122,6 @@ func TestValidatePodDisruptionBudgetStatus(t *testing.T) { } } -func TestValidatePodDisruptionBudgetUpdate(t *testing.T) { - c1 := intstr.FromString("10%") - c2 := intstr.FromInt(1) - c3 := intstr.FromInt(2) - oldPdb := &policy.PodDisruptionBudget{} - pdb := &policy.PodDisruptionBudget{} - testCases := []struct { - generations []int64 - name string - specs []policy.PodDisruptionBudgetSpec - status []policy.PodDisruptionBudgetStatus - ok bool - }{ - { - name: "only update status", - generations: []int64{int64(2), int64(3)}, - specs: []policy.PodDisruptionBudgetSpec{ - { - MinAvailable: &c1, - }, - { - MinAvailable: &c1, - }, - }, - status: []policy.PodDisruptionBudgetStatus{ - { - PodDisruptionsAllowed: 10, - CurrentHealthy: 5, - ExpectedPods: 2, - }, - { - PodDisruptionsAllowed: 8, - CurrentHealthy: 5, - DesiredHealthy: 3, - }, - }, - ok: true, - }, - { - name: "update pdb spec causing clash", - generations: []int64{int64(2), int64(3)}, - specs: []policy.PodDisruptionBudgetSpec{ - { - MaxUnavailable: &c2, - }, - { - MinAvailable: &c1, - MaxUnavailable: &c3, - }, - }, - status: []policy.PodDisruptionBudgetStatus{ - { - PodDisruptionsAllowed: 10, - }, - { - PodDisruptionsAllowed: 10, - }, - }, - ok: false, - }, - { - name: "update spec and status", - generations: []int64{int64(2), int64(3)}, - specs: []policy.PodDisruptionBudgetSpec{ - { - MaxUnavailable: &c2, - }, - { - MaxUnavailable: &c3, - }, - }, - status: []policy.PodDisruptionBudgetStatus{ - { - PodDisruptionsAllowed: 10, - CurrentHealthy: 5, - ExpectedPods: 2, - }, - { - PodDisruptionsAllowed: 8, - CurrentHealthy: 5, - DesiredHealthy: 3, - }, - }, - ok: true, - }, - } - - for i, tc := range testCases { - oldPdb.Spec = tc.specs[0] - oldPdb.Generation = tc.generations[0] - oldPdb.Status = tc.status[0] - - pdb.Spec = tc.specs[1] - pdb.Generation = tc.generations[1] - pdb.Status = tc.status[1] - - errs := ValidatePodDisruptionBudgetUpdate(pdb, oldPdb) - if tc.ok && len(errs) > 0 { - t.Errorf("[%d:%s] unexpected errors: %v", i, tc.name, errs) - } else if !tc.ok && len(errs) == 0 { - t.Errorf("[%d:%s] expected errors: %v", i, tc.name, errs) - } - } -} - func TestValidatePodSecurityPolicy(t *testing.T) { validPSP := func() *policy.PodSecurityPolicy { return &policy.PodSecurityPolicy{ diff --git a/pkg/registry/policy/poddisruptionbudget/strategy.go b/pkg/registry/policy/poddisruptionbudget/strategy.go index 0ee30f3b9d7..5a4f8dc2122 100644 --- a/pkg/registry/policy/poddisruptionbudget/strategy.go +++ b/pkg/registry/policy/poddisruptionbudget/strategy.go @@ -83,9 +83,7 @@ func (podDisruptionBudgetStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (podDisruptionBudgetStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - validationErrorList := validation.ValidatePodDisruptionBudget(obj.(*policy.PodDisruptionBudget)) - updateErrorList := validation.ValidatePodDisruptionBudgetUpdate(obj.(*policy.PodDisruptionBudget), old.(*policy.PodDisruptionBudget)) - return append(validationErrorList, updateErrorList...) + return validation.ValidatePodDisruptionBudget(obj.(*policy.PodDisruptionBudget)) } // AllowUnconditionalUpdate is the default update policy for PodDisruptionBudget objects. Status update should