add cluster.hosts flag

had to work around a bunch of viper nonsense with handling string slices

changed the validator.Check to use reflect.DeepEqual

fixed a bug where env vars need to be reset between tests

had to change the structure of pilosa.config to enable the passing of hosts as a
slice of strings (it was a slice of structs which each just contained the string
host.) so we lose some generality, but we weren't using it anyway.

cmd/root.go

@@ -99,7 +99,27 @@ func setAllConfig(v *viper.Viper, flags *flag.FlagSet, envPrefix string) error {
 		if flagErr != nil {
 			return
 		}
-		value := v.GetString(f.Name)
+		var value string
+		if f.Value.Type() == "stringSlice" {
+			// special handling is needed for stringSlice as v.GetString will
+			// always return "" in the case that the value is an actual string
+			// slice from a config file rather than a comma separated string
+			// from a flag or env var.
+			vss := v.GetStringSlice(f.Name)
+			value = strings.Join(vss, ",")
+		} else {
+			value = v.GetString(f.Name)
+		}
+		fmt.Printf("Visiting '%v' with value '%v', changed: '%v', new value: '%v'\n", f.Name, f.Value, f.Changed, value)
+		if f.Changed {
+			// If f.Changed is true, that means the value has already been set
+			// by a flag, and we don't need to ask viper for it since the flag
+			// is the highest priority. This works around a problem with string
+			// slices where f.Value.Set(csvString) would cause the elements of
+			// csvString to be appended to the existing value rather than
+			// replacing it.
+			return
+		}
 		flagErr = f.Value.Set(value)
 	})
 	return flagErr

cmd/server.go

@@ -78,6 +78,7 @@ on the configured port.`,
 	flags.StringVarP(&Serve.Config.DataDir, "data-dir", "d", "~/.pilosa", "Directory to store pilosa data files.")
 	flags.StringVarP(&Serve.Config.Host, "bind", "", ":10101", "Default URI on which pilosa should listen.")
 	flags.IntVarP(&Serve.Config.Cluster.ReplicaN, "cluster.replicas", "", 1, "Number hosts each piece of data should be stored on.")
+	flags.StringSliceVarP(&Serve.Config.Cluster.Nodes, "cluster.hosts", "", []string{}, "Comma separated list of hosts in cluster.")
 	flags.StringVarP(&Serve.CPUProfile, "cpu-profile", "", "", "Where to store CPU profile.")
 	flags.DurationVarP(&Serve.CPUTime, "cpu-time", "", 30*time.Second, "CPU profile duration.")
 

cmd/server_test.go

@@ -3,6 +3,7 @@ package cmd_test
 import (
 	"fmt"
 	"io/ioutil"
+	"reflect"
 	"strings"
 	"sync"
 	"testing"
@@ -29,7 +30,7 @@ func (v *validator) Check(actual, expected interface{}) {
 	if v.err != nil {
 		return
 	}
-	if actual != expected {
+	if !reflect.DeepEqual(actual, expected) {
 		v.err = fmt.Errorf("Actual: '%v' is not equal to '%v'", actual, expected)
 	}
 }
@@ -47,7 +48,7 @@ func TestServerConfig(t *testing.T) {
 	failErr(t, err, "making data dir")
 	tests := []commandTest{
 		{
-			args: []string{"server", "--data-dir", actualDataDir},
+			args: []string{"server", "--data-dir", actualDataDir, "--cluster.hosts", "example.com:10101,example.com:10110"},
 			env:  map[string]string{"PILOSA_DATA_DIR": "/tmp/myEnvDatadir"},
 			cfgFileContent: `
 data-dir = "/tmp/myFileDatadir"
@@ -55,18 +56,54 @@ bind = "localhost:0"
 
 [cluster]
   replicas = 2
