flag: improve StringFlags by support set default value when init (#8447)

* flag: improve StringFlags by support set default value when init

when init flagSet, set default value should be moved to StringFlags init
func, which is more friendly

personal proposal

* flag: code improved for StringFlags

etcdmain/config.go

@@ -110,8 +110,8 @@ func newConfig() *config {
 			embed.ClusterStateFlagExisting,
 		),
 		fallback: flags.NewStringsFlag(
-			fallbackFlagExit,
 			fallbackFlagProxy,
+			fallbackFlagExit,
 		),
 		proxy: flags.NewStringsFlag(
 			proxyFlagOff,
@@ -149,28 +149,19 @@ func newConfig() *config {
 	fs.Var(flags.NewURLsValue(embed.DefaultAdvertiseClientURLs), "advertise-client-urls", "List of this member's client URLs to advertise to the public.")
 	fs.StringVar(&cfg.Durl, "discovery", cfg.Durl, "Discovery URL used to bootstrap the cluster.")
 	fs.Var(cfg.fallback, "discovery-fallback", fmt.Sprintf("Valid values include %s", strings.Join(cfg.fallback.Values, ", ")))
-	if err := cfg.fallback.Set(fallbackFlagProxy); err != nil {
-		// Should never happen.
-		plog.Panicf("unexpected error setting up discovery-fallback flag: %v", err)
-	}
+
 	fs.StringVar(&cfg.Dproxy, "discovery-proxy", cfg.Dproxy, "HTTP proxy to use for traffic to discovery service.")
 	fs.StringVar(&cfg.DNSCluster, "discovery-srv", cfg.DNSCluster, "DNS domain used to bootstrap initial cluster.")
 	fs.StringVar(&cfg.InitialCluster, "initial-cluster", cfg.InitialCluster, "Initial cluster configuration for bootstrapping.")
 	fs.StringVar(&cfg.InitialClusterToken, "initial-cluster-token", cfg.InitialClusterToken, "Initial cluster token for the etcd cluster during bootstrap.")
 	fs.Var(cfg.clusterState, "initial-cluster-state", "Initial cluster state ('new' or 'existing').")
-	if err := cfg.clusterState.Set(embed.ClusterStateFlagNew); err != nil {
-		// Should never happen.
-		plog.Panicf("unexpected error setting up clusterStateFlag: %v", err)
-	}
+
 	fs.BoolVar(&cfg.StrictReconfigCheck, "strict-reconfig-check", cfg.StrictReconfigCheck, "Reject reconfiguration requests that would cause quorum loss.")
 	fs.BoolVar(&cfg.EnableV2, "enable-v2", true, "Accept etcd V2 client requests.")
 
 	// proxy
 	fs.Var(cfg.proxy, "proxy", fmt.Sprintf("Valid values include %s", strings.Join(cfg.proxy.Values, ", ")))
-	if err := cfg.proxy.Set(proxyFlagOff); err != nil {
-		// Should never happen.
-		plog.Panicf("unexpected error setting up proxyFlag: %v", err)
-	}
+
 	fs.UintVar(&cfg.ProxyFailureWaitMs, "proxy-failure-wait", cfg.ProxyFailureWaitMs, "Time (in milliseconds) an endpoint will be held in a failed state.")
 	fs.UintVar(&cfg.ProxyRefreshIntervalMs, "proxy-refresh-interval", cfg.ProxyRefreshIntervalMs, "Time (in milliseconds) of the endpoints refresh interval.")
 	fs.UintVar(&cfg.ProxyDialTimeoutMs, "proxy-dial-timeout", cfg.ProxyDialTimeoutMs, "Time (in milliseconds) for a dial to timeout.")

pkg/flags/strings.go

@@ -18,8 +18,11 @@ import "errors"
 
 // NewStringsFlag creates a new string flag for which any one of the given
 // strings is a valid value, and any other value is an error.
+//
+// valids[0] will be default value. Caller must be sure len(valids)!=0 or
+// it will panic.
 func NewStringsFlag(valids ...string) *StringsFlag {
-	return &StringsFlag{Values: valids}
+	return &StringsFlag{Values: valids, val: valids[0]}
 }
 
 // StringsFlag implements the flag.Value interface.

pkg/flags/strings_test.go

@@ -32,11 +32,14 @@ func TestStringsSet(t *testing.T) {
 		// unrecognized values
 		{[]string{"abc", "def"}, "ghi", false},
 		{[]string{"on", "off"}, "", false},
-		{[]string{}, "asdf", false},
 	}
 
 	for i, tt := range tests {
 		sf := NewStringsFlag(tt.vals...)
+		if sf.val != tt.vals[0] {
+			t.Errorf("#%d: want default val=%v,but got %v", i, tt.vals[0], sf.val)
+		}
+
 		err := sf.Set(tt.val)
 		if tt.pass != (err == nil) {
 			t.Errorf("#%d: want pass=%t, but got err=%v", i, tt.pass, err)