Merge pull request #1676 from jonboulle/doc_initial_cluster

etcdserver: validate and document initial-advertise-peer-urls

Documentation/0.5/clustering.md

@@ -10,7 +10,7 @@ This guide will walk you through configuring a three machine etcd cluster with t
 
 ## Static
 
-As we know the cluster members, their addresses and the size of the cluster before starting we can use an offline bootstrap configuration. Each machine will get either the following command line or environment variables:
+As we know the cluster members, their addresses and the size of the cluster before starting, we can use an offline bootstrap configuration by setting the `initial-cluster` flag. Each machine will get either the following command line or environment variables:
 
 ```
 ETCD_INITIAL_CLUSTER="infra0=http://10.0.1.10:2380,infra1=http://10.0.1.11:2380,infra2=http://10.0.1.12:2380"
@@ -22,6 +22,8 @@ ETCD_INITIAL_CLUSTER_STATE=new
   -initial-cluster-state new
 ```
 
+Note that the URLs specified in `initial-cluster` are the _advertised peer URLs_, i.e. they should match the value of `initial-advertise-peer-urls` on the respective nodes.
+
 If you are spinning up multiple clusters (or creating and destroying a single cluster) with same configuration for testing purpose, it is highly recommended that you specify a unique `initial-cluster-token` for the different clusters. By doing this, etcd can generate unique cluster IDs and member IDs for the clusters even if they otherwise have the exact same configuration. This can protect you from cross-cluster-interaction, which might corrupt your clusters.
 
 On each machine you would start etcd with these flags:
@@ -45,11 +47,11 @@ $ etcd -name infra2 -initial-advertise-peer-urls https://10.0.1.12:2380 \
   -initial-cluster-state new
 ```
 
-The command line parameters starting with `-initial-cluster` will be ignored on subsequent runs of etcd. You are free to remove the environment variables or command line flags after the initial bootstrap process. If you need to make changes to the configuration later see our guide on [runtime configuration](runtime-configuration.md).
+The command line parameters starting with `-initial-cluster` will be ignored on subsequent runs of etcd. You are free to remove the environment variables or command line flags after the initial bootstrap process. If you need to make changes to the configuration later (for example, adding or removing members to/from the cluster), see the [runtime configuration](runtime-configuration.md) guide.
 
 ### Error Cases
 
-In the following case we have not included our new host in the list of enumerated nodes. If this is a new cluster, the node must be added to the list of initial cluster members.
+In the following example, we have not included our new host in the list of enumerated nodes. If this is a new cluster, the node _must_ be added to the list of initial cluster members.
 
 ```
 $ etcd -name infra1 -initial-advertise-peer-urls http://10.0.1.11:2380 \
@@ -59,13 +61,13 @@ etcd: infra1 not listed in the initial cluster config
 exit 1
 ```
 
-In this case we are attempting to map a node (infra0) on a different address (127.0.0.1:2380) than its enumerated address in the cluster list (10.0.1.10:2380). If this node is to listen on multiple addresses, all addresses must be reflected in the "initial-cluster" configuration directive.
+In this example, we are attempting to map a node (infra0) on a different address (127.0.0.1:2380) than its enumerated address in the cluster list (10.0.1.10:2380). If this node is to listen on multiple addresses, all addresses _must_ be reflected in the "initial-cluster" configuration directive.
 
 ```
 $ etcd -name infra0 -initial-advertise-peer-urls http://127.0.0.1:2380 \
   -initial-cluster infra0=http://10.0.1.10:2380,infra1=http://10.0.1.11:2380,infra2=http://10.0.1.12:2380 \
   -initial-cluster-state=new
