From 589656108eff41abe5bf9e5653c59340e1318aba Mon Sep 17 00:00:00 2001 From: Harsh Singh Date: Fri, 11 Oct 2019 11:13:25 +0530 Subject: [PATCH] Replaced sortable list with native golang slice. --- pkg/scheduler/core/extender_test.go | 11 +++++---- pkg/scheduler/core/generic_scheduler.go | 12 ++++----- pkg/scheduler/util/BUILD | 1 - pkg/scheduler/util/utils.go | 33 +++---------------------- pkg/scheduler/util/utils_test.go | 33 ------------------------- 5 files changed, 16 insertions(+), 74 deletions(-) diff --git a/pkg/scheduler/core/extender_test.go b/pkg/scheduler/core/extender_test.go index b7f8f782a44..8dd690a79e1 100644 --- a/pkg/scheduler/core/extender_test.go +++ b/pkg/scheduler/core/extender_test.go @@ -19,6 +19,7 @@ package core import ( "fmt" "reflect" + "sort" "testing" "time" @@ -202,7 +203,7 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender( // and get cached node info by given node name. nodeInfoCopy := f.cachedNodeNameToInfo[node.GetName()].Clone() - potentialVictims := util.SortableList{CompFunc: util.MoreImportantPod} + var potentialVictims []*v1.Pod removePod := func(rp *v1.Pod) { nodeInfoCopy.RemovePod(rp) @@ -215,11 +216,11 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender( podPriority := podutil.GetPodPriority(pod) for _, p := range nodeInfoCopy.Pods() { if podutil.GetPodPriority(p) < podPriority { - potentialVictims.Items = append(potentialVictims.Items, p) + potentialVictims = append(potentialVictims, p) removePod(p) } } - potentialVictims.Sort() + sort.Slice(potentialVictims, func(i, j int) bool { return util.MoreImportantPod(potentialVictims[i], potentialVictims[j]) }) // If the new pod does not fit after removing all the lower priority pods, // we are almost done and this node is not suitable for preemption. @@ -248,8 +249,8 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender( // For now, assume all potential victims to be non-violating. // Now we try to reprieve non-violating victims. - for _, p := range potentialVictims.Items { - reprievePod(p.(*v1.Pod)) + for _, p := range potentialVictims { + reprievePod(p) } return victims, numViolatingVictim, true, nil diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index 1dd238f6091..29c09ba616f 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -1047,9 +1047,9 @@ func (g *genericScheduler) selectNodesForPreemption( // preempted. // 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 []interface{}, pdbs []*policy.PodDisruptionBudget) (violatingPods, nonViolatingPods []*v1.Pod) { +func filterPodsWithPDBViolation(pods []*v1.Pod, pdbs []*policy.PodDisruptionBudget) (violatingPods, nonViolatingPods []*v1.Pod) { for _, obj := range pods { - pod := obj.(*v1.Pod) + pod := obj pdbForPodIsViolated := false // A pod with no labels will not match any PDB. So, no need to check. if len(pod.Labels) != 0 { @@ -1105,7 +1105,7 @@ func (g *genericScheduler) selectVictimsOnNode( queue internalqueue.SchedulingQueue, pdbs []*policy.PodDisruptionBudget, ) ([]*v1.Pod, int, bool) { - potentialVictims := util.SortableList{CompFunc: util.MoreImportantPod} + var potentialVictims []*v1.Pod removePod := func(rp *v1.Pod) error { if err := nodeInfo.RemovePod(rp); err != nil { @@ -1140,7 +1140,7 @@ func (g *genericScheduler) selectVictimsOnNode( podPriority := podutil.GetPodPriority(pod) for _, p := range nodeInfo.Pods() { if podutil.GetPodPriority(p) < podPriority { - potentialVictims.Items = append(potentialVictims.Items, p) + potentialVictims = append(potentialVictims, p) if err := removePod(p); err != nil { return nil, 0, false } @@ -1161,11 +1161,11 @@ func (g *genericScheduler) selectVictimsOnNode( } var victims []*v1.Pod numViolatingVictim := 0 - potentialVictims.Sort() + sort.Slice(potentialVictims, func(i, j int) bool { return util.MoreImportantPod(potentialVictims[i], potentialVictims[j]) }) // Try to reprieve as many pods as possible. We first try to reprieve the PDB // violating victims and then other non-violating ones. In both cases, we start // from the highest priority victims. - violatingVictims, nonViolatingVictims := filterPodsWithPDBViolation(potentialVictims.Items, pdbs) + violatingVictims, nonViolatingVictims := filterPodsWithPDBViolation(potentialVictims, pdbs) reprievePod := func(p *v1.Pod) (bool, error) { if err := addPod(p); err != nil { return false, err diff --git a/pkg/scheduler/util/BUILD b/pkg/scheduler/util/BUILD index b885c176bb7..4518ec6c45e 100644 --- a/pkg/scheduler/util/BUILD +++ b/pkg/scheduler/util/BUILD @@ -14,7 +14,6 @@ go_test( ], embed = [":go_default_library"], deps = [ - "//pkg/api/v1/pod:go_default_library", "//pkg/scheduler/apis/extender/v1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/scheduler/util/utils.go b/pkg/scheduler/util/utils.go index 7dc2a7fa161..c1d5c3f5d97 100644 --- a/pkg/scheduler/util/utils.go +++ b/pkg/scheduler/util/utils.go @@ -17,7 +17,6 @@ limitations under the License. package util import ( - "sort" "time" v1 "k8s.io/api/core/v1" @@ -92,39 +91,15 @@ func GetEarliestPodStartTime(victims *extenderv1.Victims) *metav1.Time { return earliestPodStartTime } -// SortableList is a list that implements sort.Interface. -type SortableList struct { - Items []interface{} - CompFunc lessFunc -} - -var _ = sort.Interface(&SortableList{}) - -func (l *SortableList) Len() int { return len(l.Items) } - -func (l *SortableList) Less(i, j int) bool { - return l.CompFunc(l.Items[i], l.Items[j]) -} - -func (l *SortableList) Swap(i, j int) { - l.Items[i], l.Items[j] = l.Items[j], l.Items[i] -} - -// Sort sorts the items in the list using the given CompFunc. Item1 is placed -// before Item2 when CompFunc(Item1, Item2) returns true. -func (l *SortableList) Sort() { - sort.Sort(l) -} - // MoreImportantPod return true when priority of the first pod is higher than // the second one. If two pods' priorities are equal, compare their StartTime. // It takes arguments of the type "interface{}" to be used with SortableList, // but expects those arguments to be *v1.Pod. -func MoreImportantPod(pod1, pod2 interface{}) bool { - p1 := podutil.GetPodPriority(pod1.(*v1.Pod)) - p2 := podutil.GetPodPriority(pod2.(*v1.Pod)) +func MoreImportantPod(pod1, pod2 *v1.Pod) bool { + p1 := podutil.GetPodPriority(pod1) + p2 := podutil.GetPodPriority(pod2) if p1 != p2 { return p1 > p2 } - return GetPodStartTime(pod1.(*v1.Pod)).Before(GetPodStartTime(pod2.(*v1.Pod))) + return GetPodStartTime(pod1).Before(GetPodStartTime(pod2)) } diff --git a/pkg/scheduler/util/utils_test.go b/pkg/scheduler/util/utils_test.go index 7d210d6952c..fe37baf634c 100644 --- a/pkg/scheduler/util/utils_test.go +++ b/pkg/scheduler/util/utils_test.go @@ -25,44 +25,11 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" - "k8s.io/kubernetes/pkg/api/v1/pod" extenderv1 "k8s.io/kubernetes/pkg/scheduler/apis/extender/v1" ) // TestSortableList tests SortableList by storing pods in the list and sorting // them by their priority. -func TestSortableList(t *testing.T) { - higherPriority := func(pod1, pod2 interface{}) bool { - return pod.GetPodPriority(pod1.(*v1.Pod)) > pod.GetPodPriority(pod2.(*v1.Pod)) - } - podList := SortableList{CompFunc: higherPriority} - // Add a few Pods with different priorities from lowest to highest priority. - for i := 0; i < 10; i++ { - var p = int32(i) - pod := &v1.Pod{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "container", - Image: "image", - }, - }, - Priority: &p, - }, - } - podList.Items = append(podList.Items, pod) - } - podList.Sort() - if len(podList.Items) != 10 { - t.Errorf("expected length of list was 10, got: %v", len(podList.Items)) - } - var prevPriority = int32(10) - for _, p := range podList.Items { - if *p.(*v1.Pod).Spec.Priority >= prevPriority { - t.Errorf("Pods are not soreted. Current pod pririty is %v, while previous one was %v.", *p.(*v1.Pod).Spec.Priority, prevPriority) - } - } -} func TestGetContainerPorts(t *testing.T) { tests := []struct {