diff --git a/cmd/minikube/cmd/root.go b/cmd/minikube/cmd/root.go index 025c030806..c7009d1d5f 100644 --- a/cmd/minikube/cmd/root.go +++ b/cmd/minikube/cmd/root.go @@ -44,13 +44,18 @@ const ( showLibmachineLogs = "show-libmachine-logs" ) +var viperWhiteList = []string{ + "v", + "alsologtostderr", + "log_dir", +} + // RootCmd represents the base command when called without any subcommands var RootCmd = &cobra.Command{ Use: "minikube", Short: "Minikube is a tool for managing local Kubernetes clusters.", Long: `Minikube is a CLI tool that provisions and manages single-node Kubernetes clusters optimized for development workflows.`, PersistentPreRun: func(cmd *cobra.Command, args []string) { - setFlagsUsingViper() for _, path := range dirs { if err := os.MkdirAll(path, 0777); err != nil { glog.Exitf("Error creating minikube directory: %s", err) @@ -77,7 +82,8 @@ func Execute() { // Handle config values for flags used in external packages (e.g. glog) // by setting them directly, using values from viper when not passed in as args func setFlagsUsingViper() { - setFlagValues := func(a *pflag.Flag) { + for _, config := range viperWhiteList { + var a = pflag.Lookup(config) viper.SetDefault(a.Name, a.DefValue) // If the flag is set, override viper value if a.Changed { @@ -87,8 +93,6 @@ func setFlagsUsingViper() { // then to values from the config.yml a.Value.Set(viper.GetString(a.Name)) } - - pflag.VisitAll(setFlagValues) } func init() { @@ -101,17 +105,20 @@ func init() { // initConfig reads in config file and ENV variables if set. func initConfig() { configPath := constants.MakeMiniPath("config") - - // Bind all viper values to env variables - viper.SetEnvPrefix(constants.MinikubeEnvPrefix) - viper.AutomaticEnv() - viper.SetConfigName("config") viper.AddConfigPath(configPath) err := viper.ReadInConfig() if err != nil { glog.Warningf("Error reading config file at %s: %s", configPath, err) } + setupViper() +} + +func setupViper() { + viper.SetEnvPrefix(constants.MinikubeEnvPrefix) + viper.AutomaticEnv() + viper.SetDefault(config.WantUpdateNotification, true) viper.SetDefault(config.ReminderWaitPeriodInHours, 24) + setFlagsUsingViper() } diff --git a/cmd/minikube/cmd/root_test.go b/cmd/minikube/cmd/root_test.go index ab254632d6..80df354047 100644 --- a/cmd/minikube/cmd/root_test.go +++ b/cmd/minikube/cmd/root_test.go @@ -17,7 +17,9 @@ limitations under the License. package cmd import ( + "bytes" "os" + "strings" "testing" "github.com/spf13/cobra" @@ -27,6 +29,76 @@ import ( "k8s.io/minikube/pkg/minikube/tests" ) +var yamlExampleConfig = []byte(`v: 999 +alsologtostderr: true +log_dir: "/etc/hosts" +log-flush-frequency: "3s" +`) + +const configName = ".test_minikube_config.yml" + +type configTest struct { + Name string + EnvValue string + ConfigValue string + FlagValue string + ExpectedValue string +} + +var configTests = []configTest{ + { + Name: "v", + ExpectedValue: "0", + }, + { + Name: "v", + ConfigValue: "999", + ExpectedValue: "999", + }, + { + Name: "v", + FlagValue: "0", + ExpectedValue: "0", + }, + { + Name: "v", + EnvValue: "123", + ExpectedValue: "123", + }, + { + Name: "v", + FlagValue: "3", + ExpectedValue: "3", + }, + // Flag should override config and env + { + Name: "v", + FlagValue: "3", + ConfigValue: "222", + EnvValue: "888", + ExpectedValue: "3", + }, + // Env should override config + { + Name: "v", + EnvValue: "2", + ConfigValue: "999", + ExpectedValue: "2", + }, + // Config should not override flags not on whitelist + { + Name: "log-flush-frequency", + ConfigValue: "6s", + ExpectedValue: "5s", + }, + // Env should not override flags not on whitelist + { + Name: "log_backtrace_at", + EnvValue: ":2", + ExpectedValue: ":0", + }, +} + func runCommand(f func(*cobra.Command, []string)) { cmd := cobra.Command{} var args []string @@ -48,55 +120,54 @@ func TestPreRunDirectories(t *testing.T) { } } +func initTestConfig(config string) { + viper.SetConfigType("yml") + r := bytes.NewReader([]byte(config)) + viper.ReadConfig(r) +} + +func TestViperConfig(t *testing.T) { + defer viper.Reset() + initTestConfig("v: 999") + if viper.GetString("v") != "999" { + t.Fatalf("Viper did not read test config file") + } +} + func getEnvVarName(name string) string { - return constants.MinikubeEnvPrefix + name + return constants.MinikubeEnvPrefix + "_" + strings.ToUpper(name) } -func TestEnvVariable(t *testing.T) { - defer os.Unsetenv("WANTUPDATENOTIFICATION") - initConfig() - os.Setenv(getEnvVarName("WANTUPDATENOTIFICATION"), "true") - if !viper.GetBool("WantUpdateNotification") { - t.Fatalf("Viper did not respect environment variable") +func setValues(tt configTest) { + if tt.FlagValue != "" { + pflag.Set(tt.Name, tt.FlagValue) + } + if tt.EnvValue != "" { + os.Setenv(getEnvVarName(tt.Name), tt.EnvValue) + } + if tt.ConfigValue != "" { + initTestConfig(tt.Name + ": " + tt.ConfigValue) } } -func cleanup() { - pflag.Set("v", "0") - pflag.Lookup("v").Changed = false +func unsetValues(tt configTest) { + var f = pflag.Lookup(tt.Name) + f.Value.Set(f.DefValue) + f.Changed = false + + os.Unsetenv(getEnvVarName(tt.Name)) + + viper.Reset() } -func TestFlagShouldOverrideConfig(t *testing.T) { - defer cleanup() - viper.Set("v", "1337") - pflag.Set("v", "100") - setFlagsUsingViper() - if viper.GetInt("v") != 100 { - viper.Debug() - t.Fatal("Value from viper config overrode explicit flag value") - } -} - -func TestConfigShouldOverrideDefault(t *testing.T) { - defer cleanup() - viper.Set("v", "1337") - setFlagsUsingViper() - if viper.GetInt("v") != 1337 { - viper.Debug() - t.Fatalf("Value from viper config did not override default flag value") - } -} - -func TestFallbackToDefaultFlag(t *testing.T) { - setFlagsUsingViper() - - if viper.GetInt("stderrthreshold") != 2 { - t.Logf("stderrthreshold %s", viper.GetInt("stderrthreshold")) - t.Fatalf("The default flag value was overwritten") - } - - if viper.GetString("log-flush-frequency") != "5s" { - t.Logf("log flush frequency: %s", viper.GetString("log-flush-frequency")) - t.Fatalf("The default flag value was overwritten") +func TestViperAndFlags(t *testing.T) { + for _, tt := range configTests { + setValues(tt) + setupViper() + var actual = pflag.Lookup(tt.Name).Value.String() + if actual != tt.ExpectedValue { + t.Errorf("pflag.Value(%s) => %s, wanted %s [%+v]", tt.Name, actual, tt.ExpectedValue, tt) + } + unsetValues(tt) } }