Move Plugin Args defaults to versioned packages

Signed-off-by: Aldo Culquicondor <[email protected]>

cmd/kube-scheduler/app/options/BUILD

@@ -71,6 +71,7 @@ go_test(
     deps = [
         "//cmd/kube-scheduler/app/config:go_default_library",
         "//pkg/scheduler/apis/config:go_default_library",
+        "//pkg/scheduler/framework/plugins/interpodaffinity:go_default_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/util/rand:go_default_library",

cmd/kube-scheduler/app/options/deprecated.go

@@ -91,6 +91,8 @@ func (o *DeprecatedOptions) Validate() []error {
 // 1. --use-legacy-policy-config to use a policy file.
 // 2. --policy-configmap to use a policy config map value.
 // 3. --algorithm-provider to use a named algorithm provider.
+//
+// This function is only called when no config file is provided.
 func (o *DeprecatedOptions) ApplyTo(cfg *kubeschedulerconfig.KubeSchedulerConfiguration) error {
 	if o == nil {
 		return nil
@@ -120,20 +122,19 @@ func (o *DeprecatedOptions) ApplyTo(cfg *kubeschedulerconfig.KubeSchedulerConfig
 		}
 	}
 
-	// The following deprecated options affect the only existing profile that is
-	// added by default.
+	// Deprecated flags have an effect iff no config file was provided, in which
+	// case this function expects a default KubeSchedulerConfiguration instance,
+	// which has a single profile.
 	profile := &cfg.Profiles[0]
 	if len(o.SchedulerName) > 0 {
 		profile.SchedulerName = o.SchedulerName
 	}
-	if o.HardPodAffinitySymmetricWeight != interpodaffinity.DefaultHardPodAffinityWeight {
-		plCfg := kubeschedulerconfig.PluginConfig{
-			Name: interpodaffinity.Name,
-			Args: &kubeschedulerconfig.InterPodAffinityArgs{
-				HardPodAffinityWeight: o.HardPodAffinitySymmetricWeight,
-			},
-		}
-		profile.PluginConfig = append(profile.PluginConfig, plCfg)
+	plCfg := kubeschedulerconfig.PluginConfig{
+		Name: interpodaffinity.Name,
+		Args: &kubeschedulerconfig.InterPodAffinityArgs{
+			HardPodAffinityWeight: o.HardPodAffinitySymmetricWeight,
+		},
 	}
+	profile.PluginConfig = append(profile.PluginConfig, plCfg)
 	return nil
 }

cmd/kube-scheduler/app/options/options.go

@@ -49,7 +49,6 @@ import (
 	kubeschedulerconfig "k8s.io/kubernetes/pkg/scheduler/apis/config"
 	kubeschedulerscheme "k8s.io/kubernetes/pkg/scheduler/apis/config/scheme"
 	"k8s.io/kubernetes/pkg/scheduler/apis/config/validation"
-	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity"
 )
 
 // Options has all the params needed to run a Scheduler
@@ -104,7 +103,7 @@ func NewOptions() (*Options, error) {
 			UseLegacyPolicyConfig:          false,
 			PolicyConfigMapNamespace:       metav1.NamespaceSystem,
 			SchedulerName:                  corev1.DefaultSchedulerName,
-			HardPodAffinitySymmetricWeight: interpodaffinity.DefaultHardPodAffinityWeight,
+			HardPodAffinitySymmetricWeight: 1,
 		},
 		Metrics: metrics.NewOptions(),
 	}

cmd/kube-scheduler/app/options/options_test.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
+	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -728,7 +729,17 @@ profiles:
 				PodInitialBackoffSeconds: defaultPodInitialBackoffSeconds,
 				PodMaxBackoffSeconds:     defaultPodMaxBackoffSeconds,
 				Profiles: []kubeschedulerconfig.KubeSchedulerProfile{
-					{SchedulerName: "my-nice-scheduler"},
+					{
+						SchedulerName: "my-nice-scheduler",
+						PluginConfig: []kubeschedulerconfig.PluginConfig{
+							{
+								Name: interpodaffinity.Name,
+								Args: &kubeschedulerconfig.InterPodAffinityArgs{
+									HardPodAffinityWeight: 1,
+								},
+							},
+						},
+					},
 				},
 			},
 		},

pkg/scheduler/apis/config/scheme/scheme_test.go

@@ -233,8 +233,9 @@ profiles:
 					PluginConfig: []config.PluginConfig{
 						{
 							Name: "InterPodAffinity",
-							// TODO(acondor): Set default values.
-							Args: &config.InterPodAffinityArgs{},
+							Args: &config.InterPodAffinityArgs{
+								HardPodAffinityWeight: 1,
+							},
 						},
 						{
 							Name: "NodeResourcesFit",

pkg/scheduler/apis/config/v1alpha2/BUILD

@@ -39,6 +39,7 @@ go_test(
         "//pkg/scheduler/apis/config:go_default_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/util/runtime:go_default_library",
         "//staging/src/k8s.io/component-base/config/v1alpha1:go_default_library",
         "//staging/src/k8s.io/kube-scheduler/config/v1alpha2:go_default_library",
         "//vendor/github.com/google/go-cmp/cmp:go_default_library",

pkg/scheduler/apis/config/v1alpha2/defaults.go

@@ -156,3 +156,12 @@ func SetDefaults_KubeSchedulerConfiguration(obj *v1alpha2.KubeSchedulerConfigura
 		obj.EnableContentionProfiling = &enableContentionProfiling
 	}
 }
+
+func SetDefaults_InterPodAffinityArgs(obj *v1alpha2.InterPodAffinityArgs) {
+	// Note that an object is created manually in cmd/kube-scheduler/app/options/deprecated.go
+	// DeprecatedOptions#ApplyTo.
+	// Update that object if a new default field is added here.
+	if obj.HardPodAffinityWeight == nil {
+		obj.HardPodAffinityWeight = pointer.Int32Ptr(1)
+	}
+}

pkg/scheduler/apis/config/v1alpha2/defaults_test.go

@@ -22,6 +22,8 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	componentbaseconfig "k8s.io/component-base/config/v1alpha1"
 	"k8s.io/kube-scheduler/config/v1alpha2"
 	"k8s.io/utils/pointer"
@@ -275,3 +277,47 @@ func TestSchedulerDefaults(t *testing.T) {
 		})
 	}
 }
