koord-descheduler: support evicting all bare pods in migration contro…

…ller (#2102)

Signed-off-by: songtao98 <songtao2603060@gmail.com>

pkg/descheduler/apis/config/types_pluginargs.go

@@ -37,6 +37,9 @@ type MigrationControllerArgs struct {
 	// EvictFailedBarePods allows pods without ownerReferences and in failed phase to be evicted.
 	EvictFailedBarePods bool
 
+	// EvictAllBarePods allows all pods without ownerReferences to be evicted.
+	EvictAllBarePods bool
+
 	// EvictLocalStoragePods allows pods using local storage to be evicted.
 	EvictLocalStoragePods bool
 

pkg/descheduler/apis/config/v1alpha2/types_pluginargs.go

@@ -39,6 +39,9 @@ type MigrationControllerArgs struct {
 	// EvictFailedBarePods allows pods without ownerReferences and in failed phase to be evicted.
 	EvictFailedBarePods bool `json:"evictFailedBarePods"`
 
+	// EvictAllBarePods allows all pods without ownerReferences to be evicted.
+	EvictAllBarePods bool `json:"evictAllBarePods"`
+
 	// EvictLocalStoragePods allows pods using local storage to be evicted.
 	EvictLocalStoragePods bool `json:"evictLocalStoragePods"`
 

pkg/descheduler/controllers/migration/arbitrator/filter.go

@@ -42,6 +42,7 @@ import (
 	"github.com/koordinator-sh/koordinator/pkg/descheduler/fieldindex"
 	"github.com/koordinator-sh/koordinator/pkg/descheduler/framework"
 	"github.com/koordinator-sh/koordinator/pkg/descheduler/framework/plugins/kubernetes/defaultevictor"
+	nodeutil "github.com/koordinator-sh/koordinator/pkg/descheduler/node"
 	podutil "github.com/koordinator-sh/koordinator/pkg/descheduler/pod"
 	pkgutil "github.com/koordinator-sh/koordinator/pkg/util"
 	utilclient "github.com/koordinator-sh/koordinator/pkg/util/client"
@@ -90,11 +91,13 @@ func (f *filter) initFilters(args *deschedulerconfig.MigrationControllerArgs, ha
 		EvictFailedBarePods:     args.EvictFailedBarePods,
 		LabelSelector:           args.LabelSelector,
 	}
+	var priority *int32
 	if args.PriorityThreshold != nil {
 		defaultEvictorArgs.PriorityThreshold = &k8sdeschedulerapi.PriorityThreshold{
 			Name:  args.PriorityThreshold.Name,
 			Value: args.PriorityThreshold.Value,
 		}
+		priority = args.PriorityThreshold.Value
 	}
 	defaultEvictor, err := defaultevictor.New(defaultEvictorArgs, handle)
 	if err != nil {
@@ -106,8 +109,19 @@ func (f *filter) initFilters(args *deschedulerconfig.MigrationControllerArgs, ha
 		includedNamespaces = sets.NewString(args.Namespaces.Include...)
 		excludedNamespaces = sets.NewString(args.Namespaces.Exclude...)
 	}
-
-	filterPlugin := defaultEvictor.(framework.FilterPlugin)
+	nodeGetter := func() ([]*corev1.Node, error) {
+		nodes, err := nodeutil.ReadyNodes(context.TODO(), handle.ClientSet(), handle.SharedInformerFactory().Core().V1().Nodes(), defaultEvictorArgs.NodeSelector)
+		if err != nil {
+			return nil, err
+		}
+		return nodes, nil
+	}
+	filterPlugin, err := evictionsutil.NewEvictorFilter(nodeGetter, handle.GetPodsAssignedToNodeFunc(), args.EvictLocalStoragePods,
+		args.EvictSystemCriticalPods, args.IgnorePvcPods, args.EvictFailedBarePods, args.EvictAllBarePods,
+		evictionsutil.WithLabelSelector(args.LabelSelector), evictionsutil.WithPriorityThreshold(priority))
+	if err != nil {
+		return err
+	}
 	wrapFilterFuncs := podutil.WrapFilterFuncs(
 		util.FilterPodWithMaxEvictionCost,
 		filterPlugin.Filter,

pkg/descheduler/evictions/evictions.go

@@ -32,7 +32,6 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/events"
 	"k8s.io/klog/v2"
-	"k8s.io/utils/pointer"
 
 	"github.com/koordinator-sh/koordinator/pkg/descheduler/framework"
 	"github.com/koordinator-sh/koordinator/pkg/descheduler/metrics"
@@ -195,14 +194,14 @@ func EvictPod(ctx context.Context, client clientset.Interface, pod *corev1.Pod,
 type Options struct {
 	priority      *int32
 	nodeFit       bool
-	labelSelector labels.Selector
+	labelSelector *metav1.LabelSelector
 }
 
 // WithPriorityThreshold sets a threshold for pod's priority class.
 // Any pod whose priority class is lower is evictable.
-func WithPriorityThreshold(priority int32) func(opts *Options) {
+func WithPriorityThreshold(priority *int32) func(opts *Options) {
 	return func(opts *Options) {
-		opts.priority = pointer.Int32(priority)
+		opts.priority = priority
 	}
 }
 
@@ -218,7 +217,7 @@ func WithNodeFit(nodeFit bool) func(opts *Options) {
 
 // WithLabelSelector sets whether or not to apply label filtering when evicting.
 // Any pod matching the label selector is considered evictable.
-func WithLabelSelector(labelSelector labels.Selector) func(opts *Options) {
+func WithLabelSelector(labelSelector *metav1.LabelSelector) func(opts *Options) {
 	return func(opts *Options) {
 		opts.labelSelector = labelSelector
 	}
@@ -239,32 +238,35 @@ func NewEvictorFilter(
 	evictSystemCriticalPods bool,
 	ignorePvcPods bool,
 	evictFailedBarePods bool,
+	evictAllBarePods bool,
 	opts ...func(opts *Options),
-) *EvictorFilter {
+) (*EvictorFilter, error) {
 	options := &Options{}
 	for _, opt := range opts {
 		opt(options)
 	}
 
 	ev := &EvictorFilter{}
-	if evictFailedBarePods {
-		ev.constraints = append(ev.constraints, func(pod *corev1.Pod) error {
-			ownerRefList := podutil.OwnerRef(pod)
-			// Enable evictFailedBarePods to evict bare pods in failed phase
-			if len(ownerRefList) == 0 && pod.Status.Phase != corev1.PodFailed {
-				return fmt.Errorf("pod does not have any ownerRefs and is not in failed phase")
-			}
-			return nil
-		})
-	} else {
-		ev.constraints = append(ev.constraints, func(pod *corev1.Pod) error {
-			ownerRefList := podutil.OwnerRef(pod)
-			// Moved from IsEvictable function for backward compatibility
-			if len(ownerRefList) == 0 {
-				return fmt.Errorf("pod does not have any ownerRefs")
-			}
-			return nil
-		})
+	if !evictAllBarePods {
+		if evictFailedBarePods {
+			ev.constraints = append(ev.constraints, func(pod *corev1.Pod) error {
+				ownerRefList := podutil.OwnerRef(pod)
+				// Enable evictFailedBarePods to evict bare pods in failed phase
+				if len(ownerRefList) == 0 && pod.Status.Phase != corev1.PodFailed {
+					return fmt.Errorf("pod does not have any ownerRefs and is not in failed phase")
+				}
+				return nil
+			})
+		} else {
+			ev.constraints = append(ev.constraints, func(pod *corev1.Pod) error {
+				ownerRefList := podutil.OwnerRef(pod)
+				// Moved from IsEvictable function for backward compatibility
+				if len(ownerRefList) == 0 {
+					return fmt.Errorf("pod does not have any ownerRefs")
+				}
+				return nil
+			})
+		}
 	}
 	if !evictSystemCriticalPods {
 		ev.constraints = append(ev.constraints, func(pod *corev1.Pod) error {
@@ -312,16 +314,20 @@ func NewEvictorFilter(
 			return nil
 		})
 	}
-	if options.labelSelector != nil && !options.labelSelector.Empty() {
+	selector, err := metav1.LabelSelectorAsSelector(options.labelSelector)
+	if err != nil {
+		return nil, fmt.Errorf("could not get selector from label selector")
+	}
+	if options.labelSelector != nil && !selector.Empty() {
 		ev.constraints = append(ev.constraints, func(pod *corev1.Pod) error {
-			if !options.labelSelector.Matches(labels.Set(pod.Labels)) {
+			if !selector.Matches(labels.Set(pod.Labels)) {
 				return fmt.Errorf("pod labels do not match the labelSelector filter in the policy parameter")
 			}
 			return nil
 		})
 	}
 
-	return ev
+	return ev, nil
 }
 
 // Filter decides when a pod is evictable

pkg/descheduler/evictions/evictions_test.go

@@ -92,6 +92,7 @@ func TestIsEvictable(t *testing.T) {
 		pods                    []*corev1.Pod
 		nodes                   []*corev1.Node
 		evictFailedBarePods     bool
+		evictAllBarePods        bool
 		evictLocalStoragePods   bool
 		evictSystemCriticalPods bool
 		ignorePVCPods           bool
@@ -125,6 +126,22 @@ func TestIsEvictable(t *testing.T) {
 			},
 			evictFailedBarePods: true,
 			result:              true,
+		}, {
+			description:         "Normal pod eviction with no ownerRefs and evictAllBarePods enabled",
+			pods:                []*corev1.Pod{test.BuildTestPod("bare_pod_normal_but_can_be_evicted", 400, 0, n1.Name, nil)},
+			evictFailedBarePods: false,
+			evictAllBarePods:    true,
+			result:              true,
+		}, {
+			description: "Failed pod eviction with no ownerRefs and evictAllBarePods enabled",
+			pods: []*corev1.Pod{
+				test.BuildTestPod("bare_pod_failed_but_can_be_evicted", 400, 0, n1.Name, func(pod *corev1.Pod) {
+					pod.Status.Phase = corev1.PodFailed
+				}),
+			},
+			evictFailedBarePods: false,
+			evictAllBarePods:    true,
+			result:              true,
 		}, {
 			description: "Normal pod eviction with normal ownerRefs",
 			pods: []*corev1.Pod{
@@ -716,21 +733,16 @@ func TestIsEvictable(t *testing.T) {
 
 			var opts []func(opts *Options)
 			if tt.priorityThreshold != nil {
-				opts = append(opts, WithPriorityThreshold(*tt.priorityThreshold))
+				opts = append(opts, WithPriorityThreshold(tt.priorityThreshold))
 			}
 			if tt.nodeFit {
 				opts = append(opts, WithNodeFit(true))
 			}
 			if tt.labelSelector != nil {
-				selector, err := metav1.LabelSelectorAsSelector(tt.labelSelector)
-				if err != nil {
-					t.Errorf("failed to get label selectors: %v", err)
-				} else {
-					opts = append(opts, WithLabelSelector(selector))
-				}
+				opts = append(opts, WithLabelSelector(tt.labelSelector))
 			}
 
-			evictorFilter := NewEvictorFilter(
+			evictorFilter, err := NewEvictorFilter(
 				func() ([]*corev1.Node, error) {
 					return nodes, nil
 				},
@@ -739,8 +751,12 @@ func TestIsEvictable(t *testing.T) {
 				tt.evictSystemCriticalPods,
 				tt.ignorePVCPods,
 				tt.evictFailedBarePods,
+				tt.evictAllBarePods,
 				opts...,
 			)
+			if err != nil {
+				t.Errorf("Failed to get evictorFilter, err: %s", err.Error())
+			}
 
 			result := evictorFilter.Filter(tt.pods[0])
 			if result != tt.result {