PR feedback to refactor validateDeschedulerConfiguration error handling

kubernetes-sigs · May 11, 2023 · 12b0c5c · 12b0c5c
1 parent ba196a5
commit 12b0c5c
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 18 deletions.
diff --git a/pkg/descheduler/policyconfig.go b/pkg/descheduler/policyconfig.go
@@ -21,6 +21,8 @@ import (
 	"fmt"
 	"os"
 
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
@@ -133,27 +135,22 @@ func setDefaultEvictor(profile api.DeschedulerProfile, client clientset.Interfac
 }
 
 func validateDeschedulerConfiguration(in api.DeschedulerPolicy, registry pluginregistry.Registry) error {
-	var errorsInProfiles error
+	var errorsInProfiles []error
 	for _, profile := range in.Profiles {
 		for _, pluginConfig := range profile.PluginConfigs {
-			if _, ok := registry[pluginConfig.Name]; ok {
-				pluginUtilities := registry[pluginConfig.Name]
-				if pluginUtilities.PluginArgValidator == nil {
-					continue
-				}
-				err := pluginUtilities.PluginArgValidator(pluginConfig.Args)
-				if err != nil {
-					if errorsInProfiles == nil {
-						errorsInProfiles = fmt.Errorf("in profile %s: %s", profile.Name, err.Error())
-					} else {
-						errorsInProfiles = fmt.Errorf("%w: %s", errorsInProfiles, fmt.Sprintf("in profile %s: %s", profile.Name, err.Error()))
-					}
-				}
-			} else {
-				errorsInProfiles = fmt.Errorf("%w: %s", errorsInProfiles, fmt.Sprintf("in profile %s: plugin %s in pluginConfig not registered", profile.Name, pluginConfig.Name))
+			if _, ok := registry[pluginConfig.Name]; !ok {
+				errorsInProfiles = append(errorsInProfiles, fmt.Errorf("in profile %s: plugin %s in pluginConfig not registered", profile.Name, pluginConfig.Name))
 				continue
 			}
+
+			pluginUtilities := registry[pluginConfig.Name]
+			if pluginUtilities.PluginArgValidator == nil {
+				continue
+			}
+			if err := pluginUtilities.PluginArgValidator(pluginConfig.Args); err != nil {
+				errorsInProfiles = append(errorsInProfiles, fmt.Errorf("in profile %s: %s", profile.Name, err.Error()))
+			}
 		}
 	}
-	return errorsInProfiles
+	return utilerrors.NewAggregate(errorsInProfiles)
 }
diff --git a/pkg/descheduler/policyconfig_test.go b/pkg/descheduler/policyconfig_test.go
@@ -985,7 +985,7 @@ 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"),
+			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]"),
 		},
 	}