From 27fa7a70a12dd45cc2af23bc89f4a516a255c26a Mon Sep 17 00:00:00 2001
From: Amir Alavi <amir.alavi@zendesk.com>
Date: Tue, 9 Aug 2022 10:29:46 -0400
Subject: [PATCH] NodeAffinity plugin to use the existing validation methods

---
 pkg/apis/componentconfig/types_pluginargs.go  |  2 +-
 .../validation/validation_pluginargs.go       | 27 ++++++------
 .../validation/validation_pluginargs_test.go  | 41 +++++++++++++++++++
 .../plugins/removefailedpods/failedpods.go    |  2 +-
 4 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/pkg/apis/componentconfig/types_pluginargs.go b/pkg/apis/componentconfig/types_pluginargs.go
index 3e1dd4437..f9bd66d14 100644
--- a/pkg/apis/componentconfig/types_pluginargs.go
+++ b/pkg/apis/componentconfig/types_pluginargs.go
@@ -35,7 +35,7 @@ type RemovePodsViolatingNodeTaintsArgs struct {
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
-// RemoveFailedPodsArgs holds arguments used to configure RemoveFailedPodsArgs plugin.
+// RemoveFailedPodsArgs holds arguments used to configure RemoveFailedPods plugin.
 type RemoveFailedPodsArgs struct {
 	metav1.TypeMeta
 
diff --git a/pkg/apis/componentconfig/validation/validation_pluginargs.go b/pkg/apis/componentconfig/validation/validation_pluginargs.go
index a9f9f4e0a..edd77c232 100644
--- a/pkg/apis/componentconfig/validation/validation_pluginargs.go
+++ b/pkg/apis/componentconfig/validation/validation_pluginargs.go
@@ -50,6 +50,20 @@ func ValidateRemovePodsHavingTooManyRestartsArgs(args *componentconfig.RemovePod
 	)
 }
 
+// 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),
+	)
+}
+
 // errorsAggregate converts all arg validation errors to a single error interface.
 // if no errors, it will return nil.
 func errorsAggregate(errors ...error) error {
@@ -75,19 +89,6 @@ func validateLabelSelectorArgs(labelSelector *metav1.LabelSelector) error {
 	return nil
 }
 
-// ValidateRemovePodsViolatingNodeAffinityArgs validates RemovePodsViolatingNodeAffinity arguments
-func ValidateRemovePodsViolatingNodeAffinityArgs(args *componentconfig.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")
-	}
-	return nil
-}
-
 func validatePodRestartThreshold(podRestartThreshold int32) error {
 	if podRestartThreshold < 1 {
 		return fmt.Errorf("PodsHavingTooManyRestarts threshold not set")
diff --git a/pkg/apis/componentconfig/validation/validation_pluginargs_test.go b/pkg/apis/componentconfig/validation/validation_pluginargs_test.go
index 4e8f2fecc..71931d58f 100644
--- a/pkg/apis/componentconfig/validation/validation_pluginargs_test.go
+++ b/pkg/apis/componentconfig/validation/validation_pluginargs_test.go
@@ -83,3 +83,44 @@ 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")
+			}
+		})
+	}
+}
diff --git a/pkg/framework/plugins/removefailedpods/failedpods.go b/pkg/framework/plugins/removefailedpods/failedpods.go
index ae7cdf648..57e2b3866 100644
--- a/pkg/framework/plugins/removefailedpods/failedpods.go
+++ b/pkg/framework/plugins/removefailedpods/failedpods.go
@@ -35,7 +35,7 @@ import (
 
 const PluginName = "RemoveFailedPods"
 
-// RemoveFailedPods evicts pods on the node which violate NoSchedule Taints on nodes
+// RemoveFailedPods evicts pods in failed status phase that match the given args criteria
 type RemoveFailedPods struct {
 	handle    framework.Handle
 	args      *componentconfig.RemoveFailedPodsArgs