github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-25 04:33:26 +00:00
Code Issues Projects Releases Wiki Activity

api: add UnhealthyPodEvictionPolicy for PDBs

Browse Source
This commit is contained in:
Filip Křepinský 2022-11-10 16:29:32 +01:00
parent 909af807ee
commit a429797f2e
10 changed files with 771 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
pkg/apis/policy/types.go
View File

@ -42,8 +42,56 @@ type PodDisruptionBudgetSpec struct {
// by specifying 0. This is a mutually exclusive setting with "minAvailable".
// +optional
MaxUnavailable *intstr.IntOrString
// UnhealthyPodEvictionPolicy defines the criteria for when unhealthy pods
// should be considered for eviction. Current implementation considers healthy pods,
// as pods that have status.conditions item with type="Ready",status="True".
//
// Valid policies are IfHealthyBudget and AlwaysAllow.
// If no policy is specified, the default behavior will be used,
// which corresponds to the IfHealthyBudget policy.
//
// IfHealthyBudget policy means that running pods (status.phase="Running"),
// but not yet healthy can be evicted only if the guarded application is not
// disrupted (status.currentHealthy is at least equal to status.desiredHealthy).
// Healthy pods will be subject to the PDB for eviction.
//
// AlwaysAllow policy means that all running pods (status.phase="Running"),
// but not yet healthy are considered disrupted and can be evicted regardless
// of whether the criteria in a PDB is met. This means perspective running
// pods of a disrupted application might not get a chance to become healthy.
// Healthy pods will be subject to the PDB for eviction.
//
// Additional policies may be added in the future.
// Clients making eviction decisions should disallow eviction of unhealthy pods
// if they encounter an unrecognized policy in this field.
//
// This field is alpha-level. The eviction API uses this field when
// the feature gate PDBUnhealthyPodEvictionPolicy is enabled (disabled by default).
// +optional
UnhealthyPodEvictionPolicy *UnhealthyPodEvictionPolicyType
}
// UnhealthyPodEvictionPolicyType defines the criteria for when unhealthy pods
// should be considered for eviction.
// +enum
type UnhealthyPodEvictionPolicyType string
const (
// IfHealthyBudget policy means that running pods (status.phase="Running"),
// but not yet healthy can be evicted only if the guarded application is not
// disrupted (status.currentHealthy is at least equal to status.desiredHealthy).
// Healthy pods will be subject to the PDB for eviction.
IfHealthyBudget UnhealthyPodEvictionPolicyType = "IfHealthyBudget"
// AlwaysAllow policy means that all running pods (status.phase="Running"),
// but not yet healthy are considered disrupted and can be evicted regardless
// of whether the criteria in a PDB is met. This means perspective running
// pods of a disrupted application might not get a chance to become healthy.
// Healthy pods will be subject to the PDB for eviction.
AlwaysAllow UnhealthyPodEvictionPolicyType = "AlwaysAllow"
)
// PodDisruptionBudgetStatus represents information about the status of a
// PodDisruptionBudget. Status may trail the actual state of a system.
type PodDisruptionBudgetStatus struct {

9
pkg/apis/policy/validation/validation.go
View File

@ -44,6 +44,11 @@ const (
seccompAllowedProfilesAnnotationKey = "seccomp.security.alpha.kubernetes.io/allowedProfileNames"
)
var supportedUnhealthyPodEvictionPolicies = sets.NewString(
string(policy.IfHealthyBudget),
string(policy.AlwaysAllow),
)
type PodDisruptionBudgetValidationOptions struct {
AllowInvalidLabelValueInSelector bool
}
@ -78,6 +83,10 @@ func ValidatePodDisruptionBudgetSpec(spec policy.PodDisruptionBudgetSpec, opts P
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, labelSelectorValidationOptions, fldPath.Child("selector"))...)
if spec.UnhealthyPodEvictionPolicy != nil && !supportedUnhealthyPodEvictionPolicies.Has(string(*spec.UnhealthyPodEvictionPolicy)) {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("unhealthyPodEvictionPolicy"), *spec.UnhealthyPodEvictionPolicy, supportedUnhealthyPodEvictionPolicies.List()))
}
return allErrs
}

57
pkg/apis/policy/validation/validation_test.go
View File

