Merge pull request #2582 from xiang90/cluster-check

etcdserver: loose member validation for joining existing cluster

etcdserver/config.go

@@ -49,7 +49,7 @@ type ServerConfig struct {
 // VerifyBootstrapConfig sanity-checks the initial config for bootstrap case
 // and returns an error for things that should never happen.
 func (c *ServerConfig) VerifyBootstrap() error {
-	if err := c.verifyLocalMember(); err != nil {
+	if err := c.verifyLocalMember(true); err != nil {
 		return err
 	}
 	if err := c.Cluster.Validate(); err != nil {
@@ -64,7 +64,10 @@ func (c *ServerConfig) VerifyBootstrap() error {
 // VerifyJoinExisting sanity-checks the initial config for join existing cluster
 // case and returns an error for things that should never happen.
 func (c *ServerConfig) VerifyJoinExisting() error {
-	if err := c.verifyLocalMember(); err != nil {
+	// no need for strict checking since the member have announced its
+	// peer urls to the cluster before starting and do not have to set
+	// it in the configuration again.
+	if err := c.verifyLocalMember(false); err != nil {
 		return err
 	}
 	if err := c.Cluster.Validate(); err != nil {
@@ -76,9 +79,10 @@ func (c *ServerConfig) VerifyJoinExisting() error {
 	return nil
 }
 
-// verifyLocalMember verifies that the local member is valid and is listed
-// in the cluster correctly.
-func (c *ServerConfig) verifyLocalMember() error {
+// verifyLocalMember verifies the configured member is in configured
+// cluster. If strict is set, it also verifies the configured member
+// has the same peer urls as configured advertised peer urls.
+func (c *ServerConfig) verifyLocalMember(strict bool) error {
 	m := c.Cluster.MemberByName(c.Name)
 	// Make sure the cluster at least contains the local server.
 	if m == nil {
@@ -92,8 +96,10 @@ func (c *ServerConfig) verifyLocalMember() error {
 	// TODO: Remove URLStringsEqual after improvement of using hostnames #2150 #2123
 	apurls := c.PeerURLs.StringSlice()
 	sort.Strings(apurls)
-	if !netutil.URLStringsEqual(apurls, m.PeerURLs) {
-		return fmt.Errorf("%s has different advertised URLs in the cluster and advertised peer URLs list", c.Name)
+	if strict {
+		if !netutil.URLStringsEqual(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

@@ -22,6 +22,9 @@ import (
 )
 
 func mustNewURLs(t *testing.T, urls []string) []url.URL {
+	if len(urls) == 0 {
+		return nil
+	}
 	u, err := types.NewURLs(urls)
 	if err != nil {
 		t.Fatalf("error creating new URLs from %q: %v", urls, err)
@@ -65,12 +68,14 @@ func TestConfigVerifyLocalMember(t *testing.T) {
 	tests := []struct {
 		clusterSetting string
 		apurls         []string
+		strict         bool
 		shouldError    bool
 	}{
 		{
 			// Node must exist in cluster
 			"",
 			nil,
+			true,
 
 			true,
 		},
@@ -78,6 +83,7 @@ func TestConfigVerifyLocalMember(t *testing.T) {
 			// Initial cluster set
 			"node1=http://localhost:7001,node2=http://localhost:7002",
 			[]string{"http://localhost:7001"},
+			true,
 
 			false,
 		},
@@ -85,6 +91,7 @@ func TestConfigVerifyLocalMember(t *testing.T) {
 			// Default initial cluster
 			"node1=http://localhost:2380,node1=http://localhost:7001",
 			[]string{"http://localhost:2380", "http://localhost:7001"},
+			true,
 
 			false,
 		},
@@ -92,6 +99,7 @@ func TestConfigVerifyLocalMember(t *testing.T) {
 			// Advertised peer URLs must match those in cluster-state
 			"node1=http://localhost:7001",
 			[]string{"http://localhost:12345"},
+			true,
 
 			true,
 		},
@@ -99,9 +107,26 @@ func TestConfigVerifyLocalMember(t *testing.T) {
 			// Advertised peer URLs must match those in cluster-state
 			"node1=http://localhost:7001,node1=http://localhost:12345",
 			[]string{"http://localhost:12345"},
+			true,
 
 			true,
 		},
+		{
+			// Advertised peer URLs must match those in cluster-state
+			"node1=http://localhost:7001",
+			[]string{},
+			true,
+
+			true,
+		},
+		{
+			// do not care about the urls if strict is not set
+			"node1=http://localhost:7001",
+			[]string{},
+			false,
+
+			false,
+		},
 	}
 
 	for i, tt := range tests {
@@ -116,7 +141,7 @@ func TestConfigVerifyLocalMember(t *testing.T) {
 		if tt.apurls != nil {
 			cfg.PeerURLs = mustNewURLs(t, tt.apurls)
 		}
-		err = cfg.verifyLocalMember()
+		err = cfg.verifyLocalMember(tt.strict)
 		if (err == nil) && tt.shouldError {
 			t.Errorf("%#v", *cluster)
 			t.Errorf("#%d: Got no error where one was expected", i)