+  hosts = [
+   "localhost:19444",
+   ]
 `,
 			validation: func() error {
 				v := validator{}
 				v.Check(cmd.Serve.Config.DataDir, actualDataDir)
 				v.Check(cmd.Serve.Config.Host, "localhost:0")
 				v.Check(cmd.Serve.Config.Cluster.ReplicaN, 2)
+				v.Check(cmd.Serve.Config.Cluster.Nodes, []string{"example.com:10101", "example.com:10110"})
+				return v.Error()
+			},
+		},
+		{
+			args: []string{"server"},
+			env:  map[string]string{"PILOSA_CLUSTER.HOSTS": "example.com:1110,example.com:1111"},
+			cfgFileContent: `
+[cluster]
+  hosts = [
+   "localhost:19444",
+   ]
+`,
+			validation: func() error {
+				v := validator{}
+				v.Check(cmd.Serve.Config.Cluster.Nodes, []string{"example.com:1110", "example.com:1111"})
+				return v.Error()
+			},
+		},
+		{
+			args: []string{"server"},
+			env:  map[string]string{},
+			cfgFileContent: `
+[cluster]
+  hosts = [
+   "localhost:19444",
+   ]
+
+`,
+			validation: func() error {
+				v := validator{}
+				v.Check(cmd.Serve.Config.Cluster.Nodes, []string{"localhost:19444"})
 				return v.Error()
 			},
 		},
 	}
+
 	for i, test := range tests {
-		com := setupCommand(t, test.args, test.env, test.cfgFileContent)
+		com := test.setupCommand(t)
 		wait := sync.Mutex{}
 		wait.Lock()
 		var execErr error
@@ -84,32 +121,39 @@ bind = "localhost:0"
 		if err := test.validation(); err != nil {
 			t.Fatalf("Failed test %d due to: %v", i, err)
 		}
+		test.reset()
 	}
 }
 
-func setupCommand(t *testing.T, args []string, env map[string]string, cfgFileContent string) *cobra.Command {
+func (ct commandTest) setupCommand(t *testing.T) *cobra.Command {
 	// make config file
 	cfgFile, err := ioutil.TempFile("", "")
 	failErr(t, err, "making temp file")
-	_, err = cfgFile.WriteString(cfgFileContent)
+	_, err = cfgFile.WriteString(ct.cfgFileContent)
 	failErr(t, err, "writing config to temp file")
 
 	// set up config file args/env
-	env["PILOSA_CONFIG"] = cfgFile.Name()
-	args = append(args[:1], append([]string{"--config=" + cfgFile.Name()}, args[1:]...)...)
+	ct.env["PILOSA_CONFIG"] = cfgFile.Name()
+	ct.args = append(ct.args[:1], append([]string{"--config=" + cfgFile.Name()}, ct.args[1:]...)...)
 
 	// set up env
-	for name, val := range env {
+	for name, val := range ct.env {
 		err = os.Setenv(name, val)
 		failErr(t, err, fmt.Sprintf("setting environment variable '%s' to '%s'", name, val))
 	}
 
 	// make command and set args
 	rc := cmd.NewRootCommand(strings.NewReader(""), ioutil.Discard, ioutil.Discard)
-	rc.SetArgs(args)
+	rc.SetArgs(ct.args)
 
 	err = cfgFile.Close()
 	failErr(t, err, "closing config file")
 
 	return rc
 }
+
+func (ct commandTest) reset() {
+	for name, _ := range ct.env {
+		os.Setenv(name, "")
+	}
+}

config.go

@@ -13,9 +13,9 @@ type Config struct {
 	Host    string `toml:"host"`
 
 	Cluster struct {
-		ReplicaN        int           `toml:"replicas"`
-		Nodes           []*ConfigNode `toml:"node"`
-		PollingInterval Duration      `toml:"polling-interval"`
+		ReplicaN        int      `toml:"replicas"`
+		Nodes           []string `toml:"hosts"`
+		PollingInterval Duration `toml:"polling-interval"`
 	} `toml:"cluster"`
 
 	Plugins struct {
@@ -27,10 +27,6 @@ type Config struct {
 	} `toml:"anti-entropy"`
 }
 
-type ConfigNode struct {
-	Host string `toml:"host"`
-}
-
 // NewConfig returns an instance of Config with default options.
 func NewConfig() *Config {
 	c := &Config{
@@ -38,6 +34,7 @@ func NewConfig() *Config {
 	}
 	c.Cluster.ReplicaN = DefaultReplicaN
 	c.Cluster.PollingInterval = Duration(DefaultPollingInterval)
+	c.Cluster.Nodes = []string{}
 	c.AntiEntropy.Interval = Duration(DefaultAntiEntropyInterval)
 	return c
 }
@@ -45,7 +42,7 @@ func NewConfig() *Config {
 func NewConfigForHosts(hosts []string) *Config {
 	conf := NewConfig()
 	for _, hostport := range hosts {
-		conf.Cluster.Nodes = append(conf.Cluster.Nodes, &ConfigNode{Host: hostport})
+		conf.Cluster.Nodes = append(conf.Cluster.Nodes, hostport)
 	}
 	return conf
 }
@@ -55,8 +52,8 @@ func (c *Config) PilosaCluster() *Cluster {
 	cluster := NewCluster()
 	cluster.ReplicaN = c.Cluster.ReplicaN
 
-	for _, n := range c.Cluster.Nodes {
-		cluster.Nodes = append(cluster.Nodes, &Node{Host: n.Host})
+	for _, hostport := range c.Cluster.Nodes {
+		cluster.Nodes = append(cluster.Nodes, &Node{Host: hostport})
 	}
 
 	return cluster