RemovePodsViolatingNodeAffinity defaulting + moving arguments to its corresponding plugin

pkg/apis/componentconfig/types_pluginargs.go

@@ -35,17 +35,6 @@ type RemovePodsViolatingNodeTaintsArgs struct {
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
-// RemovePodsViolatingNodeAffinityArgs holds arguments used to configure RemovePodsViolatingNodeAffinity plugin.
-type RemovePodsViolatingNodeAffinityArgs struct {
-	metav1.TypeMeta
-
-	Namespaces       *api.Namespaces
-	LabelSelector    *metav1.LabelSelector
-	NodeAffinityType []string
-}
-
-// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
-
 // RemovePodsViolatingTopologySpreadConstraintArgs holds arguments used to configure RemovePodsViolatingTopologySpreadConstraint plugin.
 type RemovePodsViolatingTopologySpreadConstraintArgs struct {
 	metav1.TypeMeta

pkg/apis/componentconfig/validation/validation_pluginargs.go

@@ -40,20 +40,6 @@ func ValidateRemovePodsViolatingNodeTaintsArgs(args *componentconfig.RemovePodsV
 	)
 }
 
-// ValidateRemovePodsViolatingNodeAffinityArgs validates RemovePodsViolatingNodeAffinity arguments
-func ValidateRemovePodsViolatingNodeAffinityArgs(args *componentconfig.RemovePodsViolatingNodeAffinityArgs) error {
-	var err error
-	if args == nil || len(args.NodeAffinityType) == 0 {
-		err = fmt.Errorf("nodeAffinityType needs to be set")
-	}
-
-	return errorsAggregate(
-		err,
-		validateNamespaceArgs(args.Namespaces),
-		validateLabelSelectorArgs(args.LabelSelector),
-	)
-}
-
 // ValidateRemovePodsViolatingTopologySpreadConstraintArgs validates RemovePodsViolatingTopologySpreadConstraint arguments
 func ValidateRemovePodsViolatingTopologySpreadConstraintArgs(args *componentconfig.RemovePodsViolatingTopologySpreadConstraintArgs) error {
 	return errorsAggregate(

pkg/apis/componentconfig/validation/validation_pluginargs_test.go

@@ -84,44 +84,3 @@ func TestValidateRemovePodsViolatingNodeTaintsArgs(t *testing.T) {
 		})
 	}
 }
-
-func TestValidateRemovePodsViolatingNodeAffinityArgs(t *testing.T) {
-	testCases := []struct {
-		description string
-		args        *componentconfig.RemovePodsViolatingNodeAffinityArgs
-		expectError bool
-	}{
-		{
-			description: "nil NodeAffinityType args, expects errors",
-			args: &componentconfig.RemovePodsViolatingNodeAffinityArgs{
-				NodeAffinityType: nil,
-			},
-			expectError: true,
-		},
-		{
-			description: "empty NodeAffinityType args, expects errors",
-			args: &componentconfig.RemovePodsViolatingNodeAffinityArgs{
-				NodeAffinityType: []string{},
-			},
-			expectError: true,
-		},
-		{
-			description: "valid NodeAffinityType args, no errors",
-			args: &componentconfig.RemovePodsViolatingNodeAffinityArgs{
-				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
-			},
-			expectError: false,
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.description, func(t *testing.T) {
-			err := ValidateRemovePodsViolatingNodeAffinityArgs(tc.args)
-
-			hasError := err != nil
-			if tc.expectError != hasError {
-				t.Error("unexpected arg validation behavior")
-			}
-		})
-	}
-}

pkg/apis/componentconfig/zz_generated.deepcopy.go

