From 8e284389a5d1650c662917329471250c586a2868 Mon Sep 17 00:00:00 2001 From: vadasambar Date: Tue, 4 Jul 2023 10:32:14 +0530 Subject: [PATCH 1/5] feat: make `loadConfig` and `loadConfigFile` public functions - so that users who import kube-scheduler libraries can use these functions to read kube scheduler config --- cmd/kube-scheduler/app/options/configfile.go | 6 +++--- cmd/kube-scheduler/app/options/options.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/kube-scheduler/app/options/configfile.go b/cmd/kube-scheduler/app/options/configfile.go index 76fe84deb18..0f445e4f01e 100644 --- a/cmd/kube-scheduler/app/options/configfile.go +++ b/cmd/kube-scheduler/app/options/configfile.go @@ -30,16 +30,16 @@ import ( configv1beta3 "k8s.io/kubernetes/pkg/scheduler/apis/config/v1beta3" ) -func loadConfigFromFile(logger klog.Logger, file string) (*config.KubeSchedulerConfiguration, error) { +func LoadConfigFromFile(logger klog.Logger, file string) (*config.KubeSchedulerConfiguration, error) { data, err := os.ReadFile(file) if err != nil { return nil, err } - return loadConfig(logger, data) + return LoadConfig(logger, data) } -func loadConfig(logger klog.Logger, data []byte) (*config.KubeSchedulerConfiguration, error) { +func LoadConfig(logger klog.Logger, data []byte) (*config.KubeSchedulerConfiguration, error) { // The UniversalDecoder runs defaulting and returns the internal type by default. obj, gvk, err := scheme.Codecs.UniversalDecoder().Decode(data, nil, nil) if err != nil { diff --git a/cmd/kube-scheduler/app/options/options.go b/cmd/kube-scheduler/app/options/options.go index ac72f587c23..8f787396efb 100644 --- a/cmd/kube-scheduler/app/options/options.go +++ b/cmd/kube-scheduler/app/options/options.go @@ -204,7 +204,7 @@ func (o *Options) ApplyTo(logger klog.Logger, c *schedulerappconfig.Config) erro o.ApplyLeaderElectionTo(o.ComponentConfig) c.ComponentConfig = *o.ComponentConfig } else { - cfg, err := loadConfigFromFile(logger, o.ConfigFile) + cfg, err := LoadConfigFromFile(logger, o.ConfigFile) if err != nil { return err } From b3373ae2738db8250e6d1c3d2470accf487d6b0d Mon Sep 17 00:00:00 2001 From: vadasambar Date: Tue, 4 Jul 2023 11:19:30 +0530 Subject: [PATCH 2/5] refactor: make only `LoadConfigFromFile` public - `loadConfig` can be made public again when needed (not required now) --- cmd/kube-scheduler/app/options/configfile.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/kube-scheduler/app/options/configfile.go b/cmd/kube-scheduler/app/options/configfile.go index 0f445e4f01e..e91a91714a3 100644 --- a/cmd/kube-scheduler/app/options/configfile.go +++ b/cmd/kube-scheduler/app/options/configfile.go @@ -36,10 +36,10 @@ func LoadConfigFromFile(logger klog.Logger, file string) (*config.KubeSchedulerC return nil, err } - return LoadConfig(logger, data) + return loadConfig(logger, data) } -func LoadConfig(logger klog.Logger, data []byte) (*config.KubeSchedulerConfiguration, error) { +func loadConfig(logger klog.Logger, data []byte) (*config.KubeSchedulerConfiguration, error) { // The UniversalDecoder runs defaulting and returns the internal type by default. obj, gvk, err := scheme.Codecs.UniversalDecoder().Decode(data, nil, nil) if err != nil { From 72aeb961322f69e4d1a53a7c53a1b1472e307818 Mon Sep 17 00:00:00 2001 From: vadasambar Date: Tue, 4 Jul 2023 11:19:40 +0530 Subject: [PATCH 3/5] test: add tests for `LoadConfigFromFile` --- .../app/options/configfile_test.go | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 cmd/kube-scheduler/app/options/configfile_test.go diff --git a/cmd/kube-scheduler/app/options/configfile_test.go b/cmd/kube-scheduler/app/options/configfile_test.go new file mode 100644 index 00000000000..cdf8e119d84 --- /dev/null +++ b/cmd/kube-scheduler/app/options/configfile_test.go @@ -0,0 +1,136 @@ +/* +Copyright 2018 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 options + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/scheduler/apis/config" +) + +const ( + apiVersionMissing = "'apiVersion' is missing" + apiVersionTooOld = "no kind \"KubeSchedulerConfiguration\" is registered for" + + " version \"kubescheduler.config.k8s.io/v1alpha1\"" + fileNotFound = "no such file or directory" + + // schedulerConfigMinimalCorrect is the minimal + // correct scheduler config + schedulerConfigMinimalCorrect = ` +apiVersion: kubescheduler.config.k8s.io/v1 +kind: KubeSchedulerConfiguration` + + // schedulerConfigDecodeErr is the scheduler config + // which throws decoding error when we try to load it + schedulerConfigDecodeErr = ` +kind: KubeSchedulerConfiguration` + + // schedulerConfigVersionTooOld is the scheduler config + // which throws error because the config version 'v1alpha1' + // is too old + schedulerConfigVersionTooOld = ` +apiVersion: kubescheduler.config.k8s.io/v1alpha1 +kind: KubeSchedulerConfiguration +` +) + +func TestLoadConfigFromFile(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "scheduler-configs") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + correctConfigFile := filepath.Join(tmpDir, "correct_config.yaml") + if err := os.WriteFile(correctConfigFile, + []byte(schedulerConfigMinimalCorrect), + os.FileMode(0600)); err != nil { + t.Fatal(err) + } + + decodeErrConfigFile := filepath.Join(tmpDir, "decode_err_no_version_config.yaml") + if err := os.WriteFile(decodeErrConfigFile, + []byte(schedulerConfigDecodeErr), + os.FileMode(0600)); err != nil { + t.Fatal(err) + } + + versionTooOldConfigFile := filepath.Join(tmpDir, "version_too_old_config.yaml") + if err := os.WriteFile(versionTooOldConfigFile, + []byte(schedulerConfigVersionTooOld), + os.FileMode(0600)); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + path string + expectedErr error + expectedConfig *config.KubeSchedulerConfiguration + }{ + { + name: "Empty scheduler config file path", + path: "", + expectedErr: fmt.Errorf(fileNotFound), + expectedConfig: nil, + }, + { + name: "Correct scheduler config", + path: correctConfigFile, + expectedErr: nil, + expectedConfig: &config.KubeSchedulerConfiguration{}, + }, + { + name: "Scheduler config with decode error", + path: decodeErrConfigFile, + expectedErr: fmt.Errorf(apiVersionMissing), + expectedConfig: nil, + }, + { + name: "Scheduler config version too old", + path: versionTooOldConfigFile, + expectedErr: fmt.Errorf(apiVersionTooOld), + expectedConfig: nil, + }, + } + + logger := klog.FromContext(context.Background()) + + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d: %s", i, test.name), func(t *testing.T) { + cfg, err := LoadConfigFromFile(logger, test.path) + if test.expectedConfig == nil { + assert.Nil(t, cfg) + } else { + assert.NotNil(t, cfg) + } + + if test.expectedErr == nil { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, test.expectedErr.Error()) + } + }) + + } +} From 5c18810f3575d92f726970929a80484d47d2b338 Mon Sep 17 00:00:00 2001 From: vadasambar Date: Tue, 4 Jul 2023 11:43:50 +0530 Subject: [PATCH 4/5] chore: update license date for configfile tests --- cmd/kube-scheduler/app/options/configfile_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kube-scheduler/app/options/configfile_test.go b/cmd/kube-scheduler/app/options/configfile_test.go index cdf8e119d84..eb7bf84e3a3 100644 --- a/cmd/kube-scheduler/app/options/configfile_test.go +++ b/cmd/kube-scheduler/app/options/configfile_test.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Kubernetes Authors. +Copyright 2023 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. From c52911e59ad9483b936863710254b8772980d8c8 Mon Sep 17 00:00:00 2001 From: vadasambar Date: Tue, 11 Jul 2023 10:59:36 +0530 Subject: [PATCH 5/5] docs: add comment describing `LoadConfigFromFile` --- cmd/kube-scheduler/app/options/configfile.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/kube-scheduler/app/options/configfile.go b/cmd/kube-scheduler/app/options/configfile.go index e91a91714a3..6072248c3dc 100644 --- a/cmd/kube-scheduler/app/options/configfile.go +++ b/cmd/kube-scheduler/app/options/configfile.go @@ -30,6 +30,7 @@ import ( configv1beta3 "k8s.io/kubernetes/pkg/scheduler/apis/config/v1beta3" ) +// LoadConfigFromFile loads scheduler config from the specified file path func LoadConfigFromFile(logger klog.Logger, file string) (*config.KubeSchedulerConfiguration, error) { data, err := os.ReadFile(file) if err != nil {