Skip to content

Commit

Permalink
refactor: rename GetSchedulerConfig -> ConfigFromPath
Browse files Browse the repository at this point in the history
- `scheduler.ConfigFromPath` is shorter and feels less vague than `scheduler.GetSchedulerConfig`
- move test config to a new package `test` under `config` package
Signed-off-by: vadasambar <surajrbanakar@gmail.com>
  • Loading branch information
vadasambar committed Jun 29, 2023
1 parent 61ce656 commit ea715bb
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 66 deletions.
49 changes: 0 additions & 49 deletions cluster-autoscaler/config/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,53 +36,4 @@ const (
DefaultScaleDownUnreadyTimeKey = "scaledownunreadytime"
// DefaultMaxNodeProvisionTimeKey identifies MaxNodeProvisionTime autoscaling option
DefaultMaxNodeProvisionTimeKey = "maxnodeprovisiontime"

// Custom scheduler configs for testing

// SchedulerConfigNodeResourcesFitDisabled is scheduler config
// with `NodeResourcesFit` plugin disabled
SchedulerConfigNodeResourcesFitDisabled = `
apiVersion: kubescheduler.config.k8s.io/v1
kind: KubeSchedulerConfiguration
profiles:
- pluginConfig:
plugins:
multiPoint:
disabled:
- name: NodeResourcesFit
weight: 1
schedulerName: custom-scheduler`

// SchedulerConfigTaintTolerationDisabled is scheduler config
// with `TaintToleration` plugin disabled
SchedulerConfigTaintTolerationDisabled = `
apiVersion: kubescheduler.config.k8s.io/v1
kind: KubeSchedulerConfiguration
profiles:
- pluginConfig:
plugins:
multiPoint:
disabled:
- name: TaintToleration
weight: 1
schedulerName: custom-scheduler`

// 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`

// SchedulerConfigInvalid is invalid scheduler config
// because we specify percentageOfNodesToScore > 100
SchedulerConfigInvalid = `
apiVersion: kubescheduler.config.k8s.io/v1
kind: KubeSchedulerConfiguration
# percentageOfNodesToScore has to be between 0 and 100
percentageOfNodesToScore: 130`
)
68 changes: 68 additions & 0 deletions cluster-autoscaler/config/test/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
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.
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 test

