Skip to content

Commit

Permalink
need to use local variable so that pluginNameToConfig map can keep co…
Browse files Browse the repository at this point in the history 
…rrect contents

because the type of PluginConfig.Args is not pointer.

It also fixed framework_test so that it can test PluginConfig initialization.
  • Loading branch information
@everpeace
everpeace committed Sep 11, 2019
1 parent 35cf6b6 commit 439a682
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 66 deletions.
4 changes: 3 additions & 1 deletion pkg/scheduler/framework/v1alpha1/framework.go
View file
Expand Up @@ -582,7 +582,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
Expand Down
189 changes: 124 additions & 65 deletions pkg/scheduler/framework/v1alpha1/framework_test.go
View file
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1alpha1

import (
"fmt"
"reflect"
"testing"

Expand All @@ -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 {
Expand Down Expand Up @@ -89,21 +105,22 @@ 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,
scoreWithNormalizePlugin2: 2,
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.
Expand Down Expand Up @@ -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")
}
Expand All @@ -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
}{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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.
Expand All @@ -218,59 +260,88 @@ 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,
},
{
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,
},
}

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)
}
Expand All @@ -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...)
}
Expand All @@ -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
}

0 comments on commit 439a682

Please sign in to comment.