From 120b0887231391d8bf33fdf21b71c20aecd39b55 Mon Sep 17 00:00:00 2001
From: Barak Michener <me@barakmich.com>
Date: Mon, 6 Oct 2014 19:42:13 -0400
Subject: [PATCH] Split config into a separate file and add sanity check and
 test

---
 etcdserver/config.go      | 44 +++++++++++++++++++++++++++++++++++++++
 etcdserver/config_test.go | 32 ++++++++++++++++++++++++++++
 etcdserver/server.go      | 21 ++++---------------
 3 files changed, 80 insertions(+), 17 deletions(-)
 create mode 100644 etcdserver/config.go
 create mode 100644 etcdserver/config_test.go

diff --git a/etcdserver/config.go b/etcdserver/config.go
new file mode 100644
index 000000000..22f489b70
--- /dev/null
+++ b/etcdserver/config.go
@@ -0,0 +1,44 @@
+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)
+	}
+
+	if c.ClusterState == ClusterStateValueNew {
+		// 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
+}
diff --git a/etcdserver/config_test.go b/etcdserver/config_test.go
new file mode 100644
index 000000000..2b605cdf1
--- /dev/null
+++ b/etcdserver/config_test.go
@@ -0,0 +1,32 @@
+package etcdserver
+
+import (
+	"testing"
+)
+
+func TestConfigVerify(t *testing.T) {
+	cluster := &Cluster{}
+	cfg := ServerConfig{
+		Name:         "node1",
+		Cluster:      cluster,
+		ClusterState: ClusterStateValueNew,
+	}
+
+	err := cfg.Verify()
+	if err == nil {
+		t.Error("Did not get error for lacking self in cluster.")
+	}
+
+	cluster.Set("node1=http://localhost:7001,node2=http://localhost:7001")
+	err = cfg.Verify()
+	if err == nil {
+		t.Error("Did not get error for double URL in cluster.")
+	}
+
+	cluster.Set("node1=http://localhost:7001,node2=http://localhost:7002")
+	err = cfg.Verify()
+	if err != nil {
+		t.Errorf("Got unexpected error %v", err)
+	}
+
+}
diff --git a/etcdserver/server.go b/etcdserver/server.go
index 4ad33265a..33c571ae4 100644
--- a/etcdserver/server.go
+++ b/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 != "" {