Merge pull request #374 from ingvagabund/namespace-as-pointer

Promote Namespaces field to a pointer

pkg/api/types.go

@@ -60,7 +60,7 @@ type StrategyParameters struct {
 	PodsHavingTooManyRestarts         *PodsHavingTooManyRestarts
 	MaxPodLifeTimeSeconds             *uint
 	RemoveDuplicates                  *RemoveDuplicates
-	Namespaces                        Namespaces
+	Namespaces                        *Namespaces
 	ThresholdPriority                 *int32
 	ThresholdPriorityClassName        string
 }

pkg/api/v1alpha1/types.go

@@ -58,7 +58,7 @@ type StrategyParameters struct {
 	PodsHavingTooManyRestarts         *PodsHavingTooManyRestarts         `json:"podsHavingTooManyRestarts,omitempty"`
 	MaxPodLifeTimeSeconds             *uint                              `json:"maxPodLifeTimeSeconds,omitempty"`
 	RemoveDuplicates                  *RemoveDuplicates                  `json:"removeDuplicates,omitempty"`
-	Namespaces                        Namespaces                         `json:"namespaces"`
+	Namespaces                        *Namespaces                        `json:"namespaces"`
 	ThresholdPriority                 *int32                             `json:"thresholdPriority"`
 	ThresholdPriorityClassName        string                             `json:"thresholdPriorityClassName"`
 }

pkg/api/v1alpha1/zz_generated.conversion.go

