Merge pull request #87853 from alculquicondor/fix/options_test

Do lenient decoding only for kubescheduler.config.k8s.io/v1alpha1

cmd/kube-scheduler/app/options/BUILD

@@ -40,7 +40,7 @@ go_library(
         "//staging/src/k8s.io/component-base/cli/flag:go_default_library",
         "//staging/src/k8s.io/component-base/codec:go_default_library",
         "//staging/src/k8s.io/component-base/config:go_default_library",
-        "//staging/src/k8s.io/kube-scheduler/config/v1alpha1:go_default_library",
+        "//staging/src/k8s.io/kube-scheduler/config/v1alpha2:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
     ],
@@ -73,10 +73,10 @@ 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/diff:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/rand:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/server/options:go_default_library",
         "//staging/src/k8s.io/component-base/config:go_default_library",
+        "//vendor/github.com/google/go-cmp/cmp:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
     ],
 )

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

@@ -41,9 +41,8 @@ func loadConfigFromFile(file string) (*kubeschedulerconfig.KubeSchedulerConfigur
 }
 
 func loadConfig(data []byte) (*kubeschedulerconfig.KubeSchedulerConfiguration, error) {
-	configObj := &kubeschedulerconfig.KubeSchedulerConfiguration{}
 	// The UniversalDecoder runs defaulting and returns the internal type by default.
-	err := runtime.DecodeInto(kubeschedulerscheme.Codecs.UniversalDecoder(), data, configObj)
+	obj, gvk, err := kubeschedulerscheme.Codecs.UniversalDecoder().Decode(data, nil, nil)
 	if err != nil {
 		// Try strict decoding first. If that fails decode with a lenient
 		// decoder, which has only v1alpha1 registered, and log a warning.
@@ -56,18 +55,20 @@ func loadConfig(data []byte) (*kubeschedulerconfig.KubeSchedulerConfiguration, e
 		_, lenientCodecs, lenientErr := codec.NewLenientSchemeAndCodecs(
 			kubeschedulerconfig.AddToScheme,
 			kubeschedulerconfigv1alpha1.AddToScheme,
-			kubeschedulerconfigv1alpha2.AddToScheme,
 		)
 		if lenientErr != nil {
 			return nil, lenientErr
 		}
-		if lenientErr = runtime.DecodeInto(lenientCodecs.UniversalDecoder(), data, configObj); lenientErr != nil {
-			return nil, fmt.Errorf("failed lenient decoding: %v", err)
+		obj, gvk, lenientErr = lenientCodecs.UniversalDecoder().Decode(data, nil, nil)
+		if lenientErr != nil {
+			return nil, err
 		}
 		klog.Warningf("using lenient decoding as strict decoding failed: %v", err)
 	}
-
-	return configObj, nil
+	if cfgObj, ok := obj.(*kubeschedulerconfig.KubeSchedulerConfiguration); ok {
+		return cfgObj, nil
+	}
+	return nil, fmt.Errorf("couldn't decode as KubeSchedulerConfiguration, got %s: ", gvk)
 }
 
 // WriteConfigFile writes the config into the given file name as YAML.

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

@@ -40,7 +40,7 @@ import (
 	cliflag "k8s.io/component-base/cli/flag"
 	componentbaseconfig "k8s.io/component-base/config"
 	"k8s.io/klog"
-	kubeschedulerconfigv1alpha1 "k8s.io/kube-scheduler/config/v1alpha1"
+	kubeschedulerconfigv1alpha2 "k8s.io/kube-scheduler/config/v1alpha2"
 	schedulerappconfig "k8s.io/kubernetes/cmd/kube-scheduler/app/config"
 	"k8s.io/kubernetes/pkg/client/leaderelectionconfig"
 	"k8s.io/kubernetes/pkg/master/ports"
@@ -129,10 +129,10 @@ func splitHostIntPort(s string) (string, int, error) {
 }
 
 func newDefaultComponentConfig() (*kubeschedulerconfig.KubeSchedulerConfiguration, error) {
-	cfgv1alpha1 := kubeschedulerconfigv1alpha1.KubeSchedulerConfiguration{}
-	kubeschedulerscheme.Scheme.Default(&cfgv1alpha1)
+	versionedCfg := kubeschedulerconfigv1alpha2.KubeSchedulerConfiguration{}
+	kubeschedulerscheme.Scheme.Default(&versionedCfg)
 	cfg := kubeschedulerconfig.KubeSchedulerConfiguration{}
-	if err := kubeschedulerscheme.Scheme.Convert(&cfgv1alpha1, &cfg, nil); err != nil {
+	if err := kubeschedulerscheme.Scheme.Convert(&versionedCfg, &cfg, nil); err != nil {
 		return nil, err
 	}
 	return &cfg, nil

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

@@ -24,15 +24,14 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
-	"reflect"
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/util/diff"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
 	componentbaseconfig "k8s.io/component-base/config"
 	kubeschedulerconfig "k8s.io/kubernetes/pkg/scheduler/apis/config"
@@ -73,7 +72,7 @@ func TestSchedulerOptions(t *testing.T) {
 	configFile := filepath.Join(tmpDir, "scheduler.yaml")
 	configKubeconfig := filepath.Join(tmpDir, "config.kubeconfig")
 	if err := ioutil.WriteFile(configFile, []byte(fmt.Sprintf(`
-apiVersion: kubescheduler.config.k8s.io/v1alpha1
+apiVersion: kubescheduler.config.k8s.io/v1alpha2
 kind: KubeSchedulerConfiguration
 clientConnection:
   kubeconfig: "%s"
@@ -103,8 +102,8 @@ users:
 		t.Fatal(err)
 	}
 
-	oldconfigFile := filepath.Join(tmpDir, "scheduler_old.yaml")
-	if err := ioutil.WriteFile(oldconfigFile, []byte(fmt.Sprintf(`
+	oldConfigFile := filepath.Join(tmpDir, "scheduler_old.yaml")
+	if err := ioutil.WriteFile(oldConfigFile, []byte(fmt.Sprintf(`
 apiVersion: componentconfig/v1alpha1
 kind: KubeSchedulerConfiguration
 clientConnection:
@@ -114,9 +113,9 @@ leaderElection:
 		t.Fatal(err)
 	}
 
-	invalidconfigFile := filepath.Join(tmpDir, "scheduler_invalid_wrong_api_version.yaml")
-	if err := ioutil.WriteFile(invalidconfigFile, []byte(fmt.Sprintf(`
-apiVersion: componentconfig/v1alpha2
+	unknownVersionConfig := filepath.Join(tmpDir, "scheduler_invalid_wrong_api_version.yaml")
+	if err := ioutil.WriteFile(unknownVersionConfig, []byte(fmt.Sprintf(`
+apiVersion: kubescheduler.config.k8s.io/unknown
 kind: KubeSchedulerConfiguration
 clientConnection:
   kubeconfig: "%s"
@@ -125,8 +124,18 @@ leaderElection:
 		t.Fatal(err)
 	}
 
-	unknownFieldConfig := filepath.Join(tmpDir, "scheduler_invalid_unknown_field.yaml")
-	if err := ioutil.WriteFile(unknownFieldConfig, []byte(fmt.Sprintf(`
+	noVersionConfig := filepath.Join(tmpDir, "scheduler_invalid_no_version.yaml")
+	if err := ioutil.WriteFile(noVersionConfig, []byte(fmt.Sprintf(`
+kind: KubeSchedulerConfiguration
+clientConnection:
+  kubeconfig: "%s"
+leaderElection:
+  leaderElect: true`, configKubeconfig)), os.FileMode(0600)); err != nil {
+		t.Fatal(err)
+	}
+
+	unknownFieldConfigLenient := filepath.Join(tmpDir, "scheduler_invalid_unknown_field_lenient.yaml")
+	if err := ioutil.WriteFile(unknownFieldConfigLenient, []byte(fmt.Sprintf(`
 apiVersion: kubescheduler.config.k8s.io/v1alpha1
 kind: KubeSchedulerConfiguration
 clientConnection:
@@ -137,9 +146,33 @@ foo: bar`, configKubeconfig)), os.FileMode(0600)); err != nil {
 		t.Fatal(err)
 	}
 
+	unknownFieldConfig := filepath.Join(tmpDir, "scheduler_invalid_unknown_field.yaml")
+	if err := ioutil.WriteFile(unknownFieldConfig, []byte(fmt.Sprintf(`
+apiVersion: kubescheduler.config.k8s.io/v1alpha2
+kind: KubeSchedulerConfiguration
+clientConnection:
+  kubeconfig: "%s"
+leaderElection:
+  leaderElect: true
+foo: bar`, configKubeconfig)), os.FileMode(0600)); err != nil {
+		t.Fatal(err)
+	}
+
+	duplicateFieldConfigLenient := filepath.Join(tmpDir, "scheduler_invalid_duplicate_fields_lenient.yaml")
+	if err := ioutil.WriteFile(duplicateFieldConfigLenient, []byte(fmt.Sprintf(`
+apiVersion: kubescheduler.config.k8s.io/v1alpha1
+kind: KubeSchedulerConfiguration
+clientConnection:
+  kubeconfig: "%s"
+leaderElection:
+  leaderElect: true
+  leaderElect: false`, configKubeconfig)), os.FileMode(0600)); err != nil {
+		t.Fatal(err)
+	}
+
 	duplicateFieldConfig := filepath.Join(tmpDir, "scheduler_invalid_duplicate_fields.yaml")
 	if err := ioutil.WriteFile(duplicateFieldConfig, []byte(fmt.Sprintf(`
-apiVersion: kubescheduler.config.k8s.io/v1alpha1
+apiVersion: kubescheduler.config.k8s.io/v1alpha2
 kind: KubeSchedulerConfiguration
 clientConnection:
   kubeconfig: "%s"
@@ -176,7 +209,7 @@ users:
 	// plugin config
 	pluginconfigFile := filepath.Join(tmpDir, "plugin.yaml")
 	if err := ioutil.WriteFile(pluginconfigFile, []byte(fmt.Sprintf(`
-apiVersion: kubescheduler.config.k8s.io/v1alpha1
+apiVersion: kubescheduler.config.k8s.io/v1alpha2
 kind: KubeSchedulerConfiguration
 clientConnection:
   kubeconfig: "%s"
@@ -293,7 +326,7 @@ pluginConfig:
 		{
 			name: "config file in componentconfig/v1alpha1",
 			options: &Options{
-				ConfigFile: oldconfigFile,
+				ConfigFile: oldConfigFile,
 				ComponentConfig: func() kubeschedulerconfig.KubeSchedulerConfiguration {
 					cfg, err := newDefaultComponentConfig()
 					if err != nil {
@@ -306,9 +339,14 @@ pluginConfig:
 		},
 
 		{
-			name:          "invalid config file in componentconfig/v1alpha2",
-			options:       &Options{ConfigFile: invalidconfigFile},
-			expectedError: "no kind \"KubeSchedulerConfiguration\" is registered for version \"componentconfig/v1alpha2\"",
+			name:          "unknown version kubescheduler.config.k8s.io/unknown",
+			options:       &Options{ConfigFile: unknownVersionConfig},
+			expectedError: "no kind \"KubeSchedulerConfiguration\" is registered for version \"kubescheduler.config.k8s.io/unknown\"",
+		},
+		{
+			name:          "config file with no version",
+			options:       &Options{ConfigFile: noVersionConfig},
+			expectedError: "Object 'apiVersion' is missing",
 		},
 		{
 			name: "kubeconfig flag",
@@ -522,9 +560,9 @@ pluginConfig:
 			expectedError: "no configuration has been provided",
 		},
 		{
-			name: "unknown field",
+			name: "unknown field lenient (v1alpha1)",
 			options: &Options{
-				ConfigFile: unknownFieldConfig,
+				ConfigFile: unknownFieldConfigLenient,
 			},
 			// TODO (obitech): Remove this comment and add a new test for v1alpha2, when it's available, as this should fail then.
 			// expectedError: "found unknown field: foo",
@@ -565,9 +603,17 @@ pluginConfig:
 			},
 		},
 		{
-			name: "duplicate fields",
+			name: "unknown field",
 			options: &Options{
-				ConfigFile: duplicateFieldConfig,
+				ConfigFile: unknownFieldConfig,
+			},
+			expectedError: "found unknown field: foo",
+			checkErrFn:    runtime.IsStrictDecodingError,
+		},
+		{
+			name: "duplicate fields lenient (v1alpha1)",
+			options: &Options{
+				ConfigFile: duplicateFieldConfigLenient,
 			},
 			// TODO (obitech): Remove this comment and add a new test for v1alpha2, when it's available, as this should fail then.
 			// expectedError: `key "leaderElect" already set`,
@@ -607,6 +653,14 @@ pluginConfig:
 				Plugins:                  nil,
 			},
 		},
+		{
+			name: "duplicate fields",
+			options: &Options{
+				ConfigFile: duplicateFieldConfig,
+			},
+			expectedError: `key "leaderElect" already set`,
+			checkErrFn:    runtime.IsStrictDecodingError,
+		},
 	}
 
 	for _, tc := range testcases {
@@ -618,7 +672,7 @@ pluginConfig:
 			if err != nil {
 				if tc.expectedError != "" || tc.checkErrFn != nil {
 					if tc.expectedError != "" {
-						assert.Contains(t, err.Error(), tc.expectedError, tc.name)
+						assert.Contains(t, err.Error(), tc.expectedError)
 					}
 					if tc.checkErrFn != nil {
 						assert.True(t, tc.checkErrFn(err), "got error: %v", err)
@@ -629,8 +683,8 @@ pluginConfig:
 				return
 			}
 
-			if !reflect.DeepEqual(config.ComponentConfig, tc.expectedConfig) {
-				t.Errorf("config.diff:\n%s", diff.ObjectReflectDiff(tc.expectedConfig, config.ComponentConfig))
+			if diff := cmp.Diff(tc.expectedConfig, config.ComponentConfig); diff != "" {
+				t.Errorf("incorrect config (-want, +got):\n%s", diff)
 			}
 
 			// ensure we have a client

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

@@ -40,9 +40,9 @@ func init() {
 
 // AddToScheme builds the kubescheduler scheme using all known versions of the kubescheduler api.
 func AddToScheme(scheme *runtime.Scheme) {
-	utilruntime.Must(kubeschedulerconfig.AddToScheme(Scheme))
-	utilruntime.Must(kubeschedulerconfigv1.AddToScheme(Scheme))
-	utilruntime.Must(kubeschedulerconfigv1alpha1.AddToScheme(Scheme))
-	utilruntime.Must(kubeschedulerconfigv1alpha2.AddToScheme(Scheme))
-	utilruntime.Must(scheme.SetVersionPriority(kubeschedulerconfigv1alpha1.SchemeGroupVersion))
+	utilruntime.Must(kubeschedulerconfig.AddToScheme(scheme))
+	utilruntime.Must(kubeschedulerconfigv1.AddToScheme(scheme))
+	utilruntime.Must(kubeschedulerconfigv1alpha1.AddToScheme(scheme))
+	utilruntime.Must(kubeschedulerconfigv1alpha2.AddToScheme(scheme))
+	utilruntime.Must(scheme.SetVersionPriority(kubeschedulerconfigv1alpha2.SchemeGroupVersion, kubeschedulerconfigv1alpha1.SchemeGroupVersion))
 }