@ -98,6 +98,63 @@ func TestValidateMinAvailablePodAndMaxUnavailableDisruptionBudgetSpec(t *testing
}
}
func TestValidateUnhealthyPodEvictionPolicyDisruptionBudgetSpec(t *testing.T) {
c1 := intstr.FromString("10%")
alwaysAllowPolicy := policy.AlwaysAllow
invalidPolicy := policy.UnhealthyPodEvictionPolicyType("Invalid")
testCases := []struct {
name string
pdbSpec policy.PodDisruptionBudgetSpec
expectErr bool
}{
{
name: "valid nil UnhealthyPodEvictionPolicy",
pdbSpec: policy.PodDisruptionBudgetSpec{
MinAvailable: &c1,
UnhealthyPodEvictionPolicy: nil,
},
expectErr: false,
},
{
name: "valid UnhealthyPodEvictionPolicy",
pdbSpec: policy.PodDisruptionBudgetSpec{
MinAvailable: &c1,
UnhealthyPodEvictionPolicy: &alwaysAllowPolicy,
},
expectErr: false,
},
{
name: "empty UnhealthyPodEvictionPolicy",
pdbSpec: policy.PodDisruptionBudgetSpec{
MinAvailable: &c1,
UnhealthyPodEvictionPolicy: new(policy.UnhealthyPodEvictionPolicyType),
},
expectErr: true,
},
{
name: "invalid UnhealthyPodEvictionPolicy",
pdbSpec: policy.PodDisruptionBudgetSpec{
MinAvailable: &c1,
UnhealthyPodEvictionPolicy: &invalidPolicy,
},
expectErr: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
errs := ValidatePodDisruptionBudgetSpec(tc.pdbSpec, PodDisruptionBudgetValidationOptions{true}, field.NewPath("foo"))
if len(errs) == 0 && tc.expectErr {
t.Errorf("unexpected success for %v", tc.pdbSpec)
}
if len(errs) != 0 && !tc.expectErr {
t.Errorf("unexpected failure for %v", tc.pdbSpec)
}
})
}
}
func TestValidatePodDisruptionBudgetStatus(t *testing.T) {
const expectNoErrors = false
const expectErrors = true

9
pkg/features/kube_features.go
View File

@ -625,6 +625,13 @@ const (
// Permits kubelet to run with swap enabled
NodeSwap featuregate.Feature = "NodeSwap"
// owner: @mortent, @atiratree, @ravig
// kep: http://kep.k8s.io/3018
// alpha: v1.26
//
// Enables PDBUnhealthyPodEvictionPolicy for PodDisruptionBudgets
PDBUnhealthyPodEvictionPolicy featuregate.Feature = "PDBUnhealthyPodEvictionPolicy"
// owner: @haircommander
// kep: https://kep.k8s.io/2364
// alpha: v1.23
@ -1045,6 +1052,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
NodeSwap: {Default: false, PreRelease: featuregate.Alpha},
PDBUnhealthyPodEvictionPolicy: {Default: false, PreRelease: featuregate.Alpha},
PodAndContainerStatsFromCRI: {Default: false, PreRelease: featuregate.Alpha},
PodDeletionCost: {Default: true, PreRelease: featuregate.Beta},

28
pkg/registry/core/pod/storage/eviction.go
View File

@ -226,10 +226,24 @@ func (r *EvictionREST) Create(ctx context.Context, name string, obj runtime.Obje
pdb := &pdbs[0]
pdbName = pdb.Name
// If the pod is not ready, it doesn't count towards healthy and we should not decrement
if !podutil.IsPodReady(pod) && pdb.Status.CurrentHealthy >= pdb.Status.DesiredHealthy && pdb.Status.DesiredHealthy > 0 {
updateDeletionOptions = true
return nil
// IsPodReady is the current implementation of IsHealthy
// If the pod is healthy, it should be guarded by the PDB.
if !podutil.IsPodReady(pod) {
if feature.DefaultFeatureGate.Enabled(features.PDBUnhealthyPodEvictionPolicy) {
if pdb.Spec.UnhealthyPodEvictionPolicy != nil && *pdb.Spec.UnhealthyPodEvictionPolicy == policyv1.AlwaysAllow {
// Delete the unhealthy pod, it doesn't count towards currentHealthy and desiredHealthy and we should not decrement disruptionsAllowed.
updateDeletionOptions = true
return nil
}
}
// default nil and IfHealthyBudget policy
if pdb.Status.CurrentHealthy >= pdb.Status.DesiredHealthy && pdb.Status.DesiredHealthy > 0 {
// Delete the unhealthy pod, it doesn't count towards currentHealthy and desiredHealthy and we should not decrement disruptionsAllowed.
// Application guarded by the PDB is not disrupted at the moment and deleting unhealthy (unready) pod will not disrupt it.
updateDeletionOptions = true
return nil
}
// confirm no disruptions allowed in checkAndDecrement
}
refresh := false
@ -264,12 +278,12 @@ func (r *EvictionREST) Create(ctx context.Context, name string, obj runtime.Obje
}
// At this point there was either no PDB or we succeeded in decrementing or
// the pod was unready and we have enough healthy replicas
// the pod was unhealthy (unready) and we have enough healthy replicas
deleteOptions := originalDeleteOptions
// Set deleteOptions.Preconditions.ResourceVersion to ensure
// the pod hasn't been considered ready since we calculated
// the pod hasn't been considered healthy (ready) since we calculated
if updateDeletionOptions {
// Take a copy so we can compare to client-provied Options later.
deleteOptions = deleteOptions.DeepCopy()
@ -351,7 +365,7 @@ func shouldEnforceResourceVersion(pod *api.Pod) bool {
return false
}
// Return true for all other pods to ensure we don't race against a pod becoming
// ready and violating PDBs.
// healthy (ready) and violating PDBs.
return true
}

378
pkg/registry/core/pod/storage/eviction_test.go
View File

@ -19,6 +19,9 @@ package storage
import (
"context"
"errors"
"fmt"
"reflect"
"strings"
"testing"
policyv1 "k8s.io/api/policy/v1"
@ -31,21 +34,25 @@ import (
"k8s.io/apimachinery/pkg/watch"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/kubernetes/fake"
featuregatetesting "k8s.io/component-base/featuregate/testing"
podapi "k8s.io/kubernetes/pkg/api/pod"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/policy"
"k8s.io/kubernetes/pkg/features"
)
func TestEviction(t *testing.T) {
testcases := []struct {
name string
pdbs []runtime.Object
policies []*policyv1.UnhealthyPodEvictionPolicyType
eviction *policy.Eviction
badNameInURL bool
expectError bool
expectError string
expectDeleted bool
podPhase api.PodPhase
podName string
@ -58,9 +65,10 @@ func TestEviction(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t1", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: true,
expectError: "Cannot evict pod as it would violate the pod's disruption budget.: TooManyRequests: The disruption budget foo needs 0 healthy pods and has 0 currently",
podPhase: api.PodRunning,
podName: "t1",
policies: []*policyv1.UnhealthyPodEvictionPolicyType{nil, unhealthyPolicyPtr(policyv1.IfHealthyBudget)}, // AlwaysAllow would terminate the pod since Running pods are not guarded by this policy
},
{
name: "matching pdbs with no disruptions allowed, pod pending",
@ -70,7 +78,7 @@ func TestEviction(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t2", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: false,
expectError: "",
podPhase: api.PodPending,
expectDeleted: true,
podName: "t2",
@ -83,7 +91,7 @@ func TestEviction(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t3", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: false,
expectError: "",
podPhase: api.PodSucceeded,
expectDeleted: true,
podName: "t3",
@ -96,7 +104,7 @@ func TestEviction(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t4", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: false,
expectError: "",
podPhase: api.PodFailed,
expectDeleted: true,
podName: "t4",
@ -132,91 +140,109 @@ func TestEviction(t *testing.T) {
}},
badNameInURL: true,
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t7", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: true,
expectError: "name in URL does not match name in Eviction object: BadRequest",
podName: "t7",
},
}
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()
pod := validNewPod()
pod.Name = tc.podName
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
if _, err := storage.Create(testContext, pod, nil, &metav1.CreateOptions{}); err != nil {
t.Error(err)
for _, unhealthyPodEvictionPolicy := range []*policyv1.UnhealthyPodEvictionPolicyType{nil, unhealthyPolicyPtr(policyv1.IfHealthyBudget), unhealthyPolicyPtr(policyv1.AlwaysAllow)} {
for _, tc := range testcases {
if len(tc.policies) > 0 && !hasUnhealthyPolicy(tc.policies, unhealthyPodEvictionPolicy) {
// unhealthyPodEvictionPolicy is not covered by this test
continue
}
t.Run(fmt.Sprintf("%v with %v policy", tc.name, unhealthyPolicyStr(unhealthyPodEvictionPolicy)), func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, true)()
if tc.podPhase != "" {
pod.Status.Phase = tc.podPhase
_, _, err := statusStorage.Update(testContext, pod.Name, rest.DefaultUpdatedObjectInfo(pod), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
if err != nil {
t.Errorf("Unexpected error: %v", err)
// same test runs multiple times, make copy of objects to have unique ones
evictionCopy := tc.eviction.DeepCopy()
var pdbsCopy []runtime.Object
for _, pdb := range tc.pdbs {
pdbCopy := pdb.DeepCopyObject()
pdbCopy.(*policyv1.PodDisruptionBudget).Spec.UnhealthyPodEvictionPolicy = unhealthyPodEvictionPolicy
pdbsCopy = append(pdbsCopy, pdbCopy)
}
}
client := fake.NewSimpleClientset(tc.pdbs...)
evictionRest := newEvictionStorage(storage.Store, client.PolicyV1())
testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceDefault)
storage, _, statusStorage, server := newStorage(t)
defer server.Terminate(t)
defer storage.Store.DestroyFunc()
name := pod.Name
if tc.badNameInURL {
name += "bad-name"
}
pod := validNewPod()
pod.Name = tc.podName
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
if _, err := storage.Create(testContext, pod, nil, &metav1.CreateOptions{}); err != nil {
t.Error(err)
}
_, err := evictionRest.Create(testContext, name, tc.eviction, nil, &metav1.CreateOptions{})
//_, err = evictionRest.Create(testContext, name, tc.eviction, nil, &metav1.CreateOptions{})
if (err != nil) != tc.expectError {
t.Errorf("expected error=%v, got %v; name %v", tc.expectError, err, pod.Name)
return
}
if tc.badNameInURL {
if err == nil {
t.Error("expected error here, but got nil")
if tc.podPhase != "" {
pod.Status.Phase = tc.podPhase
_, _, err := statusStorage.Update(testContext, pod.Name, rest.DefaultUpdatedObjectInfo(pod), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
}
client := fake.NewSimpleClientset(pdbsCopy...)
evictionRest := newEvictionStorage(storage.Store, client.PolicyV1())
name := pod.Name
if tc.badNameInURL {
name += "bad-name"
}
_, err := evictionRest.Create(testContext, name, evictionCopy, nil, &metav1.CreateOptions{})
gotErr := errToString(err)
if gotErr != tc.expectError {
t.Errorf("error mismatch: expected %v, got %v; name %v", tc.expectError, gotErr, pod.Name)
return
}
if err.Error() != "name in URL does not match name in Eviction object" {
t.Errorf("got unexpected error: %v", err)
if tc.badNameInURL {
if err == nil {
t.Error("expected error here, but got nil")
return
}
if err.Error() != "name in URL does not match name in Eviction object" {
t.Errorf("got unexpected error: %v", err)
}
}
}
if tc.expectError {
return
}
existingPod, err := storage.Get(testContext, pod.Name, &metav1.GetOptions{})
if tc.expectDeleted {
if !apierrors.IsNotFound(err) {
t.Errorf("expected to be deleted, lookup returned %#v", existingPod)
if tc.expectError != "" {
return
}
return
} else if apierrors.IsNotFound(err) {
t.Errorf("expected graceful deletion, got %v", err)
return
}
if err != nil {
t.Errorf("%#v", err)
return
}
existingPod, err := storage.Get(testContext, pod.Name, &metav1.GetOptions{})
if tc.expectDeleted {
if !apierrors.IsNotFound(err) {
t.Errorf("expected to be deleted, lookup returned %#v", existingPod)
}
return
} else if apierrors.IsNotFound(err) {
t.Errorf("expected graceful deletion, got %v", err)
return
}
if existingPod.(*api.Pod).DeletionTimestamp == nil {
t.Errorf("expected gracefully deleted pod with deletionTimestamp set, got %#v", existingPod)
}
})
if err != nil {
t.Errorf("%#v", err)
return
}
if existingPod.(*api.Pod).DeletionTimestamp == nil {
t.Errorf("expected gracefully deleted pod with deletionTimestamp set, got %#v", existingPod)
}
})
}
}
}
func TestEvictionIngorePDB(t *testing.T) {
func TestEvictionIgnorePDB(t *testing.T) {
testcases := []struct {
name string
pdbs []runtime.Object
policies []*policyv1.UnhealthyPodEvictionPolicyType
eviction *policy.Eviction
expectError bool
expectError string
podPhase api.PodPhase
podName string
expectedDeleteCount int
@ -231,12 +257,12 @@ func TestEvictionIngorePDB(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t1", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: false,
expectError: "",
podPhase: api.PodPending,
podName: "t1",
expectedDeleteCount: 3,
},
// This test case is critical. If it is removed or broken we may
// This test case is critical. If it is removed or broken we may
// regress and allow a pod to be deleted without checking PDBs when the
// pod should not be deleted.
{
@ -247,10 +273,30 @@ func TestEvictionIngorePDB(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t2", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: true,
expectError: "Cannot evict pod as it would violate the pod's disruption budget.: TooManyRequests: The disruption budget foo needs 0 healthy pods and has 0 currently",
podPhase: api.PodPending,
podName: "t2",
expectedDeleteCount: 1,
policies: []*policyv1.UnhealthyPodEvictionPolicyType{nil, unhealthyPolicyPtr(policyv1.IfHealthyBudget)}, // AlwaysAllow does not continueToPDBs, but straight to deletion
},
{
name: "pdbs No disruptions allowed, pod pending, first delete conflict, pod becomes running, skip PDB check, conflict",
pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t2", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: "Cannot evict pod as it would violate the pod's disruption budget.: TooManyRequests: The disruption budget foo is still being processed by the server.",
podPhase: api.PodPending,
podName: "t2",
podTerminating: false,
expectedDeleteCount: 2,
prc: &api.PodCondition{
Type: api.PodReady,
Status: api.ConditionFalse,
},
policies: []*policyv1.UnhealthyPodEvictionPolicyType{unhealthyPolicyPtr(policyv1.AlwaysAllow)}, // nil, IfHealthyBudget continueToPDBs
},
{
name: "pdbs disruptions allowed, pod pending, first delete conflict, pod becomes running, continueToPDBs",
@ -260,10 +306,11 @@ func TestEvictionIngorePDB(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 1},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t3", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: false,
expectError: "",
podPhase: api.PodPending,
podName: "t3",
expectedDeleteCount: 2,
policies: []*policyv1.UnhealthyPodEvictionPolicyType{nil, unhealthyPolicyPtr(policyv1.IfHealthyBudget)}, // AlwaysAllow does not continueToPDBs, but straight to deletion if pod not healthy (ready)
},
{
name: "pod pending, always conflict on delete",
@ -273,7 +320,7 @@ func TestEvictionIngorePDB(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t4", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: true,
expectError: `Operation cannot be fulfilled on tests "2": message: Conflict`,
podPhase: api.PodPending,
podName: "t4",
expectedDeleteCount: EvictionsRetry.Steps,
@ -286,7 +333,7 @@ func TestEvictionIngorePDB(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t5", Namespace: "default"}, DeleteOptions: metav1.NewRVDeletionPrecondition("userProvided")},
expectError: true,
expectError: `Operation cannot be fulfilled on tests "2": message: Conflict`,
podPhase: api.PodPending,
podName: "t5",
expectedDeleteCount: 1,
@ -299,7 +346,7 @@ func TestEvictionIngorePDB(t *testing.T) {
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t6", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(300)},
expectError: false,
expectError: "",
podName: "t6",
expectedDeleteCount: 1,
podTerminating: true,
@ -310,14 +357,14 @@ func TestEvictionIngorePDB(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
Status: policyv1.PodDisruptionBudgetStatus{
// This simulates 3 pods desired, our pod healthy, unhealthy pod is not ours.
// This simulates 3 pods expected, our pod healthy, unhealthy pod is not ours.
DisruptionsAllowed: 0,
CurrentHealthy: 2,
DesiredHealthy: 2,
},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t7", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: true,
expectError: "Cannot evict pod as it would violate the pod's disruption budget.: TooManyRequests: The disruption budget foo needs 2 healthy pods and has 2 currently",
podName: "t7",
expectedDeleteCount: 0,
podTerminating: false,
@ -327,20 +374,42 @@ func TestEvictionIngorePDB(t *testing.T) {
Status: api.ConditionTrue,
},
},
{
name: "matching pdbs with disruptions allowed, pod running, pod healthy, healthy pod ours, deletes pod by honoring the PDB",
pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
Status: policyv1.PodDisruptionBudgetStatus{
DisruptionsAllowed: 1,
CurrentHealthy: 3,
DesiredHealthy: 3,
},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t8", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: "",
podName: "t8",
expectedDeleteCount: 1,
podTerminating: false,
podPhase: api.PodRunning,
prc: &api.PodCondition{
Type: api.PodReady,
Status: api.ConditionTrue,
},
},
{
name: "matching pdbs with no disruptions allowed, pod running, pod unhealthy, unhealthy pod ours",
pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
Status: policyv1.PodDisruptionBudgetStatus{
// This simulates 3 pods desired, our pod unhealthy
// This simulates 3 pods expected, our pod unhealthy
DisruptionsAllowed: 0,
CurrentHealthy: 2,
DesiredHealthy: 2,
},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t8", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: false,
expectError: "",
podName: "t8",
expectedDeleteCount: 1,
podTerminating: false,
@ -350,6 +419,25 @@ func TestEvictionIngorePDB(t *testing.T) {
Status: api.ConditionFalse,
},
},
{
name: "matching pdbs with no disruptions allowed, pod running, pod unhealthy, unhealthy pod ours, skips the PDB check and deletes",
pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t8", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: "",
podName: "t8",
expectedDeleteCount: 1,
podTerminating: false,
podPhase: api.PodRunning,
prc: &api.PodCondition{
Type: api.PodReady,
Status: api.ConditionFalse,
},
policies: []*policyv1.UnhealthyPodEvictionPolicyType{unhealthyPolicyPtr(policyv1.AlwaysAllow)}, // nil, IfHealthyBudget would not skip the PDB check
},
{
// This case should return the 529 retry error.
name: "matching pdbs with no disruptions allowed, pod running, pod unhealthy, unhealthy pod ours, resource version conflict",
@ -357,14 +445,14 @@ func TestEvictionIngorePDB(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
Status: policyv1.PodDisruptionBudgetStatus{
// This simulates 3 pods desired, our pod unhealthy
// This simulates 3 pods expected, our pod unhealthy
DisruptionsAllowed: 0,
CurrentHealthy: 2,
DesiredHealthy: 2,
},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t9", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: true,
expectError: "Cannot evict pod as it would violate the pod's disruption budget.: TooManyRequests: The disruption budget foo is still being processed by the server.",
podName: "t9",
expectedDeleteCount: 1,
podTerminating: false,
@ -381,14 +469,14 @@ func TestEvictionIngorePDB(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
Status: policyv1.PodDisruptionBudgetStatus{
// This simulates 3 pods desired, our pod unhealthy
// This simulates 3 pods expected, our pod unhealthy
DisruptionsAllowed: 0,
CurrentHealthy: 2,
DesiredHealthy: 2,
},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t10", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: true,
expectError: "test designed to error: BadRequest",
podName: "t10",
expectedDeleteCount: 1,
podTerminating: false,
@ -400,50 +488,68 @@ func TestEvictionIngorePDB(t *testing.T) {
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceDefault)
ms := &mockStore{
deleteCount: 0,
for _, unhealthyPodEvictionPolicy := range []*policyv1.UnhealthyPodEvictionPolicyType{unhealthyPolicyPtr(policyv1.AlwaysAllow), nil, unhealthyPolicyPtr(policyv1.IfHealthyBudget)} {
for _, tc := range testcases {
if len(tc.policies) > 0 && !hasUnhealthyPolicy(tc.policies, unhealthyPodEvictionPolicy) {
// unhealthyPodEvictionPolicy is not covered by this test
continue
}
t.Run(fmt.Sprintf("%v with %v policy", tc.name, unhealthyPolicyStr(unhealthyPodEvictionPolicy)), func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, true)()
pod := validNewPod()
pod.Name = tc.podName
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
if tc.podPhase != "" {
pod.Status.Phase = tc.podPhase
}
if tc.podTerminating {
currentTime := metav1.Now()
pod.ObjectMeta.DeletionTimestamp = &currentTime
}
// Setup pod condition
if tc.prc != nil {
if !podapi.UpdatePodCondition(&pod.Status, tc.prc) {
t.Fatalf("Unable to update pod ready condition")
// same test runs 3 times, make copy of objects to have unique ones
evictionCopy := tc.eviction.DeepCopy()
prcCopy := tc.prc.DeepCopy()
var pdbsCopy []runtime.Object
for _, pdb := range tc.pdbs {
pdbCopy := pdb.DeepCopyObject()
pdbCopy.(*policyv1.PodDisruptionBudget).Spec.UnhealthyPodEvictionPolicy = unhealthyPodEvictionPolicy
pdbsCopy = append(pdbsCopy, pdbCopy)
}
}
client := fake.NewSimpleClientset(tc.pdbs...)
evictionRest := newEvictionStorage(ms, client.PolicyV1())
testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceDefault)
ms := &mockStore{
deleteCount: 0,
}
name := pod.Name
ms.pod = pod
pod := validNewPod()
pod.Name = tc.podName
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
if tc.podPhase != "" {
pod.Status.Phase = tc.podPhase
}
_, err := evictionRest.Create(testContext, name, tc.eviction, nil, &metav1.CreateOptions{})
if (err != nil) != tc.expectError {
t.Errorf("expected error=%v, got %v; name %v", tc.expectError, err, pod.Name)
return
}
if tc.podTerminating {
currentTime := metav1.Now()
pod.ObjectMeta.DeletionTimestamp = &currentTime
}
if tc.expectedDeleteCount != ms.deleteCount {
t.Errorf("expected delete count=%v, got %v; name %v", tc.expectedDeleteCount, ms.deleteCount, pod.Name)
}
// Setup pod condition
if tc.prc != nil {
if !podapi.UpdatePodCondition(&pod.Status, prcCopy) {
t.Fatalf("Unable to update pod ready condition")
}
}
})
client := fake.NewSimpleClientset(pdbsCopy...)
evictionRest := newEvictionStorage(ms, client.PolicyV1())
name := pod.Name
ms.pod = pod
_, err := evictionRest.Create(testContext, name, evictionCopy, nil, &metav1.CreateOptions{})
gotErr := errToString(err)
if gotErr != tc.expectError {
t.Errorf("error mismatch: expected %v, got %v; name %v", tc.expectError, gotErr, pod.Name)
return
}
if tc.expectedDeleteCount != ms.deleteCount {
t.Errorf("expected delete count=%v, got %v; name %v", tc.expectedDeleteCount, ms.deleteCount, pod.Name)
}
})
}
}
}
@ -692,3 +798,41 @@ func (ms *mockStore) ConvertToTable(ctx context.Context, object runtime.Object,
func (ms *mockStore) Destroy() {
}
func unhealthyPolicyPtr(unhealthyPodEvictionPolicy policyv1.UnhealthyPodEvictionPolicyType) *policyv1.UnhealthyPodEvictionPolicyType {
return &unhealthyPodEvictionPolicy
}
func unhealthyPolicyStr(unhealthyPodEvictionPolicy *policyv1.UnhealthyPodEvictionPolicyType) string {
if unhealthyPodEvictionPolicy == nil {
return "nil"
}
return string(*unhealthyPodEvictionPolicy)
}
func hasUnhealthyPolicy(unhealthyPolicies []*policyv1.UnhealthyPodEvictionPolicyType, unhealthyPolicy *policyv1.UnhealthyPodEvictionPolicyType) bool {
for _, p := range unhealthyPolicies {
if reflect.DeepEqual(unhealthyPolicy, p) {
return true
}
}
return false
}
func errToString(err error) string {
result := ""
if err != nil {
result = err.Error()
if statusErr, ok := err.(*apierrors.StatusError); ok {
result += fmt.Sprintf(": %v", statusErr.ErrStatus.Reason)
if statusErr.ErrStatus.Details != nil {
for _, cause := range statusErr.ErrStatus.Details.Causes {
if !strings.HasSuffix(err.Error(), cause.Message) {
result += fmt.Sprintf(": %v", cause.Message)
}
}
}
}
}
return result
}

26
pkg/registry/policy/poddisruptionbudget/strategy.go
View File

@ -26,9 +26,11 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/apis/policy"
"k8s.io/kubernetes/pkg/apis/policy/validation"
"k8s.io/kubernetes/pkg/features"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
)
@ -68,6 +70,8 @@ func (podDisruptionBudgetStrategy) PrepareForCreate(ctx context.Context, obj run
podDisruptionBudget.Status = policy.PodDisruptionBudgetStatus{}
podDisruptionBudget.Generation = 1
dropDisabledFields(&podDisruptionBudget.Spec, nil)
}
// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@ -83,6 +87,8 @@ func (podDisruptionBudgetStrategy) PrepareForUpdate(ctx context.Context, obj, ol
if !apiequality.Semantic.DeepEqual(oldPodDisruptionBudget.Spec, newPodDisruptionBudget.Spec) {
newPodDisruptionBudget.Generation = oldPodDisruptionBudget.Generation + 1
}
dropDisabledFields(&newPodDisruptionBudget.Spec, &oldPodDisruptionBudget.Spec)
}
// Validate validates a new PodDisruptionBudget.
@ -182,3 +188,23 @@ func hasInvalidLabelValueInLabelSelector(pdb *policy.PodDisruptionBudget) bool {
}
return false
}
// dropDisabledFields removes disabled fields from the pod disruption budget spec.
// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pod disruption budget spec.
func dropDisabledFields(pdbSpec, oldPDBSpec *policy.PodDisruptionBudgetSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.PDBUnhealthyPodEvictionPolicy) {
if !unhealthyPodEvictionPolicyInUse(oldPDBSpec) {
pdbSpec.UnhealthyPodEvictionPolicy = nil
}
}
}
func unhealthyPodEvictionPolicyInUse(oldPDBSpec *policy.PodDisruptionBudgetSpec) bool {
if oldPDBSpec == nil {
return false
}
if oldPDBSpec.UnhealthyPodEvictionPolicy != nil {
return true
}
return false
}

244
pkg/registry/policy/poddisruptionbudget/strategy_test.go
View File

@ -17,15 +17,176 @@ limitations under the License.
package poddisruptionbudget
import (
"reflect"
"testing"
"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/apis/policy"
"k8s.io/kubernetes/pkg/features"
)
type unhealthyPodEvictionPolicyStrategyTestCase struct {
name string
enableUnhealthyPodEvictionPolicy bool
disablePDBUnhealthyPodEvictionPolicyFeatureGateAfterCreate bool
unhealthyPodEvictionPolicy *policy.UnhealthyPodEvictionPolicyType
expectedUnhealthyPodEvictionPolicy *policy.UnhealthyPodEvictionPolicyType
expectedValidationErr bool
updateUnhealthyPodEvictionPolicy *policy.UnhealthyPodEvictionPolicyType
expectedUpdateUnhealthyPodEvictionPolicy *policy.UnhealthyPodEvictionPolicyType
expectedValidationUpdateErr bool
}
func TestPodDisruptionBudgetStrategy(t *testing.T) {
tests := map[string]bool{
"PodDisruptionBudget strategy with PDBUnhealthyPodEvictionPolicy feature gate disabled": false,
"PodDisruptionBudget strategy with PDBUnhealthyPodEvictionPolicy feature gate enabled": true,
}
for name, enableUnhealthyPodEvictionPolicy := range tests {
t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, enableUnhealthyPodEvictionPolicy)()
testPodDisruptionBudgetStrategy(t)
})
}
healthyPolicyTests := []unhealthyPodEvictionPolicyStrategyTestCase{
{
name: "PodDisruptionBudget strategy with FeatureGate disabled should remove unhealthyPodEvictionPolicy",
enableUnhealthyPodEvictionPolicy: false,
unhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.IfHealthyBudget),
updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.IfHealthyBudget),
},
{
name: "PodDisruptionBudget strategy with FeatureGate disabled should remove invalid unhealthyPodEvictionPolicy",
enableUnhealthyPodEvictionPolicy: false,
unhealthyPodEvictionPolicy: unhealthyPolicyPtr("Invalid"),
updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr("Invalid"),
},
{
name: "PodDisruptionBudget strategy with FeatureGate enabled",
enableUnhealthyPodEvictionPolicy: true,
},
{
name: "PodDisruptionBudget strategy with FeatureGate enabled should respect unhealthyPodEvictionPolicy",
enableUnhealthyPodEvictionPolicy: true,
unhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.AlwaysAllow),
expectedUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.AlwaysAllow),
updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.IfHealthyBudget),
expectedUpdateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.IfHealthyBudget),
},
{
name: "PodDisruptionBudget strategy with FeatureGate enabled should fail invalid unhealthyPodEvictionPolicy",
enableUnhealthyPodEvictionPolicy: true,
unhealthyPodEvictionPolicy: unhealthyPolicyPtr("Invalid"),
expectedValidationErr: true,
},
{
name: "PodDisruptionBudget strategy with FeatureGate enabled should fail invalid unhealthyPodEvictionPolicy when updated",
enableUnhealthyPodEvictionPolicy: true,
updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr("Invalid"),
expectedValidationUpdateErr: true,
},
{
name: "PodDisruptionBudget strategy with unhealthyPodEvictionPolicy should be updated when feature gate is disabled",
enableUnhealthyPodEvictionPolicy: true,
disablePDBUnhealthyPodEvictionPolicyFeatureGateAfterCreate: true,
unhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.AlwaysAllow),
expectedUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.AlwaysAllow),
updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.IfHealthyBudget),
expectedUpdateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.IfHealthyBudget),
},
{
name: "PodDisruptionBudget strategy with unhealthyPodEvictionPolicy should not be updated to invalid when feature gate is disabled",
enableUnhealthyPodEvictionPolicy: true,
disablePDBUnhealthyPodEvictionPolicyFeatureGateAfterCreate: true,
unhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.AlwaysAllow),
expectedUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.AlwaysAllow),
updateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr("Invalid"),
expectedValidationUpdateErr: true,
expectedUpdateUnhealthyPodEvictionPolicy: unhealthyPolicyPtr(policy.AlwaysAllow),
},
}
for _, tc := range healthyPolicyTests {
t.Run(tc.name, func(t *testing.T) {
testPodDisruptionBudgetStrategyWithUnhealthyPodEvictionPolicy(t, tc)
})
}
}
func testPodDisruptionBudgetStrategyWithUnhealthyPodEvictionPolicy(t *testing.T, tc unhealthyPodEvictionPolicyStrategyTestCase) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, tc.enableUnhealthyPodEvictionPolicy)()
ctx := genericapirequest.NewDefaultContext()
if !Strategy.NamespaceScoped() {
t.Errorf("PodDisruptionBudget must be namespace scoped")
}
if Strategy.AllowCreateOnUpdate() {
t.Errorf("PodDisruptionBudget should not allow create on update")
}
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: &minAvailable,
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
UnhealthyPodEvictionPolicy: tc.unhealthyPodEvictionPolicy,
},
}
Strategy.PrepareForCreate(ctx, pdb)
errs := Strategy.Validate(ctx, pdb)
if len(errs) != 0 {
if !tc.expectedValidationErr {
t.Errorf("Unexpected error validating %v", errs)
}
return // no point going further when we have invalid PDB
}
if len(errs) == 0 && tc.expectedValidationErr {
t.Errorf("Expected error validating")
}
if !reflect.DeepEqual(pdb.Spec.UnhealthyPodEvictionPolicy, tc.expectedUnhealthyPodEvictionPolicy) {
t.Errorf("Unexpected UnhealthyPodEvictionPolicy set: expected %v, got %v", tc.expectedUnhealthyPodEvictionPolicy, pdb.Spec.UnhealthyPodEvictionPolicy)
}
if tc.disablePDBUnhealthyPodEvictionPolicyFeatureGateAfterCreate {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, false)()
}
newPdb := &policy.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{Name: pdb.Name, Namespace: pdb.Namespace},
Spec: pdb.Spec,
}
if tc.updateUnhealthyPodEvictionPolicy != nil {
newPdb.Spec.UnhealthyPodEvictionPolicy = tc.updateUnhealthyPodEvictionPolicy
}
// Nothing in Spec changes: OK
Strategy.PrepareForUpdate(ctx, newPdb, pdb)
errs = Strategy.ValidateUpdate(ctx, newPdb, pdb)
if len(errs) != 0 {
if !tc.expectedValidationUpdateErr {
t.Errorf("Unexpected error updating PodDisruptionBudget %v", errs)
}
return // no point going further when we have invalid PDB
}
if len(errs) == 0 && tc.expectedValidationUpdateErr {
t.Errorf("Expected error updating PodDisruptionBudget")
}
if !reflect.DeepEqual(newPdb.Spec.UnhealthyPodEvictionPolicy, tc.expectedUpdateUnhealthyPodEvictionPolicy) {
t.Errorf("Unexpected UnhealthyPodEvictionPolicy set: expected %v, got %v", tc.expectedUpdateUnhealthyPodEvictionPolicy, newPdb.Spec.UnhealthyPodEvictionPolicy)
}
}
func testPodDisruptionBudgetStrategy(t *testing.T) {
ctx := genericapirequest.NewDefaultContext()
if !Strategy.NamespaceScoped() {
t.Errorf("PodDisruptionBudget must be namespace scoped")
@ -210,3 +371,86 @@ func TestPodDisruptionBudgetStatusValidationByApiVersion(t *testing.T) {
})
}
}
func TestDropDisabledFields(t *testing.T) {
tests := map[string]struct {
oldSpec *policy.PodDisruptionBudgetSpec
newSpec *policy.PodDisruptionBudgetSpec
expectNewSpec *policy.PodDisruptionBudgetSpec
enableUnhealthyPodEvictionPolicy bool
}{
"disabled clears unhealthyPodEvictionPolicy": {
enableUnhealthyPodEvictionPolicy: false,
oldSpec: nil,
newSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
expectNewSpec: specWithUnhealthyPodEvictionPolicy(nil),
},
"disabled does not allow updating unhealthyPodEvictionPolicy": {
enableUnhealthyPodEvictionPolicy: false,
oldSpec: specWithUnhealthyPodEvictionPolicy(nil),
newSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
expectNewSpec: specWithUnhealthyPodEvictionPolicy(nil),
},
"disabled preserves old unhealthyPodEvictionPolicy when both old and new have it": {
enableUnhealthyPodEvictionPolicy: false,
oldSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
newSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
expectNewSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
},
"disabled allows updating unhealthyPodEvictionPolicy": {
enableUnhealthyPodEvictionPolicy: false,
oldSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
newSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.AlwaysAllow)),
expectNewSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.AlwaysAllow)),
},
"enabled preserve unhealthyPodEvictionPolicy": {
enableUnhealthyPodEvictionPolicy: true,
oldSpec: nil,
newSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
expectNewSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
},
"enabled allows updating unhealthyPodEvictionPolicy": {
enableUnhealthyPodEvictionPolicy: true,
oldSpec: specWithUnhealthyPodEvictionPolicy(nil),
newSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
expectNewSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
},
"enabled preserve unhealthyPodEvictionPolicy when both old and new have it": {
enableUnhealthyPodEvictionPolicy: true,
oldSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
newSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
expectNewSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
},
"enabled updates unhealthyPodEvictionPolicy": {
enableUnhealthyPodEvictionPolicy: true,
oldSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.IfHealthyBudget)),
newSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.AlwaysAllow)),
expectNewSpec: specWithUnhealthyPodEvictionPolicy(unhealthyPolicyPtr(policy.AlwaysAllow)),
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PDBUnhealthyPodEvictionPolicy, tc.enableUnhealthyPodEvictionPolicy)()
oldSpecBefore := tc.oldSpec.DeepCopy()
dropDisabledFields(tc.newSpec, tc.oldSpec)
if !reflect.DeepEqual(tc.newSpec, tc.expectNewSpec) {
t.Error(cmp.Diff(tc.newSpec, tc.expectNewSpec))
}
if !reflect.DeepEqual(tc.oldSpec, oldSpecBefore) {
t.Error(cmp.Diff(tc.oldSpec, oldSpecBefore))
}
})
}
}
func unhealthyPolicyPtr(unhealthyPodEvictionPolicy policy.UnhealthyPodEvictionPolicyType) *policy.UnhealthyPodEvictionPolicyType {
return &unhealthyPodEvictionPolicy
}
func specWithUnhealthyPodEvictionPolicy(unhealthyPodEvictionPolicy *policy.UnhealthyPodEvictionPolicyType) *policy.PodDisruptionBudgetSpec {
return &policy.PodDisruptionBudgetSpec{
UnhealthyPodEvictionPolicy: unhealthyPodEvictionPolicy,
}
}

