Merge pull request #83558 from hprateek43/sortable_list_package

Move Sortable List into its own package
This commit is contained in:
Kubernetes Prow Robot 2019-10-12 11:40:38 -07:00 committed by GitHub
commit aee99ce807
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 16 additions and 74 deletions

View File

@ -19,6 +19,7 @@ package core
import ( import (
"fmt" "fmt"
"reflect" "reflect"
"sort"
"testing" "testing"
"time" "time"
@ -202,7 +203,7 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender(
// and get cached node info by given node name. // and get cached node info by given node name.
nodeInfoCopy := f.cachedNodeNameToInfo[node.GetName()].Clone() nodeInfoCopy := f.cachedNodeNameToInfo[node.GetName()].Clone()
potentialVictims := util.SortableList{CompFunc: util.MoreImportantPod} var potentialVictims []*v1.Pod
removePod := func(rp *v1.Pod) { removePod := func(rp *v1.Pod) {
nodeInfoCopy.RemovePod(rp) nodeInfoCopy.RemovePod(rp)
@ -215,11 +216,11 @@ func (f *FakeExtender) selectVictimsOnNodeByExtender(
podPriority := podutil.GetPodPriority(pod) podPriority := podutil.GetPodPriority(pod)
for _, p := range nodeInfoCopy.Pods() { for _, p := range nodeInfoCopy.Pods() {
if podutil.GetPodPriority(p) < podPriority { if podutil.GetPodPriority(p) < podPriority {
potentialVictims.Items = append(potentialVictims.Items, p) potentialVictims = append(potentialVictims, p)
removePod(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, // 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. // 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. // For now, assume all potential victims to be non-violating.
// Now we try to reprieve non-violating victims. // Now we try to reprieve non-violating victims.
for _, p := range potentialVictims.Items { for _, p := range potentialVictims {
reprievePod(p.(*v1.Pod)) reprievePod(p)
} }
return victims, numViolatingVictim, true, nil return victims, numViolatingVictim, true, nil

View File

@ -1052,9 +1052,9 @@ func (g *genericScheduler) selectNodesForPreemption(
// preempted. // preempted.
// This function is stable and does not change the order of received pods. So, if it // 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. // 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 { for _, obj := range pods {
pod := obj.(*v1.Pod) pod := obj
pdbForPodIsViolated := false pdbForPodIsViolated := false
// A pod with no labels will not match any PDB. So, no need to check. // A pod with no labels will not match any PDB. So, no need to check.
if len(pod.Labels) != 0 { if len(pod.Labels) != 0 {
@ -1110,7 +1110,7 @@ func (g *genericScheduler) selectVictimsOnNode(
queue internalqueue.SchedulingQueue, queue internalqueue.SchedulingQueue,
pdbs []*policy.PodDisruptionBudget, pdbs []*policy.PodDisruptionBudget,
) ([]*v1.Pod, int, bool) { ) ([]*v1.Pod, int, bool) {
potentialVictims := util.SortableList{CompFunc: util.MoreImportantPod} var potentialVictims []*v1.Pod
removePod := func(rp *v1.Pod) error { removePod := func(rp *v1.Pod) error {
if err := nodeInfo.RemovePod(rp); err != nil { if err := nodeInfo.RemovePod(rp); err != nil {
@ -1145,7 +1145,7 @@ func (g *genericScheduler) selectVictimsOnNode(
podPriority := podutil.GetPodPriority(pod) podPriority := podutil.GetPodPriority(pod)
for _, p := range nodeInfo.Pods() { for _, p := range nodeInfo.Pods() {
if podutil.GetPodPriority(p) < podPriority { if podutil.GetPodPriority(p) < podPriority {
potentialVictims.Items = append(potentialVictims.Items, p) potentialVictims = append(potentialVictims, p)
if err := removePod(p); err != nil { if err := removePod(p); err != nil {
return nil, 0, false return nil, 0, false
} }
@ -1166,11 +1166,11 @@ func (g *genericScheduler) selectVictimsOnNode(
} }
var victims []*v1.Pod var victims []*v1.Pod
numViolatingVictim := 0 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 // 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 // violating victims and then other non-violating ones. In both cases, we start
// from the highest priority victims. // from the highest priority victims.
violatingVictims, nonViolatingVictims := filterPodsWithPDBViolation(potentialVictims.Items, pdbs) violatingVictims, nonViolatingVictims := filterPodsWithPDBViolation(potentialVictims, pdbs)
reprievePod := func(p *v1.Pod) (bool, error) { reprievePod := func(p *v1.Pod) (bool, error) {
if err := addPod(p); err != nil { if err := addPod(p); err != nil {
return false, err return false, err

View File

@ -14,7 +14,6 @@ go_test(
], ],
embed = [":go_default_library"], embed = [":go_default_library"],
deps = [ deps = [
"//pkg/api/v1/pod:go_default_library",
"//pkg/scheduler/apis/extender/v1: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/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

View File

@ -17,7 +17,6 @@ limitations under the License.
package util package util
import ( import (
"sort"
"time" "time"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
@ -92,39 +91,15 @@ func GetEarliestPodStartTime(victims *extenderv1.Victims) *metav1.Time {
return earliestPodStartTime 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 // MoreImportantPod return true when priority of the first pod is higher than
// the second one. If two pods' priorities are equal, compare their StartTime. // the second one. If two pods' priorities are equal, compare their StartTime.
// It takes arguments of the type "interface{}" to be used with SortableList, // It takes arguments of the type "interface{}" to be used with SortableList,
// but expects those arguments to be *v1.Pod. // but expects those arguments to be *v1.Pod.
func MoreImportantPod(pod1, pod2 interface{}) bool { func MoreImportantPod(pod1, pod2 *v1.Pod) bool {
p1 := podutil.GetPodPriority(pod1.(*v1.Pod)) p1 := podutil.GetPodPriority(pod1)
p2 := podutil.GetPodPriority(pod2.(*v1.Pod)) p2 := podutil.GetPodPriority(pod2)
if p1 != p2 { if p1 != p2 {
return p1 > p2 return p1 > p2
} }
return GetPodStartTime(pod1.(*v1.Pod)).Before(GetPodStartTime(pod2.(*v1.Pod))) return GetPodStartTime(pod1).Before(GetPodStartTime(pod2))
} }

View File

@ -25,44 +25,11 @@ import (
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/diff"
"k8s.io/kubernetes/pkg/api/v1/pod"
extenderv1 "k8s.io/kubernetes/pkg/scheduler/apis/extender/v1" extenderv1 "k8s.io/kubernetes/pkg/scheduler/apis/extender/v1"
) )
// TestSortableList tests SortableList by storing pods in the list and sorting // TestSortableList tests SortableList by storing pods in the list and sorting
// them by their priority. // 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) { func TestGetContainerPorts(t *testing.T) {
tests := []struct { tests := []struct {