Browse Source

Remove code dup and use coreos/pkg/flagutil

Fixes #278
Eugene Yakubovich 9 years ago
parent
commit
4888d90bf0

+ 4 - 0
Godeps/Godeps.json

@@ -54,6 +54,10 @@
 			"Comment": "v2-26-ga606a1e",
 			"Rev": "a606a1e936df81b70d85448221c7b1c6d8a74ef1"
 		},
+		{
+			"ImportPath": "github.com/coreos/pkg/flagutil",
+			"Rev": "b0d3d81732aea341039d191d8618a3466fe54395"
+		},
 		{
 			"ImportPath": "github.com/golang/glog",
 			"Rev": "d1c4472bf2efd3826f2b5bdcc02d8416798d678c"

+ 33 - 0
Godeps/_workspace/src/github.com/coreos/pkg/flagutil/env.go

@@ -0,0 +1,33 @@
+package flagutil
+
+import (
+	"flag"
+	"fmt"
+	"os"
+	"strings"
+)
+
+// 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, and any dashes are replaced by underscores. Environment
+// variables additionally are prefixed by the given string followed by
+// and underscore. For example, if prefix=PREFIX: some-flag => PREFIX_SOME_FLAG
+func SetFlagsFromEnv(fs *flag.FlagSet, prefix string) (err error) {
+	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 := prefix + "_" + strings.ToUpper(strings.Replace(f.Name, "-", "_", -1))
+			val := os.Getenv(key)
+			if val != "" {
+				if serr := fs.Set(f.Name, val); serr != nil {
+					err = fmt.Errorf("invalid value %q for %s: %v", val, key, serr)
+				}
+			}
+		}
+	})
+	return err
+}

+ 64 - 0
Godeps/_workspace/src/github.com/coreos/pkg/flagutil/env_test.go

@@ -0,0 +1,64 @@
+package flagutil
+
+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("MYPROJ_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("MYPROJ_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
+	err := SetFlagsFromEnv(fs, "MYPROJ")
+	if err != nil {
+		t.Errorf("err=%v, want nil", err)
+	}
+	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)
+		}
+	}
+}
+
+func TestSetFlagsFromEnvBad(t *testing.T) {
+	// now verify that an error is propagated
+	fs := flag.NewFlagSet("testing", flag.ExitOnError)
+	fs.Int("x", 0, "")
+	os.Setenv("MYPROJ_X", "not_a_number")
+	if err := SetFlagsFromEnv(fs, "MYPROJ"); err == nil {
+		t.Errorf("err=nil, want != nil")
+	}
+}

+ 44 - 0
Godeps/_workspace/src/github.com/coreos/pkg/flagutil/types.go

@@ -0,0 +1,44 @@
+package flagutil
+
+import (
+	"errors"
+	"fmt"
+	"net"
+	"strings"
+)
+
+// IPv4Flag parses a string into a net.IP after asserting that it
+// is an IPv4 address. This type implements the flag.Value interface.
+type IPv4Flag struct {
+	val net.IP
+}
+
+func (f *IPv4Flag) IP() net.IP {
+	return f.val
+}
+
+func (f *IPv4Flag) Set(v string) error {
+	ip := net.ParseIP(v)
+	if ip == nil || ip.To4() == nil {
+		return errors.New("not an IPv4 address")
+	}
+	f.val = ip
+	return nil
+}
+
+func (f *IPv4Flag) String() string {
+	return f.val.String()
+}
+
+// StringSliceFlag parses a comma-delimited list of strings into
+// a []string. This type implements the flag.Value interface.
+type StringSliceFlag []string
+
+func (ss *StringSliceFlag) String() string {
+	return fmt.Sprintf("%+v", *ss)
+}
+
+func (ss *StringSliceFlag) Set(v string) error {
+	*ss = strings.Split(v, ",")
+	return nil
+}

+ 57 - 0
Godeps/_workspace/src/github.com/coreos/pkg/flagutil/types_test.go

@@ -0,0 +1,57 @@
+package flagutil
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestIPv4FlagSetInvalidArgument(t *testing.T) {
+	tests := []string{
+		"",
+		"foo",
+		"::",
+		"127.0.0.1:4328",
+	}
+
+	for i, tt := range tests {
+		var f IPv4Flag
+		if err := f.Set(tt); err == nil {
+			t.Errorf("case %d: expected non-nil error", i)
+		}
+	}
+}
+
+func TestIPv4FlagSetValidArgument(t *testing.T) {
+	tests := []string{
+		"127.0.0.1",
+		"0.0.0.0",
+	}
+
+	for i, tt := range tests {
+		var f IPv4Flag
+		if err := f.Set(tt); err != nil {
+			t.Errorf("case %d: err=%v", i, err)
+		}
+	}
+}
+
+func TestStringSliceFlag(t *testing.T) {
+	tests := []struct {
+		input string
+		want  []string
+	}{
+		{input: "", want: []string{""}},
+		{input: "foo", want: []string{"foo"}},
+		{input: "foo,bar", want: []string{"foo", "bar"}},
+	}
+
+	for i, tt := range tests {
+		var f StringSliceFlag
+		if err := f.Set(tt.input); err != nil {
+			t.Errorf("case %d: err=%v", i, err)
+		}
+		if !reflect.DeepEqual(tt.want, []string(f)) {
+			t.Errorf("case %d: want=%v got=%v", i, tt.want, f)
+		}
+	}
+}

+ 3 - 23
main.go

@@ -26,8 +26,10 @@ import (
 	"syscall"
 
 	"github.com/coreos/flannel/Godeps/_workspace/src/github.com/coreos/go-systemd/daemon"
+	"github.com/coreos/flannel/Godeps/_workspace/src/github.com/coreos/pkg/flagutil"
 	log "github.com/coreos/flannel/Godeps/_workspace/src/github.com/golang/glog"
 	"github.com/coreos/flannel/Godeps/_workspace/src/golang.org/x/net/context"
+
 	"github.com/coreos/flannel/backend"
 	"github.com/coreos/flannel/network"
 	"github.com/coreos/flannel/pkg/ip"
@@ -79,28 +81,6 @@ func init() {
 	flag.BoolVar(&opts.version, "version", false, "print version and exit")
 }
 
-// TODO: This is yet another copy (others found in etcd, fleet) -- Pull it out!
-// flagsFromEnv 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 given prefix, and any dashes are replaced by
-// underscores - for example: some-flag => PREFIX_SOME_FLAG
-func flagsFromEnv(prefix string, 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 := strings.ToUpper(prefix + "_" + strings.Replace(f.Name, "-", "_", -1))
-			val := os.Getenv(key)
-			if val != "" {
-				fs.Set(f.Name, val)
-			}
-		}
-	})
-}
-
 func writeSubnetFile(path string, nw ip.IP4Net, sn *backend.SubnetDef) error {
 	dir, name := filepath.Split(path)
 	os.MkdirAll(dir, 0755)
@@ -262,7 +242,7 @@ func main() {
 		os.Exit(0)
 	}
 
-	flagsFromEnv("FLANNELD", flag.CommandLine)
+	flagutil.SetFlagsFromEnv(flag.CommandLine, "FLANNELD")
 
 	sm, err := newSubnetManager()
 	if err != nil {