-etcd: infra0 has different advertised URLs in the cluster and advertised peer URLs list
+etcd: error setting up initial cluster: infra0 has different advertised URLs in the cluster and advertised peer URLs list
 exit 1
 ```
 
@@ -81,7 +83,7 @@ exit 1
 
 ## Discovery
 
-In a number of cases you might not know the IPs of your cluster peers ahead of time. This is common when utilizing cloud providers or when your network uses DHCP. In these cases you can use an existing etcd cluster to bootstrap a new one. We call this process "discovery".
+In a number of cases, you might not know the IPs of your cluster peers ahead of time. This is common when utilizing cloud providers or when your network uses DHCP. In these cases, rather than specifying a static configuration, you can use an existing etcd cluster to bootstrap a new one. We call this process "discovery".
 
 ### Lifetime of a Discovery URL
 
@@ -99,7 +101,7 @@ Discovery uses an existing cluster to bootstrap itself. If you are using your ow
 $ curl -X PUT https://myetcd.local/v2/keys/discovery/6c007a14875d53d9bf0ef5a6fc0257c817f0fb83/_config/size -d value=3
 ```
 
-By setting the size key to the URL, you create a discovery URL with expected-cluster-size of 3.
+By setting the size key to the URL, you create a discovery URL with an expected cluster size of 3.
 
 If you bootstrap an etcd cluster using discovery service with more than the expected number of etcd members, the extra etcd processes will [fall back][fall-back] to being [proxies][proxy] by default.
 
@@ -124,14 +126,14 @@ This will cause each member to register itself with the custom etcd discovery se
 
 ### Public discovery service
 
-If you do not have access to an existing cluster you can use the public discovery service hosted at discovery.etcd.io.  You can create a private discovery URL using the "new" endpoint like so:
+If you do not have access to an existing cluster, you can use the public discovery service hosted at `discovery.etcd.io`.  You can create a private discovery URL using the "new" endpoint like so:
 
 ```
 $ curl https://discovery.etcd.io/new?size=3
 https://discovery.etcd.io/3e86b59982e49066c5d813af1c2e2579cbf573de
 ```
 
-This will create the cluster with an initial expected size of 3 members. If you do not specify a size a default of 3 will be used.
+This will create the cluster with an initial expected size of 3 members. If you do not specify a size, a default of 3 will be used.
 
 If you bootstrap an etcd cluster using discovery service with more than the expected number of etcd members, the extra etcd processes will [fall back][fall-back] to being [proxies][proxy] by default.
 
@@ -169,6 +171,7 @@ You can use the environment variable `ETCD_DISCOVERY_PROXY` to cause etcd to use
 
 #### Discovery Server Errors
 
+
 ```
 $ etcd -name infra0 -initial-advertise-peer-urls http://10.0.1.10:2380 \
   -discovery https://discovery.etcd.io/3e86b59982e49066c5d813af1c2e2579cbf573de
@@ -178,10 +181,13 @@ exit 1
 
 #### User Errors
 
+This error will occur if the discovery cluster already has the configured number of members, and `discovery-fallback` is explicitly disabled
+
 ```
 $ etcd -name infra0 -initial-advertise-peer-urls http://10.0.1.10:2380 \
-  -discovery https://discovery.etcd.io/3e86b59982e49066c5d813af1c2e2579cbf573de
-etcd: error: the cluster using discovery https://discovery.etcd.io/3e86b59982e49066c5d813af1c2e2579cbf573de has already started with all 5 members
+  -discovery https://discovery.etcd.io/3e86b59982e49066c5d813af1c2e2579cbf573de \
+  -discovery-fallback exit
+etcd: discovery: cluster is full
 exit 1
 ```
 
@@ -193,7 +199,7 @@ ignored on this machine.
 ```
 $ etcd -name infra0 -initial-advertise-peer-urls http://10.0.1.10:2380 \
   -discovery https://discovery.etcd.io/3e86b59982e49066c5d813af1c2e2579cbf573de
