From 1ca7c031ffb4a311a85f35f4dd5d1ca3a22bf7ee Mon Sep 17 00:00:00 2001 From: Barak Michener Date: Mon, 13 Oct 2014 18:40:25 -0400 Subject: [PATCH] first round of comments Conflicts: etcdserver/config.go etcdserver/config_test.go etcdserver/server.go main.go --- etcdserver/config.go | 9 ++++---- etcdserver/config_test.go | 4 +--- etcdserver/server.go | 16 +++++++------- integration/cluster_test.go | 4 +++- main.go | 42 +++++++++++++++++-------------------- 5 files changed, 35 insertions(+), 40 deletions(-) diff --git a/etcdserver/config.go b/etcdserver/config.go index b4bf03f4c..6bff1eac4 100644 --- a/etcdserver/config.go +++ b/etcdserver/config.go @@ -27,7 +27,8 @@ import ( // ServerConfig holds the configuration of etcd as taken from the command line or discovery. type ServerConfig struct { - LocalMember Member + NodeID uint64 + Name string DiscoveryURL string ClientURLs types.URLs DataDir string @@ -47,14 +48,14 @@ func (c *ServerConfig) VerifyBootstrapConfig() error { // Make sure the cluster at least contains the local server. isOk := false for _, m := range c.Cluster.members { - if m.ID == c.LocalMember.ID { + if m.ID == c.NodeID { isOk = true } } if !isOk { return fmt.Errorf("couldn't find local ID in cluster config") } - if c.LocalMember.ID == raft.None { + if c.NodeID == raft.None { return fmt.Errorf("could not use %x as member id", raft.None) } @@ -75,8 +76,6 @@ func (c *ServerConfig) WALDir() string { return path.Join(c.DataDir, "wal") } func (c *ServerConfig) SnapDir() string { return path.Join(c.DataDir, "snap") } -func (c *ServerConfig) ID() uint64 { return c.LocalMember.ID } - func (c *ServerConfig) ShouldDiscover() bool { return c.DiscoveryURL != "" } diff --git a/etcdserver/config_test.go b/etcdserver/config_test.go index 3e7563cf2..1af9797f5 100644 --- a/etcdserver/config_test.go +++ b/etcdserver/config_test.go @@ -51,9 +51,7 @@ func TestBootstrapConfigVerify(t *testing.T) { } cfg := ServerConfig{ - LocalMember: Member{ - ID: 0x7350a9cd4dc16f76, - }, + NodeID: 0x7350a9cd4dc16f76, DiscoveryURL: tt.disc, Cluster: cluster, ClusterState: tt.clst, diff --git a/etcdserver/server.go b/etcdserver/server.go index f0cf8b9bc..3a528fb87 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -180,7 +180,7 @@ func NewServer(cfg *ServerConfig) *EtcdServer { log.Fatalf("etcdserver: %v", err) } if cfg.ShouldDiscover() { - d, err := discovery.New(cfg.DiscoveryURL, cfg.ID(), cfg.Cluster.String()) + d, err := discovery.New(cfg.DiscoveryURL, cfg.NodeID, cfg.Cluster.String()) if err != nil { log.Fatalf("etcdserver: cannot init discovery %v", err) } @@ -213,17 +213,17 @@ func NewServer(cfg *ServerConfig) *EtcdServer { cls := &clusterStore{Store: st, id: cid} sstats := &stats.ServerStats{ - Name: cfg.LocalMember.Attributes.Name, - ID: idAsHex(cfg.ID()), + Name: cfg.Name, + ID: idAsHex(cfg.NodeID), } - lstats := stats.NewLeaderStats(idAsHex(cfg.ID())) + lstats := stats.NewLeaderStats(idAsHex(cfg.NodeID)) s := &EtcdServer{ store: st, node: n, id: id, clusterID: cid, - attributes: Attributes{Name: cfg.LocalMember.Attributes.Name, ClientURLs: cfg.ClientURLs.StringSlice()}, + attributes: Attributes{Name: cfg.Name, ClientURLs: cfg.ClientURLs.StringSlice()}, storage: struct { *wal.WAL *snap.Snapshotter @@ -640,7 +640,7 @@ func startNode(cfg *ServerConfig) (id, cid uint64, n raft.Node, w *wal.WAL) { // TODO: remove the discoveryURL when it becomes part of the source for // generating nodeID. cfg.Cluster.GenID([]byte(cfg.DiscoveryURL)) - metadata := pbutil.MustMarshal(&pb.Metadata{NodeID: cfg.ID(), ClusterID: cfg.Cluster.ID()}) + metadata := pbutil.MustMarshal(&pb.Metadata{NodeID: cfg.NodeID, ClusterID: cfg.Cluster.ID()}) if w, err = wal.Create(cfg.WALDir(), metadata); err != nil { log.Fatal(err) } @@ -653,9 +653,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.ID(), cfg.Cluster.ID() + id, cid = cfg.NodeID, cfg.Cluster.ID() log.Printf("etcdserver: start node %d in cluster %d", id, cid) - n = raft.StartNode(cfg.ID(), peers, 10, 1) + n = raft.StartNode(cfg.NodeID, peers, 10, 1) return } diff --git a/integration/cluster_test.go b/integration/cluster_test.go index 536d839bd..abc55206e 100644 --- a/integration/cluster_test.go +++ b/integration/cluster_test.go @@ -103,7 +103,9 @@ func (c *cluster) Launch(t *testing.T) { if err != nil { t.Fatal(err) } - m.LocalMember = *etcdserver.NewMemberFromURLs(c.name(i), listenUrls) + localMember := *etcdserver.NewMemberFromURLs(c.name(i), listenUrls) + m.NodeID = localMember.ID + m.Name = localMember.Name m.ClientURLs, err = types.NewURLs([]string{"http://" + cln.Addr().String()}) if err != nil { t.Fatal(err) diff --git a/main.go b/main.go index ed0d640b9..40684ce99 100644 --- a/main.go +++ b/main.go @@ -167,7 +167,8 @@ func startEtcd(self *etcdserver.Member) { log.Fatal(err.Error()) } cfg := &etcdserver.ServerConfig{ - LocalMember: *self, + NodeID: self.ID, + Name: *name, ClientURLs: acurls, DataDir: *dir, SnapCount: *snapCount, @@ -276,11 +277,12 @@ func setupCluster() (*etcdserver.Member, error) { if set["discovery"] && set["initial-cluster"] { return nil, fmt.Errorf("both discovery and bootstrap-config are set") } + + apurls, err := pkg.URLsFromFlags(flag.CommandLine, "advertise-peer-urls", "addr", peerTLSInfo) + if err != nil { + return nil, err + } if set["discovery"] { - apurls, err := pkg.URLsFromFlags(fs, "advertise-peer-urls", "peer-addr", peerTLSInfo) - if err != nil { - return nil, err - } m = etcdserver.NewMemberFromURLs(*name, apurls) cluster = etcdserver.NewCluster() cluster.Add(*m) @@ -288,29 +290,23 @@ func setupCluster() (*etcdserver.Member, error) { } else if set["initial-cluster"] { // We're statically configured, and cluster has appropriately been set. // Try to configure by indexing the static cluster by name. - if set["name"] { - for _, c := range cluster.Members() { - if c.Name == *name { - return c, nil - } + for _, c := range cluster.Members() { + if c.Name == *name { + return c, nil } - log.Println("etcd: cannot find the passed name", *name, "in initial-cluster. Trying advertise-peer-urls") } + log.Println("etcd: cannot find the passed name", *name, "in initial-cluster. Trying advertise-peer-urls") + // Try to configure by looking for a matching machine based on the peer urls. - if set["advertise-peer-urls"] { - apurls, err := pkg.URLsFromFlags(flag.CommandLine, "advertise-peer-urls", "addr", peerTLSInfo) - if err != nil { - return nil, err + m = etcdserver.NewMemberFromURLs(*name, apurls) + for _, c := range cluster.Members() { + if c.ID == m.ID { + return c, nil } - m = etcdserver.NewMemberFromURLs(*name, apurls) - for _, c := range cluster.Members() { - if c.ID == m.ID { - return c, nil - } - } - log.Println("etcd: Could not find a matching peer for the local instance in initial-cluster.") } + log.Println("etcd: Could not find a matching peer for the local instance in initial-cluster.") return nil, fmt.Errorf("etcd: Bootstrapping a static cluster, but local name or local peer urls are not defined") } - return nil, fmt.Errorf("etcd: Flag configuration not set for discovery or initial-cluster") + m = etcdserver.NewMemberFromURLs(*name, apurls) + return m, nil }