Merge pull request #1347 from coreos/gen_nodeID

etcdserver: etcdserver generates the ID when adding new member.

etcdserver/cluster.go

@@ -100,7 +100,7 @@ func (c *Cluster) SetMembersFromString(s string) error {
 			return fmt.Errorf("Empty URL given for %q", name)
 		}
 
-		m := newMember(name, types.URLs(*flags.NewURLsValue(strings.Join(urls, ","))), c.name, nil)
+		m := NewMember(name, types.URLs(*flags.NewURLsValue(strings.Join(urls, ","))), c.name, nil)
 		err := c.Add(*m)
 		if err != nil {
 			return err
@@ -110,7 +110,7 @@ func (c *Cluster) SetMembersFromString(s string) error {
 }
 
 func (c *Cluster) AddMemberFromURLs(name string, urls types.URLs) (*Member, error) {
-	m := newMember(name, urls, c.name, nil)
+	m := NewMember(name, urls, c.name, nil)
 	err := c.Add(*m)
 	if err != nil {
 		return nil, err

etcdserver/etcdhttp/http.go

@@ -34,6 +34,7 @@ import (
 	etcdErr "github.com/coreos/etcd/error"
 	"github.com/coreos/etcd/etcdserver"
 	"github.com/coreos/etcd/etcdserver/etcdserverpb"
+	"github.com/coreos/etcd/pkg/types"
 	"github.com/coreos/etcd/raft/raftpb"
 	"github.com/coreos/etcd/store"
 )
@@ -62,6 +63,7 @@ func NewClientHandler(server *etcdserver.EtcdServer) http.Handler {
 		stats:        server,
 		timer:        server,
 		timeout:      defaultServerTimeout,
+		clock:        clockwork.NewRealClock(),
 	}
 	mux := http.NewServeMux()
 	mux.HandleFunc(keysPrefix, sh.serveKeys)
@@ -84,6 +86,7 @@ func NewPeerHandler(server *etcdserver.EtcdServer) http.Handler {
 		server:       server,
 		stats:        server,
 		clusterStore: server.ClusterStore,
+		clock:        clockwork.NewRealClock(),
 	}
 	mux := http.NewServeMux()
 	mux.HandleFunc(raftPrefix, sh.serveRaft)
@@ -98,6 +101,7 @@ type serverHandler struct {
 	stats        etcdserver.Stats
 	timer        etcdserver.RaftTimer
 	clusterStore etcdserver.ClusterStore
+	clock        clockwork.Clock
 }
 
 func (h serverHandler) serveKeys(w http.ResponseWriter, r *http.Request) {
@@ -145,39 +149,40 @@ func (h serverHandler) serveMachines(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h serverHandler) serveAdminMembers(w http.ResponseWriter, r *http.Request) {
-	if !allowMethod(w, r.Method, "PUT", "DELETE") {
+	if !allowMethod(w, r.Method, "POST", "DELETE") {
 		return
 	}
 	ctx, cancel := context.WithTimeout(context.Background(), defaultServerTimeout)
 	defer cancel()
-	idStr := strings.TrimPrefix(r.URL.Path, adminMembersPrefix)
-	id, err := strconv.ParseUint(idStr, 16, 64)
-	if err != nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return
-	}
 
 	switch r.Method {
-	case "PUT":
+	case "POST":
 		if err := r.ParseForm(); err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 		peerURLs := r.PostForm["PeerURLs"]
-		log.Printf("etcdhttp: add node %x with peer urls %v", id, peerURLs)
-		m := etcdserver.Member{
-			ID: id,
-			RaftAttributes: etcdserver.RaftAttributes{
-				PeerURLs: peerURLs,
-			},
+		validURLs, err := types.NewURLs(peerURLs)
+		if err != nil {
+			http.Error(w, "bad peer urls", http.StatusBadRequest)
+			return
 		}
-		if err := h.server.AddMember(ctx, m); err != nil {
-			log.Printf("etcdhttp: error adding node %x: %v", id, err)
+		now := h.clock.Now()
+		m := etcdserver.NewMember("", validURLs, "", &now)
+		if err := h.server.AddMember(ctx, *m); err != nil {
+			log.Printf("etcdhttp: error adding node %x: %v", m.ID, err)
 			writeError(w, err)
 			return
 		}
+		log.Printf("etcdhttp: added node %x with peer urls %v", m.ID, peerURLs)
 		w.WriteHeader(http.StatusCreated)
 	case "DELETE":
+		idStr := strings.TrimPrefix(r.URL.Path, adminMembersPrefix)
+		id, err := strconv.ParseUint(idStr, 16, 64)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
 		log.Printf("etcdhttp: remove node %x", id)
 		if err := h.server.RemoveMember(ctx, id); err != nil {
 			log.Printf("etcdhttp: error removing node %x: %v", id, err)

etcdserver/etcdhttp/http_test.go

@@ -1427,8 +1427,8 @@ func TestServeAdminMembersFail(t *testing.T) {
 		{
 			// parse id error
 			&http.Request{
-				URL:    mustNewURL(t, path.Join(adminMembersPrefix, "badID")),
-				Method: "PUT",
+				URL:    mustNewURL(t, adminMembersPrefix),
+				Method: "POST",
 			},
 			&resServer{},
 
@@ -1437,19 +1437,32 @@ func TestServeAdminMembersFail(t *testing.T) {
 		{
 			// parse body error
 			&http.Request{
-				URL:    mustNewURL(t, path.Join(adminMembersPrefix, "1")),
-				Method: "PUT",
+				URL:    mustNewURL(t, adminMembersPrefix),
+				Method: "POST",
 			},
 			&resServer{},
 
 			http.StatusBadRequest,
 		},
+		{
+			// bad url
+			&http.Request{
+				URL:    mustNewURL(t, adminMembersPrefix),
+				Method: "POST",
+				Body:   ioutil.NopCloser(strings.NewReader(url.Values{"PeerURLs": []string{"http://bad"}}.Encode())),
+				Header: map[string][]string{"Content-Type": []string{"application/x-www-form-urlencoded"}},
+			},
+			&errServer{},
+
+			http.StatusBadRequest,
+		},
 		{
 			// etcdserver.AddMember error
 			&http.Request{
-				URL:    mustNewURL(t, path.Join(adminMembersPrefix, "1")),
-				Method: "PUT",
-				Body:   ioutil.NopCloser(strings.NewReader("")),
+				URL:    mustNewURL(t, adminMembersPrefix),
+				Method: "POST",
+				Body:   ioutil.NopCloser(strings.NewReader(url.Values{"PeerURLs": []string{"http://127.0.0.1:1"}}.Encode())),
+				Header: map[string][]string{"Content-Type": []string{"application/x-www-form-urlencoded"}},
 			},
 			&errServer{
 				errors.New("blah"),
@@ -1473,6 +1486,7 @@ func TestServeAdminMembersFail(t *testing.T) {
 	for i, tt := range tests {
 		h := &serverHandler{
 			server: tt.server,
+			clock:  clockwork.NewFakeClock(),
 		}
 		rw := httptest.NewRecorder()
 		h.serveAdminMembers(rw, tt.req)
@@ -1511,10 +1525,10 @@ func (s *serverRecorder) RemoveMember(_ context.Context, id uint64) error {
 }
 
 func TestServeAdminMembersPut(t *testing.T) {
-	u := mustNewURL(t, path.Join(adminMembersPrefix, "BEEF"))
-	form := url.Values{"PeerURLs": []string{"http://a", "http://b"}}
+	u := mustNewURL(t, adminMembersPrefix)
+	form := url.Values{"PeerURLs": []string{"http://127.0.0.1:1"}}
 	body := strings.NewReader(form.Encode())
-	req, err := http.NewRequest("PUT", u.String(), body)
+	req, err := http.NewRequest("POST", u.String(), body)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1522,6 +1536,7 @@ func TestServeAdminMembersPut(t *testing.T) {
 	s := &serverRecorder{}
 	h := &serverHandler{
 		server: s,
+		clock:  clockwork.NewFakeClock(),
 	}
 	rw := httptest.NewRecorder()
 
@@ -1536,9 +1551,9 @@ func TestServeAdminMembersPut(t *testing.T) {
 		t.Errorf("got body=%q, want %q", g, "")
 	}
 	wm := etcdserver.Member{
-		ID: 0xBEEF,
+		ID: 3064321551348478165,
 		RaftAttributes: etcdserver.RaftAttributes{
-			PeerURLs: []string{"http://a", "http://b"},
+			PeerURLs: []string{"http://127.0.0.1:1"},
 		},
 	}
 	wactions := []action{{name: "AddMember", params: []interface{}{wm}}}

etcdserver/member.go

@@ -48,8 +48,8 @@ type Member struct {
 }
 
 // newMember creates a Member without an ID and generates one based on the
-// name, peer URLs. This is used for bootstrapping.
-func newMember(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member {
+// name, peer URLs. This is used for bootstrapping/adding new member.
+func NewMember(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member {
 	m := &Member{
 		RaftAttributes: RaftAttributes{PeerURLs: peerURLs.StringSlice()},
 		Attributes:     Attributes{Name: name},

etcdserver/member_test.go

@@ -35,17 +35,17 @@ func TestMemberTime(t *testing.T) {
 		mem *Member
 		id  uint64
 	}{
-		{newMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
+		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
 		// Same ID, different name (names shouldn't matter)
-		{newMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
+		{NewMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
 		// Same ID, different Time
-		{newMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 2448790162483548276},
+		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 2448790162483548276},
 		// Different cluster name
-		{newMember("mcm1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "etcd", timeParse("1984-12-23T15:04:05Z")), 6973882743191604649},
-		{newMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 1466075294948436910},
+		{NewMember("mcm1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "etcd", timeParse("1984-12-23T15:04:05Z")), 6973882743191604649},
+		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 1466075294948436910},
 		// Order shouldn't matter
-		{newMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "10.0.0.2:2379"}}, "", nil), 16552244735972308939},
-		{newMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.2:2379"}, {Scheme: "http", Host: "10.0.0.1:2379"}}, "", nil), 16552244735972308939},
+		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "10.0.0.2:2379"}}, "", nil), 16552244735972308939},
+		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.2:2379"}, {Scheme: "http", Host: "10.0.0.1:2379"}}, "", nil), 16552244735972308939},
 	}
 	for i, tt := range tests {
 		if tt.mem.ID != tt.id {