diff --git a/pkg/api/sort.go b/pkg/api/sort.go index 6d2a39d4b..276bd0a6b 100644 --- a/pkg/api/sort.go +++ b/pkg/api/sort.go @@ -15,7 +15,7 @@ package api import "sort" -func SortProfilesByName(profiles []Profile) []Profile { +func SortDeschedulerProfileByName(profiles []DeschedulerProfile) []DeschedulerProfile { sort.Slice(profiles, func(i, j int) bool { return profiles[i].Name < profiles[j].Name }) diff --git a/pkg/api/types.go b/pkg/api/types.go index f5f4e9a78..f94538464 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -28,7 +28,7 @@ type DeschedulerPolicy struct { metav1.TypeMeta // Profiles - Profiles []Profile + Profiles []DeschedulerProfile // NodeSelector for a set of nodes to operate over NodeSelector *string @@ -57,7 +57,7 @@ type PriorityThreshold struct { Name string } -type Profile struct { +type DeschedulerProfile struct { Name string PluginConfigs []PluginConfig Plugins Plugins diff --git a/pkg/api/v1alpha1/conversion.go b/pkg/api/v1alpha1/conversion.go index 44f7a3bc0..63998c16d 100644 --- a/pkg/api/v1alpha1/conversion.go +++ b/pkg/api/v1alpha1/conversion.go @@ -142,7 +142,7 @@ func V1alpha1ToInternal( ignorePvcPods = *deschedulerPolicy.IgnorePVCPods } - var profiles []api.Profile + var profiles []api.DeschedulerProfile // Build profiles for name, strategy := range deschedulerPolicy.Strategies { @@ -184,7 +184,7 @@ func V1alpha1ToInternal( return fmt.Errorf("unknown strategy name: %v", name) } - profile := api.Profile{ + profile := api.DeschedulerProfile{ Name: fmt.Sprintf("strategy-%v-profile", name), PluginConfigs: []api.PluginConfig{ { diff --git a/pkg/api/v1alpha2/sort.go b/pkg/api/v1alpha2/sort.go index a3f659df3..7330d642d 100644 --- a/pkg/api/v1alpha2/sort.go +++ b/pkg/api/v1alpha2/sort.go @@ -18,7 +18,7 @@ package v1alpha2 import "sort" -func SortProfilesByName(profiles []Profile) []Profile { +func SortDeschedulerProfileByName(profiles []DeschedulerProfile) []DeschedulerProfile { sort.Slice(profiles, func(i, j int) bool { return profiles[i].Name < profiles[j].Name }) diff --git a/pkg/api/v1alpha2/types.go b/pkg/api/v1alpha2/types.go index b1b47467f..ea8318b73 100644 --- a/pkg/api/v1alpha2/types.go +++ b/pkg/api/v1alpha2/types.go @@ -27,7 +27,7 @@ type DeschedulerPolicy struct { metav1.TypeMeta `json:",inline"` // Profiles - Profiles []Profile `json:"profiles,omitempty"` + Profiles []DeschedulerProfile `json:"profiles,omitempty"` // NodeSelector for a set of nodes to operate over NodeSelector *string `json:"nodeSelector,omitempty"` @@ -39,7 +39,7 @@ type DeschedulerPolicy struct { MaxNoOfPodsToEvictPerNamespace *uint `json:"maxNoOfPodsToEvictPerNamespace,omitempty"` } -type Profile struct { +type DeschedulerProfile struct { Name string `json:"name"` PluginConfigs []PluginConfig `json:"pluginConfig"` Plugins Plugins `json:"plugins"` diff --git a/pkg/api/v1alpha2/zz_generated.conversion.go b/pkg/api/v1alpha2/zz_generated.conversion.go index 0852ba0e3..14080b798 100644 --- a/pkg/api/v1alpha2/zz_generated.conversion.go +++ b/pkg/api/v1alpha2/zz_generated.conversion.go @@ -36,6 +36,16 @@ func init() { // RegisterConversions adds conversion functions to the given scheme. // Public to allow building arbitrary schemes. func RegisterConversions(s *runtime.Scheme) error { + if err := s.AddGeneratedConversionFunc((*DeschedulerProfile)(nil), (*api.DeschedulerProfile)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha2_DeschedulerProfile_To_api_DeschedulerProfile(a.(*DeschedulerProfile), b.(*api.DeschedulerProfile), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*api.DeschedulerProfile)(nil), (*DeschedulerProfile)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_api_DeschedulerProfile_To_v1alpha2_DeschedulerProfile(a.(*api.DeschedulerProfile), b.(*DeschedulerProfile), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*api.PluginConfig)(nil), (*PluginConfig)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_api_PluginConfig_To_v1alpha2_PluginConfig(a.(*api.PluginConfig), b.(*PluginConfig), scope) }); err != nil { @@ -61,16 +71,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*Profile)(nil), (*api.Profile)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha2_Profile_To_api_Profile(a.(*Profile), b.(*api.Profile), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*api.Profile)(nil), (*Profile)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_api_Profile_To_v1alpha2_Profile(a.(*api.Profile), b.(*Profile), scope) - }); err != nil { - return err - } if err := s.AddConversionFunc((*api.DeschedulerPolicy)(nil), (*DeschedulerPolicy)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_api_DeschedulerPolicy_To_v1alpha2_DeschedulerPolicy(a.(*api.DeschedulerPolicy), b.(*DeschedulerPolicy), scope) }); err != nil { @@ -92,9 +92,9 @@ func RegisterConversions(s *runtime.Scheme) error { func autoConvert_v1alpha2_DeschedulerPolicy_To_api_DeschedulerPolicy(in *DeschedulerPolicy, out *api.DeschedulerPolicy, s conversion.Scope) error { if in.Profiles != nil { in, out := &in.Profiles, &out.Profiles - *out = make([]api.Profile, len(*in)) + *out = make([]api.DeschedulerProfile, len(*in)) for i := range *in { - if err := Convert_v1alpha2_Profile_To_api_Profile(&(*in)[i], &(*out)[i], s); err != nil { + if err := Convert_v1alpha2_DeschedulerProfile_To_api_DeschedulerProfile(&(*in)[i], &(*out)[i], s); err != nil { return err } } @@ -110,9 +110,9 @@ func autoConvert_v1alpha2_DeschedulerPolicy_To_api_DeschedulerPolicy(in *Desched func autoConvert_api_DeschedulerPolicy_To_v1alpha2_DeschedulerPolicy(in *api.DeschedulerPolicy, out *DeschedulerPolicy, s conversion.Scope) error { if in.Profiles != nil { in, out := &in.Profiles, &out.Profiles - *out = make([]Profile, len(*in)) + *out = make([]DeschedulerProfile, len(*in)) for i := range *in { - if err := Convert_api_Profile_To_v1alpha2_Profile(&(*in)[i], &(*out)[i], s); err != nil { + if err := Convert_api_DeschedulerProfile_To_v1alpha2_DeschedulerProfile(&(*in)[i], &(*out)[i], s); err != nil { return err } } @@ -125,6 +125,54 @@ func autoConvert_api_DeschedulerPolicy_To_v1alpha2_DeschedulerPolicy(in *api.Des return nil } +func autoConvert_v1alpha2_DeschedulerProfile_To_api_DeschedulerProfile(in *DeschedulerProfile, out *api.DeschedulerProfile, s conversion.Scope) error { + out.Name = in.Name + if in.PluginConfigs != nil { + in, out := &in.PluginConfigs, &out.PluginConfigs + *out = make([]api.PluginConfig, len(*in)) + for i := range *in { + if err := Convert_v1alpha2_PluginConfig_To_api_PluginConfig(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.PluginConfigs = nil + } + if err := Convert_v1alpha2_Plugins_To_api_Plugins(&in.Plugins, &out.Plugins, s); err != nil { + return err + } + return nil +} + +// Convert_v1alpha2_DeschedulerProfile_To_api_DeschedulerProfile is an autogenerated conversion function. +func Convert_v1alpha2_DeschedulerProfile_To_api_DeschedulerProfile(in *DeschedulerProfile, out *api.DeschedulerProfile, s conversion.Scope) error { + return autoConvert_v1alpha2_DeschedulerProfile_To_api_DeschedulerProfile(in, out, s) +} + +func autoConvert_api_DeschedulerProfile_To_v1alpha2_DeschedulerProfile(in *api.DeschedulerProfile, out *DeschedulerProfile, s conversion.Scope) error { + out.Name = in.Name + if in.PluginConfigs != nil { + in, out := &in.PluginConfigs, &out.PluginConfigs + *out = make([]PluginConfig, len(*in)) + for i := range *in { + if err := Convert_api_PluginConfig_To_v1alpha2_PluginConfig(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.PluginConfigs = nil + } + if err := Convert_api_Plugins_To_v1alpha2_Plugins(&in.Plugins, &out.Plugins, s); err != nil { + return err + } + return nil +} + +// Convert_api_DeschedulerProfile_To_v1alpha2_DeschedulerProfile is an autogenerated conversion function. +func Convert_api_DeschedulerProfile_To_v1alpha2_DeschedulerProfile(in *api.DeschedulerProfile, out *DeschedulerProfile, s conversion.Scope) error { + return autoConvert_api_DeschedulerProfile_To_v1alpha2_DeschedulerProfile(in, out, s) +} + func autoConvert_v1alpha2_PluginConfig_To_api_PluginConfig(in *PluginConfig, out *api.PluginConfig, s conversion.Scope) error { out.Name = in.Name if err := runtime.Convert_runtime_RawExtension_To_runtime_Object(&in.Args, &out.Args, s); err != nil { @@ -227,51 +275,3 @@ func autoConvert_api_Plugins_To_v1alpha2_Plugins(in *api.Plugins, out *Plugins, func Convert_api_Plugins_To_v1alpha2_Plugins(in *api.Plugins, out *Plugins, s conversion.Scope) error { return autoConvert_api_Plugins_To_v1alpha2_Plugins(in, out, s) } - -func autoConvert_v1alpha2_Profile_To_api_Profile(in *Profile, out *api.Profile, s conversion.Scope) error { - out.Name = in.Name - if in.PluginConfigs != nil { - in, out := &in.PluginConfigs, &out.PluginConfigs - *out = make([]api.PluginConfig, len(*in)) - for i := range *in { - if err := Convert_v1alpha2_PluginConfig_To_api_PluginConfig(&(*in)[i], &(*out)[i], s); err != nil { - return err - } - } - } else { - out.PluginConfigs = nil - } - if err := Convert_v1alpha2_Plugins_To_api_Plugins(&in.Plugins, &out.Plugins, s); err != nil { - return err - } - return nil -} - -// Convert_v1alpha2_Profile_To_api_Profile is an autogenerated conversion function. -func Convert_v1alpha2_Profile_To_api_Profile(in *Profile, out *api.Profile, s conversion.Scope) error { - return autoConvert_v1alpha2_Profile_To_api_Profile(in, out, s) -} - -func autoConvert_api_Profile_To_v1alpha2_Profile(in *api.Profile, out *Profile, s conversion.Scope) error { - out.Name = in.Name - if in.PluginConfigs != nil { - in, out := &in.PluginConfigs, &out.PluginConfigs - *out = make([]PluginConfig, len(*in)) - for i := range *in { - if err := Convert_api_PluginConfig_To_v1alpha2_PluginConfig(&(*in)[i], &(*out)[i], s); err != nil { - return err - } - } - } else { - out.PluginConfigs = nil - } - if err := Convert_api_Plugins_To_v1alpha2_Plugins(&in.Plugins, &out.Plugins, s); err != nil { - return err - } - return nil -} - -// Convert_api_Profile_To_v1alpha2_Profile is an autogenerated conversion function. -func Convert_api_Profile_To_v1alpha2_Profile(in *api.Profile, out *Profile, s conversion.Scope) error { - return autoConvert_api_Profile_To_v1alpha2_Profile(in, out, s) -} diff --git a/pkg/api/v1alpha2/zz_generated.deepcopy.go b/pkg/api/v1alpha2/zz_generated.deepcopy.go index 63f375381..93302328b 100644 --- a/pkg/api/v1alpha2/zz_generated.deepcopy.go +++ b/pkg/api/v1alpha2/zz_generated.deepcopy.go @@ -31,7 +31,7 @@ func (in *DeschedulerPolicy) DeepCopyInto(out *DeschedulerPolicy) { out.TypeMeta = in.TypeMeta if in.Profiles != nil { in, out := &in.Profiles, &out.Profiles - *out = make([]Profile, len(*in)) + *out = make([]DeschedulerProfile, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -72,6 +72,30 @@ func (in *DeschedulerPolicy) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeschedulerProfile) DeepCopyInto(out *DeschedulerProfile) { + *out = *in + if in.PluginConfigs != nil { + in, out := &in.PluginConfigs, &out.PluginConfigs + *out = make([]PluginConfig, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + in.Plugins.DeepCopyInto(&out.Plugins) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeschedulerProfile. +func (in *DeschedulerProfile) DeepCopy() *DeschedulerProfile { + if in == nil { + return nil + } + out := new(DeschedulerProfile) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PluginConfig) DeepCopyInto(out *PluginConfig) { *out = *in @@ -137,27 +161,3 @@ func (in *Plugins) DeepCopy() *Plugins { in.DeepCopyInto(out) return out } - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Profile) DeepCopyInto(out *Profile) { - *out = *in - if in.PluginConfigs != nil { - in, out := &in.PluginConfigs, &out.PluginConfigs - *out = make([]PluginConfig, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - in.Plugins.DeepCopyInto(&out.Plugins) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Profile. -func (in *Profile) DeepCopy() *Profile { - if in == nil { - return nil - } - out := new(Profile) - in.DeepCopyInto(out) - return out -} diff --git a/pkg/api/zz_generated.deepcopy.go b/pkg/api/zz_generated.deepcopy.go index 0858e240f..ebbdaf283 100644 --- a/pkg/api/zz_generated.deepcopy.go +++ b/pkg/api/zz_generated.deepcopy.go @@ -31,7 +31,7 @@ func (in *DeschedulerPolicy) DeepCopyInto(out *DeschedulerPolicy) { out.TypeMeta = in.TypeMeta if in.Profiles != nil { in, out := &in.Profiles, &out.Profiles - *out = make([]Profile, len(*in)) + *out = make([]DeschedulerProfile, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -72,6 +72,30 @@ func (in *DeschedulerPolicy) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeschedulerProfile) DeepCopyInto(out *DeschedulerProfile) { + *out = *in + if in.PluginConfigs != nil { + in, out := &in.PluginConfigs, &out.PluginConfigs + *out = make([]PluginConfig, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + in.Plugins.DeepCopyInto(&out.Plugins) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeschedulerProfile. +func (in *DeschedulerProfile) DeepCopy() *DeschedulerProfile { + if in == nil { + return nil + } + out := new(DeschedulerProfile) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Namespaces) DeepCopyInto(out *Namespaces) { *out = *in @@ -187,30 +211,6 @@ func (in *PriorityThreshold) DeepCopy() *PriorityThreshold { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Profile) DeepCopyInto(out *Profile) { - *out = *in - if in.PluginConfigs != nil { - in, out := &in.PluginConfigs, &out.PluginConfigs - *out = make([]PluginConfig, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - in.Plugins.DeepCopyInto(&out.Plugins) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Profile. -func (in *Profile) DeepCopy() *Profile { - if in == nil { - return nil - } - out := new(Profile) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in ResourceThresholds) DeepCopyInto(out *ResourceThresholds) { { diff --git a/pkg/descheduler/descheduler.go b/pkg/descheduler/descheduler.go index 51fdffc52..01f6f1809 100644 --- a/pkg/descheduler/descheduler.go +++ b/pkg/descheduler/descheduler.go @@ -27,7 +27,6 @@ import ( componentbaseconfig "k8s.io/component-base/config" "k8s.io/klog/v2" - v1 "k8s.io/api/core/v1" policy "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -48,23 +47,12 @@ import ( eutils "sigs.k8s.io/descheduler/pkg/descheduler/evictions/utils" nodeutil "sigs.k8s.io/descheduler/pkg/descheduler/node" podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod" - "sigs.k8s.io/descheduler/pkg/framework" "sigs.k8s.io/descheduler/pkg/framework/pluginregistry" - "sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor" + frameworkprofile "sigs.k8s.io/descheduler/pkg/framework/profile" "sigs.k8s.io/descheduler/pkg/utils" "sigs.k8s.io/descheduler/pkg/version" ) -type enabledDeschedulePluginEntry struct { - Plugin framework.DeschedulePlugin - Profile string -} - -type enabledBalancePluginEntry struct { - Plugin framework.BalancePlugin - Profile string -} - func Run(ctx context.Context, rs *options.DeschedulerServer) error { metrics.Register() @@ -216,64 +204,6 @@ func cachedClient( return fakeClient, nil } -// evictorImpl implements the Evictor interface so plugins -// can evict a pod without importing a specific pod evictor -type evictorImpl struct { - podEvictor *evictions.PodEvictor - evictorFilter framework.EvictorPlugin -} - -var _ framework.Evictor = &evictorImpl{} - -// Filter checks if a pod can be evicted -func (ei *evictorImpl) Filter(pod *v1.Pod) bool { - return ei.evictorFilter.Filter(pod) -} - -// PreEvictionFilter checks if pod can be evicted right before eviction -func (ei *evictorImpl) PreEvictionFilter(pod *v1.Pod) bool { - return ei.evictorFilter.PreEvictionFilter(pod) -} - -// Evict evicts a pod (no pre-check performed) -func (ei *evictorImpl) Evict(ctx context.Context, pod *v1.Pod, opts evictions.EvictOptions) bool { - return ei.podEvictor.EvictPod(ctx, pod, opts) -} - -func (ei *evictorImpl) NodeLimitExceeded(node *v1.Node) bool { - return ei.podEvictor.NodeLimitExceeded(node) -} - -// handleImpl implements the framework handle which gets passed to plugins -type handleImpl struct { - clientSet clientset.Interface - getPodsAssignedToNodeFunc podutil.GetPodsAssignedToNodeFunc - sharedInformerFactory informers.SharedInformerFactory - evictor *evictorImpl -} - -var _ framework.Handle = &handleImpl{} - -// ClientSet retrieves kube client set -func (hi *handleImpl) ClientSet() clientset.Interface { - return hi.clientSet -} - -// GetPodsAssignedToNodeFunc retrieves GetPodsAssignedToNodeFunc implementation -func (hi *handleImpl) GetPodsAssignedToNodeFunc() podutil.GetPodsAssignedToNodeFunc { - return hi.getPodsAssignedToNodeFunc -} - -// SharedInformerFactory retrieves shared informer factory -func (hi *handleImpl) SharedInformerFactory() informers.SharedInformerFactory { - return hi.sharedInformerFactory -} - -// Evictor retrieves evictor so plugins can filter and evict pods -func (hi *handleImpl) Evictor() framework.Evictor { - return hi.evictor -} - func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer, deschedulerPolicy *api.DeschedulerPolicy, evictionPolicyGroupVersion string) error { sharedInformerFactory := informers.NewSharedInformerFactory(rs.Client, 0) podInformer := sharedInformerFactory.Core().V1().Pods().Informer() @@ -308,6 +238,8 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer eventBroadcaster, eventRecorder := utils.GetRecorderAndBroadcaster(ctx, eventClient) defer eventBroadcaster.Shutdown() + cycleSharedInformerFactory := sharedInformerFactory + wait.NonSlidingUntil(func() { loopStartDuration := time.Now() defer metrics.DeschedulerLoopDuration.With(map[string]string{}).Observe(time.Since(loopStartDuration).Seconds()) @@ -324,7 +256,7 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer return } - var podEvictorClient clientset.Interface + var client clientset.Interface // When the dry mode is enable, collect all the relevant objects (mostly pods) under a fake client. // So when evicting pods while running multiple strategies in a row have the cummulative effect // as is when evicting pods for real. @@ -337,7 +269,9 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer return } + // create a new instance of the shared informer factor from the cached client fakeSharedInformerFactory := informers.NewSharedInformerFactory(fakeClient, 0) + // register the pod informer, otherwise it will not get running getPodsAssignedToNode, err = podutil.BuildGetPodsAssignedToNodeFunc(fakeSharedInformerFactory.Core().V1().Pods().Informer()) if err != nil { klog.Errorf("build get pods assigned to node function error: %v", err) @@ -349,14 +283,15 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer fakeSharedInformerFactory.Start(fakeCtx.Done()) fakeSharedInformerFactory.WaitForCacheSync(fakeCtx.Done()) - podEvictorClient = fakeClient + client = fakeClient + cycleSharedInformerFactory = fakeSharedInformerFactory } else { - podEvictorClient = rs.Client + client = rs.Client } klog.V(3).Infof("Building a pod evictor") podEvictor := evictions.NewPodEvictor( - podEvictorClient, + client, evictionPolicyGroupVersion, rs.DryRun, deschedulerPolicy.MaxNoOfPodsToEvictPerNode, @@ -366,88 +301,31 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer eventRecorder, ) - var enabledDeschedulePlugins []enabledDeschedulePluginEntry - var enabledBalancePlugins []enabledBalancePluginEntry - - // Build plugins for _, profile := range deschedulerPolicy.Profiles { - pc, _ := GetPluginConfig(defaultevictor.PluginName, profile.PluginConfigs) - if pc == nil { - klog.ErrorS(fmt.Errorf("unable to get plugin config"), "skipping plugin", "plugin", defaultevictor.PluginName, "profile", profile.Name) - continue - } - evictorFilter, err := defaultevictor.New( - pc.Args, - &handleImpl{ - clientSet: rs.Client, - getPodsAssignedToNodeFunc: getPodsAssignedToNode, - sharedInformerFactory: sharedInformerFactory, - }, + currProfile, err := frameworkprofile.NewProfile( + profile, + pluginregistry.PluginRegistry, + frameworkprofile.WithClientSet(client), + frameworkprofile.WithSharedInformerFactory(cycleSharedInformerFactory), + frameworkprofile.WithPodEvictor(podEvictor), + frameworkprofile.WithGetPodsAssignedToNodeFnc(getPodsAssignedToNode), ) if err != nil { - klog.ErrorS(fmt.Errorf("unable to construct a plugin"), "skipping plugin", "plugin", defaultevictor.PluginName) + klog.ErrorS(err, "unable to create a profile", "profile", profile.Name) continue } - handle := &handleImpl{ - clientSet: rs.Client, - getPodsAssignedToNodeFunc: getPodsAssignedToNode, - sharedInformerFactory: sharedInformerFactory, - evictor: &evictorImpl{ - podEvictor: podEvictor, - evictorFilter: evictorFilter.(framework.EvictorPlugin), - }, - } - // Assuming only a list of enabled extension points. - // Later, when a default list of plugins and their extension points is established, - // compute the list of enabled extension points as (DefaultEnabled + Enabled - Disabled) - for _, plugin := range append(profile.Plugins.Deschedule.Enabled, profile.Plugins.Balance.Enabled...) { - pc, _ := GetPluginConfig(plugin, profile.PluginConfigs) - if pc == nil { - klog.ErrorS(fmt.Errorf("unable to get plugin config"), "skipping plugin", "plugin", plugin) - continue - } - registryPlugin, ok := pluginregistry.PluginRegistry[plugin] - pgFnc := registryPlugin.PluginBuilder - if !ok { - klog.ErrorS(fmt.Errorf("unable to find plugin in the pluginsMap"), "skipping plugin", "plugin", plugin) - } - pg, err := pgFnc(pc.Args, handle) - if err != nil { - klog.ErrorS(err, "unable to initialize a plugin", "pluginName", plugin) - } - if pg != nil { - // pg can be of any of each type, or both - enabledDeschedulePlugins, enabledBalancePlugins = includeProfilePluginsByType(enabledDeschedulePlugins, enabledBalancePlugins, pg, profile.Name) - } - } - } - // Execute extension points - for _, pg := range enabledDeschedulePlugins { - // TODO: strategyName should be accessible from within the strategy using a framework - // handle or function which the Evictor has access to. For migration/in-progress framework - // work, we are currently passing this via context. To be removed - // (See discussion thread https://github.com/kubernetes-sigs/descheduler/pull/885#discussion_r919962292) - strategyStart := time.Now() - childCtx := context.WithValue(ctx, "strategyName", pg.Plugin.Name()) - status := pg.Plugin.Deschedule(childCtx, nodes) - metrics.DeschedulerStrategyDuration.With(map[string]string{"strategy": pg.Plugin.Name(), "profile": pg.Profile}).Observe(time.Since(strategyStart).Seconds()) + // First deschedule + status := currProfile.RunDeschedulePlugins(ctx, nodes) if status != nil && status.Err != nil { - klog.ErrorS(status.Err, "plugin finished with error", "pluginName", pg.Plugin.Name()) + klog.ErrorS(status.Err, "running deschedule extension point failed with error", "profile", profile.Name) + continue } - } - - for _, pg := range enabledBalancePlugins { - // TODO: strategyName should be accessible from within the strategy using a framework - // handle or function which the Evictor has access to. For migration/in-progress framework - // work, we are currently passing this via context. To be removed - // (See discussion thread https://github.com/kubernetes-sigs/descheduler/pull/885#discussion_r919962292) - strategyStart := time.Now() - childCtx := context.WithValue(ctx, "strategyName", pg.Plugin.Name()) - status := pg.Plugin.Balance(childCtx, nodes) - metrics.DeschedulerStrategyDuration.With(map[string]string{"strategy": pg.Plugin.Name(), "profile": pg.Profile}).Observe(time.Since(strategyStart).Seconds()) + // Then balance + status = currProfile.RunBalancePlugins(ctx, nodes) if status != nil && status.Err != nil { - klog.ErrorS(status.Err, "plugin finished with error", "pluginName", pg.Plugin.Name()) + klog.ErrorS(status.Err, "running balance extension point failed with error", "profile", profile.Name) + continue } } @@ -462,28 +340,6 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer return nil } -func includeProfilePluginsByType(enabledDeschedulePlugins []enabledDeschedulePluginEntry, enabledBalancePlugins []enabledBalancePluginEntry, pg framework.Plugin, profile string) ([]enabledDeschedulePluginEntry, []enabledBalancePluginEntry) { - enabledDeschedulePlugins = includeDeschedule(enabledDeschedulePlugins, pg, profile) - enabledBalancePlugins = includeBalance(enabledBalancePlugins, pg, profile) - return enabledDeschedulePlugins, enabledBalancePlugins -} - -func includeDeschedule(enabledDeschedulePlugins []enabledDeschedulePluginEntry, pg framework.Plugin, profile string) []enabledDeschedulePluginEntry { - _, ok := pg.(framework.DeschedulePlugin) - if ok { - enabledDeschedulePlugins = append(enabledDeschedulePlugins, enabledDeschedulePluginEntry{Plugin: pg.(framework.DeschedulePlugin), Profile: profile}) - } - return enabledDeschedulePlugins -} - -func includeBalance(enabledBalancePlugins []enabledBalancePluginEntry, pg framework.Plugin, profile string) []enabledBalancePluginEntry { - _, ok := pg.(framework.BalancePlugin) - if ok { - enabledBalancePlugins = append(enabledBalancePlugins, enabledBalancePluginEntry{Plugin: pg.(framework.BalancePlugin), Profile: profile}) - } - return enabledBalancePlugins -} - func GetPluginConfig(pluginName string, pluginConfigs []api.PluginConfig) (*api.PluginConfig, int) { for idx, pluginConfig := range pluginConfigs { if pluginConfig.Name == pluginName { diff --git a/pkg/descheduler/descheduler_test.go b/pkg/descheduler/descheduler_test.go index 5ee71d130..d6e0d271c 100644 --- a/pkg/descheduler/descheduler_test.go +++ b/pkg/descheduler/descheduler_test.go @@ -17,6 +17,7 @@ import ( "sigs.k8s.io/descheduler/pkg/api" "sigs.k8s.io/descheduler/pkg/api/v1alpha1" "sigs.k8s.io/descheduler/pkg/framework/pluginregistry" + "sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor" "sigs.k8s.io/descheduler/pkg/framework/plugins/removeduplicates" "sigs.k8s.io/descheduler/pkg/framework/plugins/removepodsviolatingnodetaints" "sigs.k8s.io/descheduler/test" @@ -41,6 +42,8 @@ func (s scope) Meta() *conversion.Meta { func TestTaintsUpdated(t *testing.T) { pluginregistry.PluginRegistry = pluginregistry.NewRegistry() pluginregistry.Register(removepodsviolatingnodetaints.PluginName, removepodsviolatingnodetaints.New, &removepodsviolatingnodetaints.RemovePodsViolatingNodeTaintsArgs{}, removepodsviolatingnodetaints.ValidateRemovePodsViolatingNodeTaintsArgs, removepodsviolatingnodetaints.SetDefaults_RemovePodsViolatingNodeTaintsArgs, pluginregistry.PluginRegistry) + pluginregistry.Register(defaultevictor.PluginName, defaultevictor.New, &defaultevictor.DefaultEvictorArgs{}, defaultevictor.ValidateDefaultEvictorArgs, defaultevictor.SetDefaults_DefaultEvictorArgs, pluginregistry.PluginRegistry) + ctx := context.Background() n1 := test.BuildTestNode("n1", 2000, 3000, 10, nil) n2 := test.BuildTestNode("n2", 2000, 3000, 10, nil) @@ -110,6 +113,8 @@ func TestTaintsUpdated(t *testing.T) { func TestDuplicate(t *testing.T) { pluginregistry.PluginRegistry = pluginregistry.NewRegistry() pluginregistry.Register(removeduplicates.PluginName, removeduplicates.New, &removeduplicates.RemoveDuplicatesArgs{}, removeduplicates.ValidateRemoveDuplicatesArgs, removeduplicates.SetDefaults_RemoveDuplicatesArgs, pluginregistry.PluginRegistry) + pluginregistry.Register(defaultevictor.PluginName, defaultevictor.New, &defaultevictor.DefaultEvictorArgs{}, defaultevictor.ValidateDefaultEvictorArgs, defaultevictor.SetDefaults_DefaultEvictorArgs, pluginregistry.PluginRegistry) + ctx := context.Background() node1 := test.BuildTestNode("n1", 2000, 3000, 10, nil) node2 := test.BuildTestNode("n2", 2000, 3000, 10, nil) @@ -177,7 +182,7 @@ func TestRootCancel(t *testing.T) { client := fakeclientset.NewSimpleClientset(n1, n2) eventClient := fakeclientset.NewSimpleClientset(n1, n2) dp := &api.DeschedulerPolicy{ - Profiles: []api.Profile{}, // no strategies needed for this test + Profiles: []api.DeschedulerProfile{}, // no strategies needed for this test } rs, err := options.NewDeschedulerServer() @@ -212,7 +217,7 @@ func TestRootCancelWithNoInterval(t *testing.T) { client := fakeclientset.NewSimpleClientset(n1, n2) eventClient := fakeclientset.NewSimpleClientset(n1, n2) dp := &api.DeschedulerPolicy{ - Profiles: []api.Profile{}, // no strategies needed for this test + Profiles: []api.DeschedulerProfile{}, // no strategies needed for this test } rs, err := options.NewDeschedulerServer() diff --git a/pkg/descheduler/policyconfig.go b/pkg/descheduler/policyconfig.go index 071fa46c2..c9b91e8ee 100644 --- a/pkg/descheduler/policyconfig.go +++ b/pkg/descheduler/policyconfig.go @@ -87,7 +87,7 @@ func setDefaultsPluginConfig(pluginConfig *api.PluginConfig, registry pluginregi } } -func setDefaultEvictor(profile api.Profile, client clientset.Interface) api.Profile { +func setDefaultEvictor(profile api.DeschedulerProfile, client clientset.Interface) api.DeschedulerProfile { newPluginConfig := api.PluginConfig{ Name: defaultevictor.PluginName, Args: &defaultevictor.DefaultEvictorArgs{ diff --git a/pkg/descheduler/policyconfig_test.go b/pkg/descheduler/policyconfig_test.go index 2717048cc..3bba1d908 100644 --- a/pkg/descheduler/policyconfig_test.go +++ b/pkg/descheduler/policyconfig_test.go @@ -93,7 +93,7 @@ func TestV1alpha1ToV1alpha2(t *testing.T) { }, }, result: &api.DeschedulerPolicy{ - Profiles: []api.Profile{ + Profiles: []api.DeschedulerProfile{ { Name: fmt.Sprintf("strategy-%s-profile", removeduplicates.PluginName), PluginConfigs: []api.PluginConfig{ @@ -201,7 +201,7 @@ func TestV1alpha1ToV1alpha2(t *testing.T) { }, }, result: &api.DeschedulerPolicy{ - Profiles: []api.Profile{ + Profiles: []api.DeschedulerProfile{ { Name: fmt.Sprintf("strategy-%s-profile", nodeutilization.HighNodeUtilizationPluginName), PluginConfigs: []api.PluginConfig{ @@ -458,7 +458,7 @@ func TestV1alpha1ToV1alpha2(t *testing.T) { }, }, result: &api.DeschedulerPolicy{ - Profiles: []api.Profile{ + Profiles: []api.DeschedulerProfile{ { Name: fmt.Sprintf("strategy-%s-profile", nodeutilization.HighNodeUtilizationPluginName), PluginConfigs: []api.PluginConfig{ @@ -692,7 +692,7 @@ func TestV1alpha1ToV1alpha2(t *testing.T) { } if err == nil { // sort to easily compare deepequality - result.Profiles = api.SortProfilesByName(result.Profiles) + result.Profiles = api.SortDeschedulerProfileByName(result.Profiles) diff := cmp.Diff(tc.result, result) if diff != "" { t.Errorf("test '%s' failed. Results are not deep equal. mismatch (-want +got):\n%s", tc.description, diff) @@ -738,7 +738,7 @@ strategies: - "testleaderelection-a" `), result: &api.DeschedulerPolicy{ - Profiles: []api.Profile{ + Profiles: []api.DeschedulerProfile{ { Name: fmt.Sprintf("strategy-%s-profile", podlifetime.PluginName), PluginConfigs: []api.PluginConfig{ @@ -792,7 +792,7 @@ profiles: - "RemovePodsHavingTooManyRestarts" `), result: &api.DeschedulerPolicy{ - Profiles: []api.Profile{ + Profiles: []api.DeschedulerProfile{ { Name: "ProfileName", PluginConfigs: []api.PluginConfig{ @@ -860,7 +860,7 @@ func TestValidateDeschedulerConfiguration(t *testing.T) { { description: "multiple errors", deschedulerPolicy: api.DeschedulerPolicy{ - Profiles: []api.Profile{ + Profiles: []api.DeschedulerProfile{ { Name: removefailedpods.PluginName, Plugins: api.Plugins{ @@ -947,7 +947,7 @@ profiles: - "RemovePodsHavingTooManyRestarts" `), result: &api.DeschedulerPolicy{ - Profiles: []api.Profile{ + Profiles: []api.DeschedulerProfile{ { Name: "ProfileName", PluginConfigs: []api.PluginConfig{ diff --git a/pkg/framework/fake/plugin/fake.go b/pkg/framework/fake/plugin/fake.go new file mode 100644 index 000000000..78b7d1bc6 --- /dev/null +++ b/pkg/framework/fake/plugin/fake.go @@ -0,0 +1,138 @@ +/* +Copyright 2023 The Kubernetes Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugin + +import ( + "context" + "fmt" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/descheduler/pkg/framework" + "sigs.k8s.io/descheduler/pkg/framework/pluginregistry" +) + +// +k8s:deepcopy-gen=true +// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object + +// FakePluginArgs holds arguments used to configure FakePlugin plugin. +type FakePluginArgs struct { + metav1.TypeMeta `json:",inline"` +} + +func ValidateFakePluginArgs(obj runtime.Object) error { + return nil +} + +func SetDefaults_FakePluginArgs(obj runtime.Object) {} + +var ( + _ framework.EvictorPlugin = &FakePlugin{} + _ framework.DeschedulePlugin = &FakePlugin{} + _ framework.BalancePlugin = &FakePlugin{} +) + +// FakePlugin is a configurable plugin used for testing +type FakePlugin struct { + PluginName string + + // ReactionChain is the list of reactors that will be attempted for every + // request in the order they are tried. + ReactionChain []Reactor + + args runtime.Object + handle framework.Handle +} + +func NewPluginFncFromFake(fp *FakePlugin) pluginregistry.PluginBuilder { + return func(args runtime.Object, handle framework.Handle) (framework.Plugin, error) { + fakePluginArgs, ok := args.(*FakePluginArgs) + if !ok { + return nil, fmt.Errorf("want args to be of type FakePluginArgs, got %T", args) + } + + fp.handle = handle + fp.args = fakePluginArgs + + return fp, nil + } +} + +// New builds plugin from its arguments while passing a handle +func New(args runtime.Object, handle framework.Handle) (framework.Plugin, error) { + fakePluginArgs, ok := args.(*FakePluginArgs) + if !ok { + return nil, fmt.Errorf("want args to be of type FakePluginArgs, got %T", args) + } + + ev := &FakePlugin{} + ev.handle = handle + ev.args = fakePluginArgs + + return ev, nil +} + +// Name retrieves the plugin name +func (d *FakePlugin) Name() string { + return d.PluginName +} + +func (d *FakePlugin) PreEvictionFilter(pod *v1.Pod) bool { + return true +} + +func (d *FakePlugin) Filter(pod *v1.Pod) bool { + return true +} + +func (d *FakePlugin) handleAction(action Action) *framework.Status { + actionCopy := action.DeepCopy() + for _, reactor := range d.ReactionChain { + if !reactor.Handles(actionCopy) { + continue + } + handled, err := reactor.React(actionCopy) + if !handled { + continue + } + + return &framework.Status{ + Err: err, + } + } + return &framework.Status{ + Err: fmt.Errorf("unhandled %q action", action.GetExtensionPoint()), + } +} + +func (d *FakePlugin) Deschedule(ctx context.Context, nodes []*v1.Node) *framework.Status { + return d.handleAction(&DescheduleActionImpl{ + ActionImpl: ActionImpl{ + handle: d.handle, + extensionPoint: string(framework.DescheduleExtensionPoint), + }, + nodes: nodes, + }) +} + +func (d *FakePlugin) Balance(ctx context.Context, nodes []*v1.Node) *framework.Status { + return d.handleAction(&BalanceActionImpl{ + ActionImpl: ActionImpl{ + handle: d.handle, + extensionPoint: string(framework.BalanceExtensionPoint), + }, + nodes: nodes, + }) +} diff --git a/pkg/framework/fake/plugin/fixture.go b/pkg/framework/fake/plugin/fixture.go new file mode 100644 index 000000000..61b72505a --- /dev/null +++ b/pkg/framework/fake/plugin/fixture.go @@ -0,0 +1,135 @@ +/* +Copyright 2023 The Kubernetes Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugin + +import ( + v1 "k8s.io/api/core/v1" + "sigs.k8s.io/descheduler/pkg/framework" +) + +type Action interface { + Handle() framework.Handle + GetExtensionPoint() string + DeepCopy() Action +} + +// Reactor is an interface to allow the composition of reaction functions. +type Reactor interface { + // Handles indicates whether or not this Reactor deals with a given + // action. + Handles(action Action) bool + // React handles the action. It may choose to + // delegate by indicated handled=false. + React(action Action) (handled bool, err error) +} + +// SimpleReactor is a Reactor. Each reaction function is attached to a given extensionPoint. "*" in either field matches everything for that value. +type SimpleReactor struct { + ExtensionPoint string + Reaction ReactionFunc +} + +func (r *SimpleReactor) Handles(action Action) bool { + return r.ExtensionPoint == "*" || r.ExtensionPoint == action.GetExtensionPoint() +} + +func (r *SimpleReactor) React(action Action) (bool, error) { + return r.Reaction(action) +} + +type ReactionFunc func(action Action) (handled bool, err error) + +type DescheduleAction interface { + Action + CanDeschedule() bool + Nodes() []*v1.Node +} + +type BalanceAction interface { + Action + CanBalance() bool + Nodes() []*v1.Node +} + +type ActionImpl struct { + handle framework.Handle + extensionPoint string +} + +func (a ActionImpl) Handle() framework.Handle { + return a.handle +} + +func (a ActionImpl) GetExtensionPoint() string { + return a.extensionPoint +} + +func (a ActionImpl) DeepCopy() Action { + // The handle is expected to be accessed only throuh interface methods + // Thus, no deep copy needed. + ret := a + return ret +} + +type DescheduleActionImpl struct { + ActionImpl + nodes []*v1.Node +} + +func (d DescheduleActionImpl) CanDeschedule() bool { + return true +} + +func (d DescheduleActionImpl) Nodes() []*v1.Node { + return d.nodes +} + +func (a DescheduleActionImpl) DeepCopy() Action { + nodesCopy := []*v1.Node{} + for _, node := range a.nodes { + nodesCopy = append(nodesCopy, node.DeepCopy()) + } + return DescheduleActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + nodes: nodesCopy, + } +} + +type BalanceActionImpl struct { + ActionImpl + nodes []*v1.Node +} + +func (d BalanceActionImpl) CanBalance() bool { + return true +} + +func (d BalanceActionImpl) Nodes() []*v1.Node { + return d.nodes +} + +func (a BalanceActionImpl) DeepCopy() Action { + nodesCopy := []*v1.Node{} + for _, node := range a.nodes { + nodesCopy = append(nodesCopy, node.DeepCopy()) + } + return BalanceActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + nodes: nodesCopy, + } +} + +func (c *FakePlugin) AddReactor(extensionPoint string, reaction ReactionFunc) { + c.ReactionChain = append(c.ReactionChain, &SimpleReactor{ExtensionPoint: extensionPoint, Reaction: reaction}) +} diff --git a/pkg/framework/fake/plugin/zz_generated.deepcopy.go b/pkg/framework/fake/plugin/zz_generated.deepcopy.go new file mode 100644 index 000000000..d2e7d2d08 --- /dev/null +++ b/pkg/framework/fake/plugin/zz_generated.deepcopy.go @@ -0,0 +1,51 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by deepcopy-gen. DO NOT EDIT. + +package plugin + +import ( + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FakePluginArgs) DeepCopyInto(out *FakePluginArgs) { + *out = *in + out.TypeMeta = in.TypeMeta + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FakePluginArgs. +func (in *FakePluginArgs) DeepCopy() *FakePluginArgs { + if in == nil { + return nil + } + out := new(FakePluginArgs) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *FakePluginArgs) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} diff --git a/pkg/framework/profile/profile.go b/pkg/framework/profile/profile.go new file mode 100644 index 000000000..2360b2757 --- /dev/null +++ b/pkg/framework/profile/profile.go @@ -0,0 +1,310 @@ +/* +Copyright 2023 The Kubernetes Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package profile + +import ( + "context" + "fmt" + "time" + + "sigs.k8s.io/descheduler/metrics" + "sigs.k8s.io/descheduler/pkg/api" + "sigs.k8s.io/descheduler/pkg/descheduler/evictions" + podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod" + "sigs.k8s.io/descheduler/pkg/framework" + "sigs.k8s.io/descheduler/pkg/framework/pluginregistry" + "sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/client-go/informers" + clientset "k8s.io/client-go/kubernetes" + + "k8s.io/klog/v2" +) + +// evictorImpl implements the Evictor interface so plugins +// can evict a pod without importing a specific pod evictor +type evictorImpl struct { + podEvictor *evictions.PodEvictor + evictorFilter framework.EvictorPlugin +} + +var _ framework.Evictor = &evictorImpl{} + +// Filter checks if a pod can be evicted +func (ei *evictorImpl) Filter(pod *v1.Pod) bool { + return ei.evictorFilter.Filter(pod) +} + +// PreEvictionFilter checks if pod can be evicted right before eviction +func (ei *evictorImpl) PreEvictionFilter(pod *v1.Pod) bool { + return ei.evictorFilter.PreEvictionFilter(pod) +} + +// Evict evicts a pod (no pre-check performed) +func (ei *evictorImpl) Evict(ctx context.Context, pod *v1.Pod, opts evictions.EvictOptions) bool { + return ei.podEvictor.EvictPod(ctx, pod, opts) +} + +func (ei *evictorImpl) NodeLimitExceeded(node *v1.Node) bool { + return ei.podEvictor.NodeLimitExceeded(node) +} + +// handleImpl implements the framework handle which gets passed to plugins +type handleImpl struct { + clientSet clientset.Interface + getPodsAssignedToNodeFunc podutil.GetPodsAssignedToNodeFunc + sharedInformerFactory informers.SharedInformerFactory + evictor *evictorImpl +} + +var _ framework.Handle = &handleImpl{} + +// ClientSet retrieves kube client set +func (hi *handleImpl) ClientSet() clientset.Interface { + return hi.clientSet +} + +// GetPodsAssignedToNodeFunc retrieves GetPodsAssignedToNodeFunc implementation +func (hi *handleImpl) GetPodsAssignedToNodeFunc() podutil.GetPodsAssignedToNodeFunc { + return hi.getPodsAssignedToNodeFunc +} + +// SharedInformerFactory retrieves shared informer factory +func (hi *handleImpl) SharedInformerFactory() informers.SharedInformerFactory { + return hi.sharedInformerFactory +} + +// Evictor retrieves evictor so plugins can filter and evict pods +func (hi *handleImpl) Evictor() framework.Evictor { + return hi.evictor +} + +type profileImpl struct { + profileName string + podEvictor *evictions.PodEvictor + + deschedulePlugins []framework.DeschedulePlugin + balancePlugins []framework.BalancePlugin +} + +// Option for the handleImpl. +type Option func(*handleImplOpts) + +type handleImplOpts struct { + clientSet clientset.Interface + sharedInformerFactory informers.SharedInformerFactory + getPodsAssignedToNodeFunc podutil.GetPodsAssignedToNodeFunc + podEvictor *evictions.PodEvictor +} + +// WithClientSet sets clientSet for the scheduling frameworkImpl. +func WithClientSet(clientSet clientset.Interface) Option { + return func(o *handleImplOpts) { + o.clientSet = clientSet + } +} + +func WithSharedInformerFactory(sharedInformerFactory informers.SharedInformerFactory) Option { + return func(o *handleImplOpts) { + o.sharedInformerFactory = sharedInformerFactory + } +} + +func WithPodEvictor(podEvictor *evictions.PodEvictor) Option { + return func(o *handleImplOpts) { + o.podEvictor = podEvictor + } +} + +func WithGetPodsAssignedToNodeFnc(getPodsAssignedToNodeFunc podutil.GetPodsAssignedToNodeFunc) Option { + return func(o *handleImplOpts) { + o.getPodsAssignedToNodeFunc = getPodsAssignedToNodeFunc + } +} + +func getPluginConfig(pluginName string, pluginConfigs []api.PluginConfig) (*api.PluginConfig, int) { + for idx, pluginConfig := range pluginConfigs { + if pluginConfig.Name == pluginName { + return &pluginConfig, idx + } + } + return nil, 0 +} + +func buildPlugin(config api.DeschedulerProfile, pluginName string, handle *handleImpl, reg pluginregistry.Registry) (framework.Plugin, error) { + pc, _ := getPluginConfig(pluginName, config.PluginConfigs) + if pc == nil { + klog.ErrorS(fmt.Errorf("unable to get plugin config"), "skipping plugin", "plugin", pluginName, "profile", config.Name) + return nil, fmt.Errorf("unable to find %q plugin config", pluginName) + } + + registryPlugin, ok := reg[pluginName] + if !ok { + klog.ErrorS(fmt.Errorf("unable to find plugin in the pluginsMap"), "skipping plugin", "plugin", pluginName) + return nil, fmt.Errorf("unable to find %q plugin in the pluginsMap", pluginName) + } + pg, err := registryPlugin.PluginBuilder(pc.Args, handle) + if err != nil { + klog.ErrorS(err, "unable to initialize a plugin", "pluginName", pluginName) + return nil, fmt.Errorf("unable to initialize %q plugin: %v", pluginName, err) + } + return pg, nil +} + +func NewProfile(config api.DeschedulerProfile, reg pluginregistry.Registry, opts ...Option) (*profileImpl, error) { + hOpts := &handleImplOpts{} + for _, optFnc := range opts { + optFnc(hOpts) + } + + if hOpts.clientSet == nil { + return nil, fmt.Errorf("clientSet missing") + } + + if hOpts.sharedInformerFactory == nil { + return nil, fmt.Errorf("sharedInformerFactory missing") + } + + if hOpts.podEvictor == nil { + return nil, fmt.Errorf("podEvictor missing") + } + + evictorPlugin, err := buildPlugin(config, defaultevictor.PluginName, &handleImpl{ + clientSet: hOpts.clientSet, + getPodsAssignedToNodeFunc: hOpts.getPodsAssignedToNodeFunc, + sharedInformerFactory: hOpts.sharedInformerFactory, + }, reg) + if err != nil { + return nil, fmt.Errorf("unable to build %v plugin: %v", defaultevictor.PluginName, err) + } + if evictorPlugin == nil { + return nil, fmt.Errorf("empty plugin build for %v plugin: %v", defaultevictor.PluginName, err) + } + + handle := &handleImpl{ + clientSet: hOpts.clientSet, + getPodsAssignedToNodeFunc: hOpts.getPodsAssignedToNodeFunc, + sharedInformerFactory: hOpts.sharedInformerFactory, + evictor: &evictorImpl{ + podEvictor: hOpts.podEvictor, + evictorFilter: evictorPlugin.(framework.EvictorPlugin), + }, + } + + deschedulePlugins := []framework.DeschedulePlugin{} + balancePlugins := []framework.BalancePlugin{} + + descheduleEnabled := make(map[string]struct{}) + balanceEnabled := make(map[string]struct{}) + for _, name := range config.Plugins.Deschedule.Enabled { + descheduleEnabled[name] = struct{}{} + } + for _, name := range config.Plugins.Balance.Enabled { + balanceEnabled[name] = struct{}{} + } + + // Assuming only a list of enabled extension points. + // Later, when a default list of plugins and their extension points is established, + // compute the list of enabled extension points as (DefaultEnabled + Enabled - Disabled) + for _, plugin := range append(config.Plugins.Deschedule.Enabled, config.Plugins.Balance.Enabled...) { + pg, err := buildPlugin(config, plugin, handle, reg) + if err != nil { + return nil, fmt.Errorf("unable to build %v plugin: %v", plugin, err) + } + if pg != nil { + // pg can be of any of each type, or both + + if _, exists := descheduleEnabled[plugin]; exists { + _, ok := pg.(framework.DeschedulePlugin) + if ok { + deschedulePlugins = append(deschedulePlugins, pg.(framework.DeschedulePlugin)) + } + } + + if _, exists := balanceEnabled[plugin]; exists { + _, ok := pg.(framework.BalancePlugin) + if ok { + balancePlugins = append(balancePlugins, pg.(framework.BalancePlugin)) + } + } + } + } + + return &profileImpl{ + profileName: config.Name, + podEvictor: hOpts.podEvictor, + deschedulePlugins: deschedulePlugins, + balancePlugins: balancePlugins, + }, nil +} + +func (d profileImpl) RunDeschedulePlugins(ctx context.Context, nodes []*v1.Node) *framework.Status { + errs := []error{} + for _, pl := range d.deschedulePlugins { + evicted := d.podEvictor.TotalEvicted() + // TODO: strategyName should be accessible from within the strategy using a framework + // handle or function which the Evictor has access to. For migration/in-progress framework + // work, we are currently passing this via context. To be removed + // (See discussion thread https://github.com/kubernetes-sigs/descheduler/pull/885#discussion_r919962292) + strategyStart := time.Now() + childCtx := context.WithValue(ctx, "strategyName", pl.Name()) + status := pl.Deschedule(childCtx, nodes) + metrics.DeschedulerStrategyDuration.With(map[string]string{"strategy": pl.Name(), "profile": d.profileName}).Observe(time.Since(strategyStart).Seconds()) + + if status != nil && status.Err != nil { + errs = append(errs, fmt.Errorf("plugin %q finished with error: %v", pl.Name(), status.Err)) + } + klog.V(1).InfoS("Total number of pods evicted", "extension point", "Deschedule", "evictedPods", d.podEvictor.TotalEvicted()-evicted) + } + + aggrErr := errors.NewAggregate(errs) + if aggrErr == nil { + return &framework.Status{} + } + + return &framework.Status{ + Err: fmt.Errorf("%v", aggrErr.Error()), + } +} + +func (d profileImpl) RunBalancePlugins(ctx context.Context, nodes []*v1.Node) *framework.Status { + errs := []error{} + for _, pl := range d.balancePlugins { + evicted := d.podEvictor.TotalEvicted() + // TODO: strategyName should be accessible from within the strategy using a framework + // handle or function which the Evictor has access to. For migration/in-progress framework + // work, we are currently passing this via context. To be removed + // (See discussion thread https://github.com/kubernetes-sigs/descheduler/pull/885#discussion_r919962292) + strategyStart := time.Now() + childCtx := context.WithValue(ctx, "strategyName", pl.Name()) + status := pl.Balance(childCtx, nodes) + metrics.DeschedulerStrategyDuration.With(map[string]string{"strategy": pl.Name(), "profile": d.profileName}).Observe(time.Since(strategyStart).Seconds()) + + if status != nil && status.Err != nil { + errs = append(errs, fmt.Errorf("plugin %q finished with error: %v", pl.Name(), status.Err)) + } + klog.V(1).InfoS("Total number of pods evicted", "extension point", "Balance", "evictedPods", d.podEvictor.TotalEvicted()-evicted) + } + + aggrErr := errors.NewAggregate(errs) + if aggrErr == nil { + return &framework.Status{} + } + + return &framework.Status{ + Err: fmt.Errorf("%v", aggrErr.Error()), + } +} diff --git a/pkg/framework/profile/profile_test.go b/pkg/framework/profile/profile_test.go new file mode 100644 index 000000000..5b75cc74a --- /dev/null +++ b/pkg/framework/profile/profile_test.go @@ -0,0 +1,283 @@ +package profile + +import ( + "context" + "fmt" + "testing" + + v1 "k8s.io/api/core/v1" + policy "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/informers" + fakeclientset "k8s.io/client-go/kubernetes/fake" + core "k8s.io/client-go/testing" + + "sigs.k8s.io/descheduler/pkg/api" + "sigs.k8s.io/descheduler/pkg/descheduler/evictions" + podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod" + "sigs.k8s.io/descheduler/pkg/framework" + fakeplugin "sigs.k8s.io/descheduler/pkg/framework/fake/plugin" + "sigs.k8s.io/descheduler/pkg/framework/pluginregistry" + "sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor" + "sigs.k8s.io/descheduler/pkg/utils" + testutils "sigs.k8s.io/descheduler/test" +) + +func TestProfileTopExtensionPoints(t *testing.T) { + tests := []struct { + name string + config api.DeschedulerProfile + extensionPoint framework.ExtensionPoint + expectedEviction bool + }{ + { + name: "profile with deschedule extension point enabled single eviction", + config: api.DeschedulerProfile{ + Name: "strategy-test-profile-with-deschedule", + PluginConfigs: []api.PluginConfig{ + { + Name: defaultevictor.PluginName, + Args: &defaultevictor.DefaultEvictorArgs{ + PriorityThreshold: &api.PriorityThreshold{ + Value: nil, + }, + }, + }, + { + Name: "FakePlugin", + Args: &fakeplugin.FakePluginArgs{}, + }, + }, + Plugins: api.Plugins{ + Deschedule: api.PluginSet{ + Enabled: []string{"FakePlugin"}, + }, + Evict: api.PluginSet{ + Enabled: []string{defaultevictor.PluginName}, + }, + }, + }, + extensionPoint: framework.DescheduleExtensionPoint, + expectedEviction: true, + }, + { + name: "profile with balance extension point enabled single eviction", + config: api.DeschedulerProfile{ + Name: "strategy-test-profile-with-balance", + PluginConfigs: []api.PluginConfig{ + { + Name: defaultevictor.PluginName, + Args: &defaultevictor.DefaultEvictorArgs{ + PriorityThreshold: &api.PriorityThreshold{ + Value: nil, + }, + }, + }, + { + Name: "FakePlugin", + Args: &fakeplugin.FakePluginArgs{}, + }, + }, + Plugins: api.Plugins{ + Balance: api.PluginSet{ + Enabled: []string{"FakePlugin"}, + }, + Evict: api.PluginSet{ + Enabled: []string{defaultevictor.PluginName}, + }, + }, + }, + extensionPoint: framework.BalanceExtensionPoint, + expectedEviction: true, + }, + { + name: "profile with deschedule extension point balance enabled no eviction", + config: api.DeschedulerProfile{ + Name: "strategy-test-profile-with-deschedule", + PluginConfigs: []api.PluginConfig{ + { + Name: defaultevictor.PluginName, + Args: &defaultevictor.DefaultEvictorArgs{ + PriorityThreshold: &api.PriorityThreshold{ + Value: nil, + }, + }, + }, + { + Name: "FakePlugin", + Args: &fakeplugin.FakePluginArgs{}, + }, + }, + Plugins: api.Plugins{ + Balance: api.PluginSet{ + Enabled: []string{"FakePlugin"}, + }, + Evict: api.PluginSet{ + Enabled: []string{defaultevictor.PluginName}, + }, + }, + }, + extensionPoint: framework.DescheduleExtensionPoint, + expectedEviction: false, + }, + { + name: "profile with balance extension point deschedule enabled no eviction", + config: api.DeschedulerProfile{ + Name: "strategy-test-profile-with-deschedule", + PluginConfigs: []api.PluginConfig{ + { + Name: defaultevictor.PluginName, + Args: &defaultevictor.DefaultEvictorArgs{ + PriorityThreshold: &api.PriorityThreshold{ + Value: nil, + }, + }, + }, + { + Name: "FakePlugin", + Args: &fakeplugin.FakePluginArgs{}, + }, + }, + Plugins: api.Plugins{ + Deschedule: api.PluginSet{ + Enabled: []string{"FakePlugin"}, + }, + Evict: api.PluginSet{ + Enabled: []string{defaultevictor.PluginName}, + }, + }, + }, + extensionPoint: framework.BalanceExtensionPoint, + expectedEviction: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + n1 := testutils.BuildTestNode("n1", 2000, 3000, 10, nil) + n2 := testutils.BuildTestNode("n2", 2000, 3000, 10, nil) + nodes := []*v1.Node{n1, n2} + + p1 := testutils.BuildTestPod(fmt.Sprintf("pod_1_%s", n1.Name), 200, 0, n1.Name, nil) + p1.ObjectMeta.OwnerReferences = []metav1.OwnerReference{{}} + + fakePlugin := fakeplugin.FakePlugin{} + if test.extensionPoint == framework.DescheduleExtensionPoint { + fakePlugin.AddReactor(string(framework.DescheduleExtensionPoint), func(action fakeplugin.Action) (handled bool, err error) { + if dAction, ok := action.(fakeplugin.DescheduleAction); ok { + if dAction.Handle().Evictor().Evict(ctx, p1, evictions.EvictOptions{}) { + return true, nil + } + return true, fmt.Errorf("pod not evicted") + } + return false, nil + }) + } + if test.extensionPoint == framework.BalanceExtensionPoint { + fakePlugin.AddReactor(string(framework.BalanceExtensionPoint), func(action fakeplugin.Action) (handled bool, err error) { + if dAction, ok := action.(fakeplugin.BalanceAction); ok { + if dAction.Handle().Evictor().Evict(ctx, p1, evictions.EvictOptions{}) { + return true, nil + } + return true, fmt.Errorf("pod not evicted") + } + return false, nil + }) + } + + pluginregistry.PluginRegistry = pluginregistry.NewRegistry() + pluginregistry.Register( + "FakePlugin", + fakeplugin.NewPluginFncFromFake(&fakePlugin), + &fakeplugin.FakePluginArgs{}, + fakeplugin.ValidateFakePluginArgs, + fakeplugin.SetDefaults_FakePluginArgs, + pluginregistry.PluginRegistry, + ) + + pluginregistry.Register( + defaultevictor.PluginName, + defaultevictor.New, + &defaultevictor.DefaultEvictorArgs{}, + defaultevictor.ValidateDefaultEvictorArgs, + defaultevictor.SetDefaults_DefaultEvictorArgs, + pluginregistry.PluginRegistry, + ) + + client := fakeclientset.NewSimpleClientset(n1, n2, p1) + var evictedPods []string + client.PrependReactor("create", "pods", podEvictionReactionFuc(&evictedPods)) + + sharedInformerFactory := informers.NewSharedInformerFactory(client, 0) + podInformer := sharedInformerFactory.Core().V1().Pods().Informer() + getPodsAssignedToNode, err := podutil.BuildGetPodsAssignedToNodeFunc(podInformer) + if err != nil { + t.Fatalf("build get pods assigned to node function error: %v", err) + } + + sharedInformerFactory.Start(ctx.Done()) + sharedInformerFactory.WaitForCacheSync(ctx.Done()) + + eventClient := fakeclientset.NewSimpleClientset(n1, n2, p1) + eventBroadcaster, eventRecorder := utils.GetRecorderAndBroadcaster(ctx, eventClient) + defer eventBroadcaster.Shutdown() + + podEvictor := evictions.NewPodEvictor(client, "policy/v1", false, nil, nil, nodes, true, eventRecorder) + + prfl, err := NewProfile( + test.config, + pluginregistry.PluginRegistry, + WithClientSet(client), + WithSharedInformerFactory(sharedInformerFactory), + WithPodEvictor(podEvictor), + WithGetPodsAssignedToNodeFnc(getPodsAssignedToNode), + ) + if err != nil { + t.Fatalf("unable to create %q profile: %v", test.config.Name, err) + } + + var status *framework.Status + switch test.extensionPoint { + case framework.DescheduleExtensionPoint: + status = prfl.RunDeschedulePlugins(ctx, nodes) + case framework.BalanceExtensionPoint: + status = prfl.RunBalancePlugins(ctx, nodes) + default: + t.Fatalf("unknown %q extension point", test.extensionPoint) + } + + if status == nil { + t.Fatalf("Unexpected nil status") + } + if status.Err != nil { + t.Fatalf("Expected nil error in status, got %q instead", status.Err) + } + + if test.expectedEviction && len(evictedPods) < 1 { + t.Fatalf("Expected eviction, got none") + } + if !test.expectedEviction && len(evictedPods) > 0 { + t.Fatalf("Unexpected eviction, expected none") + } + }) + } +} + +func podEvictionReactionFuc(evictedPods *[]string) func(action core.Action) (bool, runtime.Object, error) { + return func(action core.Action) (bool, runtime.Object, error) { + if action.GetSubresource() == "eviction" { + createAct, matched := action.(core.CreateActionImpl) + if !matched { + return false, nil, fmt.Errorf("unable to convert action to core.CreateActionImpl") + } + if eviction, matched := createAct.Object.(*policy.Eviction); matched { + *evictedPods = append(*evictedPods, eviction.GetName()) + } + } + return false, nil, nil // fallback to the default reactor + } +} diff --git a/pkg/framework/types.go b/pkg/framework/types.go index b6f617dd1..46f0a5223 100644 --- a/pkg/framework/types.go +++ b/pkg/framework/types.go @@ -82,3 +82,12 @@ type EvictorPlugin interface { Filter(pod *v1.Pod) bool PreEvictionFilter(pod *v1.Pod) bool } + +type ExtensionPoint string + +const ( + DescheduleExtensionPoint ExtensionPoint = "Deschedule" + BalanceExtensionPoint ExtensionPoint = "Balance" + FilterExtensionPoint ExtensionPoint = "Filter" + PreEvictionFilterExtensionPoint ExtensionPoint = "PreEvictionFilter" +)