NodeUtilization defaulting + moving arguments to its corresponding plugin

pkg/framework/plugins/nodeutilization/highnodeutilization.go

@@ -27,7 +27,6 @@ import (
 	"sigs.k8s.io/descheduler/pkg/api"
 	nodeutil "sigs.k8s.io/descheduler/pkg/descheduler/node"
 
-	"sigs.k8s.io/descheduler/pkg/apis/componentconfig"
 	podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
 	"sigs.k8s.io/descheduler/pkg/framework"
 )
@@ -39,7 +38,7 @@ const HighNodeUtilizationPluginName = "HighNodeUtilization"
 
 type HighNodeUtilization struct {
 	handle    framework.Handle
-	args      *componentconfig.HighNodeUtilizationArgs
+	args      *HighNodeUtilizationArgs
 	podFilter func(pod *v1.Pod) bool
 }
 
@@ -47,7 +46,7 @@ var _ framework.BalancePlugin = &HighNodeUtilization{}
 
 // NewHighNodeUtilization builds plugin from its arguments while passing a handle
 func NewHighNodeUtilization(args runtime.Object, handle framework.Handle) (framework.Plugin, error) {
-	highNodeUtilizatioArgs, ok := args.(*componentconfig.HighNodeUtilizationArgs)
+	highNodeUtilizatioArgs, ok := args.(*HighNodeUtilizationArgs)
 	if !ok {
 		return nil, fmt.Errorf("want args to be of type HighNodeUtilizationArgs, got %T", args)
 	}

pkg/framework/plugins/nodeutilization/highnodeutilization_test.go

@@ -31,7 +31,6 @@ import (
 	"k8s.io/client-go/tools/events"
 
 	"sigs.k8s.io/descheduler/pkg/api"
-	"sigs.k8s.io/descheduler/pkg/apis/componentconfig"
 	"sigs.k8s.io/descheduler/pkg/descheduler/evictions"
 	podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
 	"sigs.k8s.io/descheduler/pkg/framework"
@@ -524,7 +523,7 @@ func TestHighNodeUtilization(t *testing.T) {
 				SharedInformerFactoryImpl:     sharedInformerFactory,
 			}
 
-			plugin, err := NewHighNodeUtilization(&componentconfig.HighNodeUtilizationArgs{
+			plugin, err := NewHighNodeUtilization(&HighNodeUtilizationArgs{
 				Thresholds: testCase.thresholds,
 			},
 				handle)
@@ -676,7 +675,7 @@ func TestHighNodeUtilizationWithTaints(t *testing.T) {
 				SharedInformerFactoryImpl:     sharedInformerFactory,
 			}
 
-			plugin, err := NewHighNodeUtilization(&componentconfig.HighNodeUtilizationArgs{
+			plugin, err := NewHighNodeUtilization(&HighNodeUtilizationArgs{
 				Thresholds: api.ResourceThresholds{
 					v1.ResourceCPU: 40,
 				},

pkg/framework/plugins/nodeutilization/lownodeutilization.go

@@ -24,7 +24,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog/v2"
-	"sigs.k8s.io/descheduler/pkg/apis/componentconfig"
 	nodeutil "sigs.k8s.io/descheduler/pkg/descheduler/node"
 	podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
 	"sigs.k8s.io/descheduler/pkg/framework"
@@ -37,7 +36,7 @@ const LowNodeUtilizationPluginName = "LowNodeUtilization"
 
 type LowNodeUtilization struct {
 	handle    framework.Handle
-	args      *componentconfig.LowNodeUtilizationArgs
+	args      *LowNodeUtilizationArgs
 	podFilter func(pod *v1.Pod) bool
 }
 
@@ -45,7 +44,7 @@ var _ framework.BalancePlugin = &LowNodeUtilization{}
 
 // NewLowNodeUtilization builds plugin from its arguments while passing a handle
 func NewLowNodeUtilization(args runtime.Object, handle framework.Handle) (framework.Plugin, error) {
-	lowNodeUtilizationArgsArgs, ok := args.(*componentconfig.LowNodeUtilizationArgs)
+	lowNodeUtilizationArgsArgs, ok := args.(*LowNodeUtilizationArgs)
 	if !ok {
 		return nil, fmt.Errorf("want args to be of type LowNodeUtilizationArgs, got %T", args)
 	}

pkg/framework/plugins/nodeutilization/lownodeutilization_test.go

@@ -22,7 +22,6 @@ import (
 	"testing"
 
 	"sigs.k8s.io/descheduler/pkg/api"
-	"sigs.k8s.io/descheduler/pkg/apis/componentconfig"
 	"sigs.k8s.io/descheduler/pkg/framework"
 	frameworkfake "sigs.k8s.io/descheduler/pkg/framework/fake"
 	"sigs.k8s.io/descheduler/pkg/framework/plugins/defaultevictor"
@@ -790,7 +789,7 @@ func TestLowNodeUtilization(t *testing.T) {
 				SharedInformerFactoryImpl:     sharedInformerFactory,
 			}
 
-			plugin, err := NewLowNodeUtilization(&componentconfig.LowNodeUtilizationArgs{
+			plugin, err := NewLowNodeUtilization(&LowNodeUtilizationArgs{
 
 				Thresholds:             test.thresholds,
 				TargetThresholds:       test.targetThresholds,
@@ -963,7 +962,7 @@ func TestLowNodeUtilizationWithTaints(t *testing.T) {
 				SharedInformerFactoryImpl:     sharedInformerFactory,
 			}
 
-			plugin, err := NewLowNodeUtilization(&componentconfig.LowNodeUtilizationArgs{
+			plugin, err := NewLowNodeUtilization(&LowNodeUtilizationArgs{
 
 				Thresholds: api.ResourceThresholds{
 					v1.ResourcePods: 20,

pkg/framework/plugins/nodeutilization/types.go

@@ -0,0 +1,41 @@
+/*
+Copyright 2022 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 nodeutilization
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/descheduler/pkg/api"
+)
+
+// +k8s:deepcopy-gen=true
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+type LowNodeUtilizationArgs struct {
+	metav1.TypeMeta
+
+	UseDeviationThresholds bool
+	Thresholds             api.ResourceThresholds
+	TargetThresholds       api.ResourceThresholds
+	NumberOfNodes          int
+}
+
+// +k8s:deepcopy-gen=true
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+type HighNodeUtilizationArgs struct {
+	metav1.TypeMeta
+
+	Thresholds    api.ResourceThresholds
+	NumberOfNodes int
+}

pkg/framework/plugins/nodeutilization/validation.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2022 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 nodeutilization
+
+import (
+	"fmt"
+	"sigs.k8s.io/descheduler/pkg/api"
+)
+
+func ValidateHighNodeUtilizationArgs(args *HighNodeUtilizationArgs) error {
+	return validateThresholds(args.Thresholds)
+}
+
+func ValidateLowNodeUtilizationArgs(args *LowNodeUtilizationArgs) error {
+	return validateLowNodeUtilizationThresholds(args.Thresholds, args.TargetThresholds, args.UseDeviationThresholds)
+}
+
+func validateLowNodeUtilizationThresholds(thresholds, targetThresholds api.ResourceThresholds, useDeviationThresholds bool) error {
+	// validate thresholds and targetThresholds config
+	if err := validateThresholds(thresholds); err != nil {
+		return fmt.Errorf("thresholds config is not valid: %v", err)
+	}
+	if err := validateThresholds(targetThresholds); err != nil {
+		return fmt.Errorf("targetThresholds config is not valid: %v", err)
+	}
+
+	// validate if thresholds and targetThresholds have same resources configured
+	if len(thresholds) != len(targetThresholds) {
+		return fmt.Errorf("thresholds and targetThresholds configured different resources")
+	}
+	for resourceName, value := range thresholds {
+		if targetValue, ok := targetThresholds[resourceName]; !ok {
+			return fmt.Errorf("thresholds and targetThresholds configured different resources")
+		} else if value > targetValue && !useDeviationThresholds {
+			return fmt.Errorf("thresholds' %v percentage is greater than targetThresholds'", resourceName)
+		}
+	}
+	return nil
+}
+
+// validateThresholds checks if thresholds have valid resource name and resource percentage configured
+func validateThresholds(thresholds api.ResourceThresholds) error {
+	if len(thresholds) == 0 {
+		return fmt.Errorf("no resource threshold is configured")
+	}
+	for name, percent := range thresholds {
+		if percent < MinResourcePercentage || percent > MaxResourcePercentage {
+			return fmt.Errorf("%v threshold not in [%v, %v] range", name, MinResourcePercentage, MaxResourcePercentage)
+		}
+	}
+	return nil
+}

pkg/framework/plugins/nodeutilization/validation_test.go

@@ -0,0 +1,184 @@
+/*
+Copyright 2022 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 nodeutilization
+
+import (
+	"fmt"
+	v1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/descheduler/pkg/api"
+	"testing"
+)
+
+func TestValidateLowNodeUtilizationPluginConfig(t *testing.T) {
+	var extendedResource = v1.ResourceName("example.com/foo")
+	tests := []struct {
+		name             string
+		thresholds       api.ResourceThresholds
+		targetThresholds api.ResourceThresholds
+		errInfo          error
+	}{
+		{
+			name: "passing invalid thresholds",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 120,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+			},
+			errInfo: fmt.Errorf("thresholds config is not valid: %v", fmt.Errorf(
+				"%v threshold not in [%v, %v] range", v1.ResourceMemory, MinResourcePercentage, MaxResourcePercentage)),
+		},
+		{
+			name: "thresholds and targetThresholds configured different num of resources",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 20,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+				v1.ResourcePods:   80,
+			},
+			errInfo: fmt.Errorf("thresholds and targetThresholds configured different resources"),
+		},
+		{
+			name: "thresholds and targetThresholds configured different resources",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 20,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:  80,
+				v1.ResourcePods: 80,
+			},
+			errInfo: fmt.Errorf("thresholds and targetThresholds configured different resources"),
+		},
+		{
+			name: "thresholds' CPU config value is greater than targetThresholds'",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    90,
+				v1.ResourceMemory: 20,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+			},
+			errInfo: fmt.Errorf("thresholds' %v percentage is greater than targetThresholds'", v1.ResourceCPU),
+		},
+		{
+			name: "only thresholds configured extended resource",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 20,
+				extendedResource:  20,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+			},
+			errInfo: fmt.Errorf("thresholds and targetThresholds configured different resources"),
+		},
+		{
+			name: "only targetThresholds configured extended resource",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 20,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+				extendedResource:  80,
+			},
+			errInfo: fmt.Errorf("thresholds and targetThresholds configured different resources"),
+		},
+		{
+			name: "thresholds and targetThresholds configured different extended resources",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 20,
+				extendedResource:  20,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+				"example.com/bar": 80,
+			},
+			errInfo: fmt.Errorf("thresholds and targetThresholds configured different resources"),
+		},
+		{
+			name: "thresholds' extended resource config value is greater than targetThresholds'",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 20,
+				extendedResource:  90,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+				extendedResource:  20,
+			},
+			errInfo: fmt.Errorf("thresholds' %v percentage is greater than targetThresholds'", extendedResource),
+		},
+		{
+			name: "passing valid plugin config",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 20,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+			},
+			errInfo: nil,
+		},
+		{
+			name: "passing valid plugin config with extended resource",
+			thresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    20,
+				v1.ResourceMemory: 20,
+				extendedResource:  20,
+			},
+			targetThresholds: api.ResourceThresholds{
+				v1.ResourceCPU:    80,
+				v1.ResourceMemory: 80,
+				extendedResource:  80,
+			},
+			errInfo: nil,
+		},
+	}
+
+	for _, testCase := range tests {
+		args := &LowNodeUtilizationArgs{
+
+			Thresholds:       testCase.thresholds,
+			TargetThresholds: testCase.targetThresholds,
+		}
+		validateErr := validateLowNodeUtilizationThresholds(args.Thresholds, args.TargetThresholds, false)
+
+		if validateErr == nil || testCase.errInfo == nil {
+			if validateErr != testCase.errInfo {
+				t.Errorf("expected validity of plugin config: thresholds %#v targetThresholds %#v to be %v but got %v instead",
+					testCase.thresholds, testCase.targetThresholds, testCase.errInfo, validateErr)
+			}
+		} else if validateErr.Error() != testCase.errInfo.Error() {
+			t.Errorf("expected validity of plugin config: thresholds %#v targetThresholds %#v to be %v but got %v instead",
+				testCase.thresholds, testCase.targetThresholds, testCase.errInfo, validateErr)
+		}
+	}
+}

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

@@ -0,0 +1,98 @@
+//go:build !ignore_autogenerated
+// +build !ignore_autogenerated
+
+/*
+Copyright 2022 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 nodeutilization
+
+import (
+	runtime "k8s.io/apimachinery/pkg/runtime"
+	api "sigs.k8s.io/descheduler/pkg/api"
+)
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *HighNodeUtilizationArgs) DeepCopyInto(out *HighNodeUtilizationArgs) {
+	*out = *in
+	out.TypeMeta = in.TypeMeta
+	if in.Thresholds != nil {
+		in, out := &in.Thresholds, &out.Thresholds
+		*out = make(api.ResourceThresholds, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HighNodeUtilizationArgs.
+func (in *HighNodeUtilizationArgs) DeepCopy() *HighNodeUtilizationArgs {
+	if in == nil {
+		return nil
+	}
+	out := new(HighNodeUtilizationArgs)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
+func (in *HighNodeUtilizationArgs) DeepCopyObject() runtime.Object {
+	if c := in.DeepCopy(); c != nil {
+		return c
+	}
+	return nil
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *LowNodeUtilizationArgs) DeepCopyInto(out *LowNodeUtilizationArgs) {
+	*out = *in
+	out.TypeMeta = in.TypeMeta
+	if in.Thresholds != nil {
+		in, out := &in.Thresholds, &out.Thresholds
+		*out = make(api.ResourceThresholds, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+	if in.TargetThresholds != nil {
+		in, out := &in.TargetThresholds, &out.TargetThresholds
+		*out = make(api.ResourceThresholds, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LowNodeUtilizationArgs.
+func (in *LowNodeUtilizationArgs) DeepCopy() *LowNodeUtilizationArgs {
+	if in == nil {
+		return nil
+	}
+	out := new(LowNodeUtilizationArgs)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
+func (in *LowNodeUtilizationArgs) DeepCopyObject() runtime.Object {
+	if c := in.DeepCopy(); c != nil {
+		return c
+	}
+	return nil
+}