+
+func TestPluginArgsDefaults(t *testing.T) {
+	tests := []struct {
+		name string
+		in   runtime.Object
+		want runtime.Object
+	}{
+		{
+			name: "InterPodAffinityArgs empty",
+			in:   &v1alpha2.InterPodAffinityArgs{},
+			want: &v1alpha2.InterPodAffinityArgs{
+				HardPodAffinityWeight: pointer.Int32Ptr(1),
+			},
+		},
+		{
+			name: "InterPodAffinityArgs explicit 0",
+			in: &v1alpha2.InterPodAffinityArgs{
+				HardPodAffinityWeight: pointer.Int32Ptr(0),
+			},
+			want: &v1alpha2.InterPodAffinityArgs{
+				HardPodAffinityWeight: pointer.Int32Ptr(0),
+			},
+		},
+		{
+			name: "InterPodAffinityArgs with value",
+			in: &v1alpha2.InterPodAffinityArgs{
+				HardPodAffinityWeight: pointer.Int32Ptr(5),
+			},
+			want: &v1alpha2.InterPodAffinityArgs{
+				HardPodAffinityWeight: pointer.Int32Ptr(5),
+			},
+		},
+	}
+	for _, tc := range tests {
+		scheme := runtime.NewScheme()
+		utilruntime.Must(AddToScheme(scheme))
+		t.Run(tc.name, func(t *testing.T) {
+			scheme.Default(tc.in)
+			if diff := cmp.Diff(tc.in, tc.want); diff != "" {
+				t.Errorf("Got unexpected defaults (-want, +got):\n%s", diff)
+			}
+		})
+	}
+}

pkg/scheduler/factory.go

@@ -323,16 +323,12 @@ func (c *Configurator) createFromConfig(policy schedulerapi.Policy) (*Scheduler,
 	}
 	for i := range c.profiles {
 		prof := &c.profiles[i]
-		if prof.Plugins != nil {
-			return nil, errors.New("using Plugins and Policy simultaneously is not supported")
-		}
+		// Plugins are empty when using Policy.
 		prof.Plugins = &schedulerapi.Plugins{}
 		prof.Plugins.Append(&defPlugins)
 
-		if len(prof.PluginConfig) != 0 {
-			return nil, errors.New("using PluginConfig and Policy simultaneously is not supported")
-		}
-		prof.PluginConfig = append(prof.PluginConfig, defPluginConfig...)
+		// PluginConfig is ignored when using Policy.
+		prof.PluginConfig = defPluginConfig
 	}
 
 	return c.create()

