From e42d65da125b1fabd218d46cde8df9e856144598 Mon Sep 17 00:00:00 2001 From: Barak Michener Date: Wed, 22 Oct 2014 17:31:27 -0400 Subject: [PATCH] etcdserver: Check the initial cluster settings after checking if the WAL exists --- etcdserver/config.go | 14 ++++++-------- etcdserver/config_test.go | 2 +- etcdserver/server.go | 14 ++++++++------ integration/cluster_test.go | 1 - main.go | 23 ++++++++--------------- 5 files changed, 23 insertions(+), 31 deletions(-) diff --git a/etcdserver/config.go b/etcdserver/config.go index 12dfa8437..1bdb0c34a 100644 --- a/etcdserver/config.go +++ b/etcdserver/config.go @@ -27,7 +27,6 @@ import ( // ServerConfig holds the configuration of etcd as taken from the command line or discovery. type ServerConfig struct { - NodeID uint64 Name string DiscoveryURL string ClientURLs types.URLs @@ -41,7 +40,12 @@ type ServerConfig struct { // VerifyBootstrapConfig sanity-checks the initial config and returns an error // for things that should never happen. func (c *ServerConfig) VerifyBootstrapConfig() error { - if c.NodeID == raft.None { + m := c.Cluster.FindName(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) + } + if m.ID == raft.None { return fmt.Errorf("could not use %x as member id", raft.None) } @@ -49,12 +53,6 @@ func (c *ServerConfig) VerifyBootstrapConfig() error { return fmt.Errorf("initial cluster state unset and no wal or discovery URL found") } - // Make sure the cluster at least contains the local server. - m := c.Cluster.FindID(c.NodeID) - if m == nil { - return fmt.Errorf("couldn't find local ID in cluster config") - } - // No identical IPs in the cluster peer list urlMap := make(map[string]bool) for _, m := range c.Cluster.Members() { diff --git a/etcdserver/config_test.go b/etcdserver/config_test.go index 46b2d6835..f08042091 100644 --- a/etcdserver/config_test.go +++ b/etcdserver/config_test.go @@ -51,7 +51,7 @@ func TestBootstrapConfigVerify(t *testing.T) { } cfg := ServerConfig{ - NodeID: 0x7350a9cd4dc16f76, + Name: "node1", DiscoveryURL: tt.disc, Cluster: cluster, ClusterState: tt.clst, diff --git a/etcdserver/server.go b/etcdserver/server.go index 34f956b26..22912b865 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -181,8 +181,9 @@ func NewServer(cfg *ServerConfig) *EtcdServer { if err := cfg.VerifyBootstrapConfig(); err != nil { log.Fatalf("etcdserver: %v", err) } + m := cfg.Cluster.FindName(cfg.Name) if cfg.ShouldDiscover() { - d, err := discovery.New(cfg.DiscoveryURL, cfg.NodeID, cfg.Cluster.String()) + d, err := discovery.New(cfg.DiscoveryURL, m.ID, cfg.Cluster.String()) if err != nil { log.Fatalf("etcdserver: cannot init discovery %v", err) } @@ -216,9 +217,9 @@ func NewServer(cfg *ServerConfig) *EtcdServer { sstats := &stats.ServerStats{ Name: cfg.Name, - ID: idAsHex(cfg.NodeID), + ID: idAsHex(id), } - lstats := stats.NewLeaderStats(idAsHex(cfg.NodeID)) + lstats := stats.NewLeaderStats(idAsHex(id)) s := &EtcdServer{ store: st, @@ -647,8 +648,9 @@ func startNode(cfg *ServerConfig) (id, cid uint64, n raft.Node, w *wal.WAL) { var err error // TODO: remove the discoveryURL when it becomes part of the source for // generating nodeID. + member := cfg.Cluster.FindName(cfg.Name) cfg.Cluster.GenID([]byte(cfg.DiscoveryURL)) - metadata := pbutil.MustMarshal(&pb.Metadata{NodeID: cfg.NodeID, ClusterID: cfg.Cluster.ID()}) + metadata := pbutil.MustMarshal(&pb.Metadata{NodeID: member.ID, ClusterID: cfg.Cluster.ID()}) if w, err = wal.Create(cfg.WALDir(), metadata); err != nil { log.Fatal(err) } @@ -661,9 +663,9 @@ func startNode(cfg *ServerConfig) (id, cid uint64, n raft.Node, w *wal.WAL) { } peers[i] = raft.Peer{ID: id, Context: ctx} } - id, cid = cfg.NodeID, cfg.Cluster.ID() + id, cid = member.ID, cfg.Cluster.ID() log.Printf("etcdserver: start node %d in cluster %d", id, cid) - n = raft.StartNode(cfg.NodeID, peers, 10, 1) + n = raft.StartNode(member.ID, peers, 10, 1) return } diff --git a/integration/cluster_test.go b/integration/cluster_test.go index 97dcad06b..b4cfd78cc 100644 --- a/integration/cluster_test.go +++ b/integration/cluster_test.go @@ -105,7 +105,6 @@ func (c *cluster) Launch(t *testing.T) { m.PeerListeners = []net.Listener{lns[i]} cln := newLocalListener(t) m.ClientListeners = []net.Listener{cln} - m.NodeID = clusterCfg.FindName(c.name(i)).ID m.Name = c.name(i) m.ClientURLs, err = types.NewURLs([]string{"http://" + cln.Addr().String()}) if err != nil { diff --git a/main.go b/main.go index e3262bdc2..56074f3f9 100644 --- a/main.go +++ b/main.go @@ -141,8 +141,7 @@ func main() { // startEtcd launches the etcd server and HTTP handlers for client/server communication. func startEtcd() { - self, err := setupCluster() - if err != nil { + if err := setupCluster(); err != nil { log.Fatalf("etcd: setupCluster returned error %v", err) } @@ -164,7 +163,6 @@ func startEtcd() { log.Fatal(err.Error()) } cfg := &etcdserver.ServerConfig{ - NodeID: self.ID, Name: *name, ClientURLs: acurls, DataDir: *dir, @@ -262,37 +260,32 @@ func startProxy() { } } -// setupCluster sets cluster to a temporary value if you are using -// discovery, or to the static configuration for bootstrapped clusters. -// Returns the local member on success. -func setupCluster() (*etcdserver.Member, error) { +// setupCluster sets up the cluster definition for bootstrap or discovery. +func setupCluster() error { cluster = etcdserver.NewCluster(*initialClusterName) set := make(map[string]bool) flag.Visit(func(f *flag.Flag) { set[f.Name] = true }) if set["discovery"] && set["initial-cluster"] { - return nil, fmt.Errorf("both discovery and bootstrap-config are set") + return fmt.Errorf("both discovery and bootstrap-config are set") } apurls, err := pkg.URLsFromFlags(fs, "advertise-peer-urls", "addr", peerTLSInfo) if err != nil { - return nil, err + return err } switch { case set["discovery"]: cluster = etcdserver.NewCluster(*durl) - return cluster.AddMemberFromURLs(*name, apurls) + _, err := cluster.AddMemberFromURLs(*name, apurls) + return err 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. cluster.SetMembersFromString(*initialCluster) - m := cluster.FindName(*name) - if m != nil { - return m, nil - } - return nil, fmt.Errorf("cannot find the passed name %s in --initial-cluster bootstrap list.", *name) } + return nil }