etcdserver: etcdserver generates the ID when adding new member.

When adding new member, the etcdserver generates the ID based on the current time
and the given peerurls. We include time to add the uniqueness, since the node with
same peerurls should be able to (add, then remove) several times.

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)
+		validURLs, err := types.NewURLs(peerURLs)
-		m := etcdserver.Member{
+		if err != nil {
-			ID: id,
+			http.Error(w, "bad peer urls", http.StatusBadRequest)
-			RaftAttributes: etcdserver.RaftAttributes{
+			return
-				PeerURLs: peerURLs,
-			},
 		}
-		if err := h.server.AddMember(ctx, m); err != nil {
+		now := h.clock.Now()
-			log.Printf("etcdhttp: error adding node %x: %v", id, err)
+		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")),
+				URL:    mustNewURL(t, adminMembersPrefix),
-				Method: "PUT",
+				Method: "POST",
 			},
 			&resServer{},
 
@@ -1437,8 +1437,8 @@ func TestServeAdminMembersFail(t *testing.T) {
 		{
 			// parse body error
 			&http.Request{
-				URL:    mustNewURL(t, path.Join(adminMembersPrefix, "1")),
+				URL:    mustNewURL(t, adminMembersPrefix),
-				Method: "PUT",
+				Method: "POST",
 			},
 			&resServer{},
 
@@ -1447,9 +1447,10 @@ func TestServeAdminMembersFail(t *testing.T) {
 		{
 			// etcdserver.AddMember error
 			&http.Request{
-				URL:    mustNewURL(t, path.Join(adminMembersPrefix, "1")),
+				URL:    mustNewURL(t, adminMembersPrefix),
-				Method: "PUT",
+				Method: "POST",
-				Body:   ioutil.NopCloser(strings.NewReader("")),
+				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 +1474,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 +1513,10 @@ func (s *serverRecorder) RemoveMember(_ context.Context, id uint64) error {
 }
 
 func TestServeAdminMembersPut(t *testing.T) {
-	u := mustNewURL(t, path.Join(adminMembersPrefix, "BEEF"))
+	u := mustNewURL(t, adminMembersPrefix)
-	form := url.Values{"PeerURLs": []string{"http://a", "http://b"}}
+	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 +1524,7 @@ func TestServeAdminMembersPut(t *testing.T) {
 	s := &serverRecorder{}
 	h := &serverHandler{
 		server: s,
+		clock:  clockwork.NewFakeClock(),
 	}
 	rw := httptest.NewRecorder()
 
@@ -1536,9 +1539,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.
+// name, peer URLs. This is used for bootstrapping/adding new member.
-func newMember(name string, peerURLs types.URLs, clusterName string, now *time.Time) *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("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("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.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.2:2379"}, {Scheme: "http", Host: "10.0.0.1:2379"}}, "", nil), 16552244735972308939},
 	}
 	for i, tt := range tests {
 		if tt.mem.ID != tt.id {