Merge pull request #296 from ingvagabund/make-Params-a-pointer

Make DeschedulerStrategy.Params a pointer

pkg/api/types.go

@@ -41,7 +41,7 @@ type DeschedulerStrategy struct {
 	Weight int
 
 	// Strategy parameters
-	Params StrategyParameters
+	Params *StrategyParameters
 }
 
 // Only one of its members may be specified

pkg/api/v1alpha1/types.go

@@ -41,7 +41,7 @@ type DeschedulerStrategy struct {
 	Weight int `json:"weight,omitempty"`
 
 	// Strategy parameters
-	Params StrategyParameters `json:"params,omitempty"`
+	Params *StrategyParameters `json:"params,omitempty"`
 }
 
 // Only one of its members may be specified

pkg/api/v1alpha1/zz_generated.conversion.go

@@ -121,9 +121,7 @@ func Convert_api_DeschedulerPolicy_To_v1alpha1_DeschedulerPolicy(in *api.Desched
 func autoConvert_v1alpha1_DeschedulerStrategy_To_api_DeschedulerStrategy(in *DeschedulerStrategy, out *api.DeschedulerStrategy, s conversion.Scope) error {
 	out.Enabled = in.Enabled
 	out.Weight = in.Weight
-	if err := Convert_v1alpha1_StrategyParameters_To_api_StrategyParameters(&in.Params, &out.Params, s); err != nil {
+	out.Params = (*api.StrategyParameters)(unsafe.Pointer(in.Params))
-		return err
-	}
 	return nil
 }
 
@@ -135,9 +133,7 @@ func Convert_v1alpha1_DeschedulerStrategy_To_api_DeschedulerStrategy(in *Desched
 func autoConvert_api_DeschedulerStrategy_To_v1alpha1_DeschedulerStrategy(in *api.DeschedulerStrategy, out *DeschedulerStrategy, s conversion.Scope) error {
 	out.Enabled = in.Enabled
 	out.Weight = in.Weight
-	if err := Convert_api_StrategyParameters_To_v1alpha1_StrategyParameters(&in.Params, &out.Params, s); err != nil {
+	out.Params = (*StrategyParameters)(unsafe.Pointer(in.Params))
-		return err
-	}
 	return nil
 }
 

pkg/api/v1alpha1/zz_generated.deepcopy.go

@@ -59,7 +59,11 @@ func (in *DeschedulerPolicy) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *DeschedulerStrategy) DeepCopyInto(out *DeschedulerStrategy) {
 	*out = *in
-	in.Params.DeepCopyInto(&out.Params)
+	if in.Params != nil {
+		in, out := &in.Params, &out.Params
+		*out = new(StrategyParameters)
+		(*in).DeepCopyInto(*out)
+	}
 	return
 }
 

pkg/api/zz_generated.deepcopy.go

@@ -59,7 +59,11 @@ func (in *DeschedulerPolicy) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *DeschedulerStrategy) DeepCopyInto(out *DeschedulerStrategy) {
 	*out = *in
-	in.Params.DeepCopyInto(&out.Params)
+	if in.Params != nil {
+		in, out := &in.Params, &out.Params
+		*out = new(StrategyParameters)
+		(*in).DeepCopyInto(*out)
+	}
 	return
 }
 

pkg/descheduler/strategies/duplicates.go

