Merge pull request #86905 from draveness/feature/disable-scheduling-plugin

feat(scheduling): address disabled plugins in scheduling framework

pkg/scheduler/apis/config/BUILD

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
@@ -15,6 +15,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/component-base/config:go_default_library",
     ],
 )
@@ -39,3 +40,10 @@ filegroup(
     tags = ["automanaged"],
     visibility = ["//visibility:public"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["types_test.go"],
+    embed = [":go_default_library"],
+    deps = ["//vendor/github.com/google/go-cmp/cmp:go_default_library"],
+)

pkg/scheduler/apis/config/types.go

@@ -21,6 +21,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	componentbaseconfig "k8s.io/component-base/config"
 )
 
@@ -270,3 +271,52 @@ func (p *Plugins) Append(src *Plugins) {
 	p.PostBind = appendPluginSet(p.PostBind, src.PostBind)
 	p.Unreserve = appendPluginSet(p.Unreserve, src.Unreserve)
 }
+
+// Apply merges the plugin configuration from custom plugins, handling disabled sets.
+func (p *Plugins) Apply(customPlugins *Plugins) {
+	if customPlugins == nil {
+		return
+	}
+
+	p.QueueSort = mergePluginSets(p.QueueSort, customPlugins.QueueSort)
+	p.PreFilter = mergePluginSets(p.PreFilter, customPlugins.PreFilter)
+	p.Filter = mergePluginSets(p.Filter, customPlugins.Filter)
+	p.PostFilter = mergePluginSets(p.PostFilter, customPlugins.PostFilter)
+	p.Score = mergePluginSets(p.Score, customPlugins.Score)
+	p.Reserve = mergePluginSets(p.Reserve, customPlugins.Reserve)
+	p.Permit = mergePluginSets(p.Permit, customPlugins.Permit)
+	p.PreBind = mergePluginSets(p.PreBind, customPlugins.PreBind)
+	p.Bind = mergePluginSets(p.Bind, customPlugins.Bind)
+	p.PostBind = mergePluginSets(p.PostBind, customPlugins.PostBind)
+	p.Unreserve = mergePluginSets(p.Unreserve, customPlugins.Unreserve)
+}
+
+func mergePluginSets(defaultPluginSet, customPluginSet *PluginSet) *PluginSet {
+	if customPluginSet == nil {
+		customPluginSet = &PluginSet{}
+	}
+
+	if defaultPluginSet == nil {
+		defaultPluginSet = &PluginSet{}
+	}
+
+	disabledPlugins := sets.NewString()
+	for _, disabledPlugin := range customPluginSet.Disabled {
+		disabledPlugins.Insert(disabledPlugin.Name)
+	}
+
+	enabledPlugins := []Plugin{}
+	if !disabledPlugins.Has("*") {
+		for _, defaultEnabledPlugin := range defaultPluginSet.Enabled {
+			if disabledPlugins.Has(defaultEnabledPlugin.Name) {
+				continue
+			}
+
+			enabledPlugins = append(enabledPlugins, defaultEnabledPlugin)
+		}
+	}
+
+	enabledPlugins = append(enabledPlugins, customPluginSet.Enabled...)
+
+	return &PluginSet{Enabled: enabledPlugins}
+}

pkg/scheduler/apis/config/types_test.go

