Add observedGeneration to PodDisruptionBudgetStatus

pkg/apis/policy/types.go

@@ -40,17 +40,10 @@ type PodDisruptionBudgetSpec struct {
 // PodDisruptionBudgetStatus represents information about the status of a
 // PodDisruptionBudget. Status may trail the actual state of a system.
 type PodDisruptionBudgetStatus struct {
-	// Number of pod disruptions that are currently allowed.
+	// Most recent generation observed when updating this PDB status. PodDisruptionsAllowed and other
-	PodDisruptionsAllowed int32 `json:"disruptionsAllowed"`
+	// status informatio is valid only if observedGeneration equals to PDB's object generation.
-
+	// +optional
-	// current number of healthy pods
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
-	CurrentHealthy int32 `json:"currentHealthy"`
-
-	// minimum desired number of healthy pods
-	DesiredHealthy int32 `json:"desiredHealthy"`
-
-	// total number of pods counted by this disruption budget
-	ExpectedPods int32 `json:"expectedPods"`
 
 	// DisruptedPods contains information about pods whose eviction was
 	// processed by the API server eviction subresource handler but has not
@@ -64,6 +57,18 @@ type PodDisruptionBudgetStatus struct {
 	// If everything goes smooth this map should be empty for the most of the time.
 	// Large number of entries in the map may indicate problems with pod deletions.
 	DisruptedPods map[string]unversioned.Time `json:"disruptedPods" protobuf:"bytes,5,rep,name=disruptedPods"`
+
+	// Number of pod disruptions that are currently allowed.
+	PodDisruptionsAllowed int32 `json:"disruptionsAllowed"`
+
+	// current number of healthy pods
+	CurrentHealthy int32 `json:"currentHealthy"`
+
+	// minimum desired number of healthy pods
+	DesiredHealthy int32 `json:"desiredHealthy"`
+
+	// total number of pods counted by this disruption budget
+	ExpectedPods int32 `json:"expectedPods"`
 }
 
 // +genclient=true

pkg/apis/policy/v1beta1/types.go

@@ -38,17 +38,10 @@ type PodDisruptionBudgetSpec struct {
 // PodDisruptionBudgetStatus represents information about the status of a
 // PodDisruptionBudget. Status may trail the actual state of a system.
 type PodDisruptionBudgetStatus struct {
-	// Number of pod disruptions that are currently allowed.
+	// Most recent generation observed when updating this PDB status. PodDisruptionsAllowed and other
-	PodDisruptionsAllowed int32 `json:"disruptionsAllowed" protobuf:"varint,1,opt,name=disruptionsAllowed"`
+	// status informatio is valid only if observedGeneration equals to PDB's object generation.
-
+	// +optional
-	// current number of healthy pods
+	ObservedGeneration int64 `json:"observedGeneration,omitempty" protobuf:"varint,1,opt,name=observedGeneration"`
-	CurrentHealthy int32 `json:"currentHealthy" protobuf:"varint,2,opt,name=currentHealthy"`
-
-	// minimum desired number of healthy pods
-	DesiredHealthy int32 `json:"desiredHealthy" protobuf:"varint,3,opt,name=desiredHealthy"`
-
-	// total number of pods counted by this disruption budget
-	ExpectedPods int32 `json:"expectedPods" protobuf:"varint,4,opt,name=expectedPods"`
 
 	// DisruptedPods contains information about pods whose eviction was
 	// processed by the API server eviction subresource handler but has not
@@ -61,7 +54,19 @@ type PodDisruptionBudgetStatus struct {
 	// the list automatically by PodDisruptionBudget controller after some time.
 	// If everything goes smooth this map should be empty for the most of the time.
 	// Large number of entries in the map may indicate problems with pod deletions.
-	DisruptedPods map[string]unversioned.Time `json:"disruptedPods" protobuf:"bytes,5,rep,name=disruptedPods"`
+	DisruptedPods map[string]unversioned.Time `json:"disruptedPods" protobuf:"bytes,2,rep,name=disruptedPods"`
+
+	// Number of pod disruptions that are currently allowed.
+	PodDisruptionsAllowed int32 `json:"disruptionsAllowed" protobuf:"varint,3,opt,name=disruptionsAllowed"`
+
+	// current number of healthy pods
+	CurrentHealthy int32 `json:"currentHealthy" protobuf:"varint,4,opt,name=currentHealthy"`
+
+	// minimum desired number of healthy pods
+	DesiredHealthy int32 `json:"desiredHealthy" protobuf:"varint,5,opt,name=desiredHealthy"`
+
+	// total number of pods counted by this disruption budget
+	ExpectedPods int32 `json:"expectedPods" protobuf:"varint,6,opt,name=expectedPods"`
 }
 
 // +genclient=true

pkg/controller/disruption/disruption.go

@@ -669,7 +669,8 @@ func (dc *DisruptionController) updatePdbStatus(pdb *policy.PodDisruptionBudget,
 		pdb.Status.DesiredHealthy == desiredHealthy &&
 		pdb.Status.ExpectedPods == expectedCount &&
 		pdb.Status.PodDisruptionsAllowed == disruptionsAllowed &&
-		reflect.DeepEqual(pdb.Status.DisruptedPods, disruptedPods) {
+		reflect.DeepEqual(pdb.Status.DisruptedPods, disruptedPods) &&
+		pdb.Status.ObservedGeneration == pdb.Generation {
 		return nil
 	}
 
@@ -685,6 +686,7 @@ func (dc *DisruptionController) updatePdbStatus(pdb *policy.PodDisruptionBudget,
 		ExpectedPods:          expectedCount,
 		PodDisruptionsAllowed: disruptionsAllowed,
 		DisruptedPods:         disruptedPods,
+		ObservedGeneration:    pdb.Generation,
 	}
 
 	return dc.getUpdater()(&newPdb)

pkg/controller/disruption/disruption_test.go

@@ -58,14 +58,16 @@ func (ps *pdbStates) Get(key string) policy.PodDisruptionBudget {
 
 func (ps *pdbStates) VerifyPdbStatus(t *testing.T, key string, disruptionsAllowed, currentHealthy, desiredHealthy, expectedPods int32,
 	disruptedPodMap map[string]unversioned.Time) {
+	actualPDB := ps.Get(key)
 	expectedStatus := policy.PodDisruptionBudgetStatus{
 		PodDisruptionsAllowed: disruptionsAllowed,
 		CurrentHealthy:        currentHealthy,
 		DesiredHealthy:        desiredHealthy,
 		ExpectedPods:          expectedPods,
 		DisruptedPods:         disruptedPodMap,
+		ObservedGeneration:    actualPDB.Generation,
 	}
-	actualStatus := ps.Get(key).Status
+	actualStatus := actualPDB.Status
 	if !reflect.DeepEqual(actualStatus, expectedStatus) {
 		debug.PrintStack()
 		t.Fatalf("PDB %q status mismatch.  Expected %+v but got %+v.", key, expectedStatus, actualStatus)

pkg/registry/core/pod/etcd/eviction.go

@@ -116,6 +116,9 @@ func (r *EvictionREST) Create(ctx api.Context, obj runtime.Object) (runtime.Obje
 }
 
 func (r *EvictionREST) checkAndDecrement(namespace string, podName string, pdb policy.PodDisruptionBudget) (ok bool, err error) {
+	if pdb.Status.ObservedGeneration != pdb.Generation {
+		return false, nil
+	}
 	if pdb.Status.PodDisruptionsAllowed < 0 {
 		return false, fmt.Errorf("pdb disruptions allowed is negative")
 	}

pkg/registry/policy/poddisruptionbudget/etcd/etcd_test.go

@@ -91,8 +91,15 @@ func TestStatusUpdate(t *testing.T) {
 	if err := storage.Storage.Create(ctx, key, validPodDisruptionBudget, nil, 0); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
+
+	obj, err := storage.Get(ctx, "foo")
+	if err != nil {
+		t.Fatalf("failed to get pdb: %v", err)
+	}
+	obtainedPdb := obj.(*policy.PodDisruptionBudget)
+
 	update := policy.PodDisruptionBudget{
-		ObjectMeta: validPodDisruptionBudget.ObjectMeta,
+		ObjectMeta: obtainedPdb.ObjectMeta,
 		Spec: policy.PodDisruptionBudgetSpec{
 			MinAvailable: intstr.FromInt(8),
 		},
@@ -104,7 +111,7 @@ func TestStatusUpdate(t *testing.T) {
 	if _, _, err := statusStorage.Update(ctx, update.Name, rest.DefaultUpdatedObjectInfo(&update, api.Scheme)); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	obj, err := storage.Get(ctx, "foo")
+	obj, err = storage.Get(ctx, "foo")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}

pkg/registry/policy/poddisruptionbudget/strategy.go

@@ -91,9 +91,10 @@ func (podDisruptionBudgetStrategy) ValidateUpdate(ctx api.Context, obj, old runt
 	return append(validationErrorList, updateErrorList...)
 }
 
-// AllowUnconditionalUpdate is the default update policy for PodDisruptionBudget objects.
+// AllowUnconditionalUpdate is the default update policy for PodDisruptionBudget objects. Status update should
+// only be allowed if version match.
 func (podDisruptionBudgetStrategy) AllowUnconditionalUpdate() bool {
-	return true
+	return false
 }
 
 // PodDisruptionBudgetToSelectableFields returns a field set that represents the object.