Merge pull request #48706 from dixudx/fix_bug_validating_pdb_spec

Automatic merge from submit-queue

fix pdb validation bug on spec

**What this PR does / why we need it**:

When working on issue #48691, I found a bug on current [pdb validation on spec](https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/policy/validation/validation.go#L41-L42). This validation is to check `PodDisruptionBudgetSpec` instead of the whole obj. Currently when doing updates, this check will fail as always.

**Which issue this PR fixes** : fix #48691

**Special notes for your reviewer**:

**Release note**:

```release-note
fix pdb validation bug on PodDisruptionBudgetSpec
```

pkg/apis/policy/validation/validation.go

@@ -38,7 +38,7 @@ func ValidatePodDisruptionBudgetUpdate(pdb, oldPdb *policy.PodDisruptionBudget)
 	restoreGeneration := pdb.Generation
 	pdb.Generation = oldPdb.Generation
 
-	if !reflect.DeepEqual(pdb, oldPdb) {
+	if !reflect.DeepEqual(pdb.Spec, oldPdb.Spec) {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to poddisruptionbudget spec are forbidden."))
 	}
 	allErrs = append(allErrs, ValidatePodDisruptionBudgetStatus(pdb.Status, field.NewPath("status"))...)

pkg/apis/policy/validation/validation_test.go

@@ -113,3 +113,111 @@ 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,
+					MaxUnavailable: &c2,
+				},
+				{
+					MinAvailable:   &c1,
+					MaxUnavailable: &c2,
+				},
+			},
+			status: []policy.PodDisruptionBudgetStatus{
+				{
+					PodDisruptionsAllowed: 10,
+					CurrentHealthy:        5,
+					ExpectedPods:          2,
+				},
+				{
+					PodDisruptionsAllowed: 8,
+					CurrentHealthy:        5,
+					DesiredHealthy:        3,
+				},
+			},
+			ok: true,
+		},
+		{
+			name:        "only update pdb spec",
+			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,
+				},
+				{
+					MinAvailable:   &c1,
+					MaxUnavailable: &c3,
+				},
+			},
+			status: []policy.PodDisruptionBudgetStatus{
+				{
+					PodDisruptionsAllowed: 10,
+					CurrentHealthy:        5,
+					ExpectedPods:          2,
+				},
+				{
+					PodDisruptionsAllowed: 8,
+					CurrentHealthy:        5,
+					DesiredHealthy:        3,
+				},
+			},
+			ok: false,
+		},
+	}
+
+	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]
+		oldPdb.Status = tc.status[1]
+
+		errs := ValidatePodDisruptionBudgetUpdate(oldPdb, pdb)
+		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)
+		}
+	}
+}