[nodeutilization]: allow to set a metrics source as a string so it can be later extended for exclusive configuration

pkg/api/types.go

@@ -48,7 +48,11 @@ type DeschedulerPolicy struct {
 	EvictionFailureEventNotification *bool
 
 	// MetricsCollector configures collection of metrics about actual resource utilization
-	MetricsCollector MetricsCollector
+	// Deprecated. Use MetricsProviders field instead.
+	MetricsCollector *MetricsCollector
+
+	// MetricsProviders configure collection of metrics about actual resource utilization from various sources
+	MetricsProviders []MetricsProvider
 
 	// GracePeriodSeconds The duration in seconds before the object should be deleted. Value must be non-negative integer.
 	// The value zero indicates delete immediately. If this value is nil, the default grace period for the
@@ -105,12 +109,26 @@ type PluginSet struct {
 	Disabled []string
 }
 
+type MetricsSource string
+
+const (
+	// KubernetesMetrics enables metrics from a Kubernetes metrics server.
+	// Please see https://kubernetes-sigs.github.io/metrics-server/ for more.
+	KubernetesMetrics MetricsSource = "KubernetesMetrics"
+)
+
 // MetricsCollector configures collection of metrics about actual resource utilization
 type MetricsCollector struct {
-	// Enabled metrics collection from kubernetes metrics.
-	// Later, the collection can be extended to other providers.
+	// Enabled metrics collection from Kubernetes metrics.
+	// Deprecated. Use MetricsProvider.Source field instead.
 	Enabled bool
 }
 
+// MetricsProvider configures collection of metrics about actual resource utilization from a given source
+type MetricsProvider struct {
+	// Source enables metrics from Kubernetes metrics server.
+	Source MetricsSource
+}
+
 // ReferencedResourceList is an adaption of v1.ResourceList with resources as references
 type ReferencedResourceList = map[v1.ResourceName]*resource.Quantity

pkg/api/v1alpha2/types.go

@@ -46,7 +46,11 @@ type DeschedulerPolicy struct {
 	EvictionFailureEventNotification *bool `json:"evictionFailureEventNotification,omitempty"`
 
 	// MetricsCollector configures collection of metrics for actual resource utilization
-	MetricsCollector MetricsCollector `json:"metricsCollector,omitempty"`
+	// Deprecated. Use MetricsProviders field instead.
+	MetricsCollector *MetricsCollector `json:"metricsCollector,omitempty"`
+
+	// MetricsProviders configure collection of metrics about actual resource utilization from various sources
+	MetricsProviders []MetricsProvider `json:"metricsProviders,omitempty"`
 
 	// GracePeriodSeconds The duration in seconds before the object should be deleted. Value must be non-negative integer.
 	// The value zero indicates delete immediately. If this value is nil, the default grace period for the
@@ -80,9 +84,23 @@ type PluginSet struct {
 	Disabled []string `json:"disabled"`
 }
 
+type MetricsSource string
+
+const (
+	// KubernetesMetrics enables metrics from a Kubernetes metrics server.
+	// Please see https://kubernetes-sigs.github.io/metrics-server/ for more.
+	KubernetesMetrics MetricsSource = "KubernetesMetrics"
+)
+
 // MetricsCollector configures collection of metrics about actual resource utilization
 type MetricsCollector struct {
-	// Enabled metrics collection from kubernetes metrics.
-	// Later, the collection can be extended to other providers.
+	// Enabled metrics collection from Kubernetes metrics server.
+	// Deprecated. Use MetricsProvider.Source field instead.
 	Enabled bool `json:"enabled,omitempty"`
 }
+
+// MetricsProvider configures collection of metrics about actual resource utilization from a given source
+type MetricsProvider struct {
+	// Source enables metrics from Kubernetes metrics server.
+	Source MetricsSource `json:"source,omitempty"`
+}

pkg/api/v1alpha2/zz_generated.conversion.go

@@ -56,6 +56,16 @@ func RegisterConversions(s *runtime.Scheme) error {
 	}); err != nil {
 		return err
 	}
+	if err := s.AddGeneratedConversionFunc((*MetricsProvider)(nil), (*api.MetricsProvider)(nil), func(a, b interface{}, scope conversion.Scope) error {
+		return Convert_v1alpha2_MetricsProvider_To_api_MetricsProvider(a.(*MetricsProvider), b.(*api.MetricsProvider), scope)
+	}); err != nil {
+		return err
+	}
+	if err := s.AddGeneratedConversionFunc((*api.MetricsProvider)(nil), (*MetricsProvider)(nil), func(a, b interface{}, scope conversion.Scope) error {
+		return Convert_api_MetricsProvider_To_v1alpha2_MetricsProvider(a.(*api.MetricsProvider), b.(*MetricsProvider), 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 {
@@ -116,9 +126,8 @@ func autoConvert_v1alpha2_DeschedulerPolicy_To_api_DeschedulerPolicy(in *Desched
 	out.MaxNoOfPodsToEvictPerNamespace = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictPerNamespace))
 	out.MaxNoOfPodsToEvictTotal = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictTotal))
 	out.EvictionFailureEventNotification = (*bool)(unsafe.Pointer(in.EvictionFailureEventNotification))
-	if err := Convert_v1alpha2_MetricsCollector_To_api_MetricsCollector(&in.MetricsCollector, &out.MetricsCollector, s); err != nil {
-		return err
-	}
+	out.MetricsCollector = (*api.MetricsCollector)(unsafe.Pointer(in.MetricsCollector))
+	out.MetricsProviders = *(*[]api.MetricsProvider)(unsafe.Pointer(&in.MetricsProviders))
 	out.GracePeriodSeconds = (*int64)(unsafe.Pointer(in.GracePeriodSeconds))
 	return nil
 }
@@ -140,9 +149,8 @@ func autoConvert_api_DeschedulerPolicy_To_v1alpha2_DeschedulerPolicy(in *api.Des
 	out.MaxNoOfPodsToEvictPerNamespace = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictPerNamespace))
 	out.MaxNoOfPodsToEvictTotal = (*uint)(unsafe.Pointer(in.MaxNoOfPodsToEvictTotal))
 	out.EvictionFailureEventNotification = (*bool)(unsafe.Pointer(in.EvictionFailureEventNotification))