48
staging/src/k8s.io/api/policy/v1/types.go
View File

@ -47,8 +47,56 @@ type PodDisruptionBudgetSpec struct {
// by specifying 0. This is a mutually exclusive setting with "minAvailable".
// +optional
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty" protobuf:"bytes,3,opt,name=maxUnavailable"`
// UnhealthyPodEvictionPolicy defines the criteria for when unhealthy pods
// should be considered for eviction. Current implementation considers healthy pods,
// as pods that have status.conditions item with type="Ready",status="True".
//
// Valid policies are IfHealthyBudget and AlwaysAllow.
// If no policy is specified, the default behavior will be used,
// which corresponds to the IfHealthyBudget policy.
//
// IfHealthyBudget policy means that running pods (status.phase="Running"),
// but not yet healthy can be evicted only if the guarded application is not
// disrupted (status.currentHealthy is at least equal to status.desiredHealthy).
// Healthy pods will be subject to the PDB for eviction.
//
// AlwaysAllow policy means that all running pods (status.phase="Running"),
// but not yet healthy are considered disrupted and can be evicted regardless
// of whether the criteria in a PDB is met. This means perspective running
// pods of a disrupted application might not get a chance to become healthy.
// Healthy pods will be subject to the PDB for eviction.
//
// Additional policies may be added in the future.
// Clients making eviction decisions should disallow eviction of unhealthy pods
// if they encounter an unrecognized policy in this field.
//
// This field is alpha-level. The eviction API uses this field when
// the feature gate PDBUnhealthyPodEvictionPolicy is enabled (disabled by default).
// +optional
UnhealthyPodEvictionPolicy *UnhealthyPodEvictionPolicyType `json:"unhealthyPodEvictionPolicy,omitempty" protobuf:"bytes,4,opt,name=unhealthyPodEvictionPolicy"`
}
// UnhealthyPodEvictionPolicyType defines the criteria for when unhealthy pods
// should be considered for eviction.
// +enum
type UnhealthyPodEvictionPolicyType string
const (
// IfHealthyBudget policy means that running pods (status.phase="Running"),
// but not yet healthy can be evicted only if the guarded application is not
// disrupted (status.currentHealthy is at least equal to status.desiredHealthy).
// Healthy pods will be subject to the PDB for eviction.
IfHealthyBudget UnhealthyPodEvictionPolicyType = "IfHealthyBudget"
// AlwaysAllow policy means that all running pods (status.phase="Running"),
// but not yet healthy are considered disrupted and can be evicted regardless
// of whether the criteria in a PDB is met. This means perspective running
// pods of a disrupted application might not get a chance to become healthy.
// Healthy pods will be subject to the PDB for eviction.
AlwaysAllow UnhealthyPodEvictionPolicyType = "AlwaysAllow"
)
// PodDisruptionBudgetStatus represents information about the status of a
// PodDisruptionBudget. Status may trail the actual state of a system.
type PodDisruptionBudgetStatus struct {

48
staging/src/k8s.io/api/policy/v1beta1/types.go
View File

@ -45,8 +45,56 @@ type PodDisruptionBudgetSpec struct {
// by specifying 0. This is a mutually exclusive setting with "minAvailable".
// +optional
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty" protobuf:"bytes,3,opt,name=maxUnavailable"`
// UnhealthyPodEvictionPolicy defines the criteria for when unhealthy pods
// should be considered for eviction. Current implementation considers healthy pods,
// as pods that have status.conditions item with type="Ready",status="True".
//
// Valid policies are IfHealthyBudget and AlwaysAllow.
// If no policy is specified, the default behavior will be used,
// which corresponds to the IfHealthyBudget policy.
//
// IfHealthyBudget policy means that running pods (status.phase="Running"),
// but not yet healthy can be evicted only if the guarded application is not
// disrupted (status.currentHealthy is at least equal to status.desiredHealthy).
// Healthy pods will be subject to the PDB for eviction.
//
// AlwaysAllow policy means that all running pods (status.phase="Running"),
// but not yet healthy are considered disrupted and can be evicted regardless
// of whether the criteria in a PDB is met. This means perspective running
// pods of a disrupted application might not get a chance to become healthy.
// Healthy pods will be subject to the PDB for eviction.
//
// Additional policies may be added in the future.
// Clients making eviction decisions should disallow eviction of unhealthy pods
// if they encounter an unrecognized policy in this field.
//
// This field is alpha-level. The eviction API uses this field when
// the feature gate PDBUnhealthyPodEvictionPolicy is enabled (disabled by default).
// +optional
UnhealthyPodEvictionPolicy *UnhealthyPodEvictionPolicyType `json:"unhealthyPodEvictionPolicy,omitempty" protobuf:"bytes,4,opt,name=unhealthyPodEvictionPolicy"`
}
// UnhealthyPodEvictionPolicyType defines the criteria for when unhealthy pods
// should be considered for eviction.
// +enum
type UnhealthyPodEvictionPolicyType string
const (
// IfHealthyBudget policy means that running pods (status.phase="Running"),
// but not yet healthy can be evicted only if the guarded application is not
// disrupted (status.currentHealthy is at least equal to status.desiredHealthy).
// Healthy pods will be subject to the PDB for eviction.
IfHealthyBudget UnhealthyPodEvictionPolicyType = "IfHealthyBudget"
// AlwaysAllow policy means that all running pods (status.phase="Running"),
// but not yet healthy are considered disrupted and can be evicted regardless
// of whether the criteria in a PDB is met. This means perspective running
// pods of a disrupted application might not get a chance to become healthy.
// Healthy pods will be subject to the PDB for eviction.
AlwaysAllow UnhealthyPodEvictionPolicyType = "AlwaysAllow"
)
// PodDisruptionBudgetStatus represents information about the status of a
// PodDisruptionBudget. Status may trail the actual state of a system.
type PodDisruptionBudgetStatus struct {