need to use local variable so that pluginNameToConfig map can keep correct contents

because the type of PluginConfig.Args is not pointer.

It also fixed framework_test so that it can test PluginConfig initialization.
This commit is contained in:
Shingo Omura 2019-09-09 14:13:15 +09:00
parent 35cf6b6cbc
commit 439a6825e7
2 changed files with 127 additions and 66 deletions

View File

@ -582,7 +582,9 @@ func (f *framework) GetWaitingPod(uid types.UID) WaitingPod {
func pluginNameToConfig(args []config.PluginConfig) map[string]*runtime.Unknown { func pluginNameToConfig(args []config.PluginConfig) map[string]*runtime.Unknown {
pc := make(map[string]*runtime.Unknown, 0) 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 pc[p.Name] = &p.Args
} }
return pc return pc

View File

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