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..473011acc 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,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}}} 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 {