Merge pull request #82483 from everpeace/fix-scheduler-pluginconfig-initialization

The correct PluginConfig.Args is not passed to the corresponding PluginFactory in kube-scheduler when multiple PluginConfig items are defined
This commit is contained in:
Kubernetes Prow Robot 2019-09-11 21:22:49 -07:00 committed by GitHub
commit 3fd8962bd1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 127 additions and 66 deletions

View File

@ -577,7 +577,9 @@ func (f *framework) GetWaitingPod(uid types.UID) WaitingPod {
func pluginNameToConfig(args []config.PluginConfig) map[string]*runtime.Unknown {
pc := make(map[string]*runtime.Unknown, 0)
for _, p := range args {
for i := range args {
// This is needed because the type of PluginConfig.Args is not pointer type.
p := args[i]
pc[p.Name] = &p.Args
}
return pc

View File

@ -17,6 +17,7 @@ limitations under the License.
package v1alpha1
import (
"fmt"
"reflect"
"testing"
@ -38,17 +39,32 @@ const (
var _ = ScoreWithNormalizePlugin(&TestScoreWithNormalizePlugin{})
var _ = ScorePlugin(&TestScorePlugin{})
func newScoreWithNormalizePlugin1(inj injectedResult) Plugin {
return &TestScoreWithNormalizePlugin{scoreWithNormalizePlugin1, inj}
func newScoreWithNormalizePlugin1(injArgs *runtime.Unknown, f FrameworkHandle) (Plugin, error) {
var inj injectedResult
if err := DecodeInto(injArgs, &inj); err != nil {
return nil, err
}
return &TestScoreWithNormalizePlugin{scoreWithNormalizePlugin1, inj}, nil
}
func newScoreWithNormalizePlugin2(inj injectedResult) Plugin {
return &TestScoreWithNormalizePlugin{scoreWithNormalizePlugin2, inj}
func newScoreWithNormalizePlugin2(injArgs *runtime.Unknown, f FrameworkHandle) (Plugin, error) {
var inj injectedResult
if err := DecodeInto(injArgs, &inj); err != nil {
return nil, err
}
return &TestScoreWithNormalizePlugin{scoreWithNormalizePlugin2, inj}, nil
}
func newScorePlugin1(inj injectedResult) Plugin {
return &TestScorePlugin{scorePlugin1, inj}
func newScorePlugin1(injArgs *runtime.Unknown, f FrameworkHandle) (Plugin, error) {
var inj injectedResult
if err := DecodeInto(injArgs, &inj); err != nil {
return nil, err
}
return &TestScorePlugin{scorePlugin1, inj}, nil
}
func newPluginNotImplementingScore(injectedResult) Plugin {
return &PluginNotImplementingScore{}
func newPluginNotImplementingScore(_ *runtime.Unknown, _ FrameworkHandle) (Plugin, error) {
return &PluginNotImplementingScore{}, nil
}
type TestScoreWithNormalizePlugin struct {
@ -89,12 +105,14 @@ func (pl *PluginNotImplementingScore) Name() string {
return pluginNotImplementingScore
}
var defaultConstructors = map[string]func(injectedResult) Plugin{
scoreWithNormalizePlugin1: newScoreWithNormalizePlugin1,
scoreWithNormalizePlugin2: newScoreWithNormalizePlugin2,
scorePlugin1: newScorePlugin1,
pluginNotImplementingScore: newPluginNotImplementingScore,
}
var registry Registry = func() Registry {
r := make(Registry)
r.Register(scoreWithNormalizePlugin1, newScoreWithNormalizePlugin1)
r.Register(scoreWithNormalizePlugin2, newScoreWithNormalizePlugin2)
r.Register(scorePlugin1, newScorePlugin1)
r.Register(pluginNotImplementingScore, newPluginNotImplementingScore)
return r
}()
var defaultWeights = map[string]int32{
scoreWithNormalizePlugin1: 1,
@ -102,8 +120,7 @@ var defaultWeights = map[string]int32{
scorePlugin1: 1,
}
// No specific config required.
var args []config.PluginConfig
var emptyArgs []config.PluginConfig = make([]config.PluginConfig, 0)
var pc = &PluginContext{}
// Pod is only used for logging errors.
@ -150,7 +167,7 @@ func TestInitFrameworkWithScorePlugins(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := NewFramework(toRegistry(defaultConstructors, make(map[string]injectedResult)), tt.plugins, args)
_, err := NewFramework(registry, tt.plugins, emptyArgs)
if tt.initErr && err == nil {
t.Fatal("Framework initialization should fail")
}
@ -163,11 +180,11 @@ func TestInitFrameworkWithScorePlugins(t *testing.T) {
func TestRunScorePlugins(t *testing.T) {
tests := []struct {
name string
registry Registry
plugins *config.Plugins
injectedRes map[string]injectedResult
want PluginToNodeScores
name string
registry Registry
plugins *config.Plugins
pluginConfigs []config.PluginConfig
want PluginToNodeScores
// If err is true, we expect RunScorePlugin to fail.
err bool
}{
@ -179,8 +196,13 @@ func TestRunScorePlugins(t *testing.T) {
{
name: "single Score plugin",
plugins: buildConfigDefaultWeights(scorePlugin1),
injectedRes: map[string]injectedResult{
scorePlugin1: {scoreRes: 1},
pluginConfigs: []config.PluginConfig{
{
Name: scorePlugin1,
Args: runtime.Unknown{
Raw: []byte(`{ "scoreRes": 1 }`),
},
},
},
// scorePlugin1 Score returns 1, weight=1, so want=1.
want: PluginToNodeScores{
@ -191,8 +213,13 @@ func TestRunScorePlugins(t *testing.T) {
name: "single ScoreWithNormalize plugin",
//registry: registry,
plugins: buildConfigDefaultWeights(scoreWithNormalizePlugin1),
injectedRes: map[string]injectedResult{
scoreWithNormalizePlugin1: {scoreRes: 10, normalizeRes: 5},
pluginConfigs: []config.PluginConfig{
{
Name: scoreWithNormalizePlugin1,
Args: runtime.Unknown{
Raw: []byte(`{ "scoreRes": 10, "normalizeRes": 5 }`),
},
},
},
// scoreWithNormalizePlugin1 Score returns 10, but NormalizeScore overrides to 5, weight=1, so want=5
want: PluginToNodeScores{
@ -202,10 +229,25 @@ func TestRunScorePlugins(t *testing.T) {
{
name: "2 Score plugins, 2 NormalizeScore plugins",
plugins: buildConfigDefaultWeights(scorePlugin1, scoreWithNormalizePlugin1, scoreWithNormalizePlugin2),
injectedRes: map[string]injectedResult{
scorePlugin1: {scoreRes: 1},
scoreWithNormalizePlugin1: {scoreRes: 3, normalizeRes: 4},
scoreWithNormalizePlugin2: {scoreRes: 4, normalizeRes: 5},
pluginConfigs: []config.PluginConfig{
{
Name: scorePlugin1,
Args: runtime.Unknown{
Raw: []byte(`{ "scoreRes": 1 }`),
},
},
{
Name: scoreWithNormalizePlugin1,
Args: runtime.Unknown{
Raw: []byte(`{ "scoreRes": 3, "normalizeRes": 4}`),
},
},
{
Name: scoreWithNormalizePlugin2,
Args: runtime.Unknown{
Raw: []byte(`{ "scoreRes": 4, "normalizeRes": 5}`),
},
},
},
// scorePlugin1 Score returns 1, weight =1, so want=1.
// scoreWithNormalizePlugin1 Score returns 3, but NormalizeScore overrides to 4, weight=1, so want=4.
@ -218,16 +260,26 @@ func TestRunScorePlugins(t *testing.T) {
},
{
name: "score fails",
injectedRes: map[string]injectedResult{
scoreWithNormalizePlugin1: {scoreErr: true},
pluginConfigs: []config.PluginConfig{
{
Name: scoreWithNormalizePlugin1,
Args: runtime.Unknown{
Raw: []byte(`{ "scoreErr": true }`),
},
},
},
plugins: buildConfigDefaultWeights(scorePlugin1, scoreWithNormalizePlugin1),
err: true,
},
{
name: "normalize fails",
injectedRes: map[string]injectedResult{
scoreWithNormalizePlugin1: {normalizeErr: true},
pluginConfigs: []config.PluginConfig{
{
Name: scoreWithNormalizePlugin1,
Args: runtime.Unknown{
Raw: []byte(`{ "normalizeErr": true }`),
},
},
},
plugins: buildConfigDefaultWeights(scorePlugin1, scoreWithNormalizePlugin1),
err: true,
@ -235,32 +287,52 @@ func TestRunScorePlugins(t *testing.T) {
{
name: "Score plugin return score greater than MaxNodeScore",
plugins: buildConfigDefaultWeights(scorePlugin1),
injectedRes: map[string]injectedResult{
scorePlugin1: {scoreRes: MaxNodeScore + 1},
pluginConfigs: []config.PluginConfig{
{
Name: scorePlugin1,
Args: runtime.Unknown{
Raw: []byte(fmt.Sprintf(`{ "scoreRes": %d }`, MaxNodeScore+1)),
},
},
},
err: true,
},
{
name: "Score plugin return score less than MinNodeScore",
plugins: buildConfigDefaultWeights(scorePlugin1),
injectedRes: map[string]injectedResult{
scorePlugin1: {scoreRes: MinNodeScore - 1},
pluginConfigs: []config.PluginConfig{
{
Name: scorePlugin1,
Args: runtime.Unknown{
Raw: []byte(fmt.Sprintf(`{ "scoreRes": %d }`, MinNodeScore-1)),
},
},
},
err: true,
},
{
name: "ScoreWithNormalize plugin return score greater than MaxNodeScore",
plugins: buildConfigDefaultWeights(scoreWithNormalizePlugin1),
injectedRes: map[string]injectedResult{
scoreWithNormalizePlugin1: {normalizeRes: MaxNodeScore + 1},
pluginConfigs: []config.PluginConfig{
{
Name: scoreWithNormalizePlugin1,
Args: runtime.Unknown{
Raw: []byte(fmt.Sprintf(`{ "normalizeRes": %d }`, MaxNodeScore+1)),
},
},
},
err: true,
},
{
name: "ScoreWithNormalize plugin return score less than MinNodeScore",
plugins: buildConfigDefaultWeights(scoreWithNormalizePlugin1),
injectedRes: map[string]injectedResult{
scoreWithNormalizePlugin1: {normalizeRes: MinNodeScore - 1},
pluginConfigs: []config.PluginConfig{
{
Name: scoreWithNormalizePlugin1,
Args: runtime.Unknown{
Raw: []byte(fmt.Sprintf(`{ "normalizeRes": %d }`, MinNodeScore-1)),
},
},
},
err: true,
},
@ -268,9 +340,8 @@ func TestRunScorePlugins(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Inject the results for each plugin.
registry := toRegistry(defaultConstructors, tt.injectedRes)
f, err := NewFramework(registry, tt.plugins, args)
// Inject the results via Args in PluginConfig.
f, err := NewFramework(registry, tt.plugins, tt.pluginConfigs)
if err != nil {
t.Fatalf("Failed to create framework for testing: %v", err)
}
@ -294,18 +365,6 @@ func TestRunScorePlugins(t *testing.T) {
}
}
func toRegistry(constructors map[string]func(injectedResult) Plugin, injectedRes map[string]injectedResult) Registry {
registry := make(Registry)
for pl, f := range constructors {
npl := pl
nf := f
registry[pl] = func(_ *runtime.Unknown, _ FrameworkHandle) (Plugin, error) {
return nf(injectedRes[npl]), nil
}
}
return registry
}
func buildConfigDefaultWeights(ps ...string) *config.Plugins {
return buildConfigWithWeights(defaultWeights, ps...)
}
@ -319,25 +378,25 @@ func buildConfigWithWeights(weights map[string]int32, ps ...string) *config.Plug
}
type injectedResult struct {
scoreRes int
normalizeRes int
scoreErr bool
normalizeErr bool
ScoreRes int `json:"scoreRes,omitempty"`
NormalizeRes int `json:"normalizeRes,omitempty"`
ScoreErr bool `json:"scoreErr,omitempty"`
NormalizeErr bool `json:"normalizeErr,omitempty"`
}
func setScoreRes(inj injectedResult) (int, *Status) {
if inj.scoreErr {
if inj.ScoreErr {
return 0, NewStatus(Error, "injecting failure.")
}
return inj.scoreRes, nil
return inj.ScoreRes, nil
}
func injectNormalizeRes(inj injectedResult, scores NodeScoreList) *Status {
if inj.normalizeErr {
if inj.NormalizeErr {
return NewStatus(Error, "injecting failure.")
}
for i := range scores {
scores[i].Score = inj.normalizeRes
scores[i].Score = inj.NormalizeRes
}
return nil
}