@@ -246,9 +246,7 @@ func autoConvert_v1alpha1_StrategyParameters_To_api_StrategyParameters(in *Strat
 	out.PodsHavingTooManyRestarts = (*api.PodsHavingTooManyRestarts)(unsafe.Pointer(in.PodsHavingTooManyRestarts))
 	out.MaxPodLifeTimeSeconds = (*uint)(unsafe.Pointer(in.MaxPodLifeTimeSeconds))
 	out.RemoveDuplicates = (*api.RemoveDuplicates)(unsafe.Pointer(in.RemoveDuplicates))
-	if err := Convert_v1alpha1_Namespaces_To_api_Namespaces(&in.Namespaces, &out.Namespaces, s); err != nil {
-		return err
-	}
+	out.Namespaces = (*api.Namespaces)(unsafe.Pointer(in.Namespaces))
 	out.ThresholdPriority = (*int32)(unsafe.Pointer(in.ThresholdPriority))
 	out.ThresholdPriorityClassName = in.ThresholdPriorityClassName
 	return nil
@@ -265,9 +263,7 @@ func autoConvert_api_StrategyParameters_To_v1alpha1_StrategyParameters(in *api.S
 	out.PodsHavingTooManyRestarts = (*PodsHavingTooManyRestarts)(unsafe.Pointer(in.PodsHavingTooManyRestarts))
 	out.MaxPodLifeTimeSeconds = (*uint)(unsafe.Pointer(in.MaxPodLifeTimeSeconds))
 	out.RemoveDuplicates = (*RemoveDuplicates)(unsafe.Pointer(in.RemoveDuplicates))
-	if err := Convert_api_Namespaces_To_v1alpha1_Namespaces(&in.Namespaces, &out.Namespaces, s); err != nil {
-		return err
-	}
+	out.Namespaces = (*Namespaces)(unsafe.Pointer(in.Namespaces))
 	out.ThresholdPriority = (*int32)(unsafe.Pointer(in.ThresholdPriority))
 	out.ThresholdPriorityClassName = in.ThresholdPriorityClassName
 	return nil

pkg/api/v1alpha1/zz_generated.deepcopy.go

@@ -242,7 +242,11 @@ func (in *StrategyParameters) DeepCopyInto(out *StrategyParameters) {
 		*out = new(RemoveDuplicates)
 		(*in).DeepCopyInto(*out)
 	}
-	in.Namespaces.DeepCopyInto(&out.Namespaces)
+	if in.Namespaces != nil {
+		in, out := &in.Namespaces, &out.Namespaces
+		*out = new(Namespaces)
+		(*in).DeepCopyInto(*out)
+	}
 	if in.ThresholdPriority != nil {
 		in, out := &in.ThresholdPriority, &out.ThresholdPriority
 		*out = new(int32)

pkg/api/zz_generated.deepcopy.go

@@ -242,7 +242,11 @@ func (in *StrategyParameters) DeepCopyInto(out *StrategyParameters) {
 		*out = new(RemoveDuplicates)
 		(*in).DeepCopyInto(*out)
 	}
-	in.Namespaces.DeepCopyInto(&out.Namespaces)
+	if in.Namespaces != nil {
+		in, out := &in.Namespaces, &out.Namespaces
+		*out = new(Namespaces)
+		(*in).DeepCopyInto(*out)
+	}
 	if in.ThresholdPriority != nil {
 		in, out := &in.ThresholdPriority, &out.ThresholdPriority
 		*out = new(int32)

pkg/descheduler/strategies/node_affinity.go

@@ -36,7 +36,7 @@ func validatePodsViolatingNodeAffinityParams(params *api.StrategyParameters) err
 		return fmt.Errorf("NodeAffinityType is empty")
 	}
 	// At most one of include/exclude can be set
-	if len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
+	if params.Namespaces != nil && len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
 		return fmt.Errorf("only one of Include/Exclude namespaces can be set")
 	}
 	if params.ThresholdPriority != nil && params.ThresholdPriorityClassName != "" {
@@ -58,6 +58,12 @@ func RemovePodsViolatingNodeAffinity(ctx context.Context, client clientset.Inter
 		return
 	}
 
+	var includedNamespaces, excludedNamespaces []string
+	if strategy.Params.Namespaces != nil {
+		includedNamespaces = strategy.Params.Namespaces.Include
+		excludedNamespaces = strategy.Params.Namespaces.Exclude
+	}
+
 	for _, nodeAffinity := range strategy.Params.NodeAffinityType {
 		klog.V(2).Infof("Executing for nodeAffinityType: %v", nodeAffinity)
 
@@ -75,8 +81,8 @@ func RemovePodsViolatingNodeAffinity(ctx context.Context, client clientset.Inter
 							!nodeutil.PodFitsCurrentNode(pod, node) &&
 							nodeutil.PodFitsAnyNode(pod, nodes)
 					}),
-					podutil.WithNamespaces(strategy.Params.Namespaces.Include),
-					podutil.WithoutNamespaces(strategy.Params.Namespaces.Exclude),
+					podutil.WithNamespaces(includedNamespaces),
+					podutil.WithoutNamespaces(excludedNamespaces),
 				)
 				if err != nil {
 					klog.Errorf("failed to get pods from %v: %v", node.Name, err)

pkg/descheduler/strategies/node_taint.go

@@ -36,7 +36,7 @@ func validateRemovePodsViolatingNodeTaintsParams(params *api.StrategyParameters)
 	}
 
 	// At most one of include/exclude can be set
-	if len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
+	if params.Namespaces != nil && len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
 		return fmt.Errorf("only one of Include/Exclude namespaces can be set")
 	}
 	if params.ThresholdPriority != nil && params.ThresholdPriorityClassName != "" {
@@ -52,10 +52,13 @@ func RemovePodsViolatingNodeTaints(ctx context.Context, client clientset.Interfa
 		klog.V(1).Info(err)
 		return
 	}
-	var namespaces api.Namespaces
-	if strategy.Params != nil {
-		namespaces = strategy.Params.Namespaces
+
+	var includedNamespaces, excludedNamespaces []string
+	if strategy.Params != nil && strategy.Params.Namespaces != nil {
+		includedNamespaces = strategy.Params.Namespaces.Include
+		excludedNamespaces = strategy.Params.Namespaces.Exclude
 	}
+
 	thresholdPriority, err := utils.GetPriorityFromStrategyParams(ctx, client, strategy.Params)
 	if err != nil {
 		klog.V(1).Infof("failed to get threshold priority from strategy's params: %#v", err)
@@ -71,8 +74,8 @@ func RemovePodsViolatingNodeTaints(ctx context.Context, client clientset.Interfa
 			podutil.WithFilter(func(pod *v1.Pod) bool {
 				return podEvictor.IsEvictable(pod, thresholdPriority)
 			}),
-			podutil.WithNamespaces(namespaces.Include),
-			podutil.WithoutNamespaces(namespaces.Exclude),
+			podutil.WithNamespaces(includedNamespaces),
+			podutil.WithoutNamespaces(excludedNamespaces),
 		)
 		if err != nil {
 			//no pods evicted as error encountered retrieving evictable Pods

pkg/descheduler/strategies/pod_antiaffinity.go

@@ -25,7 +25,7 @@ import (
 	podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
 	"sigs.k8s.io/descheduler/pkg/utils"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
@@ -37,7 +37,7 @@ func validateRemovePodsViolatingInterPodAntiAffinityParams(params *api.StrategyP
 	}
 
 	// At most one of include/exclude can be set
-	if len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
+	if params.Namespaces != nil && len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
 		return fmt.Errorf("only one of Include/Exclude namespaces can be set")
 	}
 	if params.ThresholdPriority != nil && params.ThresholdPriorityClassName != "" {
@@ -53,10 +53,13 @@ func RemovePodsViolatingInterPodAntiAffinity(ctx context.Context, client clients
 		klog.V(1).Info(err)
 		return
 	}
-	var namespaces api.Namespaces
-	if strategy.Params != nil {
-		namespaces = strategy.Params.Namespaces
+
+	var includedNamespaces, excludedNamespaces []string
+	if strategy.Params != nil && strategy.Params.Namespaces != nil {
+		includedNamespaces = strategy.Params.Namespaces.Include
+		excludedNamespaces = strategy.Params.Namespaces.Exclude
 	}
+
 	thresholdPriority, err := utils.GetPriorityFromStrategyParams(ctx, client, strategy.Params)
 	if err != nil {
 		klog.V(1).Infof("failed to get threshold priority from strategy's params: %#v", err)
@@ -72,8 +75,8 @@ func RemovePodsViolatingInterPodAntiAffinity(ctx context.Context, client clients
 			podutil.WithFilter(func(pod *v1.Pod) bool {
 				return podEvictor.IsEvictable(pod, thresholdPriority)
 			}),
-			podutil.WithNamespaces(namespaces.Include),
-			podutil.WithoutNamespaces(namespaces.Exclude),
+			podutil.WithNamespaces(includedNamespaces),
+			podutil.WithoutNamespaces(excludedNamespaces),
 		)
 		if err != nil {
 			return

pkg/descheduler/strategies/pod_lifetime.go

@@ -37,7 +37,7 @@ func validatePodLifeTimeParams(params *api.StrategyParameters) error {
 	}
 
 	// At most one of include/exclude can be set
-	if len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
+	if params.Namespaces != nil && len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
 		return fmt.Errorf("only one of Include/Exclude namespaces can be set")
 	}
 	if params.ThresholdPriority != nil && params.ThresholdPriorityClassName != "" {
@@ -53,16 +53,23 @@ func PodLifeTime(ctx context.Context, client clientset.Interface, strategy api.D
 		klog.V(1).Info(err)
 		return
 	}
+
 	thresholdPriority, err := utils.GetPriorityFromStrategyParams(ctx, client, strategy.Params)
 	if err != nil {
 		klog.V(1).Infof("failed to get threshold priority from strategy's params: %#v", err)
 		return
 	}
 
+	var includedNamespaces, excludedNamespaces []string
+	if strategy.Params.Namespaces != nil {
+		includedNamespaces = strategy.Params.Namespaces.Include
+		excludedNamespaces = strategy.Params.Namespaces.Exclude
+	}
+
 	for _, node := range nodes {
 		klog.V(1).Infof("Processing node: %#v", node.Name)
 
-		pods := listOldPodsOnNode(ctx, client, node, strategy.Params, podEvictor, thresholdPriority)
+		pods := listOldPodsOnNode(ctx, client, node, includedNamespaces, excludedNamespaces, *strategy.Params.MaxPodLifeTimeSeconds, podEvictor, thresholdPriority)
 		for _, pod := range pods {
 			success, err := podEvictor.EvictPod(ctx, pod, node, "PodLifeTime")
 			if success {
@@ -78,7 +85,7 @@ func PodLifeTime(ctx context.Context, client clientset.Interface, strategy api.D
 	}
 }
 
-func listOldPodsOnNode(ctx context.Context, client clientset.Interface, node *v1.Node, params *api.StrategyParameters, podEvictor *evictions.PodEvictor, thresholdPriority int32) []*v1.Pod {
+func listOldPodsOnNode(ctx context.Context, client clientset.Interface, node *v1.Node, includedNamespaces, excludedNamespaces []string, maxPodLifeTimeSeconds uint, podEvictor *evictions.PodEvictor, thresholdPriority int32) []*v1.Pod {
 	pods, err := podutil.ListPodsOnANode(
 		ctx,
 		client,
@@ -86,8 +93,8 @@ func listOldPodsOnNode(ctx context.Context, client clientset.Interface, node *v1
 		podutil.WithFilter(func(pod *v1.Pod) bool {
 			return podEvictor.IsEvictable(pod, thresholdPriority)
 		}),
-		podutil.WithNamespaces(params.Namespaces.Include),
-		podutil.WithoutNamespaces(params.Namespaces.Exclude),
+		podutil.WithNamespaces(includedNamespaces),
+		podutil.WithoutNamespaces(excludedNamespaces),
 	)
 	if err != nil {
 		return nil
@@ -96,7 +103,7 @@ func listOldPodsOnNode(ctx context.Context, client clientset.Interface, node *v1
 	var oldPods []*v1.Pod
 	for _, pod := range pods {
 		podAgeSeconds := uint(v1meta.Now().Sub(pod.GetCreationTimestamp().Local()).Seconds())
-		if podAgeSeconds > *params.MaxPodLifeTimeSeconds {
+		if podAgeSeconds > maxPodLifeTimeSeconds {
 			oldPods = append(oldPods, pod)
 		}
 	}

pkg/descheduler/strategies/toomanyrestarts.go

@@ -36,7 +36,7 @@ func validateRemovePodsHavingTooManyRestartsParams(params *api.StrategyParameter
 	}
 
 	// At most one of include/exclude can be set
-	if len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
+	if params.Namespaces != nil && len(params.Namespaces.Include) > 0 && len(params.Namespaces.Exclude) > 0 {
 		return fmt.Errorf("only one of Include/Exclude namespaces can be set")
 	}
 	if params.ThresholdPriority != nil && params.ThresholdPriorityClassName != "" {
@@ -54,12 +54,19 @@ func RemovePodsHavingTooManyRestarts(ctx context.Context, client clientset.Inter
 		klog.V(1).Info(err)
 		return
 	}
+
 	thresholdPriority, err := utils.GetPriorityFromStrategyParams(ctx, client, strategy.Params)
 	if err != nil {
 		klog.V(1).Infof("failed to get threshold priority from strategy's params: %#v", err)
 		return
 	}
 
+	var includedNamespaces, excludedNamespaces []string
+	if strategy.Params.Namespaces != nil {
+		includedNamespaces = strategy.Params.Namespaces.Include
+		excludedNamespaces = strategy.Params.Namespaces.Exclude
+	}
+
 	for _, node := range nodes {
 		klog.V(1).Infof("Processing node: %s", node.Name)
 		pods, err := podutil.ListPodsOnANode(
@@ -69,8 +76,8 @@ func RemovePodsHavingTooManyRestarts(ctx context.Context, client clientset.Inter
 			podutil.WithFilter(func(pod *v1.Pod) bool {
 				return podEvictor.IsEvictable(pod, thresholdPriority)
 			}),
-			podutil.WithNamespaces(strategy.Params.Namespaces.Include),
-			podutil.WithoutNamespaces(strategy.Params.Namespaces.Exclude),
+			podutil.WithNamespaces(includedNamespaces),
+			podutil.WithoutNamespaces(excludedNamespaces),
 		)
 		if err != nil {
 			klog.Errorf("Error when list pods at node %s", node.Name)

test/e2e/e2e_test.go

@@ -20,12 +20,13 @@ import (
 	"context"
 	"math"
 	"os"
-	"sigs.k8s.io/descheduler/pkg/utils"
 	"sort"
 	"strings"
 	"testing"
 	"time"
 
+	"sigs.k8s.io/descheduler/pkg/utils"
+
 	v1 "k8s.io/api/core/v1"
 	schedulingv1 "k8s.io/api/scheduling/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -188,7 +189,7 @@ func TestLowNodeUtilization(t *testing.T) {
 	deleteRC(ctx, t, clientSet, rc)
 }
 
-func runPodLifetimeStrategy(ctx context.Context, clientset clientset.Interface, nodeInformer coreinformers.NodeInformer, namespaces deschedulerapi.Namespaces, priorityClass string, priority *int32) {
+func runPodLifetimeStrategy(ctx context.Context, clientset clientset.Interface, nodeInformer coreinformers.NodeInformer, namespaces *deschedulerapi.Namespaces, priorityClass string, priority *int32) {
 	// Run descheduler.
 	evictionPolicyGroupVersion, err := eutils.SupportEviction(clientset)
 	if err != nil || len(evictionPolicyGroupVersion) == 0 {
@@ -286,7 +287,7 @@ func TestNamespaceConstraintsInclude(t *testing.T) {
 	t.Logf("Existing pods: %v", initialPodNames)
 
 	t.Logf("set the strategy to delete pods from %v namespace", rc.Namespace)
-	runPodLifetimeStrategy(ctx, clientSet, nodeInformer, deschedulerapi.Namespaces{
+	runPodLifetimeStrategy(ctx, clientSet, nodeInformer, &deschedulerapi.Namespaces{
 		Include: []string{rc.Namespace},
 	}, "", nil)
 
@@ -357,7 +358,7 @@ func TestNamespaceConstraintsExclude(t *testing.T) {
 	t.Logf("Existing pods: %v", initialPodNames)
 
 	t.Logf("set the strategy to delete pods from namespaces except the %v namespace", rc.Namespace)
-	runPodLifetimeStrategy(ctx, clientSet, nodeInformer, deschedulerapi.Namespaces{
+	runPodLifetimeStrategy(ctx, clientSet, nodeInformer, &deschedulerapi.Namespaces{
 		Exclude: []string{rc.Namespace},
 	}, "", nil)
 
@@ -461,10 +462,10 @@ func testPriority(t *testing.T, isPriorityClass bool) {
 
 	if isPriorityClass {
 		t.Logf("set the strategy to delete pods with priority lower than priority class %s", highPriorityClass.Name)
-		runPodLifetimeStrategy(ctx, clientSet, nodeInformer, deschedulerapi.Namespaces{}, highPriorityClass.Name, nil)
+		runPodLifetimeStrategy(ctx, clientSet, nodeInformer, nil, highPriorityClass.Name, nil)
 	} else {
 		t.Logf("set the strategy to delete pods with priority lower than %d", highPriority)
-		runPodLifetimeStrategy(ctx, clientSet, nodeInformer, deschedulerapi.Namespaces{}, "", &highPriority)
+		runPodLifetimeStrategy(ctx, clientSet, nodeInformer, nil, "", &highPriority)
 	}
 
 	t.Logf("Waiting 10s")