Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #119732: Fix to honor PDB with an empty selector {} #121131

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions pkg/registry/core/pod/storage/eviction.go
Expand Up @@ -475,8 +475,7 @@ func (r *EvictionREST) getPodDisruptionBudgets(ctx context.Context, pod *api.Pod
// This object has an invalid selector, it does not match the pod
continue
}
// If a PDB with a nil or empty selector creeps in, it should match nothing, not everything.
if selector.Empty() || !selector.Matches(labels.Set(pod.Labels)) {
if !selector.Matches(labels.Set(pod.Labels)) {
continue
}

Expand Down
35 changes: 35 additions & 0 deletions pkg/registry/core/pod/storage/eviction_test.go
Expand Up @@ -143,6 +143,19 @@ func TestEviction(t *testing.T) {
expectError: "name in URL does not match name in Eviction object: BadRequest",
podName: "t7",
},
{
name: "matching pdbs with no disruptions allowed, pod running, empty selector",
pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{}},
Status: policyv1.PodDisruptionBudgetStatus{DisruptionsAllowed: 0},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t8", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
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: "t8",
policies: []*policyv1.UnhealthyPodEvictionPolicyType{nil, unhealthyPolicyPtr(policyv1.IfHealthyBudget)}, // AlwaysAllow would terminate the pod since Running pods are not guarded by this policy
},
}

for _, unhealthyPodEvictionPolicy := range []*policyv1.UnhealthyPodEvictionPolicyType{nil, unhealthyPolicyPtr(policyv1.IfHealthyBudget), unhealthyPolicyPtr(policyv1.AlwaysAllow)} {
Expand Down Expand Up @@ -486,6 +499,28 @@ func TestEvictionIgnorePDB(t *testing.T) {
Status: api.ConditionFalse,
},
},
{
name: "matching pdbs with no disruptions allowed, pod running, pod healthy, empty selector, pod not deleted by honoring the PDB",
pdbs: []runtime.Object{&policyv1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{}},
Status: policyv1.PodDisruptionBudgetStatus{
DisruptionsAllowed: 0,
CurrentHealthy: 3,
DesiredHealthy: 3,
},
}},
eviction: &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "t11", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
expectError: "Cannot evict pod as it would violate the pod's disruption budget.: TooManyRequests: The disruption budget foo needs 3 healthy pods and has 3 currently",
podName: "t11",
expectedDeleteCount: 0,
podTerminating: false,
podPhase: api.PodRunning,
prc: &api.PodCondition{
Type: api.PodReady,
Status: api.ConditionTrue,
},
},
}

for _, unhealthyPodEvictionPolicy := range []*policyv1.UnhealthyPodEvictionPolicyType{unhealthyPolicyPtr(policyv1.AlwaysAllow), nil, unhealthyPolicyPtr(policyv1.IfHealthyBudget)} {
Expand Down