From d45cf05ac6dbbd490976d98ae780fe2467159229 Mon Sep 17 00:00:00 2001 From: songtao98 Date: Thu, 13 Jun 2024 19:17:30 +0800 Subject: [PATCH] support evict all bare pods in migration controller Signed-off-by: songtao98 --- .../apis/config/types_pluginargs.go | 3 + .../apis/config/v1alpha2/types_pluginargs.go | 3 + .../v1alpha2/zz_generated.conversion.go | 2 + .../migration/arbitrator/filter.go | 18 +++++- pkg/descheduler/evictions/evictions.go | 60 ++++++++++--------- pkg/descheduler/evictions/evictions_test.go | 32 +++++++--- 6 files changed, 81 insertions(+), 37 deletions(-) diff --git a/pkg/descheduler/apis/config/types_pluginargs.go b/pkg/descheduler/apis/config/types_pluginargs.go index b2a51a162..59f54de5f 100644 --- a/pkg/descheduler/apis/config/types_pluginargs.go +++ b/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 diff --git a/pkg/descheduler/apis/config/v1alpha2/types_pluginargs.go b/pkg/descheduler/apis/config/v1alpha2/types_pluginargs.go index 32696274c..b990da91f 100644 --- a/pkg/descheduler/apis/config/v1alpha2/types_pluginargs.go +++ b/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"` diff --git a/pkg/descheduler/apis/config/v1alpha2/zz_generated.conversion.go b/pkg/descheduler/apis/config/v1alpha2/zz_generated.conversion.go index 36e82704c..26d4a0a65 100644 --- a/pkg/descheduler/apis/config/v1alpha2/zz_generated.conversion.go +++ b/pkg/descheduler/apis/config/v1alpha2/zz_generated.conversion.go @@ -542,6 +542,7 @@ func autoConvert_v1alpha2_MigrationControllerArgs_To_config_MigrationControllerA return err } out.EvictFailedBarePods = in.EvictFailedBarePods + out.EvictAllBarePods = in.EvictAllBarePods out.EvictLocalStoragePods = in.EvictLocalStoragePods out.EvictSystemCriticalPods = in.EvictSystemCriticalPods out.IgnorePvcPods = in.IgnorePvcPods @@ -582,6 +583,7 @@ func autoConvert_config_MigrationControllerArgs_To_v1alpha2_MigrationControllerA return err } out.EvictFailedBarePods = in.EvictFailedBarePods + out.EvictAllBarePods = in.EvictAllBarePods out.EvictLocalStoragePods = in.EvictLocalStoragePods out.EvictSystemCriticalPods = in.EvictSystemCriticalPods out.IgnorePvcPods = in.IgnorePvcPods diff --git a/pkg/descheduler/controllers/migration/arbitrator/filter.go b/pkg/descheduler/controllers/migration/arbitrator/filter.go index 5b5000724..c41a3b6d8 100644 --- a/pkg/descheduler/controllers/migration/arbitrator/filter.go +++ b/pkg/descheduler/controllers/migration/arbitrator/filter.go @@ -45,6 +45,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" @@ -97,11 +98,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 { @@ -113,8 +116,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, diff --git a/pkg/descheduler/evictions/evictions.go b/pkg/descheduler/evictions/evictions.go index 352b35dfe..d179bc9b2 100644 --- a/pkg/descheduler/evictions/evictions.go +++ b/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 diff --git a/pkg/descheduler/evictions/evictions_test.go b/pkg/descheduler/evictions/evictions_test.go index 444f1d1aa..e604764a1 100644 --- a/pkg/descheduler/evictions/evictions_test.go +++ b/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 {