From 1e2a7f381f6abdf9cf80681c3355851ae0a8187d Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Wed, 3 Mar 2021 19:40:47 -0800 Subject: [PATCH] Add conditions to PDB status --- pkg/apis/policy/types.go | 4 + pkg/apis/policy/validation/validation.go | 13 +- pkg/apis/policy/validation/validation_test.go | 165 ++++++++++++++++-- pkg/controller/disruption/disruption.go | 26 ++- pkg/controller/disruption/disruption_test.go | 19 ++ pkg/registry/core/pod/storage/eviction.go | 5 + .../core/pod/storage/eviction_test.go | 98 +++++++++++ .../policy/poddisruptionbudget/strategy.go | 14 +- .../src/k8s.io/api/policy/v1beta1/types.go | 36 ++++ .../src/k8s.io/component-helpers/apps/OWNERS | 8 + .../apps/poddisruptionbudget/helpers.go | 65 +++++++ 11 files changed, 424 insertions(+), 29 deletions(-) create mode 100644 staging/src/k8s.io/component-helpers/apps/OWNERS create mode 100644 staging/src/k8s.io/component-helpers/apps/poddisruptionbudget/helpers.go diff --git a/pkg/apis/policy/types.go b/pkg/apis/policy/types.go index 5dcee4cdac5..e4ef8360a73 100644 --- a/pkg/apis/policy/types.go +++ b/pkg/apis/policy/types.go @@ -77,6 +77,10 @@ type PodDisruptionBudgetStatus struct { // total number of pods counted by this disruption budget ExpectedPods int32 + + // Conditions contain conditions for PDB + // +optional + Conditions []metav1.Condition } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/policy/validation/validation.go b/pkg/apis/policy/validation/validation.go index 35281eeadbe..51e4c211ff8 100644 --- a/pkg/apis/policy/validation/validation.go +++ b/pkg/apis/policy/validation/validation.go @@ -23,8 +23,10 @@ import ( "strings" "k8s.io/api/core/v1" + policyapiv1beta1 "k8s.io/api/policy/v1beta1" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" unversionedvalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" appsvalidation "k8s.io/kubernetes/pkg/apis/apps/validation" @@ -40,7 +42,6 @@ import ( // with any errors. func ValidatePodDisruptionBudget(pdb *policy.PodDisruptionBudget) field.ErrorList { allErrs := ValidatePodDisruptionBudgetSpec(pdb.Spec, field.NewPath("spec")) - allErrs = append(allErrs, ValidatePodDisruptionBudgetStatus(pdb.Status, field.NewPath("status"))...) return allErrs } @@ -68,10 +69,16 @@ func ValidatePodDisruptionBudgetSpec(spec policy.PodDisruptionBudgetSpec, fldPat return allErrs } -// ValidatePodDisruptionBudgetStatus validates a PodDisruptionBudgetStatus and returns an ErrorList +// ValidatePodDisruptionBudgetStatusUpdate validates a PodDisruptionBudgetStatus and returns an ErrorList // with any errors. -func ValidatePodDisruptionBudgetStatus(status policy.PodDisruptionBudgetStatus, fldPath *field.Path) field.ErrorList { +func ValidatePodDisruptionBudgetStatusUpdate(status, oldStatus policy.PodDisruptionBudgetStatus, fldPath *field.Path, apiVersion schema.GroupVersion) field.ErrorList { allErrs := field.ErrorList{} + allErrs = append(allErrs, unversionedvalidation.ValidateConditions(status.Conditions, fldPath.Child("conditions"))...) + // Don't run other validations for v1beta1 since we don't want to introduce + // new validations retroactively. + if apiVersion == policyapiv1beta1.SchemeGroupVersion { + return allErrs + } allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.DisruptionsAllowed), fldPath.Child("disruptionsAllowed"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.CurrentHealthy), fldPath.Child("currentHealthy"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.DesiredHealthy), fldPath.Child("desiredHealthy"))...) diff --git a/pkg/apis/policy/validation/validation_test.go b/pkg/apis/policy/validation/validation_test.go index 4c5f68912b9..7b74a1c3b87 100644 --- a/pkg/apis/policy/validation/validation_test.go +++ b/pkg/apis/policy/validation/validation_test.go @@ -19,10 +19,13 @@ package validation import ( "fmt" "testing" + "time" "github.com/stretchr/testify/assert" "k8s.io/api/core/v1" + policyv1beta1 "k8s.io/api/policy/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation/field" api "k8s.io/kubernetes/pkg/apis/core" @@ -98,26 +101,150 @@ func TestValidateMinAvailablePodAndMaxUnavailableDisruptionBudgetSpec(t *testing } func TestValidatePodDisruptionBudgetStatus(t *testing.T) { - successCases := []policy.PodDisruptionBudgetStatus{ - {DisruptionsAllowed: 10}, - {CurrentHealthy: 5}, - {DesiredHealthy: 3}, - {ExpectedPods: 2}} - for _, c := range successCases { - errors := ValidatePodDisruptionBudgetStatus(c, field.NewPath("status")) - if len(errors) > 0 { - t.Errorf("unexpected failure %v for %v", errors, c) - } + const expectNoErrors = false + const expectErrors = true + testCases := []struct { + name string + pdbStatus policy.PodDisruptionBudgetStatus + expectErrForVersion map[schema.GroupVersion]bool + }{ + { + name: "DisruptionsAllowed: 10", + pdbStatus: policy.PodDisruptionBudgetStatus{ + DisruptionsAllowed: 10, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectNoErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "CurrentHealthy: 5", + pdbStatus: policy.PodDisruptionBudgetStatus{ + CurrentHealthy: 5, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectNoErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "DesiredHealthy: 3", + pdbStatus: policy.PodDisruptionBudgetStatus{ + DesiredHealthy: 3, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectNoErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "ExpectedPods: 2", + pdbStatus: policy.PodDisruptionBudgetStatus{ + ExpectedPods: 2, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectNoErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "DisruptionsAllowed: -10", + pdbStatus: policy.PodDisruptionBudgetStatus{ + DisruptionsAllowed: -10, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "CurrentHealthy: -5", + pdbStatus: policy.PodDisruptionBudgetStatus{ + CurrentHealthy: -5, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "DesiredHealthy: -3", + pdbStatus: policy.PodDisruptionBudgetStatus{ + DesiredHealthy: -3, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "ExpectedPods: -2", + pdbStatus: policy.PodDisruptionBudgetStatus{ + ExpectedPods: -2, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "Conditions valid", + pdbStatus: policy.PodDisruptionBudgetStatus{ + Conditions: []metav1.Condition{ + { + Type: policyv1beta1.DisruptionAllowedCondition, + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Time{ + Time: time.Now().Add(-5 * time.Minute), + }, + Reason: policyv1beta1.SufficientPodsReason, + Message: "message", + ObservedGeneration: 3, + }, + }, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectNoErrors, + policyv1beta1.SchemeGroupVersion: expectNoErrors, + }, + }, + { + name: "Conditions not valid", + pdbStatus: policy.PodDisruptionBudgetStatus{ + Conditions: []metav1.Condition{ + { + Type: policyv1beta1.DisruptionAllowedCondition, + Status: metav1.ConditionTrue, + }, + { + Type: policyv1beta1.DisruptionAllowedCondition, + Status: metav1.ConditionFalse, + }, + }, + }, + expectErrForVersion: map[schema.GroupVersion]bool{ + policy.SchemeGroupVersion: expectErrors, + policyv1beta1.SchemeGroupVersion: expectErrors, + }, + }, } - failureCases := []policy.PodDisruptionBudgetStatus{ - {DisruptionsAllowed: -10}, - {CurrentHealthy: -5}, - {DesiredHealthy: -3}, - {ExpectedPods: -2}} - for _, c := range failureCases { - errors := ValidatePodDisruptionBudgetStatus(c, field.NewPath("status")) - if len(errors) == 0 { - t.Errorf("unexpected success for %v", c) + + for _, tc := range testCases { + for apiVersion, expectErrors := range tc.expectErrForVersion { + t.Run(fmt.Sprintf("apiVersion: %s, %s", apiVersion.String(), tc.name), func(t *testing.T) { + errors := ValidatePodDisruptionBudgetStatusUpdate(tc.pdbStatus, policy.PodDisruptionBudgetStatus{}, + field.NewPath("status"), apiVersion) + errCount := len(errors) + + if errCount > 0 && !expectErrors { + t.Errorf("unexpected failure %v for %v", errors, tc.pdbStatus) + } + + if errCount == 0 && expectErrors { + t.Errorf("expected errors but didn't one for %v", tc.pdbStatus) + } + }) } } } diff --git a/pkg/controller/disruption/disruption.go b/pkg/controller/disruption/disruption.go index 99cf4ecfa22..2b7abf2780d 100644 --- a/pkg/controller/disruption/disruption.go +++ b/pkg/controller/disruption/disruption.go @@ -49,6 +49,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" + pdbhelper "k8s.io/component-helpers/apps/poddisruptionbudget" "k8s.io/klog/v2" podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/controller" @@ -63,7 +64,9 @@ import ( // If the controller is running on a different node it is important that the two nodes have synced // clock (via ntp for example). Otherwise PodDisruptionBudget controller may not provide enough // protection against unwanted pod disruptions. -const DeletionTimeout = 2 * 60 * time.Second +const ( + DeletionTimeout = 2 * 60 * time.Second +) type updater func(*policy.PodDisruptionBudget) error @@ -579,7 +582,7 @@ func (dc *DisruptionController) sync(key string) error { } if err != nil { klog.Errorf("Failed to sync pdb %s/%s: %v", pdb.Namespace, pdb.Name, err) - return dc.failSafe(pdb) + return dc.failSafe(pdb, err) } return nil @@ -774,9 +777,21 @@ func (dc *DisruptionController) buildDisruptedPodMap(pods []*v1.Pod, pdb *policy // implement the "fail open" part of the design since if we manage to update // this field correctly, we will prevent the /evict handler from approving an // eviction when it may be unsafe to do so. -func (dc *DisruptionController) failSafe(pdb *policy.PodDisruptionBudget) error { +func (dc *DisruptionController) failSafe(pdb *policy.PodDisruptionBudget, err error) error { newPdb := pdb.DeepCopy() newPdb.Status.DisruptionsAllowed = 0 + + if newPdb.Status.Conditions == nil { + newPdb.Status.Conditions = make([]metav1.Condition, 0) + } + apimeta.SetStatusCondition(&newPdb.Status.Conditions, metav1.Condition{ + Type: policy.DisruptionAllowedCondition, + Status: metav1.ConditionFalse, + Reason: policy.SyncFailedReason, + Message: err.Error(), + ObservedGeneration: newPdb.Status.ObservedGeneration, + }) + return dc.getUpdater()(newPdb) } @@ -797,7 +812,8 @@ func (dc *DisruptionController) updatePdbStatus(pdb *policy.PodDisruptionBudget, pdb.Status.ExpectedPods == expectedCount && pdb.Status.DisruptionsAllowed == disruptionsAllowed && apiequality.Semantic.DeepEqual(pdb.Status.DisruptedPods, disruptedPods) && - pdb.Status.ObservedGeneration == pdb.Generation { + pdb.Status.ObservedGeneration == pdb.Generation && + pdbhelper.ConditionsAreUpToDate(pdb) { return nil } @@ -811,6 +827,8 @@ func (dc *DisruptionController) updatePdbStatus(pdb *policy.PodDisruptionBudget, ObservedGeneration: pdb.Generation, } + pdbhelper.UpdateDisruptionAllowedCondition(newPdb) + return dc.getUpdater()(newPdb) } diff --git a/pkg/controller/disruption/disruption_test.go b/pkg/controller/disruption/disruption_test.go index b72a53adf00..8e47266964b 100644 --- a/pkg/controller/disruption/disruption_test.go +++ b/pkg/controller/disruption/disruption_test.go @@ -32,6 +32,7 @@ import ( policy "k8s.io/api/policy/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/meta/testrestmapper" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -73,6 +74,8 @@ 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]metav1.Time) { actualPDB := ps.Get(key) + actualConditions := actualPDB.Status.Conditions + actualPDB.Status.Conditions = nil expectedStatus := policy.PodDisruptionBudgetStatus{ DisruptionsAllowed: disruptionsAllowed, CurrentHealthy: currentHealthy, @@ -86,6 +89,22 @@ func (ps *pdbStates) VerifyPdbStatus(t *testing.T, key string, disruptionsAllowe debug.PrintStack() t.Fatalf("PDB %q status mismatch. Expected %+v but got %+v.", key, expectedStatus, actualStatus) } + + cond := apimeta.FindStatusCondition(actualConditions, policy.DisruptionAllowedCondition) + if cond == nil { + t.Fatalf("Expected condition %q, but didn't find it", policy.DisruptionAllowedCondition) + } + if disruptionsAllowed > 0 { + if cond.Status != metav1.ConditionTrue { + t.Fatalf("Expected condition %q to have status %q, but was %q", + policy.DisruptionAllowedCondition, metav1.ConditionTrue, cond.Status) + } + } else { + if cond.Status != metav1.ConditionFalse { + t.Fatalf("Expected condition %q to have status %q, but was %q", + policy.DisruptionAllowedCondition, metav1.ConditionFalse, cond.Status) + } + } } func (ps *pdbStates) VerifyDisruptionAllowed(t *testing.T, key string, disruptionsAllowed int32) { diff --git a/pkg/registry/core/pod/storage/eviction.go b/pkg/registry/core/pod/storage/eviction.go index 2cea974599e..ec1899b614d 100644 --- a/pkg/registry/core/pod/storage/eviction.go +++ b/pkg/registry/core/pod/storage/eviction.go @@ -33,6 +33,7 @@ import ( "k8s.io/apiserver/pkg/util/dryrun" policyclient "k8s.io/client-go/kubernetes/typed/policy/v1beta1" "k8s.io/client-go/util/retry" + pdbhelper "k8s.io/component-helpers/apps/poddisruptionbudget" podutil "k8s.io/kubernetes/pkg/api/pod" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/policy" @@ -331,6 +332,10 @@ func (r *EvictionREST) checkAndDecrement(namespace string, podName string, pdb p } pdb.Status.DisruptionsAllowed-- + if pdb.Status.DisruptionsAllowed == 0 { + pdbhelper.UpdateDisruptionAllowedCondition(&pdb) + } + // If this is a dry-run, we don't need to go any further than that. if dryRun == true { return nil diff --git a/pkg/registry/core/pod/storage/eviction_test.go b/pkg/registry/core/pod/storage/eviction_test.go index 8107506abd1..b15cea5daee 100644 --- a/pkg/registry/core/pod/storage/eviction_test.go +++ b/pkg/registry/core/pod/storage/eviction_test.go @@ -23,6 +23,7 @@ import ( policyv1beta1 "k8s.io/api/policy/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -505,6 +506,103 @@ func TestEvictionDryRun(t *testing.T) { } } +func TestEvictionPDBStatus(t *testing.T) { + testcases := []struct { + name string + pdb *policyv1beta1.PodDisruptionBudget + expectedDisruptionsAllowed int32 + expectedReason string + }{ + { + name: "pdb status is updated after eviction", + pdb: &policyv1beta1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, + Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1beta1.PodDisruptionBudgetStatus{ + DisruptionsAllowed: 1, + Conditions: []metav1.Condition{ + { + Type: policyv1beta1.DisruptionAllowedCondition, + Reason: policyv1beta1.SufficientPodsReason, + Status: metav1.ConditionTrue, + }, + }, + }, + }, + expectedDisruptionsAllowed: 0, + expectedReason: policyv1beta1.InsufficientPodsReason, + }, + { + name: "condition reason is only updated if AllowedDisruptions becomes 0", + pdb: &policyv1beta1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, + Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}}, + Status: policyv1beta1.PodDisruptionBudgetStatus{ + DisruptionsAllowed: 3, + Conditions: []metav1.Condition{ + { + Type: policyv1beta1.DisruptionAllowedCondition, + Reason: policyv1beta1.SufficientPodsReason, + Status: metav1.ConditionTrue, + }, + }, + }, + }, + expectedDisruptionsAllowed: 2, + expectedReason: policyv1beta1.SufficientPodsReason, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceDefault) + storage, _, statusStorage, server := newStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + + client := fake.NewSimpleClientset(tc.pdb) + for _, podName := range []string{"foo-1", "foo-2"} { + pod := validNewPod() + pod.Labels = map[string]string{"a": "true"} + pod.ObjectMeta.Name = podName + pod.Spec.NodeName = "foo" + newPod, err := storage.Create(testContext, pod, nil, &metav1.CreateOptions{}) + if err != nil { + t.Error(err) + } + (newPod.(*api.Pod)).Status.Phase = api.PodRunning + _, _, err = statusStorage.Update(testContext, pod.Name, rest.DefaultUpdatedObjectInfo(newPod), + nil, nil, false, &metav1.UpdateOptions{}) + if err != nil { + t.Error(err) + } + } + + evictionRest := newEvictionStorage(storage.Store, client.PolicyV1beta1()) + eviction := &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo-1", Namespace: "default"}, DeleteOptions: &metav1.DeleteOptions{}} + _, err := evictionRest.Create(testContext, "foo-1", eviction, nil, &metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to run eviction: %v", err) + } + + existingPDB, err := client.PolicyV1beta1().PodDisruptionBudgets(metav1.NamespaceDefault).Get(context.TODO(), tc.pdb.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("%#v", err) + return + } + + if want, got := tc.expectedDisruptionsAllowed, existingPDB.Status.DisruptionsAllowed; got != want { + t.Errorf("expected DisruptionsAllowed to be %d, but got %d", want, got) + } + + cond := apimeta.FindStatusCondition(existingPDB.Status.Conditions, policyv1beta1.DisruptionAllowedCondition) + if want, got := tc.expectedReason, cond.Reason; want != got { + t.Errorf("expected Reason to be %q, but got %q", want, got) + } + }) + } +} + func resource(resource string) schema.GroupResource { return schema.GroupResource{Group: "", Resource: resource} } diff --git a/pkg/registry/policy/poddisruptionbudget/strategy.go b/pkg/registry/policy/poddisruptionbudget/strategy.go index 5a4f8dc2122..b9ed2bf40c1 100644 --- a/pkg/registry/policy/poddisruptionbudget/strategy.go +++ b/pkg/registry/policy/poddisruptionbudget/strategy.go @@ -21,7 +21,9 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/policy" @@ -109,7 +111,13 @@ func (podDisruptionBudgetStatusStrategy) PrepareForUpdate(ctx context.Context, o // ValidateUpdate is the default update validation for an end user updating status func (podDisruptionBudgetStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - // TODO: Validate status updates. - return field.ErrorList{} - // return validation.ValidatePodDisruptionBudgetStatusUpdate(obj.(*policy.PodDisruptionBudget), old.(*policy.PodDisruptionBudget)) + var apiVersion schema.GroupVersion + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + apiVersion = schema.GroupVersion{ + Group: requestInfo.APIVersion, + Version: requestInfo.APIGroup, + } + } + return validation.ValidatePodDisruptionBudgetStatusUpdate(obj.(*policy.PodDisruptionBudget).Status, + old.(*policy.PodDisruptionBudget).Status, field.NewPath("status"), apiVersion) } diff --git a/staging/src/k8s.io/api/policy/v1beta1/types.go b/staging/src/k8s.io/api/policy/v1beta1/types.go index 24998051f81..212abaef377 100644 --- a/staging/src/k8s.io/api/policy/v1beta1/types.go +++ b/staging/src/k8s.io/api/policy/v1beta1/types.go @@ -77,8 +77,44 @@ type PodDisruptionBudgetStatus struct { // total number of pods counted by this disruption budget ExpectedPods int32 `json:"expectedPods" protobuf:"varint,6,opt,name=expectedPods"` + + // Conditions contain conditions for PDB. The disruption controller sets the + // DisruptionAllowed condition. The following are known values for the reason field + // (additional reasons could be added in the future): + // - SyncFailed: The controller encountered an error and wasn't able to compute + // the number of allowed disruptions. Therefore no disruptions are + // allowed and the status of the condition will be False. + // - InsufficientPods: The number of pods are either at or below the number + // required by the PodDisruptionBudget. No disruptions are + // allowed and the status of the condition will be False. + // - SufficientPods: There are more pods than required by the PodDisruptionBudget. + // The condition will be True, and the number of allowed + // disruptions are provided by the disruptionsAllowed property. + // + // +optional + // +patchMergeKey=type + // +patchStrategy=merge + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type", protobuf:"bytes,7,rep,name=conditions"` } +const ( + // DisruptionAllowedCondition is a condition set by the disruption controller + // that signal whether any of the pods covered by the PDB can be disrupted. + DisruptionAllowedCondition = "DisruptionAllowed" + + // SyncFailedReason is set on the DisruptionAllowed condition if reconcile + // of the PDB failed and therefore disruption of pods are not allowed. + SyncFailedReason = "SyncFailed" + // SufficientPodsReason is set on the DisruptionAllowed condition if there are + // more pods covered by the PDB than required and at least one can be disrupted. + SufficientPodsReason = "SufficientPods" + // InsufficientPodsReason is set on the DisruptionAllowed condition if the number + // of pods are equal to or fewer than required by the PDB. + InsufficientPodsReason = "InsufficientPods" +) + // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +k8s:prerelease-lifecycle-gen:introduced=1.5 diff --git a/staging/src/k8s.io/component-helpers/apps/OWNERS b/staging/src/k8s.io/component-helpers/apps/OWNERS new file mode 100644 index 00000000000..6476090e385 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/apps/OWNERS @@ -0,0 +1,8 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +approvers: +- sig-apps-api-approvers +reviewers: +- sig-apps-api-reviewers +labels: +- sig/apps diff --git a/staging/src/k8s.io/component-helpers/apps/poddisruptionbudget/helpers.go b/staging/src/k8s.io/component-helpers/apps/poddisruptionbudget/helpers.go new file mode 100644 index 00000000000..48629d09fd0 --- /dev/null +++ b/staging/src/k8s.io/component-helpers/apps/poddisruptionbudget/helpers.go @@ -0,0 +1,65 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package poddisruptionbudget + +import ( + policy "k8s.io/api/policy/v1beta1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// UpdateDisruptionAllowedCondition updates the DisruptionAllowed condition +// on a PodDisruptionBudget based on the value of the DisruptionsAllowed field. +func UpdateDisruptionAllowedCondition(pdb *policy.PodDisruptionBudget) { + if pdb.Status.Conditions == nil { + pdb.Status.Conditions = make([]metav1.Condition, 0) + } + if pdb.Status.DisruptionsAllowed > 0 { + apimeta.SetStatusCondition(&pdb.Status.Conditions, metav1.Condition{ + Type: policy.DisruptionAllowedCondition, + Reason: policy.SufficientPodsReason, + Status: metav1.ConditionTrue, + ObservedGeneration: pdb.Status.ObservedGeneration, + }) + } else { + apimeta.SetStatusCondition(&pdb.Status.Conditions, metav1.Condition{ + Type: policy.DisruptionAllowedCondition, + Reason: policy.InsufficientPodsReason, + Status: metav1.ConditionFalse, + ObservedGeneration: pdb.Status.ObservedGeneration, + }) + } +} + +// ConditionsAreUpToDate checks whether the status and reason for the +// DisruptionAllowed condition are set to the correct values based on the +// DisruptionsAllowed field. +func ConditionsAreUpToDate(pdb *policy.PodDisruptionBudget) bool { + cond := apimeta.FindStatusCondition(pdb.Status.Conditions, policy.DisruptionAllowedCondition) + if cond == nil { + return false + } + + if pdb.Status.ObservedGeneration != pdb.Generation { + return false + } + + if pdb.Status.DisruptionsAllowed > 0 { + return cond.Status == metav1.ConditionTrue && cond.Reason == policy.SufficientPodsReason + } + return cond.Status == metav1.ConditionFalse && cond.Reason == policy.InsufficientPodsReason +} \ No newline at end of file