Merge pull request kubernetes#469 from r2d4/config

Use config when flags are not set for logging

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()
 }

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)
 	}
 }

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) {