Readable error message on the plugin configs of the removed plugins

Several plugins are removed in the v1beta2, but the legacy scheduler
config would still have the plugin configs of those removed plugins.

It was throwing raw byte data when those plugin configs are still in
place which will hard to read and understand.

Fix it by checking the removed plugin config before the validation of
the plugin args.

Signed-off-by: Dave Chen <dave.chen@arm.com>
This commit is contained in:
Dave Chen 2021-07-05 17:27:46 +08:00
parent f9d0498f71
commit 5ea9460234
2 changed files with 41 additions and 25 deletions

View File

@ -201,31 +201,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,
@ -249,6 +224,36 @@ func validatePluginConfig(path *field.Path, apiVersion string, profile *config.K
errs = append(errs, validateScorePluginSetForConflictPlugins(
pluginsPath.Child("score"), apiVersion, profile)...)
}
seenPluginConfig := make(sets.String)
// validate plugin config at the end because we will return the aggregated errors early if type of args doesn't match.
// validate plugin config at the end will will help to collect as much as misconfiguration as possible.
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"))
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))
}
}
}
return errs
}

View File

@ -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})
badPluginsConfig := validConfig.DeepCopy()
badPluginsConfig.Profiles[0].PluginConfig = append(badPluginsConfig.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: badPluginsConfig,
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 {