Skip to content

Commit d5fd978

Browse files
authored
Merge pull request #586 from wzshiming/automated-cherry-pick-of-#583-upstream-release-0.2
Automated cherry pick of #583: Fix duplicate loading of default configuration
2 parents 9716a8d + 756630b commit d5fd978

File tree

2 files changed

+123
-20
lines changed

2 files changed

+123
-20
lines changed

pkg/config/flags.go

+35-20
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"sigs.k8s.io/kwok/pkg/consts"
2626
"sigs.k8s.io/kwok/pkg/log"
27+
"sigs.k8s.io/kwok/pkg/utils/file"
2728
"sigs.k8s.io/kwok/pkg/utils/path"
2829
"sigs.k8s.io/kwok/pkg/utils/slices"
2930
)
@@ -34,45 +35,59 @@ func InitFlags(ctx context.Context, flags *pflag.FlagSet) (context.Context, erro
3435
config := flags.StringArrayP("config", "c", []string{defaultConfigPath}, "config path")
3536
_ = flags.Parse(os.Args[1:])
3637

37-
existDefaultConfig := false
38-
configPath, err := path.Expand(defaultConfigPath)
39-
if err == nil {
40-
_, err = os.Stat(configPath)
41-
if err == nil {
42-
existDefaultConfig = true
43-
}
38+
// Expand the all config paths.
39+
defaultConfigPath, err := path.Expand(defaultConfigPath)
40+
if err != nil {
41+
return nil, err
4442
}
45-
46-
if !slices.Contains(*config, defaultConfigPath) {
47-
if existDefaultConfig {
48-
*config = append([]string{configPath}, *config...)
49-
}
50-
} else {
51-
if !existDefaultConfig {
52-
*config = slices.Filter(*config, func(s string) bool {
53-
return s != defaultConfigPath
54-
})
43+
configPaths := make([]string, 0, len(*config))
44+
for _, c := range *config {
45+
configPath, err := path.Expand(c)
46+
if err != nil {
47+
return nil, err
5548
}
49+
configPaths = append(configPaths, configPath)
5650
}
5751

52+
configPaths = loadConfig(configPaths, defaultConfigPath, file.Exists(defaultConfigPath))
53+
5854
logger := log.FromContext(ctx)
59-
objs, err := Load(ctx, *config...)
55+
objs, err := Load(ctx, configPaths...)
6056
if err != nil {
6157
return nil, err
6258
}
6359

6460
if len(objs) == 0 {
6561
logger.Debug("Load config",
66-
"path", *config,
62+
"path", configPaths,
6763
"err", "empty config",
6864
)
6965
} else {
7066
logger.Debug("Load config",
71-
"path", *config,
67+
"path", configPaths,
7268
"count", len(objs),
7369
"content", objs,
7470
)
7571
}
7672

7773
return setupContext(ctx, objs), nil
7874
}
75+
76+
// loadConfig loads the config paths.
77+
// ~/.kwok/kwok.yaml will be loaded first if it exists.
78+
func loadConfig(configPaths []string, defaultConfigPath string, existDefaultConfig bool) []string {
79+
if !slices.Contains(configPaths, defaultConfigPath) {
80+
if existDefaultConfig {
81+
// If the defaultConfigPath is not specified and the default config exists, it will be loaded first.
82+
return append([]string{defaultConfigPath}, configPaths...)
83+
}
84+
} else {
85+
if !existDefaultConfig {
86+
// If the defaultConfigPath is specified and the default config does not exist, it will be removed.
87+
return slices.Filter(configPaths, func(s string) bool {
88+
return s != defaultConfigPath
89+
})
90+
}
91+
}
92+
return configPaths
93+
}

pkg/config/flags_test.go

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
Copyright 2023 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package config
18+
19+
import (
20+
"reflect"
21+
"testing"
22+
)
23+
24+
func Test_loadConfig(t *testing.T) {
25+
type args struct {
26+
configPaths []string
27+
defaultConfigPath string
28+
existDefaultConfig bool
29+
}
30+
tests := []struct {
31+
name string
32+
args args
33+
want []string
34+
}{
35+
{
36+
name: "default config",
37+
args: args{
38+
configPaths: []string{},
39+
defaultConfigPath: "default",
40+
existDefaultConfig: true,
41+
},
42+
want: []string{"default"},
43+
},
44+
{
45+
name: "add default config",
46+
args: args{
47+
configPaths: []string{"config"},
48+
defaultConfigPath: "default",
49+
existDefaultConfig: true,
50+
},
51+
want: []string{"default", "config"},
52+
},
53+
{
54+
name: "no default config",
55+
args: args{
56+
configPaths: []string{"config"},
57+
defaultConfigPath: "default",
58+
existDefaultConfig: false,
59+
},
60+
want: []string{"config"},
61+
},
62+
{
63+
name: "no config",
64+
args: args{
65+
configPaths: []string{},
66+
defaultConfigPath: "default",
67+
existDefaultConfig: false,
68+
},
69+
want: []string{},
70+
},
71+
{
72+
name: "remove default config",
73+
args: args{
74+
configPaths: []string{"default", "config"},
75+
defaultConfigPath: "default",
76+
existDefaultConfig: false,
77+
},
78+
want: []string{"config"},
79+
},
80+
}
81+
for _, tt := range tests {
82+
t.Run(tt.name, func(t *testing.T) {
83+
if got := loadConfig(tt.args.configPaths, tt.args.defaultConfigPath, tt.args.existDefaultConfig); !reflect.DeepEqual(got, tt.want) {
84+
t.Errorf("loadConfig() = %v, want %v", got, tt.want)
85+
}
86+
})
87+
}
88+
}

0 commit comments

Comments
 (0)