From 7eeb07d96ac2fb42074d4b461fd7729f28e3a5e1 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Fri, 8 Nov 2024 12:21:50 +0100 Subject: [PATCH] Update nodes sorting function to respect available resources --- .../nodeutilization/nodeutilization.go | 18 +- .../nodeutilization/nodeutilization_test.go | 161 +++++++++--------- 2 files changed, 100 insertions(+), 79 deletions(-) diff --git a/pkg/framework/plugins/nodeutilization/nodeutilization.go b/pkg/framework/plugins/nodeutilization/nodeutilization.go index b3a352aadb..7b052aa0c6 100644 --- a/pkg/framework/plugins/nodeutilization/nodeutilization.go +++ b/pkg/framework/plugins/nodeutilization/nodeutilization.go @@ -368,8 +368,22 @@ func evictPods( // sortNodesByUsage sorts nodes based on usage according to the given plugin. func sortNodesByUsage(nodes []NodeInfo, ascending bool) { sort.Slice(nodes, func(i, j int) bool { - ti := nodes[i].usage[v1.ResourceMemory].Value() + nodes[i].usage[v1.ResourceCPU].MilliValue() + nodes[i].usage[v1.ResourcePods].Value() - tj := nodes[j].usage[v1.ResourceMemory].Value() + nodes[j].usage[v1.ResourceCPU].MilliValue() + nodes[j].usage[v1.ResourcePods].Value() + ti := resource.NewQuantity(0, resource.DecimalSI).Value() + tj := resource.NewQuantity(0, resource.DecimalSI).Value() + for resourceName := range nodes[i].usage { + if resourceName == v1.ResourceCPU { + ti += nodes[i].usage[resourceName].MilliValue() + } else { + ti += nodes[i].usage[resourceName].Value() + } + } + for resourceName := range nodes[j].usage { + if resourceName == v1.ResourceCPU { + tj += nodes[j].usage[resourceName].MilliValue() + } else { + tj += nodes[j].usage[resourceName].Value() + } + } // extended resources for name := range nodes[i].usage { diff --git a/pkg/framework/plugins/nodeutilization/nodeutilization_test.go b/pkg/framework/plugins/nodeutilization/nodeutilization_test.go index 0107d8af39..1703fd492f 100644 --- a/pkg/framework/plugins/nodeutilization/nodeutilization_test.go +++ b/pkg/framework/plugins/nodeutilization/nodeutilization_test.go @@ -25,59 +25,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -var ( - lowPriority = int32(0) - highPriority = int32(10000) - extendedResource = v1.ResourceName("example.com/foo") - testNode1 = NodeInfo{ - NodeUsage: NodeUsage{ - node: &v1.Node{ - Status: v1.NodeStatus{ - Capacity: v1.ResourceList{ - v1.ResourceCPU: *resource.NewMilliQuantity(2000, resource.DecimalSI), - v1.ResourceMemory: *resource.NewQuantity(3977868*1024, resource.BinarySI), - v1.ResourcePods: *resource.NewQuantity(29, resource.BinarySI), - }, - Allocatable: v1.ResourceList{ - v1.ResourceCPU: *resource.NewMilliQuantity(1930, resource.DecimalSI), - v1.ResourceMemory: *resource.NewQuantity(3287692*1024, resource.BinarySI), - v1.ResourcePods: *resource.NewQuantity(29, resource.BinarySI), - }, - }, - ObjectMeta: metav1.ObjectMeta{Name: "node1"}, - }, - usage: map[v1.ResourceName]*resource.Quantity{ - v1.ResourceCPU: resource.NewMilliQuantity(1730, resource.DecimalSI), - v1.ResourceMemory: resource.NewQuantity(3038982964, resource.BinarySI), - v1.ResourcePods: resource.NewQuantity(25, resource.BinarySI), - }, - }, - } - testNode2 = NodeInfo{ - NodeUsage: NodeUsage{ - node: &v1.Node{ - Status: v1.NodeStatus{ - Capacity: v1.ResourceList{ - v1.ResourceCPU: *resource.NewMilliQuantity(2000, resource.DecimalSI), - v1.ResourceMemory: *resource.NewQuantity(3977868*1024, resource.BinarySI), - v1.ResourcePods: *resource.NewQuantity(29, resource.BinarySI), - }, - Allocatable: v1.ResourceList{ - v1.ResourceCPU: *resource.NewMilliQuantity(1930, resource.DecimalSI), - v1.ResourceMemory: *resource.NewQuantity(3287692*1024, resource.BinarySI), - v1.ResourcePods: *resource.NewQuantity(29, resource.BinarySI), - }, - }, - ObjectMeta: metav1.ObjectMeta{Name: "node2"}, - }, - usage: map[v1.ResourceName]*resource.Quantity{ - v1.ResourceCPU: resource.NewMilliQuantity(1220, resource.DecimalSI), - v1.ResourceMemory: resource.NewQuantity(3038982964, resource.BinarySI), - v1.ResourcePods: resource.NewQuantity(11, resource.BinarySI), - }, - }, - } - testNode3 = NodeInfo{ +func BuildTestNodeInfo(name string, apply func(*NodeInfo)) *NodeInfo { + nodeInfo := &NodeInfo{ NodeUsage: NodeUsage{ node: &v1.Node{ Status: v1.NodeStatus{ @@ -92,15 +41,18 @@ var ( v1.ResourcePods: *resource.NewQuantity(29, resource.BinarySI), }, }, - ObjectMeta: metav1.ObjectMeta{Name: "node3"}, - }, - usage: map[v1.ResourceName]*resource.Quantity{ - v1.ResourceCPU: resource.NewMilliQuantity(1530, resource.DecimalSI), - v1.ResourceMemory: resource.NewQuantity(5038982964, resource.BinarySI), - v1.ResourcePods: resource.NewQuantity(20, resource.BinarySI), + ObjectMeta: metav1.ObjectMeta{Name: name}, }, }, } + apply(nodeInfo) + return nodeInfo +} + +var ( + lowPriority = int32(0) + highPriority = int32(10000) + extendedResource = v1.ResourceName("example.com/foo") ) func TestResourceUsagePercentages(t *testing.T) { @@ -141,26 +93,81 @@ func TestResourceUsagePercentages(t *testing.T) { t.Logf("resourceUsagePercentage: %#v\n", resourceUsagePercentage) } -func TestSortNodesByUsageDescendingOrder(t *testing.T) { - nodeList := []NodeInfo{testNode1, testNode2, testNode3} - expectedNodeList := []NodeInfo{testNode3, testNode1, testNode2} // testNode3 has the highest usage - sortNodesByUsage(nodeList, false) // ascending=false, sort nodes in descending order - - for i := 0; i < len(expectedNodeList); i++ { - if nodeList[i].NodeUsage.node.Name != expectedNodeList[i].NodeUsage.node.Name { - t.Errorf("Expected %v, got %v", expectedNodeList[i].NodeUsage.node.Name, nodeList[i].NodeUsage.node.Name) - } +func TestSortNodesByUsage(t *testing.T) { + tests := []struct { + name string + nodeInfoList []NodeInfo + expectedNodeInfoNames []string + }{ + { + name: "cpu memory pods", + nodeInfoList: []NodeInfo{ + *BuildTestNodeInfo("node1", func(nodeInfo *NodeInfo) { + nodeInfo.usage = map[v1.ResourceName]*resource.Quantity{ + v1.ResourceCPU: resource.NewMilliQuantity(1730, resource.DecimalSI), + v1.ResourceMemory: resource.NewQuantity(3038982964, resource.BinarySI), + v1.ResourcePods: resource.NewQuantity(25, resource.BinarySI), + } + }), + *BuildTestNodeInfo("node2", func(nodeInfo *NodeInfo) { + nodeInfo.usage = map[v1.ResourceName]*resource.Quantity{ + v1.ResourceCPU: resource.NewMilliQuantity(1220, resource.DecimalSI), + v1.ResourceMemory: resource.NewQuantity(3038982964, resource.BinarySI), + v1.ResourcePods: resource.NewQuantity(11, resource.BinarySI), + } + }), + *BuildTestNodeInfo("node3", func(nodeInfo *NodeInfo) { + nodeInfo.usage = map[v1.ResourceName]*resource.Quantity{ + v1.ResourceCPU: resource.NewMilliQuantity(1530, resource.DecimalSI), + v1.ResourceMemory: resource.NewQuantity(5038982964, resource.BinarySI), + v1.ResourcePods: resource.NewQuantity(20, resource.BinarySI), + } + }), + }, + expectedNodeInfoNames: []string{"node3", "node1", "node2"}, + }, + { + name: "memory", + nodeInfoList: []NodeInfo{ + *BuildTestNodeInfo("node1", func(nodeInfo *NodeInfo) { + nodeInfo.usage = map[v1.ResourceName]*resource.Quantity{ + v1.ResourceMemory: resource.NewQuantity(3038982964, resource.BinarySI), + } + }), + *BuildTestNodeInfo("node2", func(nodeInfo *NodeInfo) { + nodeInfo.usage = map[v1.ResourceName]*resource.Quantity{ + v1.ResourceMemory: resource.NewQuantity(2038982964, resource.BinarySI), + } + }), + *BuildTestNodeInfo("node3", func(nodeInfo *NodeInfo) { + nodeInfo.usage = map[v1.ResourceName]*resource.Quantity{ + v1.ResourceMemory: resource.NewQuantity(5038982964, resource.BinarySI), + } + }), + }, + expectedNodeInfoNames: []string{"node3", "node1", "node2"}, + }, } -} -func TestSortNodesByUsageAscendingOrder(t *testing.T) { - nodeList := []NodeInfo{testNode1, testNode2, testNode3} - expectedNodeList := []NodeInfo{testNode2, testNode1, testNode3} - sortNodesByUsage(nodeList, true) // ascending=true, sort nodes in ascending order + for _, tc := range tests { + t.Run(tc.name+" descending", func(t *testing.T) { + sortNodesByUsage(tc.nodeInfoList, false) // ascending=false, sort nodes in descending order - for i := 0; i < len(expectedNodeList); i++ { - if nodeList[i].NodeUsage.node.Name != expectedNodeList[i].NodeUsage.node.Name { - t.Errorf("Expected %v, got %v", expectedNodeList[i].NodeUsage.node.Name, nodeList[i].NodeUsage.node.Name) - } + for i := 0; i < len(tc.nodeInfoList); i++ { + if tc.nodeInfoList[i].NodeUsage.node.Name != tc.expectedNodeInfoNames[i] { + t.Errorf("Expected %v, got %v", tc.expectedNodeInfoNames[i], tc.nodeInfoList[i].NodeUsage.node.Name) + } + } + }) + t.Run(tc.name+" ascending", func(t *testing.T) { + sortNodesByUsage(tc.nodeInfoList, true) // ascending=true, sort nodes in ascending order + + size := len(tc.nodeInfoList) + for i := 0; i < size; i++ { + if tc.nodeInfoList[i].NodeUsage.node.Name != tc.expectedNodeInfoNames[size-i-1] { + t.Errorf("Expected %v, got %v", tc.expectedNodeInfoNames[size-i-1], tc.nodeInfoList[i].NodeUsage.node.Name) + } + } + }) } }