diff --git a/pkg/descheduler/pod/pods.go b/pkg/descheduler/pod/pods.go index 91f0f0fed2..8e7ad39dc8 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 aed6361b6f..08140d1796 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,31 @@ import ( "sigs.k8s.io/descheduler/test" ) +var ( + lowPriority = int32(0) + highPriority = int32(10000) +) + +func setHighPriority(pod *v1.Pod) { pod.Spec.Priority = &highPriority } +func setLowPriority(pod *v1.Pod) { pod.Spec.Priority = &lowPriority } + +func makeBestEffortPod(pod *v1.Pod) { + pod.Spec.Containers[0].Resources.Requests = nil + pod.Spec.Containers[0].Resources.Requests = nil + pod.Spec.Containers[0].Resources.Limits = nil + pod.Spec.Containers[0].Resources.Limits = nil +} + +func makeBurstablePod(pod *v1.Pod) { + pod.Spec.Containers[0].Resources.Limits = nil + pod.Spec.Containers[0].Resources.Limits = nil +} + +func makeGuaranteedPod(pod *v1.Pod) { + pod.Spec.Containers[0].Resources.Limits[v1.ResourceCPU] = pod.Spec.Containers[0].Resources.Requests[v1.ResourceCPU] + pod.Spec.Containers[0].Resources.Limits[v1.ResourceMemory] = pod.Spec.Containers[0].Resources.Requests[v1.ResourceMemory] +} + func TestListPodsOnANode(t *testing.T) { testCases := []struct { name string @@ -67,3 +93,41 @@ 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, setLowPriority) + + // BestEffort + p2 := test.BuildTestPod("p2", 400, 0, n1.Name, func(pod *v1.Pod) { + setHighPriority(pod) + makeBestEffortPod(pod) + }) + + // Burstable + p3 := test.BuildTestPod("p3", 400, 0, n1.Name, func(pod *v1.Pod) { + setHighPriority(pod) + makeBurstablePod(pod) + }) + + // Guaranteed + p4 := test.BuildTestPod("p4", 400, 100, n1.Name, func(pod *v1.Pod) { + setHighPriority(pod) + makeGuaranteedPod(pod) + }) + + // Best effort with nil priorities. + p5 := test.BuildTestPod("p5", 400, 100, n1.Name, makeBestEffortPod) + p5.Spec.Priority = nil + + p6 := test.BuildTestPod("p6", 400, 100, n1.Name, 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 26d609cd9d..7dd12ba32e 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 bb346ebfcd..5a9abd132c 100644 --- a/pkg/descheduler/strategies/lownodeutilization_test.go +++ b/pkg/descheduler/strategies/lownodeutilization_test.go @@ -22,15 +22,13 @@ import ( "strings" "testing" - "reflect" - v1 "k8s.io/api/core/v1" "k8s.io/api/policy/v1beta1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - serializer "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" @@ -495,44 +493,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, setLowPriority) - - // BestEffort - p2 := test.BuildTestPod("p2", 400, 0, n1.Name, func(pod *v1.Pod) { - setHighPriority(pod) - makeBestEffortPod(pod) - }) - - // Burstable - p3 := test.BuildTestPod("p3", 400, 0, n1.Name, func(pod *v1.Pod) { - setHighPriority(pod) - makeBurstablePod(pod) - }) - - // Guaranteed - p4 := test.BuildTestPod("p4", 400, 100, n1.Name, func(pod *v1.Pod) { - setHighPriority(pod) - makeGuaranteedPod(pod) - }) - - // Best effort with nil priorities. - p5 := test.BuildTestPod("p5", 400, 100, n1.Name, makeBestEffortPod) - p5.Spec.Priority = nil - - p6 := test.BuildTestPod("p6", 400, 100, n1.Name, 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