diff --git a/etcdserver/config.go b/etcdserver/config.go index 877479381..e06c498cd 100644 --- a/etcdserver/config.go +++ b/etcdserver/config.go @@ -46,9 +46,39 @@ type ServerConfig struct { ElectionTicks int } -// VerifyBootstrapConfig sanity-checks the initial config and returns an error -// for things that should never happen. -func (c *ServerConfig) VerifyBootstrapConfig() error { +// 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 { + return err + } + if err := c.Cluster.Validate(); err != nil { + return err + } + if c.Cluster.String() == "" && c.DiscoveryURL == "" { + return fmt.Errorf("initial cluster unset and no discovery URL found") + } + return nil +} + +// 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 { + return err + } + if err := c.Cluster.Validate(); err != nil { + return err + } + if c.DiscoveryURL != "" { + return fmt.Errorf("discovery URL should not be set when joining existing initial cluster") + } + return nil +} + +// verifyLocalMember verifies that the local member is valid and is listed +// in the cluster correctly. +func (c *ServerConfig) verifyLocalMember() error { m := c.Cluster.MemberByName(c.Name) // Make sure the cluster at least contains the local server. if m == nil { @@ -58,14 +88,6 @@ func (c *ServerConfig) VerifyBootstrapConfig() error { return fmt.Errorf("cannot use %x as member id", raft.None) } - if c.DiscoveryURL == "" && !c.NewCluster { - return fmt.Errorf("initial cluster state unset and no wal or discovery URL found") - } - - if err := c.Cluster.Validate(); err != nil { - return err - } - // Advertised peer URLs must match those in the cluster peer list // TODO: Remove URLStringsEqual after improvement of using hostnames #2150 #2123 apurls := c.PeerURLs.StringSlice() diff --git a/etcdserver/config_test.go b/etcdserver/config_test.go index 0a0f011a9..6ee7f5db8 100644 --- a/etcdserver/config_test.go +++ b/etcdserver/config_test.go @@ -29,74 +29,76 @@ func mustNewURLs(t *testing.T, urls []string) []url.URL { return u } -func TestBootstrapConfigVerify(t *testing.T) { +func TestConfigVerifyBootstrapWithoutClusterAndDiscoveryURLFail(t *testing.T) { + cluster, err := NewClusterFromString("", "") + if err != nil { + t.Fatalf("NewClusterFromString error: %v", err) + } + c := &ServerConfig{ + Name: "node1", + DiscoveryURL: "", + Cluster: cluster, + } + if err := c.VerifyBootstrap(); err == nil { + t.Errorf("err = nil, want not nil") + } +} + +func TestConfigVerifyExistingWithDiscoveryURLFail(t *testing.T) { + cluster, err := NewClusterFromString("", "node1=http://127.0.0.1:2380") + if err != nil { + t.Fatalf("NewClusterFromString error: %v", err) + } + c := &ServerConfig{ + Name: "node1", + DiscoveryURL: "http://127.0.0.1:4001/abcdefg", + PeerURLs: mustNewURLs(t, []string{"http://127.0.0.1:2380"}), + Cluster: cluster, + NewCluster: false, + } + if err := c.VerifyJoinExisting(); err == nil { + t.Errorf("err = nil, want not nil") + } +} + +func TestConfigVerifyLocalMember(t *testing.T) { tests := []struct { clusterSetting string - newclst bool apurls []string - disc string shouldError bool }{ { // 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 + // Initial cluster set "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", + // Default initial cluster + "node1=http://localhost:2380,node1=http://localhost:7001", + []string{"http://localhost:2380", "http://localhost:7001"}, 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, }, @@ -108,15 +110,13 @@ func TestBootstrapConfigVerify(t *testing.T) { t.Fatalf("#%d: Got unexpected error: %v", i, err) } cfg := ServerConfig{ - Name: "node1", - DiscoveryURL: tt.disc, - Cluster: cluster, - NewCluster: tt.newclst, + Name: "node1", + Cluster: cluster, } if tt.apurls != nil { cfg.PeerURLs = mustNewURLs(t, tt.apurls) } - err = cfg.VerifyBootstrapConfig() + err = cfg.verifyLocalMember() if (err == nil) && tt.shouldError { t.Errorf("%#v", *cluster) t.Errorf("#%d: Got no error where one was expected", i) diff --git a/etcdserver/server.go b/etcdserver/server.go index bfd663a9f..523622659 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -157,6 +157,9 @@ func NewServer(cfg *ServerConfig) (*EtcdServer, error) { switch { case !haveWAL && !cfg.NewCluster: + if err := cfg.VerifyJoinExisting(); err != nil { + return nil, err + } existingCluster, err := GetClusterFromRemotePeers(getRemotePeerURLs(cfg.Cluster, cfg.Name), cfg.Transport) if err != nil { return nil, fmt.Errorf("cannot fetch cluster info from peer urls: %v", err) @@ -170,7 +173,7 @@ func NewServer(cfg *ServerConfig) (*EtcdServer, error) { cfg.Print() id, n, s, w = startNode(cfg, nil) case !haveWAL && cfg.NewCluster: - if err := cfg.VerifyBootstrapConfig(); err != nil { + if err := cfg.VerifyBootstrap(); err != nil { return nil, err } m := cfg.Cluster.MemberByName(cfg.Name)