From 002ace2403e94de270932f52f88c4430239f13fb Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 24 Sep 2014 16:03:53 -0700 Subject: [PATCH 1/7] etcd: remove unnecessary flag desc --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 26cff9422..621dd01a0 100644 --- a/main.go +++ b/main.go @@ -86,7 +86,7 @@ func init() { flag.StringVar(&peerTLSInfo.KeyFile, "peer-key-file", "", "Path to the peer server TLS key file.") for _, f := range deprecated { - flag.Var(&deprecatedFlag{f}, f, "No longer supported.") + flag.Var(&deprecatedFlag{f}, f, "") } } From 314c13a8f0c45362efbadf69b7b961fdad82fb90 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 24 Sep 2014 16:06:28 -0700 Subject: [PATCH 2/7] pkg: move DeprecatedFlag to new package --- main.go | 41 +++-------------------------------------- pkg/flag.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ test | 2 +- 3 files changed, 48 insertions(+), 39 deletions(-) create mode 100644 pkg/flag.go diff --git a/main.go b/main.go index 621dd01a0..e054c9de7 100644 --- a/main.go +++ b/main.go @@ -15,6 +15,7 @@ import ( "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/etcdserver/etcdhttp" + "github.com/coreos/etcd/pkg" "github.com/coreos/etcd/proxy" "github.com/coreos/etcd/raft" "github.com/coreos/etcd/snap" @@ -86,12 +87,12 @@ func init() { flag.StringVar(&peerTLSInfo.KeyFile, "peer-key-file", "", "Path to the peer server TLS key file.") for _, f := range deprecated { - flag.Var(&deprecatedFlag{f}, f, "") + flag.Var(&pkg.DeprecatedFlag{f}, f, "") } } func main() { - flag.Usage = usageWithIgnoredFlagsFunc(flag.CommandLine, deprecated) + flag.Usage = pkg.UsageWithIgnoredFlagsFunc(flag.CommandLine, deprecated) flag.Parse() setFlagsFromEnv() @@ -349,39 +350,3 @@ func setFlagsFromEnv() { }) } - -type deprecatedFlag struct { - name string -} - -// IsBoolFlag is defined to allow the flag to be defined without an argument -func (df *deprecatedFlag) IsBoolFlag() bool { - return true -} - -func (df *deprecatedFlag) Set(s string) error { - log.Printf("WARNING: flag \"-%s\" is no longer supported.", df.name) - return nil -} - -func (df *deprecatedFlag) String() string { - return "" -} - -func usageWithIgnoredFlagsFunc(fs *flag.FlagSet, ignore []string) func() { - iMap := make(map[string]struct{}, len(ignore)) - for _, name := range ignore { - iMap[name] = struct{}{} - } - - return func() { - fs.VisitAll(func(f *flag.Flag) { - if _, ok := iMap[f.Name]; ok { - return - } - - format := " -%s=%s: %s\n" - fmt.Fprintf(os.Stderr, format, f.Name, f.DefValue, f.Usage) - }) - } -} diff --git a/pkg/flag.go b/pkg/flag.go new file mode 100644 index 000000000..225af0ce6 --- /dev/null +++ b/pkg/flag.go @@ -0,0 +1,44 @@ +package pkg + +import ( + "flag" + "fmt" + "log" + "os" +) + +type DeprecatedFlag struct { + Name string +} + +// IsBoolFlag is defined to allow the flag to be defined without an argument +func (df *DeprecatedFlag) IsBoolFlag() bool { + return true +} + +func (df *DeprecatedFlag) Set(s string) error { + log.Printf("WARNING: flag \"-%s\" is no longer supported.", df.Name) + return nil +} + +func (df *DeprecatedFlag) String() string { + return "" +} + +func UsageWithIgnoredFlagsFunc(fs *flag.FlagSet, ignore []string) func() { + iMap := make(map[string]struct{}, len(ignore)) + for _, name := range ignore { + iMap[name] = struct{}{} + } + + return func() { + fs.VisitAll(func(f *flag.Flag) { + if _, ok := iMap[f.Name]; ok { + return + } + + format := " -%s=%s: %s\n" + fmt.Fprintf(os.Stderr, format, f.Name, f.DefValue, f.Usage) + }) + } +} diff --git a/test b/test index ee93c6907..f4b0a69ad 100755 --- a/test +++ b/test @@ -17,7 +17,7 @@ source ./build # Hack: gofmt ./ will recursively check the .git directory. So use *.go for gofmt. TESTABLE_AND_FORMATTABLE="client etcdserver etcdserver/etcdhttp etcdserver/etcdserverpb functional proxy raft snap store wait wal transport" TESTABLE="$TESTABLE_AND_FORMATTABLE ./" -FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go" +FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go pkg" # user has not provided PKG override if [ -z "$PKG" ]; then From 18c300f80c4a186828ca4e71c6ab77a6a083704d Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 24 Sep 2014 16:11:47 -0700 Subject: [PATCH 3/7] etcd: pass flagset into setFlagsFromEnv --- main.go | 13 ++++++------- main_test.go | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/main.go b/main.go index e054c9de7..2f71876f7 100644 --- a/main.go +++ b/main.go @@ -95,7 +95,7 @@ func main() { flag.Usage = pkg.UsageWithIgnoredFlagsFunc(flag.CommandLine, deprecated) flag.Parse() - setFlagsFromEnv() + setFlagsFromEnv(flag.CommandLine) if string(*proxyFlag) == proxyFlagValueOff { startEtcd() @@ -329,24 +329,23 @@ func (pf *ProxyFlag) String() string { return string(*pf) } -// setFlagsFromEnv parses all registered flags in the global flagset, +// setFlagsFromEnv parses all registered flags in the given flagset, // and if they are not already set it attempts to set their values from // environment variables. Environment variables take the name of the flag but // are UPPERCASE, have the prefix "ETCD_", and any dashes are replaced by // underscores - for example: some-flag => ETCD_SOME_FLAG -func setFlagsFromEnv() { +func setFlagsFromEnv(fs *flag.FlagSet) { alreadySet := make(map[string]bool) - flag.Visit(func(f *flag.Flag) { + fs.Visit(func(f *flag.Flag) { alreadySet[f.Name] = true }) - flag.VisitAll(func(f *flag.Flag) { + fs.VisitAll(func(f *flag.Flag) { if !alreadySet[f.Name] { key := "ETCD_" + strings.ToUpper(strings.Replace(f.Name, "-", "_", -1)) val := os.Getenv(key) if val != "" { - flag.Set(f.Name, val) + fs.Set(f.Name, val) } } - }) } diff --git a/main_test.go b/main_test.go index 97647cd95..c209859da 100644 --- a/main_test.go +++ b/main_test.go @@ -30,7 +30,7 @@ func TestSetFlagsFromEnv(t *testing.T) { } // now read the env and verify flags were updated as expected - setFlagsFromEnv() + setFlagsFromEnv(flag.CommandLine) for f, want := range map[string]string{ "data-dir": "/foo/bar", "peer-bind-addr": "1.2.3.4", From f7c353a703e39bdd7b3ffa2c6115b5bd022f6ee0 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 24 Sep 2014 16:12:06 -0700 Subject: [PATCH 4/7] etcd: export SetFlagsFromEnv --- main.go | 6 +++--- main_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index 2f71876f7..a7025271d 100644 --- a/main.go +++ b/main.go @@ -95,7 +95,7 @@ func main() { flag.Usage = pkg.UsageWithIgnoredFlagsFunc(flag.CommandLine, deprecated) flag.Parse() - setFlagsFromEnv(flag.CommandLine) + SetFlagsFromEnv(flag.CommandLine) if string(*proxyFlag) == proxyFlagValueOff { startEtcd() @@ -329,12 +329,12 @@ func (pf *ProxyFlag) String() string { return string(*pf) } -// setFlagsFromEnv parses all registered flags in the given flagset, +// SetFlagsFromEnv parses all registered flags in the given flagset, // and if they are not already set it attempts to set their values from // environment variables. Environment variables take the name of the flag but // are UPPERCASE, have the prefix "ETCD_", and any dashes are replaced by // underscores - for example: some-flag => ETCD_SOME_FLAG -func setFlagsFromEnv(fs *flag.FlagSet) { +func SetFlagsFromEnv(fs *flag.FlagSet) { alreadySet := make(map[string]bool) fs.Visit(func(f *flag.Flag) { alreadySet[f.Name] = true diff --git a/main_test.go b/main_test.go index c209859da..687775812 100644 --- a/main_test.go +++ b/main_test.go @@ -30,7 +30,7 @@ func TestSetFlagsFromEnv(t *testing.T) { } // now read the env and verify flags were updated as expected - setFlagsFromEnv(flag.CommandLine) + SetFlagsFromEnv(flag.CommandLine) for f, want := range map[string]string{ "data-dir": "/foo/bar", "peer-bind-addr": "1.2.3.4", From b0617be7e3c1f387749dd68c0f84605e7ed9f612 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 24 Sep 2014 16:27:00 -0700 Subject: [PATCH 5/7] etcd: rewrite SetFlagsFromEnv test to use custom flagset --- main_test.go | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/main_test.go b/main_test.go index 687775812..200b50969 100644 --- a/main_test.go +++ b/main_test.go @@ -5,38 +5,44 @@ import "flag" import "testing" func TestSetFlagsFromEnv(t *testing.T) { + fs := flag.NewFlagSet("testing", flag.ExitOnError) + fs.String("a", "", "") + fs.String("b", "", "") + fs.String("c", "", "") + fs.Parse([]string{}) + os.Clearenv() // flags should be settable using env vars - os.Setenv("ETCD_DATA_DIR", "/foo/bar") + os.Setenv("ETCD_A", "foo") // and command-line flags - if err := flag.Set("peer-bind-addr", "1.2.3.4"); err != nil { + if err := fs.Set("b", "bar"); err != nil { t.Fatal(err) } // command-line flags take precedence over env vars - os.Setenv("ETCD_ID", "woof") - if err := flag.Set("id", "quack"); err != nil { + os.Setenv("ETCD_C", "woof") + if err := fs.Set("c", "quack"); err != nil { t.Fatal(err) } // first verify that flags are as expected before reading the env for f, want := range map[string]string{ - "data-dir": "", - "peer-bind-addr": "1.2.3.4", - "id": "quack", + "a": "", + "b": "bar", + "c": "quack", } { - if got := flag.Lookup(f).Value.String(); got != want { + if got := fs.Lookup(f).Value.String(); got != want { t.Fatalf("flag %q=%q, want %q", f, got, want) } } // now read the env and verify flags were updated as expected - SetFlagsFromEnv(flag.CommandLine) + SetFlagsFromEnv(fs) for f, want := range map[string]string{ - "data-dir": "/foo/bar", - "peer-bind-addr": "1.2.3.4", - "id": "quack", + "a": "foo", + "b": "bar", + "c": "quack", } { - if got := flag.Lookup(f).Value.String(); got != want { + if got := fs.Lookup(f).Value.String(); got != want { t.Errorf("flag %q=%q, want %q", f, got, want) } } From e30c1eeefd4f75bf73564c1443a37c6b4ce6da66 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 24 Sep 2014 16:29:47 -0700 Subject: [PATCH 6/7] pkg: move SetFlagsFromEnv to pkg package --- main.go | 23 +--------------------- main_test.go | 50 +++-------------------------------------------- pkg/flag.go | 22 +++++++++++++++++++++ pkg/flag_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ test | 4 ++-- 5 files changed, 79 insertions(+), 71 deletions(-) create mode 100644 pkg/flag_test.go diff --git a/main.go b/main.go index a7025271d..e2b4ac9bd 100644 --- a/main.go +++ b/main.go @@ -95,7 +95,7 @@ func main() { flag.Usage = pkg.UsageWithIgnoredFlagsFunc(flag.CommandLine, deprecated) flag.Parse() - SetFlagsFromEnv(flag.CommandLine) + pkg.SetFlagsFromEnv(flag.CommandLine) if string(*proxyFlag) == proxyFlagValueOff { startEtcd() @@ -328,24 +328,3 @@ func (pf *ProxyFlag) Set(s string) error { func (pf *ProxyFlag) String() string { return string(*pf) } - -// SetFlagsFromEnv parses all registered flags in the given flagset, -// and if they are not already set it attempts to set their values from -// environment variables. Environment variables take the name of the flag but -// are UPPERCASE, have the prefix "ETCD_", and any dashes are replaced by -// underscores - for example: some-flag => ETCD_SOME_FLAG -func SetFlagsFromEnv(fs *flag.FlagSet) { - alreadySet := make(map[string]bool) - fs.Visit(func(f *flag.Flag) { - alreadySet[f.Name] = true - }) - fs.VisitAll(func(f *flag.Flag) { - if !alreadySet[f.Name] { - key := "ETCD_" + strings.ToUpper(strings.Replace(f.Name, "-", "_", -1)) - val := os.Getenv(key) - if val != "" { - fs.Set(f.Name, val) - } - } - }) -} diff --git a/main_test.go b/main_test.go index 200b50969..46aa848d9 100644 --- a/main_test.go +++ b/main_test.go @@ -1,52 +1,8 @@ package main -import "os" -import "flag" -import "testing" - -func TestSetFlagsFromEnv(t *testing.T) { - fs := flag.NewFlagSet("testing", flag.ExitOnError) - fs.String("a", "", "") - fs.String("b", "", "") - fs.String("c", "", "") - fs.Parse([]string{}) - - os.Clearenv() - // flags should be settable using env vars - os.Setenv("ETCD_A", "foo") - // and command-line flags - if err := fs.Set("b", "bar"); err != nil { - t.Fatal(err) - } - // command-line flags take precedence over env vars - os.Setenv("ETCD_C", "woof") - if err := fs.Set("c", "quack"); err != nil { - t.Fatal(err) - } - - // first verify that flags are as expected before reading the env - for f, want := range map[string]string{ - "a": "", - "b": "bar", - "c": "quack", - } { - if got := fs.Lookup(f).Value.String(); got != want { - t.Fatalf("flag %q=%q, want %q", f, got, want) - } - } - - // now read the env and verify flags were updated as expected - SetFlagsFromEnv(fs) - for f, want := range map[string]string{ - "a": "foo", - "b": "bar", - "c": "quack", - } { - if got := fs.Lookup(f).Value.String(); got != want { - t.Errorf("flag %q=%q, want %q", f, got, want) - } - } -} +import ( + "testing" +) func TestProxyFlagSet(t *testing.T) { tests := []struct { diff --git a/pkg/flag.go b/pkg/flag.go index 225af0ce6..b09374268 100644 --- a/pkg/flag.go +++ b/pkg/flag.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "os" + "strings" ) type DeprecatedFlag struct { @@ -42,3 +43,24 @@ func UsageWithIgnoredFlagsFunc(fs *flag.FlagSet, ignore []string) func() { }) } } + +// SetFlagsFromEnv parses all registered flags in the given flagset, +// and if they are not already set it attempts to set their values from +// environment variables. Environment variables take the name of the flag but +// are UPPERCASE, have the prefix "ETCD_", and any dashes are replaced by +// underscores - for example: some-flag => ETCD_SOME_FLAG +func SetFlagsFromEnv(fs *flag.FlagSet) { + alreadySet := make(map[string]bool) + fs.Visit(func(f *flag.Flag) { + alreadySet[f.Name] = true + }) + fs.VisitAll(func(f *flag.Flag) { + if !alreadySet[f.Name] { + key := "ETCD_" + strings.ToUpper(strings.Replace(f.Name, "-", "_", -1)) + val := os.Getenv(key) + if val != "" { + fs.Set(f.Name, val) + } + } + }) +} diff --git a/pkg/flag_test.go b/pkg/flag_test.go new file mode 100644 index 000000000..0ae424e6d --- /dev/null +++ b/pkg/flag_test.go @@ -0,0 +1,51 @@ +package pkg + +import ( + "flag" + "os" + "testing" +) + +func TestSetFlagsFromEnv(t *testing.T) { + fs := flag.NewFlagSet("testing", flag.ExitOnError) + fs.String("a", "", "") + fs.String("b", "", "") + fs.String("c", "", "") + fs.Parse([]string{}) + + os.Clearenv() + // flags should be settable using env vars + os.Setenv("ETCD_A", "foo") + // and command-line flags + if err := fs.Set("b", "bar"); err != nil { + t.Fatal(err) + } + // command-line flags take precedence over env vars + os.Setenv("ETCD_C", "woof") + if err := fs.Set("c", "quack"); err != nil { + t.Fatal(err) + } + + // first verify that flags are as expected before reading the env + for f, want := range map[string]string{ + "a": "", + "b": "bar", + "c": "quack", + } { + if got := fs.Lookup(f).Value.String(); got != want { + t.Fatalf("flag %q=%q, want %q", f, got, want) + } + } + + // now read the env and verify flags were updated as expected + SetFlagsFromEnv(fs) + for f, want := range map[string]string{ + "a": "foo", + "b": "bar", + "c": "quack", + } { + if got := fs.Lookup(f).Value.String(); got != want { + t.Errorf("flag %q=%q, want %q", f, got, want) + } + } +} diff --git a/test b/test index f4b0a69ad..010635b58 100755 --- a/test +++ b/test @@ -15,9 +15,9 @@ COVER=${COVER:-"-cover"} source ./build # Hack: gofmt ./ will recursively check the .git directory. So use *.go for gofmt. -TESTABLE_AND_FORMATTABLE="client etcdserver etcdserver/etcdhttp etcdserver/etcdserverpb functional proxy raft snap store wait wal transport" +TESTABLE_AND_FORMATTABLE="client etcdserver etcdserver/etcdhttp etcdserver/etcdserverpb functional proxy raft snap store wait wal transport pkg" TESTABLE="$TESTABLE_AND_FORMATTABLE ./" -FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go pkg" +FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go" # user has not provided PKG override if [ -z "$PKG" ]; then From 4a65813a6667f3eb85cfc88304cb8195cafdaebf Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 24 Sep 2014 16:37:46 -0700 Subject: [PATCH 7/7] test: alphabetize list of testable packages --- test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test b/test index 010635b58..76412f8b7 100755 --- a/test +++ b/test @@ -15,7 +15,7 @@ COVER=${COVER:-"-cover"} source ./build # Hack: gofmt ./ will recursively check the .git directory. So use *.go for gofmt. -TESTABLE_AND_FORMATTABLE="client etcdserver etcdserver/etcdhttp etcdserver/etcdserverpb functional proxy raft snap store wait wal transport pkg" +TESTABLE_AND_FORMATTABLE="client etcdserver etcdserver/etcdhttp etcdserver/etcdserverpb functional pkg proxy raft snap store transport wait wal" TESTABLE="$TESTABLE_AND_FORMATTABLE ./" FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go"