From 43525f649348aa7f614aa9e357eb3e1233b9ab8e Mon Sep 17 00:00:00 2001 From: lixiang Date: Tue, 16 Jun 2020 19:19:03 +0800 Subject: [PATCH] Move sortPodsBasedOnPriority to podutil --- README.md | 3 +- pkg/descheduler/pod/pods.go | 25 ++++++++++ pkg/descheduler/pod/pods_test.go | 46 +++++++++++++++++++ .../strategies/lownodeutilization.go | 24 +--------- .../strategies/lownodeutilization_test.go | 42 ----------------- 5 files changed, 74 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 68d695f32..32bced6a3 100644 --- a/README.md +++ b/README.md @@ -238,7 +238,8 @@ When the descheduler decides to evict pods from a node, it employs the following never evicted because these pods won't be recreated. * Pods associated with DaemonSets are never evicted. * Pods with local storage are never evicted. -* Best efforts pods are evicted before burstable and guaranteed pods. +* In `LowNodeUtilization`, pods are evicted by their priority from low to high, and if they have same priority, +best effort pods are evicted before burstable and guaranteed pods. * All types of pods with the annotation descheduler.alpha.kubernetes.io/evict are evicted. This annotation is used to override checks which prevent eviction and users can select which pod is evicted. Users should know how and if the pod will be recreated. diff --git a/pkg/descheduler/pod/pods.go b/pkg/descheduler/pod/pods.go index 91f0f0fed..8e7ad39dc 100644 --- a/pkg/descheduler/pod/pods.go +++ b/pkg/descheduler/pod/pods.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/fields" clientset "k8s.io/client-go/kubernetes" "sigs.k8s.io/descheduler/pkg/utils" + "sort" ) // ListPodsOnANode lists all of the pods on a node @@ -68,3 +69,27 @@ func IsBurstablePod(pod *v1.Pod) bool { func IsGuaranteedPod(pod *v1.Pod) bool { return utils.GetPodQOS(pod) == v1.PodQOSGuaranteed } + +// SortPodsBasedOnPriorityLowToHigh sorts pods based on their priorities from low to high. +// If pods have same priorities, they will be sorted by QoS in the following order: +// BestEffort, Burstable, Guaranteed +func SortPodsBasedOnPriorityLowToHigh(pods []*v1.Pod) { + sort.Slice(pods, func(i, j int) bool { + if pods[i].Spec.Priority == nil && pods[j].Spec.Priority != nil { + return true + } + if pods[j].Spec.Priority == nil && pods[i].Spec.Priority != nil { + return false + } + if (pods[j].Spec.Priority == nil && pods[i].Spec.Priority == nil) || (*pods[i].Spec.Priority == *pods[j].Spec.Priority) { + if IsBestEffortPod(pods[i]) { + return true + } + if IsBurstablePod(pods[i]) && IsGuaranteedPod(pods[j]) { + return true + } + return false + } + return *pods[i].Spec.Priority < *pods[j].Spec.Priority + }) +} diff --git a/pkg/descheduler/pod/pods_test.go b/pkg/descheduler/pod/pods_test.go index aed6361b6..9bff1d2b4 100644 --- a/pkg/descheduler/pod/pods_test.go +++ b/pkg/descheduler/pod/pods_test.go @@ -19,6 +19,7 @@ package pod import ( "context" "fmt" + "reflect" "strings" "testing" @@ -29,6 +30,11 @@ import ( "sigs.k8s.io/descheduler/test" ) +var ( + lowPriority = int32(0) + highPriority = int32(10000) +) + func TestListPodsOnANode(t *testing.T) { testCases := []struct { name string @@ -67,3 +73,43 @@ func TestListPodsOnANode(t *testing.T) { } } } + +func TestSortPodsBasedOnPriorityLowToHigh(t *testing.T) { + n1 := test.BuildTestNode("n1", 4000, 3000, 9, nil) + + p1 := test.BuildTestPod("p1", 400, 0, n1.Name, func(pod *v1.Pod) { + test.SetPodPriority(pod, lowPriority) + }) + + // BestEffort + p2 := test.BuildTestPod("p2", 400, 0, n1.Name, func(pod *v1.Pod) { + test.SetPodPriority(pod, highPriority) + test.MakeBestEffortPod(pod) + }) + + // Burstable + p3 := test.BuildTestPod("p3", 400, 0, n1.Name, func(pod *v1.Pod) { + test.SetPodPriority(pod, highPriority) + test.MakeBurstablePod(pod) + }) + + // Guaranteed + p4 := test.BuildTestPod("p4", 400, 100, n1.Name, func(pod *v1.Pod) { + test.SetPodPriority(pod, highPriority) + test.MakeGuaranteedPod(pod) + }) + + // Best effort with nil priorities. + p5 := test.BuildTestPod("p5", 400, 100, n1.Name, test.MakeBestEffortPod) + p5.Spec.Priority = nil + + p6 := test.BuildTestPod("p6", 400, 100, n1.Name, test.MakeGuaranteedPod) + p6.Spec.Priority = nil + + podList := []*v1.Pod{p4, p3, p2, p1, p6, p5} + + SortPodsBasedOnPriorityLowToHigh(podList) + if !reflect.DeepEqual(podList[len(podList)-1], p4) { + t.Errorf("Expected last pod in sorted list to be %v which of highest priority and guaranteed but got %v", p4, podList[len(podList)-1]) + } +} diff --git a/pkg/descheduler/strategies/lownodeutilization.go b/pkg/descheduler/strategies/lownodeutilization.go index b16f84e61..9e674cbfe 100644 --- a/pkg/descheduler/strategies/lownodeutilization.go +++ b/pkg/descheduler/strategies/lownodeutilization.go @@ -247,7 +247,7 @@ func evictPodsFromTargetNodes( evictablePods = append(append(burstablePods, bestEffortPods...), guaranteedPods...) // sort the evictable Pods based on priority. This also sorts them based on QoS. If there are multiple pods with same priority, they are sorted based on QoS tiers. - sortPodsBasedOnPriority(evictablePods) + podutil.SortPodsBasedOnPriorityLowToHigh(evictablePods) evictPods(ctx, evictablePods, targetThresholds, nodeCapacity, node.usage, &totalPods, &totalCPU, &totalMem, taintsOfLowNodes, podEvictor, node.node) } else { // TODO: Remove this when we support only priority. @@ -338,28 +338,6 @@ func sortNodesByUsage(nodes []NodeUsageMap) { }) } -// sortPodsBasedOnPriority sorts pods based on priority and if their priorities are equal, they are sorted based on QoS tiers. -func sortPodsBasedOnPriority(evictablePods []*v1.Pod) { - sort.Slice(evictablePods, func(i, j int) bool { - if evictablePods[i].Spec.Priority == nil && evictablePods[j].Spec.Priority != nil { - return true - } - if evictablePods[j].Spec.Priority == nil && evictablePods[i].Spec.Priority != nil { - return false - } - if (evictablePods[j].Spec.Priority == nil && evictablePods[i].Spec.Priority == nil) || (*evictablePods[i].Spec.Priority == *evictablePods[j].Spec.Priority) { - if podutil.IsBestEffortPod(evictablePods[i]) { - return true - } - if podutil.IsBurstablePod(evictablePods[i]) && podutil.IsGuaranteedPod(evictablePods[j]) { - return true - } - return false - } - return *evictablePods[i].Spec.Priority < *evictablePods[j].Spec.Priority - }) -} - // createNodePodsMap returns nodepodsmap with evictable pods on node. func createNodePodsMap(ctx context.Context, client clientset.Interface, nodes []*v1.Node) NodePodsMap { npm := NodePodsMap{} diff --git a/pkg/descheduler/strategies/lownodeutilization_test.go b/pkg/descheduler/strategies/lownodeutilization_test.go index 4aa6ece2f..5d87570ed 100644 --- a/pkg/descheduler/strategies/lownodeutilization_test.go +++ b/pkg/descheduler/strategies/lownodeutilization_test.go @@ -22,8 +22,6 @@ import ( "strings" "testing" - "reflect" - v1 "k8s.io/api/core/v1" "k8s.io/api/policy/v1beta1" "k8s.io/apimachinery/pkg/api/resource" @@ -471,46 +469,6 @@ func TestLowNodeUtilization(t *testing.T) { } } -func TestSortPodsByPriority(t *testing.T) { - n1 := test.BuildTestNode("n1", 4000, 3000, 9, nil) - - p1 := test.BuildTestPod("p1", 400, 0, n1.Name, func(pod *v1.Pod) { - test.SetPodPriority(pod, lowPriority) - }) - - // BestEffort - p2 := test.BuildTestPod("p2", 400, 0, n1.Name, func(pod *v1.Pod) { - test.SetPodPriority(pod, highPriority) - test.MakeBestEffortPod(pod) - }) - - // Burstable - p3 := test.BuildTestPod("p3", 400, 0, n1.Name, func(pod *v1.Pod) { - test.SetPodPriority(pod, highPriority) - test.MakeBurstablePod(pod) - }) - - // Guaranteed - p4 := test.BuildTestPod("p4", 400, 100, n1.Name, func(pod *v1.Pod) { - test.SetPodPriority(pod, highPriority) - test.MakeGuaranteedPod(pod) - }) - - // Best effort with nil priorities. - p5 := test.BuildTestPod("p5", 400, 100, n1.Name, test.MakeBestEffortPod) - p5.Spec.Priority = nil - - p6 := test.BuildTestPod("p6", 400, 100, n1.Name, test.MakeGuaranteedPod) - p6.Spec.Priority = nil - - podList := []*v1.Pod{p4, p3, p2, p1, p6, p5} - - sortPodsBasedOnPriority(podList) - if !reflect.DeepEqual(podList[len(podList)-1], p4) { - t.Errorf("Expected last pod in sorted list to be %v which of highest priority and guaranteed but got %v", p4, podList[len(podList)-1]) - } -} - func TestValidateStrategyConfig(t *testing.T) { tests := []struct { name string