diff --git a/pkg/scheduler/core/BUILD b/pkg/scheduler/core/BUILD index aef9861c5ff..95c8c3b374e 100644 --- a/pkg/scheduler/core/BUILD +++ b/pkg/scheduler/core/BUILD @@ -69,6 +69,7 @@ go_test( "//pkg/scheduler/testing:go_default_library", "//pkg/scheduler/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/api/policy/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index 19cdebe761c..227222cf9d7 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -894,12 +894,17 @@ func (g *genericScheduler) selectNodesForPreemption( // This function is stable and does not change the order of received pods. So, if it // receives a sorted list, grouping will preserve the order of the input list. func filterPodsWithPDBViolation(pods []*v1.Pod, pdbs []*policy.PodDisruptionBudget) (violatingPods, nonViolatingPods []*v1.Pod) { + pdbsAllowed := make([]int32, len(pdbs)) + for i, pdb := range pdbs { + pdbsAllowed[i] = pdb.Status.DisruptionsAllowed + } + for _, obj := range pods { pod := obj pdbForPodIsViolated := false // A pod with no labels will not match any PDB. So, no need to check. if len(pod.Labels) != 0 { - for _, pdb := range pdbs { + for i, pdb := range pdbs { if pdb.Namespace != pod.Namespace { continue } @@ -912,9 +917,11 @@ func filterPodsWithPDBViolation(pods []*v1.Pod, pdbs []*policy.PodDisruptionBudg continue } // We have found a matching PDB. - if pdb.Status.DisruptionsAllowed <= 0 { + if pdbsAllowed[i] <= 0 { pdbForPodIsViolated = true break + } else { + pdbsAllowed[i]-- } } } diff --git a/pkg/scheduler/core/generic_scheduler_test.go b/pkg/scheduler/core/generic_scheduler_test.go index f71196387c8..29536164404 100644 --- a/pkg/scheduler/core/generic_scheduler_test.go +++ b/pkg/scheduler/core/generic_scheduler_test.go @@ -28,6 +28,7 @@ import ( "time" v1 "k8s.io/api/core/v1" + policy "k8s.io/api/policy/v1beta1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -1181,11 +1182,16 @@ func printNodeToVictims(nodeToVictims map[*v1.Node]*extenderv1.Victims) string { return output } -func checkPreemptionVictims(expected map[string]map[string]bool, nodeToPods map[*v1.Node]*extenderv1.Victims) error { +type victims struct { + pods sets.String + numPDBViolations int64 +} + +func checkPreemptionVictims(expected map[string]victims, nodeToPods map[*v1.Node]*extenderv1.Victims) error { if len(expected) == len(nodeToPods) { for k, victims := range nodeToPods { - if expPods, ok := expected[k.Name]; ok { - if len(victims.Pods) != len(expPods) { + if expVictims, ok := expected[k.Name]; ok { + if len(victims.Pods) != len(expVictims.pods) { return fmt.Errorf("unexpected number of pods. expected: %v, got: %v", expected, printNodeToVictims(nodeToPods)) } prevPriority := int32(math.MaxInt32) @@ -1195,10 +1201,13 @@ func checkPreemptionVictims(expected map[string]map[string]bool, nodeToPods map[ return fmt.Errorf("pod %v of node %v was not sorted by priority", p.Name, k) } prevPriority = *p.Spec.Priority - if _, ok := expPods[p.Name]; !ok { - return fmt.Errorf("pod %v was not expected. Expected: %v", p.Name, expPods) + if !expVictims.pods.Has(p.Name) { + return fmt.Errorf("pod %v was not expected. Expected: %v", p.Name, expVictims.pods) } } + if expVictims.numPDBViolations != victims.NumPDBViolations { + return fmt.Errorf("unexpected numPDBViolations. expected: %d, got: %d", expVictims.numPDBViolations, victims.NumPDBViolations) + } } else { return fmt.Errorf("unexpected machines. expected: %v, got: %v", expected, printNodeToVictims(nodeToPods)) } @@ -1277,8 +1286,9 @@ func TestSelectNodesForPreemption(t *testing.T) { nodes []string pod *v1.Pod pods []*v1.Pod + pdbs []*policy.PodDisruptionBudget filterReturnCode framework.Code - expected map[string]map[string]bool // Map from node name to a list of pods names which should be preempted. + expected map[string]victims expectedNumFilterCalled int32 }{ { @@ -1293,7 +1303,7 @@ func TestSelectNodesForPreemption(t *testing.T) { pods: []*v1.Pod{ {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a")}, Spec: v1.PodSpec{Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b")}, Spec: v1.PodSpec{Priority: &midPriority, NodeName: "machine2"}}}, - expected: map[string]map[string]bool{}, + expected: map[string]victims{}, expectedNumFilterCalled: 2, }, { @@ -1308,7 +1318,7 @@ func TestSelectNodesForPreemption(t *testing.T) { pods: []*v1.Pod{ {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a")}, Spec: v1.PodSpec{Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b")}, Spec: v1.PodSpec{Priority: &midPriority, NodeName: "machine2"}}}, - expected: map[string]map[string]bool{"machine1": {}, "machine2": {}}, + expected: map[string]victims{"machine1": {}, "machine2": {}}, expectedNumFilterCalled: 4, }, { @@ -1323,7 +1333,7 @@ func TestSelectNodesForPreemption(t *testing.T) { pods: []*v1.Pod{ {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a")}, Spec: v1.PodSpec{Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b")}, Spec: v1.PodSpec{Priority: &midPriority, NodeName: "machine2"}}}, - expected: map[string]map[string]bool{"machine1": {}}, + expected: map[string]victims{"machine1": {}}, expectedNumFilterCalled: 3, }, { @@ -1338,7 +1348,7 @@ func TestSelectNodesForPreemption(t *testing.T) { pods: []*v1.Pod{ {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine2"}}}, - expected: map[string]map[string]bool{"machine1": {"a": true}, "machine2": {"b": true}}, + expected: map[string]victims{"machine1": {pods: sets.NewString("a")}, "machine2": {pods: sets.NewString("b")}}, expectedNumFilterCalled: 4, }, { @@ -1353,7 +1363,7 @@ func TestSelectNodesForPreemption(t *testing.T) { pods: []*v1.Pod{ {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine2"}}}, - expected: map[string]map[string]bool{}, + expected: map[string]victims{}, expectedNumFilterCalled: 2, }, { @@ -1369,7 +1379,7 @@ func TestSelectNodesForPreemption(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a")}, Spec: v1.PodSpec{Containers: smallContainers, Priority: &lowPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "c", UID: types.UID("c")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine2"}}}, - expected: map[string]map[string]bool{"machine1": {"b": true}, "machine2": {"c": true}}, + expected: map[string]victims{"machine1": {pods: sets.NewString("b")}, "machine2": {pods: sets.NewString("c")}}, expectedNumFilterCalled: 5, }, { @@ -1387,7 +1397,7 @@ func TestSelectNodesForPreemption(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "c", UID: types.UID("c")}, Spec: v1.PodSpec{Containers: mediumContainers, Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "d", UID: types.UID("d")}, Spec: v1.PodSpec{Containers: smallContainers, Priority: &highPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "e", UID: types.UID("e")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &highPriority, NodeName: "machine2"}}}, - expected: map[string]map[string]bool{"machine1": {"b": true, "c": true}}, + expected: map[string]victims{"machine1": {pods: sets.NewString("b", "c")}}, expectedNumFilterCalled: 5, }, { @@ -1405,7 +1415,7 @@ func TestSelectNodesForPreemption(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "c", UID: types.UID("c")}, Spec: v1.PodSpec{Containers: mediumContainers, Priority: &midPriority, NodeName: "machine1"}, Status: v1.PodStatus{StartTime: &startTime20190105}}, {ObjectMeta: metav1.ObjectMeta{Name: "d", UID: types.UID("d")}, Spec: v1.PodSpec{Containers: smallContainers, Priority: &highPriority, NodeName: "machine1"}, Status: v1.PodStatus{StartTime: &startTime20190104}}, {ObjectMeta: metav1.ObjectMeta{Name: "e", UID: types.UID("e")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &highPriority, NodeName: "machine2"}, Status: v1.PodStatus{StartTime: &startTime20190103}}}, - expected: map[string]map[string]bool{"machine1": {"a": true, "c": true}}, + expected: map[string]victims{"machine1": {pods: sets.NewString("a", "c")}}, expectedNumFilterCalled: 5, }, { @@ -1441,7 +1451,7 @@ func TestSelectNodesForPreemption(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b")}, Spec: v1.PodSpec{Containers: smallContainers, Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "d", UID: types.UID("d")}, Spec: v1.PodSpec{Containers: smallContainers, Priority: &highPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "e", UID: types.UID("e")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &highPriority, NodeName: "machine2"}}}, - expected: map[string]map[string]bool{"machine1": {"a": true}, "machine2": {}}, + expected: map[string]victims{"machine1": {pods: sets.NewString("a")}, "machine2": {}}, expectedNumFilterCalled: 4, }, { @@ -1522,9 +1532,9 @@ func TestSelectNodesForPreemption(t *testing.T) { Status: v1.PodStatus{Phase: v1.PodRunning}, }, }, - expected: map[string]map[string]bool{ - "node-a": {"pod-a2": true}, - "node-b": {"pod-b1": true}, + expected: map[string]victims{ + "node-a": {pods: sets.NewString("pod-a2")}, + "node-b": {pods: sets.NewString("pod-b1")}, }, expectedNumFilterCalled: 6, }, @@ -1541,9 +1551,26 @@ func TestSelectNodesForPreemption(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "a", UID: types.UID("a")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b", UID: types.UID("b")}, Spec: v1.PodSpec{Containers: largeContainers, Priority: &midPriority, NodeName: "machine2"}}}, filterReturnCode: framework.Unschedulable, - expected: map[string]map[string]bool{}, + expected: map[string]victims{}, expectedNumFilterCalled: 2, }, + { + name: "preemption with violation of same pdb", + registerPlugins: []st.RegisterPluginFunc{ + st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), + st.RegisterFilterPlugin(noderesources.FitName, noderesources.NewFit), + 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"}}}, + pdbs: []*policy.PodDisruptionBudget{ + {Spec: policy.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}}, Status: policy.PodDisruptionBudgetStatus{DisruptionsAllowed: 1}}}, + expected: map[string]victims{"machine1": {pods: sets.NewString("a", "b"), numPDBViolations: 1}}, + expectedNumFilterCalled: 3, + }, } labelKeys := []string{"hostname", "zone", "region"} for _, test := range tests { @@ -1617,7 +1644,7 @@ func TestSelectNodesForPreemption(t *testing.T) { if err != nil { t.Fatal(err) } - nodeToPods, err := g.selectNodesForPreemption(context.Background(), state, test.pod, nodeInfos, nil) + nodeToPods, err := g.selectNodesForPreemption(context.Background(), state, test.pod, nodeInfos, test.pdbs) if err != nil { t.Error(err) }