@@ -116,7 +116,7 @@ func listDuplicatePodsOnANode(ctx context.Context, client clientset.Interface, n
 }
 
 func hasExcludedOwnerRefKind(ownerRefs []metav1.OwnerReference, strategy api.DeschedulerStrategy) bool {
-	if strategy.Params.RemoveDuplicates == nil {
+	if strategy.Params == nil || strategy.Params.RemoveDuplicates == nil {
 		return false
 	}
 	exclude := sets.NewString(strategy.Params.RemoveDuplicates.ExcludeOwnerKinds...)

pkg/descheduler/strategies/duplicates_test.go

@@ -132,7 +132,7 @@ func TestFindDuplicatePods(t *testing.T) {
 			maxPodsToEvict:          5,
 			pods:                    []v1.Pod{*p1, *p2, *p3},
 			expectedEvictedPodCount: 0,
-			strategy:                api.DeschedulerStrategy{Params: api.StrategyParameters{RemoveDuplicates: &api.RemoveDuplicates{ExcludeOwnerKinds: []string{"ReplicaSet"}}}},
+			strategy:                api.DeschedulerStrategy{Params: &api.StrategyParameters{RemoveDuplicates: &api.RemoveDuplicates{ExcludeOwnerKinds: []string{"ReplicaSet"}}}},
 		},
 		{
 			description:             "Three Pods in the `test` Namespace, bound to same ReplicaSet. 2 should be evicted.",

pkg/descheduler/strategies/lownodeutilization.go

@@ -49,7 +49,7 @@ const (
 func LowNodeUtilization(ctx context.Context, client clientset.Interface, strategy api.DeschedulerStrategy, nodes []*v1.Node, evictLocalStoragePods bool, podEvictor *evictions.PodEvictor) {
 	// todo: move to config validation?
 	// TODO: May be create a struct for the strategy as well, so that we don't have to pass along the all the params?
-	if strategy.Params.NodeResourceUtilizationThresholds == nil {
+	if strategy.Params == nil || strategy.Params.NodeResourceUtilizationThresholds == nil {
 		klog.V(1).Infof("NodeResourceUtilizationThresholds not set")
 		return
 	}

pkg/descheduler/strategies/lownodeutilization_test.go

@@ -702,7 +702,7 @@ func TestWithTaints(t *testing.T) {
 	ctx := context.Background()
 	strategy := api.DeschedulerStrategy{
 		Enabled: true,
-		Params: api.StrategyParameters{
+		Params: &api.StrategyParameters{
 			NodeResourceUtilizationThresholds: &api.NodeResourceUtilizationThresholds{
 				Thresholds: api.ResourceThresholds{
 					v1.ResourcePods: 20,

pkg/descheduler/strategies/node_affinity.go

@@ -30,6 +30,10 @@ import (
 )
 
 func RemovePodsViolatingNodeAffinity(ctx context.Context, client clientset.Interface, strategy api.DeschedulerStrategy, nodes []*v1.Node, evictLocalStoragePods bool, podEvictor *evictions.PodEvictor) {
+	if strategy.Params == nil {
+		klog.V(1).Infof("NodeAffinityType not set")
+		return
+	}
 	for _, nodeAffinity := range strategy.Params.NodeAffinityType {
 		klog.V(2).Infof("Executing for nodeAffinityType: %v", nodeAffinity)
 

pkg/descheduler/strategies/node_affinity_test.go

@@ -33,7 +33,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 	ctx := context.Background()
 	requiredDuringSchedulingIgnoredDuringExecutionStrategy := api.DeschedulerStrategy{
 		Enabled: true,
-		Params: api.StrategyParameters{
+		Params: &api.StrategyParameters{
 			NodeAffinityType: []string{
 				"requiredDuringSchedulingIgnoredDuringExecution",
 			},
@@ -99,7 +99,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 			description: "Invalid strategy type, should not evict any pods",
 			strategy: api.DeschedulerStrategy{
 				Enabled: true,
-				Params: api.StrategyParameters{
+				Params: &api.StrategyParameters{
 					NodeAffinityType: []string{
 						"requiredDuringSchedulingRequiredDuringExecution",
 					},

pkg/descheduler/strategies/pod_lifetime.go

@@ -31,7 +31,7 @@ import (
 
 // PodLifeTime evicts pods on nodes that were created more than strategy.Params.MaxPodLifeTimeSeconds seconds ago.
 func PodLifeTime(ctx context.Context, client clientset.Interface, strategy api.DeschedulerStrategy, nodes []*v1.Node, evictLocalStoragePods bool, podEvictor *evictions.PodEvictor) {
-	if strategy.Params.MaxPodLifeTimeSeconds == nil {
+	if strategy.Params == nil || strategy.Params.MaxPodLifeTimeSeconds == nil {
 		klog.V(1).Infof("MaxPodLifeTimeSeconds not set")
 		return
 	}

pkg/descheduler/strategies/pod_lifetime_test.go

@@ -97,7 +97,7 @@ func TestPodLifeTime(t *testing.T) {
 			description: "Two pods in the `dev` Namespace, 1 is new and 1 very is old. 1 should be evicted.",
 			strategy: api.DeschedulerStrategy{
 				Enabled: true,
-				Params: api.StrategyParameters{
+				Params: &api.StrategyParameters{
 					MaxPodLifeTimeSeconds: &maxLifeTime,
 				},
 			},
@@ -109,7 +109,7 @@ func TestPodLifeTime(t *testing.T) {
 			description: "Two pods in the `dev` Namespace, 2 are new and 0 are old. 0 should be evicted.",
 			strategy: api.DeschedulerStrategy{
 				Enabled: true,
-				Params: api.StrategyParameters{
+				Params: &api.StrategyParameters{
 					MaxPodLifeTimeSeconds: &maxLifeTime,
 				},
 			},
@@ -121,7 +121,7 @@ func TestPodLifeTime(t *testing.T) {
 			description: "Two pods in the `dev` Namespace, 1 created 605 seconds ago. 1 should be evicted.",
 			strategy: api.DeschedulerStrategy{
 				Enabled: true,
-				Params: api.StrategyParameters{
+				Params: &api.StrategyParameters{
 					MaxPodLifeTimeSeconds: &maxLifeTime,
 				},
 			},
@@ -133,7 +133,7 @@ func TestPodLifeTime(t *testing.T) {
 			description: "Two pods in the `dev` Namespace, 1 created 595 seconds ago. 0 should be evicted.",
 			strategy: api.DeschedulerStrategy{
 				Enabled: true,
-				Params: api.StrategyParameters{
+				Params: &api.StrategyParameters{
 					MaxPodLifeTimeSeconds: &maxLifeTime,
 				},
 			},

pkg/descheduler/strategies/toomanyrestarts.go

@@ -32,7 +32,7 @@ import (
 // There are too many cases leading this issue: Volume mount failed, app error due to nodes' different settings.
 // As of now, this strategy won't evict daemonsets, mirror pods, critical pods and pods with local storages.
 func RemovePodsHavingTooManyRestarts(ctx context.Context, client clientset.Interface, strategy api.DeschedulerStrategy, nodes []*v1.Node, evictLocalStoragePods bool, podEvictor *evictions.PodEvictor) {
-	if strategy.Params.PodsHavingTooManyRestarts == nil || strategy.Params.PodsHavingTooManyRestarts.PodRestartThreshold < 1 {
+	if strategy.Params == nil || strategy.Params.PodsHavingTooManyRestarts == nil || strategy.Params.PodsHavingTooManyRestarts.PodRestartThreshold < 1 {
 		klog.V(1).Infof("PodsHavingTooManyRestarts thresholds not set")
 		return
 	}

pkg/descheduler/strategies/toomanyrestarts_test.go

@@ -84,7 +84,7 @@ func TestRemovePodsHavingTooManyRestarts(t *testing.T) {
 	createStrategy := func(enabled, includingInitContainers bool, restartThresholds int32) api.DeschedulerStrategy {
 		return api.DeschedulerStrategy{
 			Enabled: enabled,
-			Params: api.StrategyParameters{
+			Params: &api.StrategyParameters{
 				PodsHavingTooManyRestarts: &api.PodsHavingTooManyRestarts{
 					PodRestartThreshold:     restartThresholds,
 					IncludingInitContainers: includingInitContainers,

test/e2e/e2e_test.go

@@ -118,7 +118,7 @@ func startEndToEndForLowNodeUtilization(ctx context.Context, clientset clientset
 
 	lowNodeUtilizationStrategy := deschedulerapi.DeschedulerStrategy{
 		Enabled: true,
-		Params: deschedulerapi.StrategyParameters{
+		Params: &deschedulerapi.StrategyParameters{
 			NodeResourceUtilizationThresholds: &deschedulerapi.NodeResourceUtilizationThresholds{
 				Thresholds:       thresholds,
 				TargetThresholds: targetThresholds,