Move sortPodsBasedOnPriority to podutil

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
+ })
+}

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])
+ }
+}

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{}

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