From 0901cb18bf3a97baf893a9b081bbbe3d03a0aea6 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Sat, 22 Jun 2024 15:02:07 +0200 Subject: [PATCH] NewPodEvictor: drop nodes parameter --- pkg/descheduler/descheduler.go | 1 - pkg/descheduler/evictions/evictions.go | 14 +---- pkg/descheduler/evictions/evictions_test.go | 52 +++++++++++++++++++ .../highnodeutilization_test.go | 2 - .../lownodeutilization_test.go | 2 - .../plugins/podlifetime/pod_lifetime_test.go | 1 - .../removeduplicates/removeduplicates_test.go | 2 - .../removefailedpods/failedpods_test.go | 1 - .../toomanyrestarts_test.go | 1 - .../pod_antiaffinity_test.go | 1 - .../node_affinity_test.go | 1 - .../node_taint_test.go | 1 - .../topologyspreadconstraint_test.go | 1 - pkg/framework/profile/profile_test.go | 7 ++- test/e2e/e2e_duplicatepods_test.go | 3 +- test/e2e/e2e_failedpods_test.go | 2 +- test/e2e/e2e_test.go | 8 ++- test/e2e/e2e_toomanyrestarts_test.go | 3 +- test/e2e/e2e_topologyspreadconstraint_test.go | 4 +- 19 files changed, 65 insertions(+), 42 deletions(-) diff --git a/pkg/descheduler/descheduler.go b/pkg/descheduler/descheduler.go index 92ffb56e7..e257db9ee 100644 --- a/pkg/descheduler/descheduler.go +++ b/pkg/descheduler/descheduler.go @@ -160,7 +160,6 @@ func (d *descheduler) runDeschedulerLoop(ctx context.Context, nodes []*v1.Node) d.rs.DryRun, d.deschedulerPolicy.MaxNoOfPodsToEvictPerNode, d.deschedulerPolicy.MaxNoOfPodsToEvictPerNamespace, - nodes, !d.rs.DisableMetrics, d.eventRecorder, ) diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index ffbf287df..b7119f54c 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/pkg/descheduler/evictions/evictions.go @@ -43,7 +43,6 @@ type ( type PodEvictor struct { client clientset.Interface - nodes []*v1.Node policyGroupVersion string dryRun bool maxPodsToEvictPerNode *uint @@ -60,26 +59,17 @@ func NewPodEvictor( dryRun bool, maxPodsToEvictPerNode *uint, maxPodsToEvictPerNamespace *uint, - nodes []*v1.Node, metricsEnabled bool, eventRecorder events.EventRecorder, ) *PodEvictor { - nodePodCount := make(nodePodEvictedCount) - namespacePodCount := make(namespacePodEvictCount) - for _, node := range nodes { - // Initialize podsEvicted till now with 0. - nodePodCount[node.Name] = 0 - } - return &PodEvictor{ client: client, - nodes: nodes, policyGroupVersion: policyGroupVersion, dryRun: dryRun, maxPodsToEvictPerNode: maxPodsToEvictPerNode, maxPodsToEvictPerNamespace: maxPodsToEvictPerNamespace, - nodepodCount: nodePodCount, - namespacePodCount: namespacePodCount, + nodepodCount: make(nodePodEvictedCount), + namespacePodCount: make(namespacePodEvictCount), metricsEnabled: metricsEnabled, eventRecorder: eventRecorder, } diff --git a/pkg/descheduler/evictions/evictions_test.go b/pkg/descheduler/evictions/evictions_test.go index 9a931c781..3f2dbe9ed 100644 --- a/pkg/descheduler/evictions/evictions_test.go +++ b/pkg/descheduler/evictions/evictions_test.go @@ -22,9 +22,12 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" + "k8s.io/client-go/tools/events" + utilpointer "k8s.io/utils/pointer" podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod" "sigs.k8s.io/descheduler/pkg/utils" "sigs.k8s.io/descheduler/test" @@ -113,3 +116,52 @@ func TestPodTypes(t *testing.T) { t.Errorf("Expected p1 to be a normal pod.") } } + +func TestNewPodEvictor(t *testing.T) { + pod1 := test.BuildTestPod("pod", 400, 0, "node", nil) + + fakeClient := fake.NewSimpleClientset(pod1) + + eventRecorder := &events.FakeRecorder{} + + podEvictor := NewPodEvictor( + fakeClient, + "policy/v1", + false, + utilpointer.Uint(1), + nil, + false, + eventRecorder, + ) + + stubNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node"}} + + // 0 evictions expected + if evictions := podEvictor.NodeEvicted(stubNode); evictions != 0 { + t.Errorf("Expected 0 node evictions, got %q instead", evictions) + } + // 0 evictions expected + if evictions := podEvictor.TotalEvicted(); evictions != 0 { + t.Errorf("Expected 0 total evictions, got %q instead", evictions) + } + + if podEvictor.NodeLimitExceeded(stubNode) { + t.Errorf("Expected NodeLimitExceeded to return false, got true instead") + } + + if !podEvictor.EvictPod(context.TODO(), pod1, EvictOptions{}) { + t.Errorf("Expected a pod eviction, got no eviction instead") + } + + // 1 node eviction expected + if evictions := podEvictor.NodeEvicted(stubNode); evictions != 1 { + t.Errorf("Expected 1 node eviction, got %q instead", evictions) + } + // 1 total eviction expected + if evictions := podEvictor.TotalEvicted(); evictions != 1 { + t.Errorf("Expected 1 total evictions, got %q instead", evictions) + } + if !podEvictor.NodeLimitExceeded(stubNode) { + t.Errorf("Expected NodeLimitExceeded to return true, got false instead") + } +} diff --git a/pkg/framework/plugins/nodeutilization/highnodeutilization_test.go b/pkg/framework/plugins/nodeutilization/highnodeutilization_test.go index bfe55594e..7083c319a 100644 --- a/pkg/framework/plugins/nodeutilization/highnodeutilization_test.go +++ b/pkg/framework/plugins/nodeutilization/highnodeutilization_test.go @@ -490,7 +490,6 @@ func TestHighNodeUtilization(t *testing.T) { false, nil, nil, - testCase.nodes, false, eventRecorder, ) @@ -642,7 +641,6 @@ func TestHighNodeUtilizationWithTaints(t *testing.T) { false, &item.evictionsExpected, nil, - item.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/nodeutilization/lownodeutilization_test.go b/pkg/framework/plugins/nodeutilization/lownodeutilization_test.go index 63ade79f0..660749445 100644 --- a/pkg/framework/plugins/nodeutilization/lownodeutilization_test.go +++ b/pkg/framework/plugins/nodeutilization/lownodeutilization_test.go @@ -892,7 +892,6 @@ func TestLowNodeUtilization(t *testing.T) { false, nil, nil, - test.nodes, false, eventRecorder, ) @@ -1064,7 +1063,6 @@ func TestLowNodeUtilizationWithTaints(t *testing.T) { false, &item.evictionsExpected, nil, - item.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/podlifetime/pod_lifetime_test.go b/pkg/framework/plugins/podlifetime/pod_lifetime_test.go index 16536cbc3..195e94fa1 100644 --- a/pkg/framework/plugins/podlifetime/pod_lifetime_test.go +++ b/pkg/framework/plugins/podlifetime/pod_lifetime_test.go @@ -562,7 +562,6 @@ func TestPodLifeTime(t *testing.T) { false, tc.maxPodsToEvictPerNode, tc.maxPodsToEvictPerNamespace, - tc.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/removeduplicates/removeduplicates_test.go b/pkg/framework/plugins/removeduplicates/removeduplicates_test.go index 6a14ba878..f0f3ecc79 100644 --- a/pkg/framework/plugins/removeduplicates/removeduplicates_test.go +++ b/pkg/framework/plugins/removeduplicates/removeduplicates_test.go @@ -319,7 +319,6 @@ func TestFindDuplicatePods(t *testing.T) { false, nil, nil, - testCase.nodes, false, eventRecorder, ) @@ -768,7 +767,6 @@ func TestRemoveDuplicatesUniformly(t *testing.T) { false, nil, nil, - testCase.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/removefailedpods/failedpods_test.go b/pkg/framework/plugins/removefailedpods/failedpods_test.go index d052fc670..8acaf77af 100644 --- a/pkg/framework/plugins/removefailedpods/failedpods_test.go +++ b/pkg/framework/plugins/removefailedpods/failedpods_test.go @@ -381,7 +381,6 @@ func TestRemoveFailedPods(t *testing.T) { false, nil, nil, - tc.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/removepodshavingtoomanyrestarts/toomanyrestarts_test.go b/pkg/framework/plugins/removepodshavingtoomanyrestarts/toomanyrestarts_test.go index bf50e89ae..4958fdf98 100644 --- a/pkg/framework/plugins/removepodshavingtoomanyrestarts/toomanyrestarts_test.go +++ b/pkg/framework/plugins/removepodshavingtoomanyrestarts/toomanyrestarts_test.go @@ -350,7 +350,6 @@ func TestRemovePodsHavingTooManyRestarts(t *testing.T) { false, tc.maxPodsToEvictPerNode, tc.maxNoOfPodsToEvictPerNamespace, - tc.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/removepodsviolatinginterpodantiaffinity/pod_antiaffinity_test.go b/pkg/framework/plugins/removepodsviolatinginterpodantiaffinity/pod_antiaffinity_test.go index 648298844..23990ab4c 100644 --- a/pkg/framework/plugins/removepodsviolatinginterpodantiaffinity/pod_antiaffinity_test.go +++ b/pkg/framework/plugins/removepodsviolatinginterpodantiaffinity/pod_antiaffinity_test.go @@ -239,7 +239,6 @@ func TestPodAntiAffinity(t *testing.T) { false, test.maxPodsToEvictPerNode, test.maxNoOfPodsToEvictPerNamespace, - test.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/removepodsviolatingnodeaffinity/node_affinity_test.go b/pkg/framework/plugins/removepodsviolatingnodeaffinity/node_affinity_test.go index 9cf0cdd2a..32f287888 100644 --- a/pkg/framework/plugins/removepodsviolatingnodeaffinity/node_affinity_test.go +++ b/pkg/framework/plugins/removepodsviolatingnodeaffinity/node_affinity_test.go @@ -365,7 +365,6 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) { false, tc.maxPodsToEvictPerNode, tc.maxNoOfPodsToEvictPerNamespace, - tc.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go b/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go index 1cbfc3e54..1c6539aba 100644 --- a/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go +++ b/pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go @@ -406,7 +406,6 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) { false, tc.maxPodsToEvictPerNode, tc.maxNoOfPodsToEvictPerNamespace, - tc.nodes, false, eventRecorder, ) diff --git a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint_test.go b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint_test.go index d7ab72682..7a75ef91c 100644 --- a/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint_test.go +++ b/pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint_test.go @@ -1462,7 +1462,6 @@ func TestTopologySpreadConstraint(t *testing.T) { false, nil, nil, - tc.nodes, false, eventRecorder, ) diff --git a/pkg/framework/profile/profile_test.go b/pkg/framework/profile/profile_test.go index 8823c57a6..8309739ab 100644 --- a/pkg/framework/profile/profile_test.go +++ b/pkg/framework/profile/profile_test.go @@ -244,7 +244,7 @@ func TestProfileDescheduleBalanceExtensionPointsEviction(t *testing.T) { eventBroadcaster, eventRecorder := utils.GetRecorderAndBroadcaster(ctx, eventClient) defer eventBroadcaster.Shutdown() - podEvictor := evictions.NewPodEvictor(client, "policy/v1", false, nil, nil, nodes, true, eventRecorder) + podEvictor := evictions.NewPodEvictor(client, "policy/v1", false, nil, nil, true, eventRecorder) prfl, err := NewProfile( test.config, @@ -306,7 +306,6 @@ func TestProfileExtensionPoints(t *testing.T) { n1 := testutils.BuildTestNode("n1", 2000, 3000, 10, nil) n2 := testutils.BuildTestNode("n2", 2000, 3000, 10, nil) - nodes := []*v1.Node{n1, n2} p1 := testutils.BuildTestPod(fmt.Sprintf("pod_1_%s", n1.Name), 200, 0, n1.Name, nil) p1.ObjectMeta.OwnerReferences = []metav1.OwnerReference{{}} @@ -393,7 +392,7 @@ func TestProfileExtensionPoints(t *testing.T) { eventBroadcaster, eventRecorder := utils.GetRecorderAndBroadcaster(ctx, eventClient) defer eventBroadcaster.Shutdown() - podEvictor := evictions.NewPodEvictor(client, "policy/v1", false, nil, nil, nodes, true, eventRecorder) + podEvictor := evictions.NewPodEvictor(client, "policy/v1", false, nil, nil, true, eventRecorder) prfl, err := NewProfile( api.DeschedulerProfile{ @@ -605,7 +604,7 @@ func TestProfileExtensionPointOrdering(t *testing.T) { eventBroadcaster, eventRecorder := utils.GetRecorderAndBroadcaster(ctx, eventClient) defer eventBroadcaster.Shutdown() - podEvictor := evictions.NewPodEvictor(client, "policy/v1", false, nil, nil, nodes, true, eventRecorder) + podEvictor := evictions.NewPodEvictor(client, "policy/v1", false, nil, nil, true, eventRecorder) prfl, err := NewProfile( api.DeschedulerProfile{ diff --git a/test/e2e/e2e_duplicatepods_test.go b/test/e2e/e2e_duplicatepods_test.go index 86a79d18a..01b4b5778 100644 --- a/test/e2e/e2e_duplicatepods_test.go +++ b/test/e2e/e2e_duplicatepods_test.go @@ -48,7 +48,7 @@ func TestRemoveDuplicates(t *testing.T) { t.Errorf("Error listing node with %v", err) } - nodes, workerNodes := splitNodesAndWorkerNodes(nodeList.Items) + _, workerNodes := splitNodesAndWorkerNodes(nodeList.Items) t.Log("Creating testing namespace") testNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "e2e-" + strings.ToLower(t.Name())}} @@ -177,7 +177,6 @@ func TestRemoveDuplicates(t *testing.T) { false, nil, nil, - nodes, false, eventRecorder, ) diff --git a/test/e2e/e2e_failedpods_test.go b/test/e2e/e2e_failedpods_test.go index b94c1f373..ca5ad5dd1 100644 --- a/test/e2e/e2e_failedpods_test.go +++ b/test/e2e/e2e_failedpods_test.go @@ -75,7 +75,7 @@ func TestFailedPods(t *testing.T) { defer jobClient.Delete(ctx, job.Name, metav1.DeleteOptions{PropagationPolicy: &deletePropagationPolicy}) waitForJobPodPhase(ctx, t, clientSet, job, v1.PodFailed) - podEvictor := initPodEvictorOrFail(t, clientSet, getPodsAssignedToNode, nodes) + podEvictor := initPodEvictorOrFail(t, clientSet, getPodsAssignedToNode) defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{ EvictLocalStoragePods: true, diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 8b92212a1..2527d4e15 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -196,7 +196,6 @@ func runPodLifetimePlugin( false, nil, maxPodsToEvictPerNamespace, - nodes, false, &events.FakeRecorder{}, ) @@ -285,7 +284,7 @@ func TestLowNodeUtilization(t *testing.T) { t.Errorf("Error listing node with %v", err) } - nodes, workerNodes := splitNodesAndWorkerNodes(nodeList.Items) + _, workerNodes := splitNodesAndWorkerNodes(nodeList.Items) t.Log("Creating testing namespace") testNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "e2e-" + strings.ToLower(t.Name())}} @@ -374,7 +373,7 @@ func TestLowNodeUtilization(t *testing.T) { waitForRCPodsRunning(ctx, t, clientSet, rc) // Run LowNodeUtilization plugin - podEvictor := initPodEvictorOrFail(t, clientSet, getPodsAssignedToNode, nodes) + podEvictor := initPodEvictorOrFail(t, clientSet, getPodsAssignedToNode) defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{ EvictLocalStoragePods: true, @@ -1570,7 +1569,7 @@ func splitNodesAndWorkerNodes(nodes []v1.Node) ([]*v1.Node, []*v1.Node) { return allNodes, workerNodes } -func initPodEvictorOrFail(t *testing.T, clientSet clientset.Interface, getPodsAssignedToNode podutil.GetPodsAssignedToNodeFunc, nodes []*v1.Node) *evictions.PodEvictor { +func initPodEvictorOrFail(t *testing.T, clientSet clientset.Interface, getPodsAssignedToNode podutil.GetPodsAssignedToNodeFunc) *evictions.PodEvictor { evictionPolicyGroupVersion, err := eutils.SupportEviction(clientSet) if err != nil || len(evictionPolicyGroupVersion) == 0 { t.Fatalf("Error creating eviction policy group: %v", err) @@ -1584,7 +1583,6 @@ func initPodEvictorOrFail(t *testing.T, clientSet clientset.Interface, getPodsAs false, nil, nil, - nodes, false, eventRecorder, ) diff --git a/test/e2e/e2e_toomanyrestarts_test.go b/test/e2e/e2e_toomanyrestarts_test.go index c74e7a8d5..63c986ee9 100644 --- a/test/e2e/e2e_toomanyrestarts_test.go +++ b/test/e2e/e2e_toomanyrestarts_test.go @@ -50,7 +50,7 @@ func TestTooManyRestarts(t *testing.T) { t.Errorf("Error listing node with %v", err) } - nodes, workerNodes := splitNodesAndWorkerNodes(nodeList.Items) + _, workerNodes := splitNodesAndWorkerNodes(nodeList.Items) t.Logf("Creating testing namespace %v", t.Name()) testNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "e2e-" + strings.ToLower(t.Name())}} @@ -167,7 +167,6 @@ func TestTooManyRestarts(t *testing.T) { false, nil, nil, - nodes, false, eventRecorder, ) diff --git a/test/e2e/e2e_topologyspreadconstraint_test.go b/test/e2e/e2e_topologyspreadconstraint_test.go index 7c0fbb366..6ecc347ae 100644 --- a/test/e2e/e2e_topologyspreadconstraint_test.go +++ b/test/e2e/e2e_topologyspreadconstraint_test.go @@ -27,7 +27,7 @@ func TestTopologySpreadConstraint(t *testing.T) { if err != nil { t.Errorf("Error listing node with %v", err) } - nodes, workerNodes := splitNodesAndWorkerNodes(nodeList.Items) + _, workerNodes := splitNodesAndWorkerNodes(nodeList.Items) t.Log("Creating testing namespace") testNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "e2e-" + strings.ToLower(t.Name())}} if _, err := clientSet.CoreV1().Namespaces().Create(ctx, testNamespace, metav1.CreateOptions{}); err != nil { @@ -138,7 +138,7 @@ func TestTopologySpreadConstraint(t *testing.T) { defer test.DeleteDeployment(ctx, t, clientSet, violatorDeployment) test.WaitForDeploymentPodsRunning(ctx, t, clientSet, violatorDeployment) - podEvictor := initPodEvictorOrFail(t, clientSet, getPodsAssignedToNode, nodes) + podEvictor := initPodEvictorOrFail(t, clientSet, getPodsAssignedToNode) // Run TopologySpreadConstraint strategy t.Logf("Running RemovePodsViolatingTopologySpreadConstraint strategy for %s", name)