diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index bdddf81fafd..a3e0f6c056a 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -936,15 +936,18 @@ func filterPodsWithPDBViolation(pods []*v1.Pod, pdbs []*policy.PodDisruptionBudg if selector.Empty() || !selector.Matches(labels.Set(pod.Labels)) { continue } + + // Existing in DisruptedPods means it has been processed in API server, + // we don't treat it as a violating case. + if _, exist := pdb.Status.DisruptedPods[pod.Name]; exist { + continue + } + // Only decrement the matched pdb when it's not in its ; + // otherwise we may over-decrement the budget number. + pdbsAllowed[i]-- // We have found a matching PDB. - if pdbsAllowed[i] <= 0 { + if pdbsAllowed[i] < 0 { pdbForPodIsViolated = true - } else { - // Only decrement the matched pdb when it's not in its ; - // otherwise we may over-decrement the budget number. - if _, exist := pdb.Status.DisruptedPods[pod.Name]; !exist { - pdbsAllowed[i]-- - } } } } diff --git a/pkg/scheduler/core/generic_scheduler_test.go b/pkg/scheduler/core/generic_scheduler_test.go index d5684c14018..0a4ccbc08dd 100644 --- a/pkg/scheduler/core/generic_scheduler_test.go +++ b/pkg/scheduler/core/generic_scheduler_test.go @@ -1498,10 +1498,28 @@ func TestSelectNodesForPreemption(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a"), Labels: map[string]string{"app": "foo"}}, Spec: v1.PodSpec{Containers: mediumContainers, Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b"), Labels: map[string]string{"app": "foo"}}, Spec: v1.PodSpec{Containers: mediumContainers, Priority: &midPriority, NodeName: "machine1"}}}, pdbs: []*policy.PodDisruptionBudget{ - {Spec: policy.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}}, Status: policy.PodDisruptionBudgetStatus{DisruptionsAllowed: 1, DisruptedPods: map[string]metav1.Time{"a": {Time: time.Now()}}}}}, + {Spec: policy.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}}, Status: policy.PodDisruptionBudgetStatus{DisruptionsAllowed: 1, DisruptedPods: map[string]metav1.Time{"b": {Time: time.Now()}}}}}, expected: map[string]victims{"machine1": {pods: sets.NewString("a", "b"), numPDBViolations: 0}}, expectedNumFilterCalled: 3, }, + { + name: "preemption with violation of the pdb with pod whose eviction was processed, the victim which belongs to DisruptedPods is treated as 'nonViolating'", + registerPlugins: []st.RegisterPluginFunc{ + st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), + st.RegisterPluginAsExtensions(noderesources.FitName, noderesources.NewFit, "Filter", "PreFilter"), + st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), + }, + nodes: []string{"machine1"}, + pod: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "pod1", UID: types.UID("pod1")}, Spec: v1.PodSpec{Containers: veryLargeContainers, Priority: &highPriority}}, + pods: []*v1.Pod{ + {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a"), Labels: map[string]string{"app": "foo"}}, Spec: v1.PodSpec{Containers: mediumContainers, Priority: &midPriority, NodeName: "machine1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b"), Labels: map[string]string{"app": "foo"}}, Spec: v1.PodSpec{Containers: mediumContainers, Priority: &midPriority, NodeName: "machine1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "c", UID: types.UID("c"), Labels: map[string]string{"app": "foo"}}, Spec: v1.PodSpec{Containers: mediumContainers, Priority: &midPriority, NodeName: "machine1"}}}, + pdbs: []*policy.PodDisruptionBudget{ + {Spec: policy.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}}, Status: policy.PodDisruptionBudgetStatus{DisruptionsAllowed: 1, DisruptedPods: map[string]metav1.Time{"c": {Time: time.Now()}}}}}, + expected: map[string]victims{"machine1": {pods: sets.NewString("a", "b", "c"), numPDBViolations: 1}}, + expectedNumFilterCalled: 4, + }, } labelKeys := []string{"hostname", "zone", "region"} for _, test := range tests {