Merge pull request #1249 from barakmich/sanity_check

Split config into a separate file and add sanity check and test

etcdserver/config.go

@@ -0,0 +1,42 @@
+package etcdserver
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/coreos/etcd/pkg/types"
+)
+
+// ServerConfig holds the configuration of etcd as taken from the command line or discovery.
+type ServerConfig struct {
+	Name         string
+	DiscoveryURL string
+	ClientURLs   types.URLs
+	DataDir      string
+	SnapCount    int64
+	Cluster      *Cluster
+	ClusterState ClusterState
+	Transport    *http.Transport
+}
+
+// Verify sanity-checks the config struct and returns an error for things that
+// should never happen.
+func (c *ServerConfig) Verify() error {
+	// Make sure the cluster at least contains the local server.
+	m := c.Cluster.FindName(c.Name)
+	if m == nil {
+		return fmt.Errorf("could not find name %v in cluster", c.Name)
+	}
+
+	// No identical IPs in the cluster peer list
+	urlMap := make(map[string]bool)
+	for _, m := range *c.Cluster {
+		for _, url := range m.PeerURLs {
+			if urlMap[url] {
+				return fmt.Errorf("duplicate url %v in server config", url)
+			}
+			urlMap[url] = true
+		}
+	}
+	return nil
+}

etcdserver/config_test.go

@@ -0,0 +1,32 @@
+package etcdserver
+
+import (
+	"testing"
+)
+
+func TestConfigVerify(t *testing.T) {
+	tests := []struct {
+		clusterSetting string
+		shouldError    bool
+	}{
+		{"", true},
+		{"node1=http://localhost:7001,node2=http://localhost:7001", true},
+		{"node1=http://localhost:7001,node2=http://localhost:7002", false},
+	}
+
+	for i, tt := range tests {
+		cluster := &Cluster{}
+		cluster.Set(tt.clusterSetting)
+		cfg := ServerConfig{
+			Name:    "node1",
+			Cluster: cluster,
+		}
+		err := cfg.Verify()
+		if (err == nil) && tt.shouldError {
+			t.Errorf("#%d: Got no error where one was expected", i)
+		}
+		if (err != nil) && !tt.shouldError {
+			t.Errorf("#%d: Got unexpected error: %v", i, err)
+		}
+	}
+}

etcdserver/server.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"log"
 	"math/rand"
-	"net/http"
 	"os"
 	"path"
 	"sync/atomic"
@@ -84,24 +83,12 @@ type RaftTimer interface {
 	Term() int64
 }
 
-type ServerConfig struct {
-	Name         string
-	DiscoveryURL string
-	ClientURLs   types.URLs
-	DataDir      string
-	SnapCount    int64
-	Cluster      *Cluster
-	ClusterState ClusterState
-	Transport    *http.Transport
-}
-
 // NewServer creates a new EtcdServer from the supplied configuration. The
 // configuration is considered static for the lifetime of the EtcdServer.
 func NewServer(cfg *ServerConfig) *EtcdServer {
-	m := cfg.Cluster.FindName(cfg.Name)
-	if m == nil {
-		// Should never happen
-		log.Fatalf("could not find name %v in cluster!", cfg.Name)
+	err := cfg.Verify()
+	if err != nil {
+		log.Fatalln(err)
 	}
 	snapdir := path.Join(cfg.DataDir, "snap")
 	if err := os.MkdirAll(snapdir, privateDirMode); err != nil {
@@ -111,7 +98,7 @@ func NewServer(cfg *ServerConfig) *EtcdServer {
 	st := store.New()
 	var w *wal.WAL
 	var n raft.Node
-	var err error
+	m := cfg.Cluster.FindName(cfg.Name)
 	waldir := path.Join(cfg.DataDir, "wal")
 	if !wal.Exist(waldir) {
 		if cfg.DiscoveryURL != "" {