pkg/scheduler/factory_test.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"errors"
 	"reflect"
-	"strings"
 	"testing"
 	"time"
 
@@ -96,87 +95,48 @@ func TestCreateFromConfig(t *testing.T) {
 			{"name" : "NodeAffinityPriority", "weight" : 2},
 			{"name" : "ImageLocalityPriority", "weight" : 1}		]
 	}`)
-	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)
-			}
+	client := fake.NewSimpleClientset()
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+	factory := newConfigFactory(client, stopCh)
 
-			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)
-			}
+	var policy schedulerapi.Policy
+	if err := runtime.DecodeInto(scheme.Codecs.UniversalDecoder(), configData, &policy); err != nil {
+		t.Errorf("Invalid configuration: %v", err)
+	}
 
-			// Verify that node label predicate/priority are converted to framework plugins.
-			var wantArgs runtime.Object = &schedulerapi.NodeLabelArgs{
-				PresentLabels:           []string{"zone"},
-				AbsentLabels:            []string{"foo"},
-				PresentLabelsPreference: []string{"l1"},
-				AbsentLabelsPreference:  []string{"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 = &schedulerapi.ServiceAffinityArgs{
-				AffinityLabels:               []string{"zone", "foo"},
-				AntiAffinityLabelsPreference: []string{"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.
+	var wantArgs runtime.Object = &schedulerapi.NodeLabelArgs{
+		PresentLabels:           []string{"zone"},
+		AbsentLabels:            []string{"foo"},
+		PresentLabelsPreference: []string{"l1"},
+		AbsentLabelsPreference:  []string{"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 = &schedulerapi.ServiceAffinityArgs{
+		AffinityLabels:               []string{"zone", "foo"},
+		AntiAffinityLabelsPreference: []string{"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, extensionPoints []string, prof *profile.Profile, cfg *schedulerapi.KubeSchedulerProfile, wantWeight int32, wantArgs runtime.Object) {

pkg/scheduler/framework/plugins/interpodaffinity/plugin.go

@@ -30,8 +30,6 @@ const (
 	// Name is the name of the plugin used in the plugin registry and configurations.
 	Name = "InterPodAffinity"
 
-	// DefaultHardPodAffinityWeight is the default HardPodAffinityWeight.
-	DefaultHardPodAffinityWeight int32 = 1
 	// MinHardPodAffinityWeight is the minimum HardPodAffinityWeight.
 	MinHardPodAffinityWeight int32 = 0
 	// MaxHardPodAffinityWeight is the maximum HardPodAffinityWeight.
@@ -79,11 +77,6 @@ func New(plArgs runtime.Object, h framework.FrameworkHandle) (framework.Plugin,
 }
 
 func getArgs(obj runtime.Object) (config.InterPodAffinityArgs, error) {
-	if obj == nil {
-		return config.InterPodAffinityArgs{
-			HardPodAffinityWeight: DefaultHardPodAffinityWeight,
-		}, nil
-	}
 	ptr, ok := obj.(*config.InterPodAffinityArgs)
 	if !ok {
 		return config.InterPodAffinityArgs{}, fmt.Errorf("want args to be of type InterPodAffinityArgs, got %T", obj)

pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go

@@ -519,7 +519,7 @@ func TestPreferredAffinity(t *testing.T) {
 			snapshot := cache.NewSnapshot(test.pods, test.nodes)
 			p := &InterPodAffinity{
 				args: config.InterPodAffinityArgs{
-					HardPodAffinityWeight: DefaultHardPodAffinityWeight,
+					HardPodAffinityWeight: 1,
 				},
 				sharedLister: snapshot,
 			}

pkg/scheduler/framework/plugins/nodelabel/node_label.go

@@ -68,9 +68,6 @@ func New(plArgs runtime.Object, handle framework.FrameworkHandle) (framework.Plu
 }
 
 func getArgs(obj runtime.Object) (config.NodeLabelArgs, error) {
-	if obj == nil {
-		return config.NodeLabelArgs{}, nil
-	}
 	ptr, ok := obj.(*config.NodeLabelArgs)
 	if !ok {
 		return config.NodeLabelArgs{}, fmt.Errorf("want args to be of type NodeLabelArgs, got %T", obj)