@@ -54,46 +54,6 @@ func (in *DeschedulerConfiguration) DeepCopyObject() runtime.Object {
 	return nil
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *RemovePodsViolatingNodeAffinityArgs) DeepCopyInto(out *RemovePodsViolatingNodeAffinityArgs) {
-	*out = *in
-	out.TypeMeta = in.TypeMeta
-	if in.Namespaces != nil {
-		in, out := &in.Namespaces, &out.Namespaces
-		*out = new(api.Namespaces)
-		(*in).DeepCopyInto(*out)
-	}
-	if in.LabelSelector != nil {
-		in, out := &in.LabelSelector, &out.LabelSelector
-		*out = new(v1.LabelSelector)
-		(*in).DeepCopyInto(*out)
-	}
-	if in.NodeAffinityType != nil {
-		in, out := &in.NodeAffinityType, &out.NodeAffinityType
-		*out = make([]string, len(*in))
-		copy(*out, *in)
-	}
-	return
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RemovePodsViolatingNodeAffinityArgs.
-func (in *RemovePodsViolatingNodeAffinityArgs) DeepCopy() *RemovePodsViolatingNodeAffinityArgs {
-	if in == nil {
-		return nil
-	}
-	out := new(RemovePodsViolatingNodeAffinityArgs)
-	in.DeepCopyInto(out)
-	return out
-}
-
-// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
-func (in *RemovePodsViolatingNodeAffinityArgs) 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 *RemovePodsViolatingNodeTaintsArgs) DeepCopyInto(out *RemovePodsViolatingNodeTaintsArgs) {
 	*out = *in

pkg/descheduler/strategy_migration.go

@@ -90,12 +90,12 @@ var pluginsMap = map[string]func(ctx context.Context, nodes []*v1.Node, params *
 		}
 	},
 	"RemovePodsViolatingNodeAffinity": func(ctx context.Context, nodes []*v1.Node, params *api.StrategyParameters, handle *handleImpl) {
-		args := &componentconfig.RemovePodsViolatingNodeAffinityArgs{
+		args := &removepodsviolatingnodeaffinity.RemovePodsViolatingNodeAffinityArgs{
 			Namespaces:       params.Namespaces,
 			LabelSelector:    params.LabelSelector,
 			NodeAffinityType: params.NodeAffinityType,
 		}
-		if err := validation.ValidateRemovePodsViolatingNodeAffinityArgs(args); err != nil {
+		if err := removepodsviolatingnodeaffinity.ValidateRemovePodsViolatingNodeAffinityArgs(args); err != nil {
 			klog.V(1).ErrorS(err, "unable to validate plugin arguments", "pluginName", removepodsviolatingnodeaffinity.PluginName)
 			return
 		}

pkg/framework/plugins/removepodsviolatingnodeaffinity/node_affinity.go

@@ -23,7 +23,6 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/klog/v2"
-	"sigs.k8s.io/descheduler/pkg/apis/componentconfig"
 	"sigs.k8s.io/descheduler/pkg/descheduler/evictions"
 	nodeutil "sigs.k8s.io/descheduler/pkg/descheduler/node"
 	podutil "sigs.k8s.io/descheduler/pkg/descheduler/pod"
@@ -35,7 +34,7 @@ const PluginName = "RemovePodsViolatingNodeAffinity"
 // RemovePodsViolatingNodeAffinity evicts pods on the node which violate node affinity
 type RemovePodsViolatingNodeAffinity struct {
 	handle    framework.Handle
-	args      *componentconfig.RemovePodsViolatingNodeAffinityArgs
+	args      *RemovePodsViolatingNodeAffinityArgs
 	podFilter podutil.FilterFunc
 }
 
@@ -43,7 +42,7 @@ var _ framework.DeschedulePlugin = &RemovePodsViolatingNodeAffinity{}
 
 // New builds plugin from its arguments while passing a handle
 func New(args runtime.Object, handle framework.Handle) (framework.Plugin, error) {
-	nodeAffinityArgs, ok := args.(*componentconfig.RemovePodsViolatingNodeAffinityArgs)
+	nodeAffinityArgs, ok := args.(*RemovePodsViolatingNodeAffinityArgs)
 	if !ok {
 		return nil, fmt.Errorf("want args to be of type RemovePodsViolatingNodeAffinityArgs, got %T", args)
 	}

pkg/framework/plugins/removepodsviolatingnodeaffinity/node_affinity_test.go

@@ -27,7 +27,6 @@ import (
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/events"
-	"sigs.k8s.io/descheduler/pkg/apis/componentconfig"
 	"sigs.k8s.io/descheduler/pkg/framework"
 
 	"sigs.k8s.io/descheduler/pkg/descheduler/evictions"
@@ -96,12 +95,12 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		expectedEvictedPodCount        uint
 		maxPodsToEvictPerNode          *uint
 		maxNoOfPodsToEvictPerNamespace *uint
-		args                           componentconfig.RemovePodsViolatingNodeAffinityArgs
+		args                           RemovePodsViolatingNodeAffinityArgs
 		nodefit                        bool
 	}{
 		{
 			description: "Invalid Affinity type, should not evict any pods",
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingRequiredDuringExecution"},
 			},
 			expectedEvictedPodCount: 0,
@@ -110,7 +109,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		},
 		{
 			description: "Pod is correctly scheduled on node, no eviction expected",
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
 			},
 			expectedEvictedPodCount: 0,
@@ -120,7 +119,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		{
 			description:             "Pod is scheduled on node without matching labels, another schedulable node available, should be evicted",
 			expectedEvictedPodCount: 1,
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
 			},
 			pods:  addPodsToNode(nodeWithoutLabels, nil),
@@ -129,7 +128,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		{
 			description:             "Pod is scheduled on node without matching labels, another schedulable node available, maxPodsToEvictPerNode set to 1, should not be evicted",
 			expectedEvictedPodCount: 1,
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
 			},
 			pods:                  addPodsToNode(nodeWithoutLabels, nil),
@@ -139,7 +138,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		{
 			description:             "Pod is scheduled on node without matching labels, another schedulable node available, maxPodsToEvictPerNode set to 1, no pod evicted since pod terminting",
 			expectedEvictedPodCount: 1,
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
 			},
 			pods:                  addPodsToNode(nodeWithoutLabels, &metav1.Time{}),
@@ -149,7 +148,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		{
 			description:             "Pod is scheduled on node without matching labels, another schedulable node available, maxNoOfPodsToEvictPerNamespace set to 1, should not be evicted",
 			expectedEvictedPodCount: 1,
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
 			},
 			pods:                           addPodsToNode(nodeWithoutLabels, nil),
@@ -159,7 +158,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		{
 			description:             "Pod is scheduled on node without matching labels, another schedulable node available, maxNoOfPodsToEvictPerNamespace set to 1, no pod evicted since pod terminting",
 			expectedEvictedPodCount: 1,
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
 			},
 			pods:                           addPodsToNode(nodeWithoutLabels, &metav1.Time{}),
@@ -169,7 +168,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		{
 			description:             "Pod is scheduled on node without matching labels, but no node where pod fits is available, should not evict",
 			expectedEvictedPodCount: 0,
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
 			},
 			pods:    addPodsToNode(nodeWithoutLabels, nil),
@@ -179,7 +178,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 		{
 			description:             "Pod is scheduled on node without matching labels, and node where pod fits is available, should evict",
 			expectedEvictedPodCount: 0,
-			args: componentconfig.RemovePodsViolatingNodeAffinityArgs{
+			args: RemovePodsViolatingNodeAffinityArgs{
 				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
 			},
 			pods:                  addPodsToNode(nodeWithoutLabels, nil),
@@ -257,7 +256,7 @@ func TestRemovePodsViolatingNodeAffinity(t *testing.T) {
 			}
 
 			plugin, err := New(
-				&componentconfig.RemovePodsViolatingNodeAffinityArgs{
+				&RemovePodsViolatingNodeAffinityArgs{
 					NodeAffinityType: tc.args.NodeAffinityType,
 				},
 				handle,

pkg/framework/plugins/removepodsviolatingnodeaffinity/types.go

@@ -0,0 +1,34 @@
+/*
+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 removepodsviolatingnodeaffinity
+
+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
+
+// RemovePodsViolatingNodeAffinityArgs holds arguments used to configure RemovePodsViolatingNodeAffinity plugin.
+type RemovePodsViolatingNodeAffinityArgs struct {
+	metav1.TypeMeta
+
+	Namespaces       *api.Namespaces
+	LabelSelector    *metav1.LabelSelector
+	NodeAffinityType []string
+}

pkg/framework/plugins/removepodsviolatingnodeaffinity/validation.go

@@ -0,0 +1,42 @@
+/*
+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 removepodsviolatingnodeaffinity
+
+import (
+	"fmt"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// ValidateRemovePodsViolatingNodeAffinityArgs validates RemovePodsViolatingNodeAffinity arguments
+func ValidateRemovePodsViolatingNodeAffinityArgs(args *RemovePodsViolatingNodeAffinityArgs) error {
+	if args == nil || len(args.NodeAffinityType) == 0 {
+		return fmt.Errorf("nodeAffinityType needs to be set")
+	}
+
+	// At most one of include/exclude can be set
+	if args.Namespaces != nil && len(args.Namespaces.Include) > 0 && len(args.Namespaces.Exclude) > 0 {
+		return fmt.Errorf("only one of Include/Exclude namespaces can be set")
+	}
+
+	if args.LabelSelector != nil {
+		if _, err := metav1.LabelSelectorAsSelector(args.LabelSelector); err != nil {
+			return fmt.Errorf("failed to get label selectors from strategy's params: %+v", err)
+		}
+	}
+
+	return nil
+}

pkg/framework/plugins/removepodsviolatingnodeaffinity/validation_test.go

@@ -0,0 +1,60 @@
+/*
+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 removepodsviolatingnodeaffinity
+
+import "testing"
+
+func TestValidateRemovePodsViolatingNodeAffinityArgs(t *testing.T) {
+	testCases := []struct {
+		description string
+		args        *RemovePodsViolatingNodeAffinityArgs
+		expectError bool
+	}{
+		{
+			description: "nil NodeAffinityType args, expects errors",
+			args: &RemovePodsViolatingNodeAffinityArgs{
+				NodeAffinityType: nil,
+			},
+			expectError: true,
+		},
+		{
+			description: "empty NodeAffinityType args, expects errors",
+			args: &RemovePodsViolatingNodeAffinityArgs{
+				NodeAffinityType: []string{},
+			},
+			expectError: true,
+		},
+		{
+			description: "valid NodeAffinityType args, no errors",
+			args: &RemovePodsViolatingNodeAffinityArgs{
+				NodeAffinityType: []string{"requiredDuringSchedulingIgnoredDuringExecution"},
+			},
+			expectError: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			err := ValidateRemovePodsViolatingNodeAffinityArgs(tc.args)
+
+			hasError := err != nil
+			if tc.expectError != hasError {
+				t.Error("unexpected arg validation behavior")
+			}
+		})
+	}
+}

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

@@ -0,0 +1,68 @@
+//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 removepodsviolatingnodeaffinity
+
+import (
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	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 *RemovePodsViolatingNodeAffinityArgs) DeepCopyInto(out *RemovePodsViolatingNodeAffinityArgs) {
+	*out = *in
+	out.TypeMeta = in.TypeMeta
+	if in.Namespaces != nil {
+		in, out := &in.Namespaces, &out.Namespaces
+		*out = new(api.Namespaces)
+		(*in).DeepCopyInto(*out)
+	}
+	if in.LabelSelector != nil {
+		in, out := &in.LabelSelector, &out.LabelSelector
+		*out = new(v1.LabelSelector)
+		(*in).DeepCopyInto(*out)
+	}
+	if in.NodeAffinityType != nil {
+		in, out := &in.NodeAffinityType, &out.NodeAffinityType
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RemovePodsViolatingNodeAffinityArgs.
+func (in *RemovePodsViolatingNodeAffinityArgs) DeepCopy() *RemovePodsViolatingNodeAffinityArgs {
+	if in == nil {
+		return nil
+	}
+	out := new(RemovePodsViolatingNodeAffinityArgs)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
+func (in *RemovePodsViolatingNodeAffinityArgs) DeepCopyObject() runtime.Object {
+	if c := in.DeepCopy(); c != nil {
+		return c
+	}
+	return nil
+}