const (
// Custom scheduler configs for testing

// SchedulerConfigNodeResourcesFitDisabled is scheduler config
// with `NodeResourcesFit` plugin disabled
SchedulerConfigNodeResourcesFitDisabled = `
apiVersion: kubescheduler.config.k8s.io/v1
kind: KubeSchedulerConfiguration
profiles:
- pluginConfig:
plugins:
multiPoint:
disabled:
- name: NodeResourcesFit
weight: 1
schedulerName: custom-scheduler`

// SchedulerConfigTaintTolerationDisabled is scheduler config
// with `TaintToleration` plugin disabled
SchedulerConfigTaintTolerationDisabled = `
apiVersion: kubescheduler.config.k8s.io/v1
kind: KubeSchedulerConfiguration
profiles:
- pluginConfig:
plugins:
multiPoint:
disabled:
- name: TaintToleration
weight: 1
schedulerName: custom-scheduler`

// 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`

// SchedulerConfigInvalid is invalid scheduler config
// because we specify percentageOfNodesToScore > 100
SchedulerConfigInvalid = `
apiVersion: kubescheduler.config.k8s.io/v1
kind: KubeSchedulerConfiguration
# percentageOfNodesToScore has to be between 0 and 100
percentageOfNodesToScore: 130`
)
2 changes: 1 addition & 1 deletion cluster-autoscaler/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func createAutoscalingOptions() config.AutoscalingOptions {
var parsedSchedConfig *scheduler_config.KubeSchedulerConfiguration
// if scheduler config flag was set by the user
if pflag.CommandLine.Changed(config.SchedulerConfigFileFlag) {
parsedSchedConfig, err = scheduler_util.GetSchedulerConfig(*schedulerConfigFile)
parsedSchedConfig, err = scheduler_util.ConfigFromPath(*schedulerConfigFile)
}
if err != nil {
klog.Fatalf("Failed to get scheduler config: %v", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"testing"
"time"

"k8s.io/autoscaler/cluster-autoscaler/config"
testconfig "k8s.io/autoscaler/cluster-autoscaler/config/test"
"k8s.io/autoscaler/cluster-autoscaler/simulator/clustersnapshot"
scheduler "k8s.io/autoscaler/cluster-autoscaler/utils/scheduler"
. "k8s.io/autoscaler/cluster-autoscaler/utils/test"
Expand Down Expand Up @@ -55,12 +55,12 @@ func TestCheckPredicate(t *testing.T) {

customConfigFile := filepath.Join(tmpDir, "custom_config.yaml")
if err := os.WriteFile(customConfigFile,
[]byte(config.SchedulerConfigNodeResourcesFitDisabled),
[]byte(testconfig.SchedulerConfigNodeResourcesFitDisabled),
os.FileMode(0600)); err != nil {
t.Fatal(err)
}

customConfig, err := scheduler.GetSchedulerConfig(customConfigFile)
customConfig, err := scheduler.ConfigFromPath(customConfigFile)
assert.NoError(t, err)
customPredicateChecker, err := NewTestPredicateCheckerWithCustomConfig(customConfig)
assert.NoError(t, err)
Expand Down Expand Up @@ -180,12 +180,12 @@ func TestFitsAnyNode(t *testing.T) {

customConfigFile := filepath.Join(tmpDir, "custom_config.yaml")
if err := os.WriteFile(customConfigFile,
[]byte(config.SchedulerConfigNodeResourcesFitDisabled),
[]byte(testconfig.SchedulerConfigNodeResourcesFitDisabled),
os.FileMode(0600)); err != nil {
t.Fatal(err)
}

customConfig, err := scheduler.GetSchedulerConfig(customConfigFile)
customConfig, err := scheduler.ConfigFromPath(customConfigFile)
assert.NoError(t, err)
customPredicateChecker, err := NewTestPredicateCheckerWithCustomConfig(customConfig)
assert.NoError(t, err)
Expand Down Expand Up @@ -304,12 +304,12 @@ func TestDebugInfo(t *testing.T) {

customConfigFile := filepath.Join(tmpDir, "custom_config.yaml")
if err := os.WriteFile(customConfigFile,
[]byte(config.SchedulerConfigTaintTolerationDisabled),
[]byte(testconfig.SchedulerConfigTaintTolerationDisabled),
os.FileMode(0600)); err != nil {
t.Fatal(err)
}

customConfig, err := scheduler.GetSchedulerConfig(customConfigFile)
customConfig, err := scheduler.ConfigFromPath(customConfigFile)
assert.NoError(t, err)
customPredicateChecker, err := NewTestPredicateCheckerWithCustomConfig(customConfig)
assert.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions cluster-autoscaler/utils/scheduler/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ func ResourceToResourceList(r *schedulerframework.Resource) apiv1.ResourceList {
return result
}

// GetSchedulerConfig loads scheduler config from a path.
func GetSchedulerConfig(path string) (*scheduler_config.KubeSchedulerConfiguration, error) {
// ConfigFromPath loads scheduler config from a path.
func ConfigFromPath(path string) (*scheduler_config.KubeSchedulerConfiguration, error) {
data, err := os.ReadFile(path)
if err != nil {
return nil, fmt.Errorf("%s: %v", schedulerConfigLoadErr, err)
Expand Down
14 changes: 7 additions & 7 deletions cluster-autoscaler/utils/scheduler/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"testing"

"k8s.io/apimachinery/pkg/api/resource"
caconfig "k8s.io/autoscaler/cluster-autoscaler/config"
testconfig "k8s.io/autoscaler/cluster-autoscaler/config/test"
. "k8s.io/autoscaler/cluster-autoscaler/utils/test"
"k8s.io/kubernetes/pkg/scheduler/apis/config"
schedulerframework "k8s.io/kubernetes/pkg/scheduler/framework"
Expand Down Expand Up @@ -107,7 +107,7 @@ func TestResourceList(t *testing.T) {
}
}

func TestGetSchedulerConfig(t *testing.T) {
func TestConfigFromPath(t *testing.T) {
// temp dir
tmpDir, err := os.MkdirTemp("", "scheduler-configs")
if err != nil {
Expand All @@ -116,25 +116,25 @@ func TestGetSchedulerConfig(t *testing.T) {
defer os.RemoveAll(tmpDir)

// Note that even if we are passing minimal config like below
// `GetSchedulerConfig` will set the rest of the default fields
// `ConfigFromPath` will set the rest of the default fields
// on its own (including default profile and default plugins)
correctConfigFile := filepath.Join(tmpDir, "correct_config.yaml")
if err := os.WriteFile(correctConfigFile,
[]byte(caconfig.SchedulerConfigMinimalCorrect),
[]byte(testconfig.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(caconfig.SchedulerConfigDecodeErr),
[]byte(testconfig.SchedulerConfigDecodeErr),
os.FileMode(0600)); err != nil {
t.Fatal(err)
}

validationErrConfigFile := filepath.Join(tmpDir, "invalid_percent_node_score_config.yaml")
if err := os.WriteFile(validationErrConfigFile,
[]byte(caconfig.SchedulerConfigInvalid),
[]byte(testconfig.SchedulerConfigInvalid),
os.FileMode(0600)); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestGetSchedulerConfig(t *testing.T) {

for i, test := range tests {
t.Run(fmt.Sprintf("case_%d: %s", i, test.name), func(t *testing.T) {
cfg, err := GetSchedulerConfig(test.path)
cfg, err := ConfigFromPath(test.path)
if test.expectedConfig == nil {
assert.Nil(t, cfg)
} else {
Expand Down

0 comments on commit ea715bb

Please sign in to comment.