Merge pull request #903 from inoc603/fix/900

fix: ineffective viper alias

Author: starnop

cmd/dfdaemon/app/root.go

@@ -49,7 +49,7 @@ var rootCmd = &cobra.Command{
 			return errors.Wrap(err, "read config file")
 		}
 
-		cfg, err := getConfigFromViper(viper.GetViper())
+		cfg, err := getConfigFromViper(cmd, viper.GetViper())
 		if err != nil {
 			return errors.Wrap(err, "get config from viper")
 		}
@@ -95,7 +95,7 @@ func init() {
 	rf.String("localrepo", filepath.Join(os.Getenv("HOME"), ".small-dragonfly/dfdaemon/data/"), "temp output dir of dfdaemon")
 	rf.String("dfpath", defaultDfgetPath, "dfget path")
 	rf.Var(netutils.NetLimit(), "ratelimit", "net speed limit")
-	rf.StringSlice("node", nil, "specify the addresses(host:port) of supernodes that will be passed to dfget.")
+	rf.StringSlice("node", []string{"127.0.0.1:8002"}, "specify the addresses(host:port) of supernodes that will be passed to dfget.")
 
 	exitOnError(bindRootFlags(viper.GetViper()), "bind root command flags")
 }
@@ -122,7 +122,6 @@ func readConfigFile(v *viper.Viper, cmd *cobra.Command) error {
 		}
 		return err
 	}
-	v.RegisterAlias("supernodes", "node")
 
 	return nil
 }
@@ -145,8 +144,15 @@ func Execute() {
 	}
 }
 
-// getConfigFromViper returns dfdaemon config from the given viper instance.
-func getConfigFromViper(v *viper.Viper) (*config.Properties, error) {
+// getConfigFromViper returns dfdaemon config from the given viper instance
+func getConfigFromViper(cmd *cobra.Command, v *viper.Viper) (*config.Properties, error) {
+	// override supernodes in config file if --node is sepecified in cli.
+	// use default value if no supernodes is configured in config file
+	if cmd.Flags().Lookup("node").Changed ||
+		len(v.GetStringSlice("supernodes")) == 0 {
+		v.Set("supernodes", v.GetStringSlice("node"))
+	}
+
 	var cfg config.Properties
 	if err := v.Unmarshal(&cfg, func(dc *mapstructure.DecoderConfig) {
 		dc.TagName = "yaml"
@@ -160,6 +166,7 @@ func getConfigFromViper(v *viper.Viper) (*config.Properties, error) {
 	}); err != nil {
 		return nil, errors.Wrap(err, "unmarshal yaml")
 	}
+
 	return &cfg, cfg.Validate()
 }
 

cmd/dfdaemon/app/root_test.go

@@ -51,6 +51,72 @@ func (ts *rootTestSuite) TestConfigNotFound() {
 	r.True(os.IsNotExist(errors.Cause(readConfigFile(v, rootCmd))))
 }
 
+func (ts *rootTestSuite) TestNodeFlag() {
+	r := ts.Require()
+	fs := afero.NewMemMapFs()
+
+	fs.Create("/dfget")
+
+	configName := "dfdaemon.yml"
+	file, err := fs.Create(configName)
+	r.Nil(err)
+	file.WriteString("supernodes:\n- 127.0.0.1:6666")
+	file.Close()
+
+	// flag not set, should use config file
+	{
+		v := viper.New()
+		v.SetFs(fs)
+		v.Set("dfpath", "/")
+		rootCmd.Flags().Set("config", configName)
+		r.Nil(bindRootFlags(v))
+		r.Nil(readConfigFile(v, rootCmd))
+		cfg, err := getConfigFromViper(rootCmd, v)
+		r.Nil(err)
+		r.Equal([]string{"127.0.0.1:6666"}, cfg.SuperNodes)
+	}
+
+	// flag not set, config file doesn't exist, should use default
+	{
+		v := viper.New()
+		v.SetFs(fs)
+		v.Set("dfpath", "/")
+		rootCmd.Flags().Set("config", "xxx")
+		r.Nil(bindRootFlags(v))
+		r.NotNil(readConfigFile(v, rootCmd))
+		cfg, err := getConfigFromViper(rootCmd, v)
+		r.Nil(err)
+		r.Equal([]string{"127.0.0.1:8002"}, cfg.SuperNodes)
+	}
+
+	// when --node flag is set, should always use the flag
+	rootCmd.Flags().Set("node", "127.0.0.1:7777")
+
+	{
+		v := viper.New()
+		v.SetFs(fs)
+		v.Set("dfpath", "/")
+		rootCmd.Flags().Set("config", "xxx")
+		r.Nil(bindRootFlags(v))
+		r.NotNil(readConfigFile(v, rootCmd))
+		cfg, err := getConfigFromViper(rootCmd, v)
+		r.Nil(err)
+		r.Equal([]string{"127.0.0.1:7777"}, cfg.SuperNodes)
+	}
+
+	{
+		v := viper.New()
+		v.SetFs(fs)
+		v.Set("dfpath", "/")
+		rootCmd.Flags().Set("config", configName)
+		r.Nil(bindRootFlags(v))
+		r.Nil(readConfigFile(v, rootCmd))
+		cfg, err := getConfigFromViper(rootCmd, v)
+		r.Nil(err)
+		r.Equal([]string{"127.0.0.1:7777"}, cfg.SuperNodes)
+	}
+}
+
 func generateFakeFilename(fs afero.Fs) string {
 	for i := 0; i < 100; i++ {
 		d := fmt.Sprintf("/dftest-%d-%d", time.Now().UnixNano(), rand.Int())
@@ -111,7 +177,7 @@ func (ts *rootTestSuite) TestDecodeWithYAML() {
 	r.Nil(err)
 	v.Set("registry_mirror.certs", []string{f.Name()})
 
-	cfg, err := getConfigFromViper(v)
+	cfg, err := getConfigFromViper(rootCmd, v)
 	r.Nil(err)
 	r.NotNil(cfg.RegistryMirror.Remote)
 	r.Equal(mockURL, cfg.RegistryMirror.Remote.String())