From b15aba547a633e6e89e1c5ff85c1fadddba6d537 Mon Sep 17 00:00:00 2001 From: obitech Date: Sun, 20 Oct 2019 16:15:36 +0200 Subject: [PATCH] Add lenient decoding path for v1alpha1 kube-scheduler config This implements a lenient path for decoding a kube-scheduler config file. The config file gets decoded with a strict serializer first, if that fails a lenient CodecFactory that has just v1alpha1 registered into it is used for decoding. The lenient path is to be dropped when support for v1alpha1 is dropped. For more information on the discussion see #82924 and the linked PRs. --- cmd/kube-scheduler/app/options/BUILD | 1 + cmd/kube-scheduler/app/options/configfile.go | 27 ++++++- .../app/options/options_test.go | 70 +++++++++++++++++-- staging/src/k8s.io/component-base/BUILD | 1 + staging/src/k8s.io/component-base/codec/BUILD | 27 +++++++ .../src/k8s.io/component-base/codec/codec.go | 39 +++++++++++ vendor/modules.txt | 1 + 7 files changed, 159 insertions(+), 7 deletions(-) create mode 100644 staging/src/k8s.io/component-base/codec/BUILD create mode 100644 staging/src/k8s.io/component-base/codec/codec.go diff --git a/cmd/kube-scheduler/app/options/BUILD b/cmd/kube-scheduler/app/options/BUILD index d5b439fbb5d..1d7d6038e06 100644 --- a/cmd/kube-scheduler/app/options/BUILD +++ b/cmd/kube-scheduler/app/options/BUILD @@ -36,6 +36,7 @@ go_library( "//staging/src/k8s.io/client-go/tools/leaderelection/resourcelock:go_default_library", "//staging/src/k8s.io/client-go/tools/record:go_default_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", "//vendor/github.com/spf13/pflag:go_default_library", diff --git a/cmd/kube-scheduler/app/options/configfile.go b/cmd/kube-scheduler/app/options/configfile.go index cdd03b4726f..04f41d45411 100644 --- a/cmd/kube-scheduler/app/options/configfile.go +++ b/cmd/kube-scheduler/app/options/configfile.go @@ -21,7 +21,10 @@ import ( "io/ioutil" "os" + "k8s.io/klog" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/component-base/codec" kubeschedulerconfig "k8s.io/kubernetes/pkg/scheduler/apis/config" kubeschedulerscheme "k8s.io/kubernetes/pkg/scheduler/apis/config/scheme" kubeschedulerconfigv1alpha1 "k8s.io/kubernetes/pkg/scheduler/apis/config/v1alpha1" @@ -38,8 +41,28 @@ func loadConfigFromFile(file string) (*kubeschedulerconfig.KubeSchedulerConfigur func loadConfig(data []byte) (*kubeschedulerconfig.KubeSchedulerConfiguration, error) { configObj := &kubeschedulerconfig.KubeSchedulerConfiguration{} - if err := runtime.DecodeInto(kubeschedulerscheme.Codecs.UniversalDecoder(), data, configObj); err != nil { - return nil, err + // The UniversalDecoder runs defaulting and returns the internal type by default. + err := runtime.DecodeInto(kubeschedulerscheme.Codecs.UniversalDecoder(), data, configObj) + if err != nil { + // Try strict decoding first. If that fails decode with a lenient + // decoder, which has only v1alpha1 registered, and log a warning. + // The lenient path is to be dropped when support for v1alpha1 is dropped. + if !runtime.IsStrictDecodingError(err) { + return nil, err + } + + var lenientErr error + _, lenientCodecs, lenientErr := codec.NewLenientSchemeAndCodecs( + kubeschedulerconfig.AddToScheme, + kubeschedulerconfigv1alpha1.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) + } + klog.Warningf("using lenient decoding as strict decoding failed: %v", err) } return configObj, nil diff --git a/cmd/kube-scheduler/app/options/options_test.go b/cmd/kube-scheduler/app/options/options_test.go index 7f5b19c2de7..325eec3e215 100644 --- a/cmd/kube-scheduler/app/options/options_test.go +++ b/cmd/kube-scheduler/app/options/options_test.go @@ -472,16 +472,76 @@ pluginConfig: options: &Options{ ConfigFile: unknownFieldConfig, }, - expectedError: "found unknown field: foo", - checkErrFn: runtime.IsStrictDecodingError, + // 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", + // checkErrFn: runtime.IsStrictDecodingError, + expectedUsername: "config", + expectedConfig: kubeschedulerconfig.KubeSchedulerConfiguration{ + SchedulerName: "default-scheduler", + AlgorithmSource: kubeschedulerconfig.SchedulerAlgorithmSource{Provider: &defaultSource}, + HardPodAffinitySymmetricWeight: 1, + HealthzBindAddress: "0.0.0.0:10251", + MetricsBindAddress: "0.0.0.0:10251", + LeaderElection: kubeschedulerconfig.KubeSchedulerLeaderElectionConfiguration{ + LeaderElectionConfiguration: componentbaseconfig.LeaderElectionConfiguration{ + LeaderElect: true, + LeaseDuration: metav1.Duration{Duration: 15 * time.Second}, + RenewDeadline: metav1.Duration{Duration: 10 * time.Second}, + RetryPeriod: metav1.Duration{Duration: 2 * time.Second}, + ResourceLock: "endpointsleases", + ResourceNamespace: "kube-system", + ResourceName: "kube-scheduler", + }, + }, + ClientConnection: componentbaseconfig.ClientConnectionConfiguration{ + Kubeconfig: configKubeconfig, + QPS: 50, + Burst: 100, + ContentType: "application/vnd.kubernetes.protobuf", + }, + BindTimeoutSeconds: &defaultBindTimeoutSeconds, + PodInitialBackoffSeconds: &defaultPodInitialBackoffSeconds, + PodMaxBackoffSeconds: &defaultPodMaxBackoffSeconds, + Plugins: nil, + }, }, { name: "duplicate fields", options: &Options{ ConfigFile: duplicateFieldConfig, }, - expectedError: `key "leaderElect" already set`, - checkErrFn: runtime.IsStrictDecodingError, + // 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`, + // checkErrFn: runtime.IsStrictDecodingError, + expectedUsername: "config", + expectedConfig: kubeschedulerconfig.KubeSchedulerConfiguration{ + SchedulerName: "default-scheduler", + AlgorithmSource: kubeschedulerconfig.SchedulerAlgorithmSource{Provider: &defaultSource}, + HardPodAffinitySymmetricWeight: 1, + HealthzBindAddress: "0.0.0.0:10251", + MetricsBindAddress: "0.0.0.0:10251", + LeaderElection: kubeschedulerconfig.KubeSchedulerLeaderElectionConfiguration{ + LeaderElectionConfiguration: componentbaseconfig.LeaderElectionConfiguration{ + LeaderElect: false, + LeaseDuration: metav1.Duration{Duration: 15 * time.Second}, + RenewDeadline: metav1.Duration{Duration: 10 * time.Second}, + RetryPeriod: metav1.Duration{Duration: 2 * time.Second}, + ResourceLock: "endpointsleases", + ResourceNamespace: "kube-system", + ResourceName: "kube-scheduler", + }, + }, + ClientConnection: componentbaseconfig.ClientConnectionConfiguration{ + Kubeconfig: configKubeconfig, + QPS: 50, + Burst: 100, + ContentType: "application/vnd.kubernetes.protobuf", + }, + BindTimeoutSeconds: &defaultBindTimeoutSeconds, + PodInitialBackoffSeconds: &defaultPodInitialBackoffSeconds, + PodMaxBackoffSeconds: &defaultPodMaxBackoffSeconds, + Plugins: nil, + }, }, } @@ -523,7 +583,7 @@ pluginConfig: return } if username != tc.expectedUsername { - t.Errorf("expected server call with user %s, got %s", tc.expectedUsername, username) + t.Errorf("expected server call with user %q, got %q", tc.expectedUsername, username) } }) } diff --git a/staging/src/k8s.io/component-base/BUILD b/staging/src/k8s.io/component-base/BUILD index d15d0769f49..c8a16fdf511 100644 --- a/staging/src/k8s.io/component-base/BUILD +++ b/staging/src/k8s.io/component-base/BUILD @@ -11,6 +11,7 @@ filegroup( ":package-srcs", "//staging/src/k8s.io/component-base/cli/flag:all-srcs", "//staging/src/k8s.io/component-base/cli/globalflag:all-srcs", + "//staging/src/k8s.io/component-base/codec:all-srcs", "//staging/src/k8s.io/component-base/config:all-srcs", "//staging/src/k8s.io/component-base/featuregate:all-srcs", "//staging/src/k8s.io/component-base/logs:all-srcs", diff --git a/staging/src/k8s.io/component-base/codec/BUILD b/staging/src/k8s.io/component-base/codec/BUILD new file mode 100644 index 00000000000..9e848249cb6 --- /dev/null +++ b/staging/src/k8s.io/component-base/codec/BUILD @@ -0,0 +1,27 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["codec.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/component-base/codec", + importpath = "k8s.io/component-base/codec", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/component-base/codec/codec.go b/staging/src/k8s.io/component-base/codec/codec.go new file mode 100644 index 00000000000..58a0235b204 --- /dev/null +++ b/staging/src/k8s.io/component-base/codec/codec.go @@ -0,0 +1,39 @@ +/* +Copyright 2019 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 codec + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" +) + +// NewLenientSchemeAndCodecs constructs a CodecFactory with strict decoding +// disabled, that has only the Schemes registered into it which are passed +// and added via AddToScheme functions. This can be used to skip strict decoding +// a specific version only. +func NewLenientSchemeAndCodecs(addToSchemeFns ...func(s *runtime.Scheme) error) (*runtime.Scheme, *serializer.CodecFactory, error) { + lenientScheme := runtime.NewScheme() + for _, s := range addToSchemeFns { + if err := s(lenientScheme); err != nil { + return nil, nil, fmt.Errorf("unable to add API to lenient scheme: %v", err) + } + } + lenientCodecs := serializer.NewCodecFactory(lenientScheme, serializer.DisableStrict) + return lenientScheme, &lenientCodecs, nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 56441d9350b..4897d38f3fa 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1667,6 +1667,7 @@ k8s.io/code-generator/third_party/forked/golang/reflect # k8s.io/component-base v0.0.0 => ./staging/src/k8s.io/component-base k8s.io/component-base/cli/flag k8s.io/component-base/cli/globalflag +k8s.io/component-base/codec k8s.io/component-base/config k8s.io/component-base/config/v1alpha1 k8s.io/component-base/config/validation