Merge pull request #103480 from chendave/pluginconfig_issue

Readable error message on the plugin configs of the removed plugins

pkg/scheduler/apis/config/validation/validation.go

@@ -202,31 +202,6 @@ func validatePluginConfig(path *field.Path, apiVersion string, profile *config.K
 		"VolumeBinding":                   ValidateVolumeBindingArgs,
 	}
 
-	seenPluginConfig := make(sets.String)
-	for i := range profile.PluginConfig {
-		pluginConfigPath := path.Child("pluginConfig").Index(i)
-		name := profile.PluginConfig[i].Name
-		args := profile.PluginConfig[i].Args
-		if seenPluginConfig.Has(name) {
-			errs = append(errs, field.Duplicate(pluginConfigPath, name))
-		} else {
-			seenPluginConfig.Insert(name)
-		}
-		if validateFunc, ok := m[name]; ok {
-			// type mismatch, no need to validate the `args`.
-			if reflect.TypeOf(args) != reflect.ValueOf(validateFunc).Type().In(1) {
-				errs = append(errs, field.Invalid(pluginConfigPath.Child("args"), args, "has to match plugin args"))
-				return errs
-			}
-			in := []reflect.Value{reflect.ValueOf(pluginConfigPath.Child("args")), reflect.ValueOf(args)}
-			res := reflect.ValueOf(validateFunc).Call(in)
-			// It's possible that validation function return a Aggregate, just append here and it will be flattened at the end of CC validation.
-			if res[0].Interface() != nil {
-				errs = append(errs, res[0].Interface().(error))
-			}
-		}
-	}
-
 	if profile.Plugins != nil {
 		stagesToPluginSet := map[string]config.PluginSet{
 			"queueSort":  profile.Plugins.QueueSort,
@@ -250,6 +225,34 @@ func validatePluginConfig(path *field.Path, apiVersion string, profile *config.K
 		errs = append(errs, validateScorePluginSetForConflictPlugins(
 			pluginsPath.Child("score"), apiVersion, profile)...)
 	}
+
+	seenPluginConfig := make(sets.String)
+
+	for i := range profile.PluginConfig {
+		pluginConfigPath := path.Child("pluginConfig").Index(i)
+		name := profile.PluginConfig[i].Name
+		args := profile.PluginConfig[i].Args
+		if seenPluginConfig.Has(name) {
+			errs = append(errs, field.Duplicate(pluginConfigPath, name))
+		} else {
+			seenPluginConfig.Insert(name)
+		}
+		if removed, removedVersion := isPluginRemoved(apiVersion, name); removed {
+			errs = append(errs, field.Invalid(pluginConfigPath, name, fmt.Sprintf("was removed in version %q (KubeSchedulerConfiguration is version %q)", removedVersion, apiVersion)))
+		} else if validateFunc, ok := m[name]; ok {
+			// type mismatch, no need to validate the `args`.
+			if reflect.TypeOf(args) != reflect.ValueOf(validateFunc).Type().In(1) {
+				errs = append(errs, field.Invalid(pluginConfigPath.Child("args"), args, "has to match plugin args"))
+			} else {
+				in := []reflect.Value{reflect.ValueOf(pluginConfigPath.Child("args")), reflect.ValueOf(args)}
+				res := reflect.ValueOf(validateFunc).Call(in)
+				// It's possible that validation function return a Aggregate, just append here and it will be flattened at the end of CC validation.
+				if res[0].Interface() != nil {
+					errs = append(errs, res[0].Interface().(error))
+				}
+			}
+		}
+	}
 	return errs
 }
 

pkg/scheduler/apis/config/validation/validation_test.go

@@ -254,6 +254,12 @@ func TestValidateKubeSchedulerConfiguration(t *testing.T) {
 	goodConflictPlugins2.Profiles[0].Plugins.Score.Enabled = append(goodConflictPlugins2.Profiles[0].Plugins.Score.Enabled, config.Plugin{Name: "NodeResourcesMostAllocated", Weight: 2})
 	goodConflictPlugins2.Profiles[0].Plugins.Score.Enabled = append(goodConflictPlugins2.Profiles[0].Plugins.Score.Enabled, config.Plugin{Name: "RequestedToCapacityRatio", Weight: 2})
 
+	deprecatedPluginsConfig := validConfig.DeepCopy()
+	deprecatedPluginsConfig.Profiles[0].PluginConfig = append(deprecatedPluginsConfig.Profiles[0].PluginConfig, config.PluginConfig{
+		Name: "NodeResourcesLeastAllocated",
+		Args: &config.NodeResourcesLeastAllocatedArgs{},
+	})
+
 	scenarios := map[string]struct {
 		expectedToFail bool
 		config         *config.KubeSchedulerConfiguration
@@ -384,6 +390,11 @@ func TestValidateKubeSchedulerConfiguration(t *testing.T) {
 			expectedToFail: false,
 			config:         goodConflictPlugins2,
 		},
+		"bad-plugins-config": {
+			expectedToFail: true,
+			config:         deprecatedPluginsConfig,
+			errorString:    "profiles[0].pluginConfig[1]: Invalid value: \"NodeResourcesLeastAllocated\": was removed in version \"kubescheduler.config.k8s.io/v1beta2\" (KubeSchedulerConfiguration is version \"kubescheduler.config.k8s.io/v1beta2\")",
+		},
 	}
 
 	for name, scenario := range scenarios {