Merge pull request #111378 from kerthcet/revert/remove-plugin-logic

Revert "Cleanup: remove validation of removedPlugins"

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

@@ -33,6 +33,8 @@ import (
 	componentbasevalidation "k8s.io/component-base/config/validation"
 	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
 	"k8s.io/kubernetes/pkg/scheduler/apis/config"
+	"k8s.io/kubernetes/pkg/scheduler/apis/config/v1beta2"
+	"k8s.io/kubernetes/pkg/scheduler/apis/config/v1beta3"
 )
 
 // ValidateKubeSchedulerConfiguration ensures validation of the KubeSchedulerConfiguration struct
@@ -115,6 +117,51 @@ func splitHostIntPort(s string) (string, int, error) {
 	return host, portInt, err
 }
 
+type invalidPlugins struct {
+	schemeGroupVersion string
+	plugins            []string
+}
+
+// invalidPluginsByVersion maintains a list of removed/deprecated plugins in each version.
+// Remember to add an entry to that list when creating a new component config
+// version (even if the list of invalid plugins is empty).
+var invalidPluginsByVersion = []invalidPlugins{
+	{
+		schemeGroupVersion: v1beta2.SchemeGroupVersion.String(),
+		plugins:            []string{},
+	},
+	{
+		schemeGroupVersion: v1beta3.SchemeGroupVersion.String(),
+		plugins:            []string{},
+	},
+}
+
+// isPluginInvalid checks if a given plugin was removed/deprecated in the given component
+// config version or earlier.
+func isPluginInvalid(apiVersion string, name string) (bool, string) {
+	for _, dp := range invalidPluginsByVersion {
+		for _, plugin := range dp.plugins {
+			if name == plugin {
+				return true, dp.schemeGroupVersion
+			}
+		}
+		if apiVersion == dp.schemeGroupVersion {
+			break
+		}
+	}
+	return false, ""
+}
+
+func validatePluginSetForInvalidPlugins(path *field.Path, apiVersion string, ps config.PluginSet) []error {
+	var errs []error
+	for i, plugin := range ps.Enabled {
+		if invalid, invalidVersion := isPluginInvalid(apiVersion, plugin.Name); invalid {
+			errs = append(errs, field.Invalid(path.Child("enabled").Index(i), plugin.Name, fmt.Sprintf("was invalid in version %q (KubeSchedulerConfiguration is version %q)", invalidVersion, apiVersion)))
+		}
+	}
+	return errs
+}
+
 func validateKubeSchedulerProfile(path *field.Path, apiVersion string, profile *config.KubeSchedulerProfile) []error {
 	var errs []error
 	if len(profile.SchedulerName) == 0 {
@@ -136,6 +183,28 @@ func validatePluginConfig(path *field.Path, apiVersion string, profile *config.K
 		"VolumeBinding":                   ValidateVolumeBindingArgs,
 	}
 
+	if profile.Plugins != nil {
+		stagesToPluginSet := map[string]config.PluginSet{
+			"queueSort":  profile.Plugins.QueueSort,
+			"preFilter":  profile.Plugins.PreFilter,
+			"filter":     profile.Plugins.Filter,
+			"postFilter": profile.Plugins.PostFilter,
+			"preScore":   profile.Plugins.PreScore,
+			"score":      profile.Plugins.Score,
+			"reserve":    profile.Plugins.Reserve,
+			"permit":     profile.Plugins.Permit,
+			"preBind":    profile.Plugins.PreBind,
+			"bind":       profile.Plugins.Bind,
+			"postBind":   profile.Plugins.PostBind,
+		}
+
+		pluginsPath := path.Child("plugins")
+		for s, p := range stagesToPluginSet {
+			errs = append(errs, validatePluginSetForInvalidPlugins(
+				pluginsPath.Child(s), apiVersion, p)...)
+		}
+	}
+
 	seenPluginConfig := make(sets.String)
 
 	for i := range profile.PluginConfig {
@@ -147,7 +216,9 @@ func validatePluginConfig(path *field.Path, apiVersion string, profile *config.K
 		} else {
 			seenPluginConfig.Insert(name)
 		}
-		if validateFunc, ok := m[name]; ok {
+		if invalid, invalidVersion := isPluginInvalid(apiVersion, name); invalid {
+			errs = append(errs, field.Invalid(pluginConfigPath, name, fmt.Sprintf("was invalid in version %q (KubeSchedulerConfiguration is version %q)", invalidVersion, 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"))

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

@@ -198,6 +198,9 @@ func TestValidateKubeSchedulerConfigurationV1beta2(t *testing.T) {
 		BindVerb:       "bar",
 	})
 
+	goodInvalidPlugins := validConfig.DeepCopy()
+	goodInvalidPlugins.Profiles[0].Plugins.Score.Enabled = append(goodInvalidPlugins.Profiles[0].Plugins.Score.Enabled, config.Plugin{Name: "PodTopologySpread", Weight: 2})
+
 	scenarios := map[string]struct {
 		expectedToFail bool
 		config         *config.KubeSchedulerConfiguration
@@ -275,6 +278,10 @@ func TestValidateKubeSchedulerConfigurationV1beta2(t *testing.T) {
 			expectedToFail: true,
 			config:         mismatchQueueSort,
 		},
+		"good-invalid-plugins": {
+			expectedToFail: false,
+			config:         goodInvalidPlugins,
+		},
 	}
 
 	for name, scenario := range scenarios {
@@ -465,6 +472,9 @@ func TestValidateKubeSchedulerConfigurationV1beta3(t *testing.T) {
 		BindVerb:       "bar",
 	})
 
+	goodInvalidPlugins := validConfig.DeepCopy()
+	goodInvalidPlugins.Profiles[0].Plugins.Score.Enabled = append(goodInvalidPlugins.Profiles[0].Plugins.Score.Enabled, config.Plugin{Name: "PodTopologySpread", Weight: 2})
+
 	scenarios := map[string]struct {
 		expectedToFail bool
 		config         *config.KubeSchedulerConfiguration
@@ -542,6 +552,10 @@ func TestValidateKubeSchedulerConfigurationV1beta3(t *testing.T) {
 			expectedToFail: true,
 			config:         mismatchQueueSort,
 		},
+		"good-invalid-plugins": {
+			expectedToFail: false,
+			config:         goodInvalidPlugins,
+		},
 	}
 
 	for name, scenario := range scenarios {