diff --git a/pkg/scheduler/factory.go b/pkg/scheduler/factory.go index 31370232676..7b112fed14e 100644 --- a/pkg/scheduler/factory.go +++ b/pkg/scheduler/factory.go @@ -317,15 +317,16 @@ func (c *Configurator) createFromConfig(policy schedulerapi.Policy) (*Scheduler, defPluginConfig = append(defPluginConfig, pluginConfigForPriorities...) for i := range c.profiles { prof := &c.profiles[i] - plugins := &schedulerapi.Plugins{} - plugins.Append(&defPlugins) - plugins.Apply(prof.Plugins) - prof.Plugins = plugins + if prof.Plugins != nil { + return nil, errors.New("using Plugins and Policy simultaneously is not supported") + } + prof.Plugins = &schedulerapi.Plugins{} + prof.Plugins.Append(&defPlugins) - var pluginConfig []schedulerapi.PluginConfig - pluginConfig = append(pluginConfig, defPluginConfig...) - pluginConfig = append(pluginConfig, prof.PluginConfig...) - prof.PluginConfig = pluginConfig + if len(prof.PluginConfig) != 0 { + return nil, errors.New("using PluginConfig and Policy simultaneously is not supported") + } + prof.PluginConfig = append(prof.PluginConfig, defPluginConfig...) } return c.create() diff --git a/pkg/scheduler/factory_test.go b/pkg/scheduler/factory_test.go index 900773c0a1e..9ed4c40a466 100644 --- a/pkg/scheduler/factory_test.go +++ b/pkg/scheduler/factory_test.go @@ -21,6 +21,7 @@ import ( "encoding/json" "errors" "reflect" + "strings" "testing" "time" @@ -77,12 +78,6 @@ func TestCreate(t *testing.T) { // It combines some configurable predicate/priorities with some pre-defined ones func TestCreateFromConfig(t *testing.T) { var configData []byte - var policy schedulerapi.Policy - - client := fake.NewSimpleClientset() - stopCh := make(chan struct{}) - defer close(stopCh) - factory := newConfigFactory(client, stopCh) configData = []byte(`{ "kind" : "Policy", @@ -103,34 +98,79 @@ func TestCreateFromConfig(t *testing.T) { {"name" : "NodeAffinityPriority", "weight" : 2}, {"name" : "ImageLocalityPriority", "weight" : 1} ] }`) - if err := runtime.DecodeInto(scheme.Codecs.UniversalDecoder(), configData, &policy); err != nil { - t.Errorf("Invalid configuration: %v", err) + cases := []struct { + name string + plugins *schedulerapi.Plugins + pluginCfgs []schedulerapi.PluginConfig + wantErr string + }{ + { + name: "just policy", + }, + { + name: "policy and plugins", + plugins: &schedulerapi.Plugins{ + Filter: &schedulerapi.PluginSet{ + Disabled: []schedulerapi.Plugin{{Name: nodelabel.Name}}, + }, + }, + wantErr: "using Plugins and Policy simultaneously is not supported", + }, + { + name: "policy and plugin config", + pluginCfgs: []schedulerapi.PluginConfig{ + {Name: queuesort.Name}, + }, + wantErr: "using PluginConfig and Policy simultaneously is not supported", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + client := fake.NewSimpleClientset() + stopCh := make(chan struct{}) + defer close(stopCh) + factory := newConfigFactory(client, stopCh) + factory.profiles[0].Plugins = tc.plugins + factory.profiles[0].PluginConfig = tc.pluginCfgs + + var policy schedulerapi.Policy + if err := runtime.DecodeInto(scheme.Codecs.UniversalDecoder(), configData, &policy); err != nil { + t.Errorf("Invalid configuration: %v", err) + } + + sched, err := factory.createFromConfig(policy) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("got err %q, want %q", err, tc.wantErr) + } + return + } + if err != nil { + t.Fatalf("createFromConfig failed: %v", err) + } + // createFromConfig is the old codepath where we only have one profile. + prof := sched.Profiles[testSchedulerName] + queueSortPls := prof.ListPlugins()["QueueSortPlugin"] + wantQueuePls := []schedulerapi.Plugin{{Name: queuesort.Name}} + if diff := cmp.Diff(wantQueuePls, queueSortPls); diff != "" { + t.Errorf("Unexpected QueueSort plugins (-want, +got): %s", diff) + } + bindPls := prof.ListPlugins()["BindPlugin"] + wantBindPls := []schedulerapi.Plugin{{Name: defaultbinder.Name}} + if diff := cmp.Diff(wantBindPls, bindPls); diff != "" { + t.Errorf("Unexpected Bind plugins (-want, +got): %s", diff) + } + + // Verify that node label predicate/priority are converted to framework plugins. + wantArgs := `{"Name":"NodeLabel","Args":{"presentLabels":["zone"],"absentLabels":["foo"],"presentLabelsPreference":["l1"],"absentLabelsPreference":["l2"]}}` + verifyPluginConvertion(t, nodelabel.Name, []string{"FilterPlugin", "ScorePlugin"}, prof, &factory.profiles[0], 6, wantArgs) + // Verify that service affinity custom predicate/priority is converted to framework plugin. + wantArgs = `{"Name":"ServiceAffinity","Args":{"affinityLabels":["zone","foo"],"antiAffinityLabelsPreference":["rack","zone"]}}` + verifyPluginConvertion(t, serviceaffinity.Name, []string{"FilterPlugin", "ScorePlugin"}, prof, &factory.profiles[0], 6, wantArgs) + // TODO(#87703): Verify all plugin configs. + }) } - sched, err := factory.createFromConfig(policy) - if err != nil { - t.Fatalf("createFromConfig failed: %v", err) - } - // createFromConfig is the old codepath where we only have one profile. - prof := sched.Profiles[testSchedulerName] - queueSortPls := prof.ListPlugins()["QueueSortPlugin"] - wantQueuePls := []schedulerapi.Plugin{{Name: queuesort.Name}} - if diff := cmp.Diff(wantQueuePls, queueSortPls); diff != "" { - t.Errorf("Unexpected QueueSort plugins (-want, +got): %s", diff) - } - bindPls := prof.ListPlugins()["BindPlugin"] - wantBindPls := []schedulerapi.Plugin{{Name: defaultbinder.Name}} - if diff := cmp.Diff(wantBindPls, bindPls); diff != "" { - t.Errorf("Unexpected Bind plugins (-want, +got): %s", diff) - } - - // Verify that node label predicate/priority are converted to framework plugins. - wantArgs := `{"Name":"NodeLabel","Args":{"presentLabels":["zone"],"absentLabels":["foo"],"presentLabelsPreference":["l1"],"absentLabelsPreference":["l2"]}}` - verifyPluginConvertion(t, nodelabel.Name, []string{"FilterPlugin", "ScorePlugin"}, prof, &factory.profiles[0], 6, wantArgs) - // Verify that service affinity custom predicate/priority is converted to framework plugin. - wantArgs = `{"Name":"ServiceAffinity","Args":{"affinityLabels":["zone","foo"],"antiAffinityLabelsPreference":["rack","zone"]}}` - verifyPluginConvertion(t, serviceaffinity.Name, []string{"FilterPlugin", "ScorePlugin"}, prof, &factory.profiles[0], 6, wantArgs) - // TODO(#87703): Verify all plugin configs. } func verifyPluginConvertion(t *testing.T, name string, extentionPoints []string, prof *profile.Profile, cfg *schedulerapi.KubeSchedulerProfile, wantWeight int32, wantArgs string) {