@@ -0,0 +1,202 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package config
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestPluginsApply(t *testing.T) {
+	tests := []struct {
+		name            string
+		customPlugins   *Plugins
+		defaultPlugins  *Plugins
+		expectedPlugins *Plugins
+	}{
+		{
+			name: "AppendCustomPlugin",
+			customPlugins: &Plugins{
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "CustomPlugin"},
+					},
+				},
+			},
+			defaultPlugins: &Plugins{
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "DefaultPlugin1"},
+						{Name: "DefaultPlugin2"},
+					},
+				},
+			},
+			expectedPlugins: &Plugins{
+				QueueSort: &PluginSet{Enabled: []Plugin{}},
+				PreFilter: &PluginSet{Enabled: []Plugin{}},
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "DefaultPlugin1"},
+						{Name: "DefaultPlugin2"},
+						{Name: "CustomPlugin"},
+					},
+				},
+				PostFilter: &PluginSet{Enabled: []Plugin{}},
+				Score:      &PluginSet{Enabled: []Plugin{}},
+				Reserve:    &PluginSet{Enabled: []Plugin{}},
+				Permit:     &PluginSet{Enabled: []Plugin{}},
+				PreBind:    &PluginSet{Enabled: []Plugin{}},
+				Bind:       &PluginSet{Enabled: []Plugin{}},
+				PostBind:   &PluginSet{Enabled: []Plugin{}},
+				Unreserve:  &PluginSet{Enabled: []Plugin{}},
+			},
+		},
+		{
+			name: "InsertAfterDefaultPlugins2",
+			customPlugins: &Plugins{
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "CustomPlugin"},
+						{Name: "DefaultPlugin2"},
+					},
+					Disabled: []Plugin{
+						{Name: "DefaultPlugin2"},
+					},
+				},
+			},
+			defaultPlugins: &Plugins{
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "DefaultPlugin1"},
+						{Name: "DefaultPlugin2"},
+					},
+				},
+			},
+			expectedPlugins: &Plugins{
+				QueueSort: &PluginSet{Enabled: []Plugin{}},
+				PreFilter: &PluginSet{Enabled: []Plugin{}},
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "DefaultPlugin1"},
+						{Name: "CustomPlugin"},
+						{Name: "DefaultPlugin2"},
+					},
+				},
+				PostFilter: &PluginSet{Enabled: []Plugin{}},
+				Score:      &PluginSet{Enabled: []Plugin{}},
+				Reserve:    &PluginSet{Enabled: []Plugin{}},
+				Permit:     &PluginSet{Enabled: []Plugin{}},
+				PreBind:    &PluginSet{Enabled: []Plugin{}},
+				Bind:       &PluginSet{Enabled: []Plugin{}},
+				PostBind:   &PluginSet{Enabled: []Plugin{}},
+				Unreserve:  &PluginSet{Enabled: []Plugin{}},
+			},
+		},
+		{
+			name: "InsertBeforeAllPlugins",
+			customPlugins: &Plugins{
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "CustomPlugin"},
+						{Name: "DefaultPlugin1"},
+						{Name: "DefaultPlugin2"},
+					},
+					Disabled: []Plugin{
+						{Name: "*"},
+					},
+				},
+			},
+			defaultPlugins: &Plugins{
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "DefaultPlugin1"},
+						{Name: "DefaultPlugin2"},
+					},
+				},
+			},
+			expectedPlugins: &Plugins{
+				QueueSort: &PluginSet{Enabled: []Plugin{}},
+				PreFilter: &PluginSet{Enabled: []Plugin{}},
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "CustomPlugin"},
+						{Name: "DefaultPlugin1"},
+						{Name: "DefaultPlugin2"},
+					},
+				},
+				PostFilter: &PluginSet{Enabled: []Plugin{}},
+				Score:      &PluginSet{Enabled: []Plugin{}},
+				Reserve:    &PluginSet{Enabled: []Plugin{}},
+				Permit:     &PluginSet{Enabled: []Plugin{}},
+				PreBind:    &PluginSet{Enabled: []Plugin{}},
+				Bind:       &PluginSet{Enabled: []Plugin{}},
+				PostBind:   &PluginSet{Enabled: []Plugin{}},
+				Unreserve:  &PluginSet{Enabled: []Plugin{}},
+			},
+		},
+		{
+			name: "ReorderDefaultPlugins",
+			customPlugins: &Plugins{
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "DefaultPlugin2"},
+						{Name: "DefaultPlugin1"},
+					},
+					Disabled: []Plugin{
+						{Name: "*"},
+					},
+				},
+			},
+			defaultPlugins: &Plugins{
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "DefaultPlugin1"},
+						{Name: "DefaultPlugin2"},
+					},
+				},
+			},
+			expectedPlugins: &Plugins{
+				QueueSort: &PluginSet{Enabled: []Plugin{}},
+				PreFilter: &PluginSet{Enabled: []Plugin{}},
+				Filter: &PluginSet{
+					Enabled: []Plugin{
+						{Name: "DefaultPlugin2"},
+						{Name: "DefaultPlugin1"},
+					},
+				},
+				PostFilter: &PluginSet{Enabled: []Plugin{}},
+				Score:      &PluginSet{Enabled: []Plugin{}},
+				Reserve:    &PluginSet{Enabled: []Plugin{}},
+				Permit:     &PluginSet{Enabled: []Plugin{}},
+				PreBind:    &PluginSet{Enabled: []Plugin{}},
+				Bind:       &PluginSet{Enabled: []Plugin{}},
+				PostBind:   &PluginSet{Enabled: []Plugin{}},
+				Unreserve:  &PluginSet{Enabled: []Plugin{}},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			test.defaultPlugins.Apply(test.customPlugins)
+			if d := cmp.Diff(test.expectedPlugins, test.defaultPlugins); d != "" {
+				t.Fatalf("plugins mismatch (-want +got):\n%s", d)
+			}
+		})
+	}
+}

pkg/scheduler/factory.go

@@ -186,11 +186,11 @@ func (c *Configurator) createFromProvider(providerName string) (*Scheduler, erro
 	}
 
 	// Combine the provided plugins with the ones from component config.
-	// TODO(#86789): address disabled plugins.
+	var defaultPlugins schedulerapi.Plugins
-	var plugins schedulerapi.Plugins
+	defaultPlugins.Append(provider.FrameworkPlugins)
-	plugins.Append(provider.FrameworkPlugins)
+	defaultPlugins.Apply(c.plugins)
-	plugins.Append(c.plugins)
+	c.plugins = &defaultPlugins
-	c.plugins = &plugins
+
 	var pluginConfig []schedulerapi.PluginConfig
 	pluginConfig = append(pluginConfig, provider.FrameworkPluginConfig...)
 	pluginConfig = append(pluginConfig, c.pluginConfig...)
@@ -297,11 +297,11 @@ func (c *Configurator) createFromConfig(policy schedulerapi.Policy) (*Scheduler,
 	}
 	// Combine all framework configurations. If this results in any duplication, framework
 	// instantiation should fail.
-	var plugins schedulerapi.Plugins
+	var defaultPlugins schedulerapi.Plugins
-	plugins.Append(pluginsForPredicates)
+	defaultPlugins.Append(pluginsForPredicates)
-	plugins.Append(pluginsForPriorities)
+	defaultPlugins.Append(pluginsForPriorities)
-	plugins.Append(c.plugins)
+	defaultPlugins.Apply(c.plugins)
-	c.plugins = &plugins
+	c.plugins = &defaultPlugins
 
 	var pluginConfig []schedulerapi.PluginConfig
 	pluginConfig = append(pluginConfig, pluginConfigForPredicates...)