-etcd: warn: ignoring discovery URL: etcd has already been initialized and has a valid log in /var/lib/etcd
+etcdserver: warn: ignoring discovery: etcd has already been initialized and has a valid log in /var/lib/etcd
 ```
 
 # 0.4 to 0.5+ Migration Guide

etcdmain/etcd.go

@@ -22,6 +22,7 @@ import (
 	"log"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"strings"
 
@@ -188,7 +189,11 @@ func Main() {
 
 // startEtcd launches the etcd server and HTTP handlers for client/server communication.
 func startEtcd() error {
-	cls, err := setupCluster()
+	apurls, err := flags.URLsFromFlags(fs, "initial-advertise-peer-urls", "addr", peerTLSInfo)
+	if err != nil {
+		return err
+	}
+	cls, err := setupCluster(apurls)
 	if err != nil {
 		return fmt.Errorf("error setting up initial cluster: %v", err)
 	}
@@ -265,6 +270,7 @@ func startEtcd() error {
 	cfg := &etcdserver.ServerConfig{
 		Name:            *name,
 		ClientURLs:      acurls,
+		PeerURLs:        apurls,
 		DataDir:         *dir,
 		SnapCount:       *snapCount,
 		Cluster:         cls,
@@ -303,7 +309,11 @@ func startEtcd() error {
 
 // startProxy launches an HTTP proxy for client communication which proxies to other etcd nodes.
 func startProxy() error {
-	cls, err := setupCluster()
+	apurls, err := flags.URLsFromFlags(fs, "initial-advertise-peer-urls", "addr", peerTLSInfo)
+	if err != nil {
+		return err
+	}
+	cls, err := setupCluster(apurls)
 	if err != nil {
 		return fmt.Errorf("error setting up initial cluster: %v", err)
 	}
@@ -363,8 +373,8 @@ func startProxy() error {
 	return nil
 }
 
-// setupCluster sets up the cluster definition for bootstrap or discovery.
-func setupCluster() (*etcdserver.Cluster, error) {
+// setupCluster sets up an initial cluster definition for bootstrap or discovery.
+func setupCluster(apurls []url.URL) (*etcdserver.Cluster, error) {
 	set := make(map[string]bool)
 	fs.Visit(func(f *flag.Flag) {
 		set[f.Name] = true
@@ -372,21 +382,18 @@ func setupCluster() (*etcdserver.Cluster, error) {
 	if set["discovery"] && set["initial-cluster"] {
 		return nil, fmt.Errorf("both discovery and bootstrap-config are set")
 	}
-	apurls, err := flags.URLsFromFlags(fs, "initial-advertise-peer-urls", "addr", peerTLSInfo)
-	if err != nil {
-		return nil, err
-	}
-
 	var cls *etcdserver.Cluster
+	var err error
 	switch {
 	case set["discovery"]:
+		// If using discovery, generate a temporary cluster based on
+		// self's advertised peer URLs
 		clusterStr := genClusterString(*name, apurls)
 		cls, err = etcdserver.NewClusterFromString(*durl, clusterStr)
 	case set["initial-cluster"]:
 		fallthrough
 	default:
 		// We're statically configured, and cluster has appropriately been set.
-		// Try to configure by indexing the static cluster by name.
 		cls, err = etcdserver.NewClusterFromString(*initialClusterToken, *initialCluster)
 	}
 	return cls, err

etcdmain/etcd_test.go

@@ -17,11 +17,20 @@
 package etcdmain
 
 import (
+	"net/url"
 	"testing"
 
 	"github.com/coreos/etcd/pkg/types"
 )
 
+func mustNewURLs(t *testing.T, urls []string) []url.URL {
+	u, err := types.NewURLs(urls)
+	if err != nil {
+		t.Fatalf("unexpected new urls error: %v", err)
+	}
+	return u
+}
+
 func TestGenClusterString(t *testing.T) {
 	tests := []struct {
 		token string
@@ -38,10 +47,7 @@ func TestGenClusterString(t *testing.T) {
 		},
 	}
 	for i, tt := range tests {
-		urls, err := types.NewURLs(tt.urls)
-		if err != nil {
-			t.Fatalf("unexpected new urls error: %v", err)
-		}
+		urls := mustNewURLs(t, tt.urls)
 		str := genClusterString(tt.token, urls)
 		if str != tt.wstr {
 			t.Errorf("#%d: cluster = %s, want %s", i, str, tt.wstr)

etcdserver/cluster.go

@@ -42,11 +42,18 @@ const (
 )
 
 type ClusterInfo interface {
+	// ID returns the cluster ID
 	ID() types.ID
+	// ClientURLs returns an aggregate set of all URLs on which this
+	// cluster is listening for client requests
 	ClientURLs() []string
 	// Members returns a slice of members sorted by their ID
 	Members() []*Member
+	// Member retrieves a particular member based on ID, or nil if the
+	// member does not exist in the cluster
 	Member(id types.ID) *Member
+	// IsIDRemoved checks whether the given ID has been removed from this
+	// cluster at some point in the past
 	IsIDRemoved(id types.ID) bool
 }
 
@@ -62,8 +69,9 @@ type Cluster struct {
 	sync.Mutex
 }
 
-// NewClusterFromString returns Cluster through given cluster token and parsing
-// members from a sets of names to IPs discovery formatted like:
+// NewClusterFromString returns a Cluster instantiated from the given cluster token
+// and cluster string, by parsing members from a set of discovery-formatted
+// names-to-IPs, like:
 // mach0=http://1.1.1.1,mach0=http://2.2.2.2,mach1=http://3.3.3.3,mach2=http://4.4.4.4
 func NewClusterFromString(token string, cluster string) (*Cluster, error) {
 	c := newCluster(token)

etcdserver/config.go

@@ -20,6 +20,8 @@ import (
 	"fmt"
 	"net/http"
 	"path"
+	"reflect"
+	"sort"
 
 	"github.com/coreos/etcd/pkg/types"
 	"github.com/coreos/etcd/raft"
@@ -31,6 +33,7 @@ type ServerConfig struct {
 	DiscoveryURL    string
 	DiscoveryProxy  string
 	ClientURLs      types.URLs
+	PeerURLs        types.URLs
 	DataDir         string
 	SnapCount       uint64
 	Cluster         *Cluster
@@ -45,7 +48,7 @@ func (c *ServerConfig) VerifyBootstrapConfig() error {
 	m := c.Cluster.MemberByName(c.Name)
 	// Make sure the cluster at least contains the local server.
 	if m == nil {
-		return fmt.Errorf("couldn't find local name %s in the initial cluster configuration", c.Name)
+		return fmt.Errorf("couldn't find local name %q in the initial cluster configuration", c.Name)
 	}
 	if uint64(m.ID) == raft.None {
 		return fmt.Errorf("cannot use %x as member id", raft.None)
@@ -65,6 +68,13 @@ func (c *ServerConfig) VerifyBootstrapConfig() error {
 			urlMap[url] = true
 		}
 	}
+
+	// Advertised peer URLs must match those in the cluster peer list
+	apurls := c.PeerURLs.StringSlice()
+	sort.Strings(apurls)
+	if !reflect.DeepEqual(apurls, m.PeerURLs) {
+		return fmt.Errorf("%s has different advertised URLs in the cluster and advertised peer URLs list", c.Name)
+	}
 	return nil
 }
 

etcdserver/config_test.go

@@ -16,12 +16,26 @@
 
 package etcdserver
 
-import "testing"
+import (
+	"net/url"
+	"testing"
+
+	"github.com/coreos/etcd/pkg/types"
+)
+
+func mustNewURLs(t *testing.T, urls []string) []url.URL {
+	u, err := types.NewURLs(urls)
+	if err != nil {
+		t.Fatalf("error creating new URLs from %q: %v", urls, err)
+	}
+	return u
+}
 
 func TestBootstrapConfigVerify(t *testing.T) {
 	tests := []struct {
 		clusterSetting string
 		newclst        bool
+		apurls         []string
 		disc           string
 		shouldError    bool
 	}{
@@ -29,35 +43,63 @@ func TestBootstrapConfigVerify(t *testing.T) {
 			// Node must exist in cluster
 			"",
 			true,
+			nil,
 			"",
+
 			true,
 		},
 		{
 			// Cannot have duplicate URLs in cluster config
 			"node1=http://localhost:7001,node2=http://localhost:7001,node2=http://localhost:7002",
 			true,
+			nil,
 			"",
+
 			true,
 		},
 		{
 			// Node defined, ClusterState OK
 			"node1=http://localhost:7001,node2=http://localhost:7002",
 			true,
+			[]string{"http://localhost:7001"},
 			"",
+
 			false,
 		},
 		{
 			// Node defined, discovery OK
 			"node1=http://localhost:7001",
 			false,
+			[]string{"http://localhost:7001"},
 			"http://discovery",
+
 			false,
 		},
 		{
 			// Cannot have ClusterState!=new && !discovery
 			"node1=http://localhost:7001",
 			false,
+			nil,
+			"",
+
+			true,
+		},
+		{
+			// Advertised peer URLs must match those in cluster-state
+			"node1=http://localhost:7001",
+			true,
+			[]string{"http://localhost:12345"},
+			"",
+
+			true,
+		},
+		{
+			// Advertised peer URLs must match those in cluster-state
+			"node1=http://localhost:7001,node1=http://localhost:12345",
+			true,
+			[]string{"http://localhost:12345"},
 			"",
+
 			true,
 		},
 	}
@@ -67,13 +109,15 @@ func TestBootstrapConfigVerify(t *testing.T) {
 		if err != nil {
 			t.Fatalf("#%d: Got unexpected error: %v", i, err)
 		}
-
 		cfg := ServerConfig{
 			Name:         "node1",
 			DiscoveryURL: tt.disc,
 			Cluster:      cluster,
 			NewCluster:   tt.newclst,
 		}
+		if tt.apurls != nil {
+			cfg.PeerURLs = mustNewURLs(t, tt.apurls)
+		}
 		err = cfg.VerifyBootstrapConfig()
 		if (err == nil) && tt.shouldError {
 			t.Errorf("%#v", *cluster)

integration/cluster_test.go

@@ -25,7 +25,6 @@ import (
 	"net/http/httptest"
 	"os"
 	"strings"
-	"sync"
 	"testing"
 	"time"
 
@@ -154,17 +153,19 @@ func NewClusterByDiscovery(t *testing.T, size int, url string) *cluster {
 }
 
 func (c *cluster) Launch(t *testing.T) {
-	var wg sync.WaitGroup
+	errc := make(chan error)
 	for _, m := range c.Members {
-		wg.Add(1)
 		// Members are launched in separate goroutines because if they boot
 		// using discovery url, they have to wait for others to register to continue.
 		go func(m *member) {
-			m.Launch(t)
-			wg.Done()
+			errc <- m.Launch()
 		}(m)
 	}
-	wg.Wait()
+	for _ = range c.Members {
+		if err := <-errc; err != nil {
+			t.Fatalf("error setting up member: %v", err)
+		}
+	}
 	// wait cluster to be stable to receive future client requests
 	c.waitClientURLsPublished(t)
 }
@@ -243,15 +244,23 @@ type member struct {
 func mustNewMember(t *testing.T, name string) *member {
 	var err error
 	m := &member{}
+
 	pln := newLocalListener(t)
 	m.PeerListeners = []net.Listener{pln}
+	m.PeerURLs, err = types.NewURLs([]string{"http://" + pln.Addr().String()})
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	cln := newLocalListener(t)
 	m.ClientListeners = []net.Listener{cln}
-	m.Name = name
 	m.ClientURLs, err = types.NewURLs([]string{"http://" + cln.Addr().String()})
 	if err != nil {
 		t.Fatal(err)
 	}
+
+	m.Name = name
+
 	m.DataDir, err = ioutil.TempDir(os.TempDir(), "etcd")
 	if err != nil {
 		t.Fatal(err)
@@ -268,10 +277,10 @@ func mustNewMember(t *testing.T, name string) *member {
 
 // Launch starts a member based on ServerConfig, PeerListeners
 // and ClientListeners.
-func (m *member) Launch(t *testing.T) {
+func (m *member) Launch() error {
 	var err error
 	if m.s, err = etcdserver.NewServer(&m.ServerConfig); err != nil {
-		t.Fatalf("failed to initialize the etcd server: %v", err)
+		return fmt.Errorf("failed to initialize the etcd server: %v", err)
 	}
 	m.s.Ticker = time.Tick(tickDuration)
 	m.s.SyncTicker = time.Tick(10 * tickDuration)
@@ -293,6 +302,7 @@ func (m *member) Launch(t *testing.T) {
 		hs.Start()
 		m.hss = append(m.hss, hs)
 	}
+	return nil
 }
 
 // Stop stops the member, but the data dir of the member is preserved.