From d00152765ad85d97d8fb87384e316cfd2ba325e6 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Wed, 22 Oct 2014 11:37:53 -0700 Subject: [PATCH] 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 | 4 ++-- etcdserver/etcdhttp/http.go | 37 ++++++++++++++++++-------------- etcdserver/etcdhttp/http_test.go | 27 ++++++++++++----------- etcdserver/member.go | 4 ++-- etcdserver/member_test.go | 14 ++++++------ 5 files changed, 47 insertions(+), 39 deletions(-) diff --git a/etcdserver/cluster.go b/etcdserver/cluster.go index 44da5d4ed..2019bc1e4 100644 --- a/etcdserver/cluster.go +++ b/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 diff --git a/etcdserver/etcdhttp/http.go b/etcdserver/etcdhttp/http.go index 6b98ad013..d9f90c247 100644 --- a/etcdserver/etcdhttp/http.go +++ b/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) diff --git a/etcdserver/etcdhttp/http_test.go b/etcdserver/etcdhttp/http_test.go index aa708049e..0b6dcea9a 100644 --- a/etcdserver/etcdhttp/http_test.go +++ b/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,8 +1437,8 @@ 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{}, @@ -1447,9 +1447,10 @@ func TestServeAdminMembersFail(t *testing.T) { { // 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 +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")) - 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 +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}}} diff --git a/etcdserver/member.go b/etcdserver/member.go index a0a256628..5d66e243a 100644 --- a/etcdserver/member.go +++ b/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}, diff --git a/etcdserver/member_test.go b/etcdserver/member_test.go index f0024eda8..a45221c3d 100644 --- a/etcdserver/member_test.go +++ b/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 {