feat: pod evictor options

pkg/descheduler/descheduler.go

@@ -97,12 +97,13 @@ func newDescheduler(rs *options.DeschedulerServer, deschedulerPolicy *api.Desche
 
 	podEvictor := evictions.NewPodEvictor(
 		nil,
-		evictionPolicyGroupVersion,
-		rs.DryRun,
-		deschedulerPolicy.MaxNoOfPodsToEvictPerNode,
-		deschedulerPolicy.MaxNoOfPodsToEvictPerNamespace,
-		!rs.DisableMetrics,
 		eventRecorder,
+		evictions.NewOptions().
+			WithPolicyGroupVersion(evictionPolicyGroupVersion).
+			WithMaxPodsToEvictPerNode(deschedulerPolicy.MaxNoOfPodsToEvictPerNode).
+			WithMaxPodsToEvictPerNamespace(deschedulerPolicy.MaxNoOfPodsToEvictPerNamespace).
+			WithDryRun(rs.DryRun).
+			WithMetricsEnabled(!rs.DisableMetrics),
 	)
 
 	return &descheduler{

pkg/descheduler/evictions/evictions.go

@@ -55,23 +55,23 @@ type PodEvictor struct {
 
 func NewPodEvictor(
 	client clientset.Interface,
-	policyGroupVersion string,
-	dryRun bool,
-	maxPodsToEvictPerNode *uint,
-	maxPodsToEvictPerNamespace *uint,
-	metricsEnabled bool,
 	eventRecorder events.EventRecorder,
+	options *Options,
 ) *PodEvictor {
+	if options == nil {
+		options = NewOptions()
+	}
+
 	return &PodEvictor{
 		client:                     client,
-		policyGroupVersion:         policyGroupVersion,
+		eventRecorder:              eventRecorder,
-		dryRun:                     dryRun,
+		policyGroupVersion:         options.policyGroupVersion,
-		maxPodsToEvictPerNode:      maxPodsToEvictPerNode,
+		dryRun:                     options.dryRun,
-		maxPodsToEvictPerNamespace: maxPodsToEvictPerNamespace,
+		maxPodsToEvictPerNode:      options.maxPodsToEvictPerNode,
+		maxPodsToEvictPerNamespace: options.maxPodsToEvictPerNamespace,
+		metricsEnabled:             options.metricsEnabled,
 		nodepodCount:               make(nodePodEvictedCount),
 		namespacePodCount:          make(namespacePodEvictCount),
-		metricsEnabled:             metricsEnabled,
-		eventRecorder:              eventRecorder,
 	}
 }
 

pkg/descheduler/evictions/evictions_test.go

@@ -126,12 +126,8 @@ func TestNewPodEvictor(t *testing.T) {
 
 	podEvictor := NewPodEvictor(
 		fakeClient,
-		"policy/v1",
-		false,
-		utilpointer.Uint(1),
-		nil,
-		false,
 		eventRecorder,
+		NewOptions().WithMaxPodsToEvictPerNode(utilpointer.Uint(1)),
 	)
 
 	stubNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node"}}

pkg/descheduler/evictions/options.go

@@ -0,0 +1,45 @@
+package evictions
+
+import (
+	policy "k8s.io/api/policy/v1"
+)
+
+type Options struct {
+	policyGroupVersion         string
+	dryRun                     bool
+	maxPodsToEvictPerNode      *uint
+	maxPodsToEvictPerNamespace *uint
+	metricsEnabled             bool
+}
+
+// NewOptions returns an Options with default values.
+func NewOptions() *Options {
+	return &Options{
+		policyGroupVersion: policy.SchemeGroupVersion.String(),
+	}
+}
+
+func (o *Options) WithPolicyGroupVersion(policyGroupVersion string) *Options {
+	o.policyGroupVersion = policyGroupVersion
+	return o
+}
+
+func (o *Options) WithDryRun(dryRun bool) *Options {
+	o.dryRun = dryRun
+	return o
+}
+
+func (o *Options) WithMaxPodsToEvictPerNode(maxPodsToEvictPerNode *uint) *Options {
+	o.maxPodsToEvictPerNode = maxPodsToEvictPerNode
+	return o
+}
+
+func (o *Options) WithMaxPodsToEvictPerNamespace(maxPodsToEvictPerNamespace *uint) *Options {
+	o.maxPodsToEvictPerNamespace = maxPodsToEvictPerNamespace
+	return o
+}
+
+func (o *Options) WithMetricsEnabled(metricsEnabled bool) *Options {
+	o.metricsEnabled = metricsEnabled
+	return o
+}

pkg/framework/plugins/nodeutilization/highnodeutilization_test.go

@@ -486,15 +486,7 @@ func TestHighNodeUtilization(t *testing.T) {
 
 			eventRecorder := &events.FakeRecorder{}
 
-			podEvictor := evictions.NewPodEvictor(
+			podEvictor := evictions.NewPodEvictor(fakeClient, eventRecorder, nil)
-				fakeClient,
-				"v1",
-				false,
-				nil,
-				nil,
-				false,
-				eventRecorder,
-			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{
 				EvictLocalStoragePods:   false,
@@ -639,12 +631,8 @@ func TestHighNodeUtilizationWithTaints(t *testing.T) {
 
 			podEvictor := evictions.NewPodEvictor(
 				fakeClient,
-				"policy/v1",
-				false,
-				&item.evictionsExpected,
-				nil,
-				false,
 				eventRecorder,
+				evictions.NewOptions().WithMaxPodsToEvictPerNode(&item.evictionsExpected),
 			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{

pkg/framework/plugins/nodeutilization/lownodeutilization_test.go

@@ -430,7 +430,7 @@ func TestLowNodeUtilization(t *testing.T) {
 				test.BuildTestNode(n2NodeName, 4000, 3000, 10, nil),
 				test.BuildTestNode(n3NodeName, 4000, 3000, 10, test.SetNodeUnschedulable),
 			},
-			// All pods are assumed to be burstable (test.BuildTestNode always sets both cpu/memory resource requests to some value)
+			// All pods are assumed to be burstable (tc.BuildTestNode always sets both cpu/memory resource requests to some value)
 			pods: []*v1.Pod{
 				test.BuildTestPod("p1", 400, 0, n1NodeName, func(pod *v1.Pod) {
 					test.SetRSOwnerRef(pod)
@@ -855,16 +855,16 @@ func TestLowNodeUtilization(t *testing.T) {
 		},
 	}
 
-	for _, test := range testCases {
+	for _, tc := range testCases {
-		t.Run(test.name, func(t *testing.T) {
+		t.Run(tc.name, func(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
 			var objs []runtime.Object
-			for _, node := range test.nodes {
+			for _, node := range tc.nodes {
 				objs = append(objs, node)
 			}
-			for _, pod := range test.pods {
+			for _, pod := range tc.pods {
 				objs = append(objs, pod)
 			}
 			fakeClient := fake.NewSimpleClientset(objs...)
@@ -878,12 +878,12 @@ func TestLowNodeUtilization(t *testing.T) {
 			}
 
 			podsForEviction := make(map[string]struct{})
-			for _, pod := range test.evictedPods {
+			for _, pod := range tc.evictedPods {
 				podsForEviction[pod] = struct{}{}
 			}
 
 			evictionFailed := false
-			if len(test.evictedPods) > 0 {
+			if len(tc.evictedPods) > 0 {
 				fakeClient.Fake.AddReactor("create", "pods", func(action core.Action) (bool, runtime.Object, error) {
 					getAction := action.(core.CreateAction)
 					obj := getAction.GetObject()
@@ -903,15 +903,7 @@ func TestLowNodeUtilization(t *testing.T) {
 
 			eventRecorder := &events.FakeRecorder{}
 
-			podEvictor := evictions.NewPodEvictor(
+			podEvictor := evictions.NewPodEvictor(fakeClient, eventRecorder, nil)
-				fakeClient,
-				policy.SchemeGroupVersion.String(),
-				false,
-				nil,
-				nil,
-				false,
-				eventRecorder,
-			)
 
 			defaultEvictorFilterArgs := &defaultevictor.DefaultEvictorArgs{
 				EvictLocalStoragePods:   false,
@@ -942,20 +934,20 @@ func TestLowNodeUtilization(t *testing.T) {
 			}
 
 			plugin, err := NewLowNodeUtilization(&LowNodeUtilizationArgs{
-				Thresholds:             test.thresholds,
+				Thresholds:             tc.thresholds,
-				TargetThresholds:       test.targetThresholds,
+				TargetThresholds:       tc.targetThresholds,
-				UseDeviationThresholds: test.useDeviationThresholds,
+				UseDeviationThresholds: tc.useDeviationThresholds,
-				EvictableNamespaces:    test.evictableNamespaces,
+				EvictableNamespaces:    tc.evictableNamespaces,
 			},
 				handle)
 			if err != nil {
 				t.Fatalf("Unable to initialize the plugin: %v", err)
 			}
-			plugin.(frameworktypes.BalancePlugin).Balance(ctx, test.nodes)
+			plugin.(frameworktypes.BalancePlugin).Balance(ctx, tc.nodes)
 
 			podsEvicted := podEvictor.TotalEvicted()
-			if test.expectedPodsEvicted != podsEvicted {
+			if tc.expectedPodsEvicted != podsEvicted {
-				t.Errorf("Expected %v pods to be evicted but %v got evicted", test.expectedPodsEvicted, podsEvicted)
+				t.Errorf("Expected %v pods to be evicted but %v got evicted", tc.expectedPodsEvicted, podsEvicted)
 			}
 			if evictionFailed {
 				t.Errorf("Pod evictions failed unexpectedly")
@@ -1076,12 +1068,8 @@ func TestLowNodeUtilizationWithTaints(t *testing.T) {
 
 			podEvictor := evictions.NewPodEvictor(
 				fakeClient,
-				policy.SchemeGroupVersion.String(),
-				false,
-				&item.evictionsExpected,
-				nil,
-				false,
 				eventRecorder,
+				evictions.NewOptions().WithMaxPodsToEvictPerNode(&item.evictionsExpected),
 			)
 
 			defaultEvictorFilterArgs := &defaultevictor.DefaultEvictorArgs{

pkg/framework/plugins/podlifetime/pod_lifetime_test.go

@@ -22,7 +22,6 @@ import (
 	"time"
 
 	v1 "k8s.io/api/core/v1"
-	policyv1 "k8s.io/api/policy/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/informers"
@@ -558,12 +557,10 @@ func TestPodLifeTime(t *testing.T) {
 
 			podEvictor := evictions.NewPodEvictor(
 				fakeClient,
-				policyv1.SchemeGroupVersion.String(),
-				false,
-				tc.maxPodsToEvictPerNode,
-				tc.maxPodsToEvictPerNamespace,
-				false,
 				eventRecorder,
+				evictions.NewOptions().
+					WithMaxPodsToEvictPerNode(tc.maxPodsToEvictPerNode).
+					WithMaxPodsToEvictPerNamespace(tc.maxPodsToEvictPerNamespace),
 			)
 
 			defaultEvictorFilterArgs := &defaultevictor.DefaultEvictorArgs{

pkg/framework/plugins/removeduplicates/removeduplicates_test.go

@@ -26,7 +26,6 @@ import (
 	frameworktypes "sigs.k8s.io/descheduler/pkg/framework/types"
 
 	v1 "k8s.io/api/core/v1"
-	policyv1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -313,15 +312,7 @@ func TestFindDuplicatePods(t *testing.T) {
 
 			eventRecorder := &events.FakeRecorder{}
 
-			podEvictor := evictions.NewPodEvictor(
+			podEvictor := evictions.NewPodEvictor(fakeClient, eventRecorder, nil)
-				fakeClient,
-				"v1",
-				false,
-				nil,
-				nil,
-				false,
-				eventRecorder,
-			)
 
 			nodeFit := testCase.nodefit
 
@@ -761,15 +752,7 @@ func TestRemoveDuplicatesUniformly(t *testing.T) {
 
 			eventRecorder := &events.FakeRecorder{}
 
-			podEvictor := evictions.NewPodEvictor(
+			podEvictor := evictions.NewPodEvictor(fakeClient, eventRecorder, nil)
-				fakeClient,
-				policyv1.SchemeGroupVersion.String(),
-				false,
-				nil,
-				nil,
-				false,
-				eventRecorder,
-			)
 
 			defaultEvictorFilterArgs := &defaultevictor.DefaultEvictorArgs{
 				EvictLocalStoragePods:   false,

pkg/framework/plugins/removefailedpods/failedpods_test.go

@@ -21,7 +21,6 @@ import (
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
-	policyv1 "k8s.io/api/policy/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/informers"
@@ -375,15 +374,7 @@ func TestRemoveFailedPods(t *testing.T) {
 
 			eventRecorder := &events.FakeRecorder{}
 
-			podEvictor := evictions.NewPodEvictor(
+			podEvictor := evictions.NewPodEvictor(fakeClient, eventRecorder, nil)
-				fakeClient,
-				policyv1.SchemeGroupVersion.String(),
-				false,
-				nil,
-				nil,
-				false,
-				eventRecorder,
-			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{
 				EvictLocalStoragePods:   false,

pkg/framework/plugins/removepodshavingtoomanyrestarts/toomanyrestarts_test.go

@@ -22,7 +22,6 @@ import (
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
-	policyv1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/informers"
@@ -346,12 +345,10 @@ func TestRemovePodsHavingTooManyRestarts(t *testing.T) {
 
 			podEvictor := evictions.NewPodEvictor(
 				fakeClient,
-				policyv1.SchemeGroupVersion.String(),
-				false,
-				tc.maxPodsToEvictPerNode,
-				tc.maxNoOfPodsToEvictPerNamespace,
-				false,
 				eventRecorder,
+				evictions.NewOptions().
+					WithMaxPodsToEvictPerNode(tc.maxPodsToEvictPerNode).
+					WithMaxPodsToEvictPerNamespace(tc.maxNoOfPodsToEvictPerNamespace),
 			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{

pkg/framework/plugins/removepodsviolatinginterpodantiaffinity/pod_antiaffinity_test.go

@@ -21,7 +21,6 @@ import (
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
-	policyv1 "k8s.io/api/policy/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/informers"
@@ -235,12 +234,10 @@ func TestPodAntiAffinity(t *testing.T) {
 
 			podEvictor := evictions.NewPodEvictor(
 				fakeClient,
-				policyv1.SchemeGroupVersion.String(),
-				false,
-				test.maxPodsToEvictPerNode,
-				test.maxNoOfPodsToEvictPerNamespace,
-				false,
 				eventRecorder,
+				evictions.NewOptions().
+					WithMaxPodsToEvictPerNode(test.maxPodsToEvictPerNode).
+					WithMaxPodsToEvictPerNamespace(test.maxNoOfPodsToEvictPerNamespace),
 			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{

pkg/framework/plugins/removepodsviolatingnodeaffinity/node_affinity_test.go

@@ -21,7 +21,6 @@ import (
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
-	policyv1 "k8s.io/api/policy/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/informers"
@@ -361,12 +360,10 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 
 			podEvictor := evictions.NewPodEvictor(
 				fakeClient,
-				policyv1.SchemeGroupVersion.String(),
-				false,
-				tc.maxPodsToEvictPerNode,
-				tc.maxNoOfPodsToEvictPerNamespace,
-				false,
 				eventRecorder,
+				evictions.NewOptions().
+					WithMaxPodsToEvictPerNode(tc.maxPodsToEvictPerNode).
+					WithMaxPodsToEvictPerNamespace(tc.maxNoOfPodsToEvictPerNamespace),
 			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{

pkg/framework/plugins/removepodsviolatingnodetaints/node_taint_test.go

@@ -22,7 +22,6 @@ import (
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
-	policyv1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/informers"
@@ -406,12 +405,10 @@ func TestDeletePodsViolatingNodeTaints(t *testing.T) {
 
 			podEvictor := evictions.NewPodEvictor(
 				fakeClient,
-				policyv1.SchemeGroupVersion.String(),
-				false,
-				tc.maxPodsToEvictPerNode,
-				tc.maxNoOfPodsToEvictPerNamespace,
-				false,
 				eventRecorder,
+				evictions.NewOptions().
+					WithMaxPodsToEvictPerNode(tc.maxPodsToEvictPerNode).
+					WithMaxPodsToEvictPerNamespace(tc.maxNoOfPodsToEvictPerNamespace),
 			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{

pkg/framework/plugins/removepodsviolatingtopologyspreadconstraint/topologyspreadconstraint_test.go

@@ -1456,15 +1456,7 @@ func TestTopologySpreadConstraint(t *testing.T) {
 
 			eventRecorder := &events.FakeRecorder{}
 
-			podEvictor := evictions.NewPodEvictor(
+			podEvictor := evictions.NewPodEvictor(fakeClient, eventRecorder, nil)
-				fakeClient,
-				"v1",
-				false,
-				nil,
-				nil,
-				false,
-				eventRecorder,
-			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{
 				EvictLocalStoragePods:   false,

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, true, eventRecorder)
+			podEvictor := evictions.NewPodEvictor(client, eventRecorder, nil)
 
 			prfl, err := NewProfile(
 				test.config,
@@ -392,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, true, eventRecorder)
+	podEvictor := evictions.NewPodEvictor(client, eventRecorder, nil)
 
 	prfl, err := NewProfile(
 		api.DeschedulerProfile{
@@ -604,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, true, eventRecorder)
+	podEvictor := evictions.NewPodEvictor(client, eventRecorder, nil)
 
 	prfl, err := NewProfile(
 		api.DeschedulerProfile{

test/e2e/e2e_duplicatepods_test.go

@@ -171,15 +171,7 @@ func TestRemoveDuplicates(t *testing.T) {
 
 			eventRecorder := &events.FakeRecorder{}
 
-			podEvictor := evictions.NewPodEvictor(
+			podEvictor := evictions.NewPodEvictor(clientSet, eventRecorder, nil)
-				clientSet,
-				evictionPolicyGroupVersion,
-				false,
-				nil,
-				nil,
-				false,
-				eventRecorder,
-			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{
 				EvictLocalStoragePods:   true,

test/e2e/e2e_test.go

@@ -192,12 +192,10 @@ func runPodLifetimePlugin(
 
 	podEvictor := evictions.NewPodEvictor(
 		clientset,
-		evictionPolicyGroupVersion,
-		false,
-		nil,
-		maxPodsToEvictPerNamespace,
-		false,
 		&events.FakeRecorder{},
+		evictions.NewOptions().
+			WithPolicyGroupVersion(evictionPolicyGroupVersion).
+			WithMaxPodsToEvictPerNamespace(maxPodsToEvictPerNamespace),
 	)
 
 	var thresholdPriority int32
@@ -1579,11 +1577,7 @@ func initPodEvictorOrFail(t *testing.T, clientSet clientset.Interface, getPodsAs
 
 	return evictions.NewPodEvictor(
 		clientSet,
-		evictionPolicyGroupVersion,
-		false,
-		nil,
-		nil,
-		false,
 		eventRecorder,
+		evictions.NewOptions().WithPolicyGroupVersion(evictionPolicyGroupVersion),
 	)
 }

test/e2e/e2e_toomanyrestarts_test.go

@@ -163,12 +163,8 @@ func TestTooManyRestarts(t *testing.T) {
 
 			podEvictor := evictions.NewPodEvictor(
 				clientSet,
-				evictionPolicyGroupVersion,
-				false,
-				nil,
-				nil,
-				false,
 				eventRecorder,
+				evictions.NewOptions().WithPolicyGroupVersion(evictionPolicyGroupVersion),
 			)
 
 			defaultevictorArgs := &defaultevictor.DefaultEvictorArgs{