diff --git a/cmd/minikube/cmd/root.go b/cmd/minikube/cmd/root.go index 8e5dcb4cac77..727f69e4fc5a 100644 --- a/cmd/minikube/cmd/root.go +++ b/cmd/minikube/cmd/root.go @@ -40,10 +40,20 @@ var dirs = [...]string{ constants.MakeMiniPath("cache", "localkube"), constants.MakeMiniPath("config")} +const ( + showLibmachineLogs = "show-libmachine-logs" +) + var ( - showLibmachineLogs bool + enableUpdateNotification = true ) +var viperWhiteList = []string{ + "v", + "alsologtostderr", + "log_dir", +} + // RootCmd represents the base command when called without any subcommands var RootCmd = &cobra.Command{ Use: "minikube", @@ -56,12 +66,17 @@ var RootCmd = &cobra.Command{ } } - log.SetDebug(showLibmachineLogs) - if !showLibmachineLogs { + shouldShowLibmachineLogs := viper.GetBool(showLibmachineLogs) + + log.SetDebug(shouldShowLibmachineLogs) + if !shouldShowLibmachineLogs { log.SetOutWriter(ioutil.Discard) log.SetErrWriter(ioutil.Discard) } - notify.MaybePrintUpdateTextFromGithub(os.Stdout) + + if enableUpdateNotification { + notify.MaybePrintUpdateTextFromGithub(os.Stdout) + } }, } @@ -73,26 +88,46 @@ 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() { + 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 { + viper.Set(a.Name, a.Value.String()) + } + // Viper will give precedence first to calls to the Set command, + // then to values from the config.yml + a.Value.Set(viper.GetString(a.Name)) + } +} + func init() { - RootCmd.PersistentFlags().BoolVarP(&showLibmachineLogs, "show-libmachine-logs", "", false, "Whether or not to show logs from libmachine.") + RootCmd.PersistentFlags().Bool(showLibmachineLogs, false, "Whether or not to show logs from libmachine.") pflag.CommandLine.AddGoFlagSet(goflag.CommandLine) + viper.BindPFlags(RootCmd.PersistentFlags()) cobra.OnInitialize(initConfig) } // 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 cc0d44b6e4df..ea9d612775d5 100644 --- a/cmd/minikube/cmd/root_test.go +++ b/cmd/minikube/cmd/root_test.go @@ -17,15 +17,86 @@ limitations under the License. package cmd import ( + "bytes" "os" + "strings" "testing" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/spf13/viper" "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/tests" ) +var yamlExampleConfig = []byte(`v: 999 +alsologtostderr: true +log_dir: "/etc/hosts" +log-flush-frequency: "3s" +`) + +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 @@ -47,15 +118,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 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 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) } } diff --git a/cmd/minikube/cmd/version.go b/cmd/minikube/cmd/version.go index cd171534fa0e..88894eda8b85 100644 --- a/cmd/minikube/cmd/version.go +++ b/cmd/minikube/cmd/version.go @@ -29,8 +29,10 @@ var versionCmd = &cobra.Command{ Short: "Print the version of minikube.", Long: `Print the version of minikube.`, PersistentPreRun: func(cmd *cobra.Command, args []string) { - // Empty func to override parent pre-run - don't want to perform - // check for new version. + // Explicitly disable update checking for the version command + enableUpdateNotification = false + + RootCmd.PersistentPreRun(cmd, args) }, Run: func(command *cobra.Command, args []string) {