Fix to honor PDB with an empty selector {}

PDB with an empty selector `{}` is selecting all the pods in a namespace.
But, during the `drain`, all the pods are getting evicted which is not expected.
This change should fix the issue and honor the pdb before evicting the pods.

Signed-off-by: Sai Ramesh Vanka <svanka@redhat.com>
This commit is contained in:
Sai Ramesh Vanka 2023-08-08 17:28:48 +05:30
parent 99190634ab
commit a6329ac453
2 changed files with 36 additions and 2 deletions

View File

@ -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
}

View File

@ -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)} {
@ -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)} {