-	if err := Convert_api_MetricsCollector_To_v1alpha2_MetricsCollector(&in.MetricsCollector, &out.MetricsCollector, s); err != nil {
-		return err
-	}
+	out.MetricsCollector = (*MetricsCollector)(unsafe.Pointer(in.MetricsCollector))
+	out.MetricsProviders = *(*[]MetricsProvider)(unsafe.Pointer(&in.MetricsProviders))
 	out.GracePeriodSeconds = (*int64)(unsafe.Pointer(in.GracePeriodSeconds))
 	return nil
 }
@@ -215,6 +223,26 @@ func Convert_api_MetricsCollector_To_v1alpha2_MetricsCollector(in *api.MetricsCo
 	return autoConvert_api_MetricsCollector_To_v1alpha2_MetricsCollector(in, out, s)
 }
 
+func autoConvert_v1alpha2_MetricsProvider_To_api_MetricsProvider(in *MetricsProvider, out *api.MetricsProvider, s conversion.Scope) error {
+	out.Source = api.MetricsSource(in.Source)
+	return nil
+}
+
+// Convert_v1alpha2_MetricsProvider_To_api_MetricsProvider is an autogenerated conversion function.
+func Convert_v1alpha2_MetricsProvider_To_api_MetricsProvider(in *MetricsProvider, out *api.MetricsProvider, s conversion.Scope) error {
+	return autoConvert_v1alpha2_MetricsProvider_To_api_MetricsProvider(in, out, s)
+}
+
+func autoConvert_api_MetricsProvider_To_v1alpha2_MetricsProvider(in *api.MetricsProvider, out *MetricsProvider, s conversion.Scope) error {
+	out.Source = MetricsSource(in.Source)
+	return nil
+}
+
+// Convert_api_MetricsProvider_To_v1alpha2_MetricsProvider is an autogenerated conversion function.
+func Convert_api_MetricsProvider_To_v1alpha2_MetricsProvider(in *api.MetricsProvider, out *MetricsProvider, s conversion.Scope) error {
+	return autoConvert_api_MetricsProvider_To_v1alpha2_MetricsProvider(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 {

pkg/api/v1alpha2/zz_generated.deepcopy.go

@@ -61,7 +61,16 @@ func (in *DeschedulerPolicy) DeepCopyInto(out *DeschedulerPolicy) {
 		*out = new(bool)
 		**out = **in
 	}
-	out.MetricsCollector = in.MetricsCollector
+	if in.MetricsCollector != nil {
+		in, out := &in.MetricsCollector, &out.MetricsCollector
+		*out = new(MetricsCollector)
+		**out = **in
+	}
+	if in.MetricsProviders != nil {
+		in, out := &in.MetricsProviders, &out.MetricsProviders
+		*out = make([]MetricsProvider, len(*in))
+		copy(*out, *in)
+	}
 	if in.GracePeriodSeconds != nil {
 		in, out := &in.GracePeriodSeconds, &out.GracePeriodSeconds
 		*out = new(int64)
@@ -128,6 +137,22 @@ func (in *MetricsCollector) DeepCopy() *MetricsCollector {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *MetricsProvider) DeepCopyInto(out *MetricsProvider) {
+	*out = *in
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MetricsProvider.
+func (in *MetricsProvider) DeepCopy() *MetricsProvider {
+	if in == nil {
+		return nil
+	}
+	out := new(MetricsProvider)
+	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

pkg/api/zz_generated.deepcopy.go

@@ -61,7 +61,16 @@ func (in *DeschedulerPolicy) DeepCopyInto(out *DeschedulerPolicy) {
 		*out = new(bool)
 		**out = **in
 	}
-	out.MetricsCollector = in.MetricsCollector
+	if in.MetricsCollector != nil {
+		in, out := &in.MetricsCollector, &out.MetricsCollector
+		*out = new(MetricsCollector)
+		**out = **in
+	}
+	if in.MetricsProviders != nil {
+		in, out := &in.MetricsProviders, &out.MetricsProviders
+		*out = make([]MetricsProvider, len(*in))
+		copy(*out, *in)
+	}
 	if in.GracePeriodSeconds != nil {
 		in, out := &in.GracePeriodSeconds, &out.GracePeriodSeconds
 		*out = new(int64)
@@ -149,6 +158,22 @@ func (in *MetricsCollector) DeepCopy() *MetricsCollector {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *MetricsProvider) DeepCopyInto(out *MetricsProvider) {
+	*out = *in
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MetricsProvider.
+func (in *MetricsProvider) DeepCopy() *MetricsProvider {
+	if in == nil {
+		return nil
+	}
+	out := new(MetricsProvider)
+	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

pkg/descheduler/descheduler.go

@@ -166,7 +166,7 @@ func newDescheduler(ctx context.Context, rs *options.DeschedulerServer, deschedu
 	}
 
 	var metricsCollector *metricscollector.MetricsCollector
-	if deschedulerPolicy.MetricsCollector.Enabled {
+	if (deschedulerPolicy.MetricsCollector != nil && deschedulerPolicy.MetricsCollector.Enabled) || (len(deschedulerPolicy.MetricsProviders) > 0 && deschedulerPolicy.MetricsProviders[0].Source == api.KubernetesMetrics) {
 		nodeSelector := labels.Everything()
 		if deschedulerPolicy.NodeSelector != nil {
 			sel, err := labels.Parse(*deschedulerPolicy.NodeSelector)
@@ -332,7 +332,7 @@ func Run(ctx context.Context, rs *options.DeschedulerServer) error {
 		return err
 	}
 
-	if deschedulerPolicy.MetricsCollector.Enabled {
+	if (deschedulerPolicy.MetricsCollector != nil && deschedulerPolicy.MetricsCollector.Enabled) || (len(deschedulerPolicy.MetricsProviders) > 0 && deschedulerPolicy.MetricsProviders[0].Source == api.KubernetesMetrics) {
 		metricsClient, err := client.CreateMetricsClient(clientConnection, "descheduler")
 		if err != nil {
 			return err
@@ -448,7 +448,7 @@ func RunDeschedulerStrategies(ctx context.Context, rs *options.DeschedulerServer
 	sharedInformerFactory.WaitForCacheSync(ctx.Done())
 	descheduler.podEvictor.WaitForEventHandlersSync(ctx)
 
-	if deschedulerPolicy.MetricsCollector.Enabled {
+	if (deschedulerPolicy.MetricsCollector != nil && deschedulerPolicy.MetricsCollector.Enabled) || (len(deschedulerPolicy.MetricsProviders) > 0 && deschedulerPolicy.MetricsProviders[0].Source == api.KubernetesMetrics) {
 		go func() {
 			klog.V(2).Infof("Starting metrics collector")
 			descheduler.metricsCollector.Run(ctx)

pkg/descheduler/descheduler_test.go

@@ -136,6 +136,10 @@ func removeDuplicatesPolicy() *api.DeschedulerPolicy {
 }
 
 func lowNodeUtilizationPolicy(thresholds, targetThresholds api.ResourceThresholds, metricsEnabled bool) *api.DeschedulerPolicy {
+	var metricsSource api.MetricsSource = ""
+	if metricsEnabled {
+		metricsSource = api.KubernetesMetrics
+	}
 	return &api.DeschedulerPolicy{
 		Profiles: []api.DeschedulerProfile{
 			{
@@ -146,8 +150,8 @@ func lowNodeUtilizationPolicy(thresholds, targetThresholds api.ResourceThreshold
 						Args: &nodeutilization.LowNodeUtilizationArgs{
 							Thresholds:       thresholds,
 							TargetThresholds: targetThresholds,
-							MetricsUtilization: nodeutilization.MetricsUtilization{
-								MetricsServer: metricsEnabled,
+							MetricsUtilization: &nodeutilization.MetricsUtilization{
+								Source: metricsSource,
 							},
 						},
 					},
@@ -837,7 +841,7 @@ func TestLoadAwareDescheduling(t *testing.T) {
 		},
 		true, // enabled metrics utilization
 	)
-	policy.MetricsCollector.Enabled = true
+	policy.MetricsProviders = []api.MetricsProvider{{Source: api.KubernetesMetrics}}
 
 	ctxCancel, cancel := context.WithCancel(ctx)
 	_, descheduler, _ := initDescheduler(

pkg/descheduler/policyconfig.go

@@ -137,11 +137,11 @@ func setDefaultEvictor(profile api.DeschedulerProfile, client clientset.Interfac
 }
 
 func validateDeschedulerConfiguration(in api.DeschedulerPolicy, registry pluginregistry.Registry) error {
-	var errorsInProfiles []error
+	var errorsInPolicy []error
 	for _, profile := range in.Profiles {
 		for _, pluginConfig := range profile.PluginConfigs {
 			if _, ok := registry[pluginConfig.Name]; !ok {
-				errorsInProfiles = append(errorsInProfiles, fmt.Errorf("in profile %s: plugin %s in pluginConfig not registered", profile.Name, pluginConfig.Name))
+				errorsInPolicy = append(errorsInPolicy, fmt.Errorf("in profile %s: plugin %s in pluginConfig not registered", profile.Name, pluginConfig.Name))
 				continue
 			}
 
@@ -150,9 +150,15 @@ func validateDeschedulerConfiguration(in api.DeschedulerPolicy, registry pluginr
 				continue
 			}
 			if err := pluginUtilities.PluginArgValidator(pluginConfig.Args); err != nil {
-				errorsInProfiles = append(errorsInProfiles, fmt.Errorf("in profile %s: %s", profile.Name, err.Error()))
+				errorsInPolicy = append(errorsInPolicy, fmt.Errorf("in profile %s: %s", profile.Name, err.Error()))
 			}
 		}
 	}
-	return utilerrors.NewAggregate(errorsInProfiles)
+	if len(in.MetricsProviders) > 1 {
+		errorsInPolicy = append(errorsInPolicy, fmt.Errorf("only a single metrics provider can be set, got %v instead", len(in.MetricsProviders)))
+	}
+	if len(in.MetricsProviders) > 0 && in.MetricsCollector != nil && in.MetricsCollector.Enabled {
+		errorsInPolicy = append(errorsInPolicy, fmt.Errorf("it is not allowed to combine metrics provider when metrics collector is enabled"))
+	}
+	return utilerrors.NewAggregate(errorsInPolicy)
 }

pkg/descheduler/policyconfig_test.go

@@ -211,6 +211,28 @@ func TestValidateDeschedulerConfiguration(t *testing.T) {
 			},
 			result: fmt.Errorf("[in profile RemoveFailedPods: only one of Include/Exclude namespaces can be set, in profile RemovePodsViolatingTopologySpreadConstraint: only one of Include/Exclude namespaces can be set]"),
 		},
+		{
+			description: "Too many metrics providers error",
+			deschedulerPolicy: api.DeschedulerPolicy{
+				MetricsProviders: []api.MetricsProvider{
+					{Source: api.KubernetesMetrics},
+					{Source: api.KubernetesMetrics},
+				},
+			},
+			result: fmt.Errorf("only a single metrics provider can be set, got 2 instead"),
+		},
+		{
+			description: "Too many metrics providers error",
+			deschedulerPolicy: api.DeschedulerPolicy{
+				MetricsCollector: &api.MetricsCollector{
+					Enabled: true,
+				},
+				MetricsProviders: []api.MetricsProvider{
+					{Source: api.KubernetesMetrics},
+				},
+			},
+			result: fmt.Errorf("it is not allowed to combine metrics provider when metrics collector is enabled"),
+		},
 	}
 
 	for _, tc := range testCases {

pkg/framework/plugins/nodeutilization/lownodeutilization.go

@@ -89,7 +89,8 @@ func NewLowNodeUtilization(args runtime.Object, handle frameworktypes.Handle) (f
 	resourceNames := getResourceNames(lowNodeUtilizationArgsArgs.Thresholds)
 
 	var usageClient usageClient
-	if lowNodeUtilizationArgsArgs.MetricsUtilization.MetricsServer {
+	// MetricsServer is deprecated, removed once dropped
+	if lowNodeUtilizationArgsArgs.MetricsUtilization != nil && (lowNodeUtilizationArgsArgs.MetricsUtilization.MetricsServer || lowNodeUtilizationArgsArgs.MetricsUtilization.Source == api.KubernetesMetrics) {
 		if handle.MetricsCollector() == nil {
 			return nil, fmt.Errorf("metrics client not initialized")
 		}

pkg/framework/plugins/nodeutilization/lownodeutilization_test.go

@@ -1257,14 +1257,19 @@ func TestLowNodeUtilization(t *testing.T) {
 				}
 				handle.MetricsCollectorImpl = collector
 
+				var metricsSource api.MetricsSource = ""
+				if metricsEnabled {
+					metricsSource = api.KubernetesMetrics
+				}
+
 				plugin, err := NewLowNodeUtilization(&LowNodeUtilizationArgs{
 					Thresholds:             tc.thresholds,
 					TargetThresholds:       tc.targetThresholds,
 					UseDeviationThresholds: tc.useDeviationThresholds,
 					EvictionLimits:         tc.evictionLimits,
 					EvictableNamespaces:    tc.evictableNamespaces,
-					MetricsUtilization: MetricsUtilization{
-						MetricsServer: metricsEnabled,
+					MetricsUtilization: &MetricsUtilization{
+						Source: metricsSource,
 					},
 				},
 					handle)

pkg/framework/plugins/nodeutilization/types.go

@@ -28,7 +28,7 @@ type LowNodeUtilizationArgs struct {
 	Thresholds             api.ResourceThresholds `json:"thresholds"`
 	TargetThresholds       api.ResourceThresholds `json:"targetThresholds"`
 	NumberOfNodes          int                    `json:"numberOfNodes,omitempty"`
-	MetricsUtilization     MetricsUtilization     `json:"metricsUtilization,omitempty"`
+	MetricsUtilization     *MetricsUtilization    `json:"metricsUtilization,omitempty"`
 
 	// Naming this one differently since namespaces are still
 	// considered while considering resources used by pods
@@ -45,9 +45,8 @@ type LowNodeUtilizationArgs struct {
 type HighNodeUtilizationArgs struct {
 	metav1.TypeMeta `json:",inline"`
 
-	Thresholds         api.ResourceThresholds `json:"thresholds"`
-	NumberOfNodes      int                    `json:"numberOfNodes,omitempty"`
-	MetricsUtilization MetricsUtilization     `json:"metricsUtilization,omitempty"`
+	Thresholds    api.ResourceThresholds `json:"thresholds"`
+	NumberOfNodes int                    `json:"numberOfNodes,omitempty"`
 
 	// Naming this one differently since namespaces are still
 	// considered while considering resources used by pods
@@ -59,5 +58,10 @@ type HighNodeUtilizationArgs struct {
 type MetricsUtilization struct {
 	// metricsServer enables metrics from a kubernetes metrics server.
 	// Please see https://kubernetes-sigs.github.io/metrics-server/ for more.
+	// Deprecated. Use MetricsSource instead.
 	MetricsServer bool `json:"metricsServer,omitempty"`
+
+	// source enables the plugin to consume metrics from a metrics source.
+	// Currently only KubernetesMetrics available.
+	Source api.MetricsSource `json:"source,omitempty"`
 }

pkg/framework/plugins/nodeutilization/zz_generated.deepcopy.go

@@ -37,7 +37,6 @@ func (in *HighNodeUtilizationArgs) DeepCopyInto(out *HighNodeUtilizationArgs) {
 			(*out)[key] = val
 		}
 	}
-	out.MetricsUtilization = in.MetricsUtilization
 	if in.EvictableNamespaces != nil {
 		in, out := &in.EvictableNamespaces, &out.EvictableNamespaces
 		*out = new(api.Namespaces)
@@ -82,7 +81,11 @@ func (in *LowNodeUtilizationArgs) DeepCopyInto(out *LowNodeUtilizationArgs) {
 			(*out)[key] = val
 		}
 	}
-	out.MetricsUtilization = in.MetricsUtilization
+	if in.MetricsUtilization != nil {
+		in, out := &in.MetricsUtilization, &out.MetricsUtilization
+		*out = new(MetricsUtilization)
+		**out = **in
+	}
 	if in.EvictableNamespaces != nil {
 		in, out := &in.EvictableNamespaces, &out.EvictableNamespaces
 		*out = new(api.Namespaces)