From b06499d0c2f1107db1d56ea4123fae3ca34e13f4 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Sun, 26 Oct 2014 13:00:42 -0700 Subject: [PATCH] etcdserver/etcdhttp: break apart HTTP handlers --- etcdserver/etcdhttp/client.go | 71 +++++++++++++++++++++--------- etcdserver/etcdhttp/client_test.go | 58 ++++++++++++------------ etcdserver/etcdhttp/http.go | 12 ----- etcdserver/etcdhttp/peer.go | 35 ++++++++++----- etcdserver/etcdhttp/peer_test.go | 25 ++++------- 5 files changed, 111 insertions(+), 90 deletions(-) diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index 8c030fa75..0e7b2baf1 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -49,28 +49,46 @@ const ( // NewClientHandler generates a muxed http.Handler with the given parameters to serve etcd client requests. func NewClientHandler(server *etcdserver.EtcdServer) http.Handler { - sh := &serverHandler{ + kh := &keysHandler{ + server: server, + timer: server, + timeout: defaultServerTimeout, + } + + sh := &statsHandler{ + stats: server, + } + + amh := &adminMembersHandler{ server: server, clusterInfo: server.Cluster, - stats: server, - timer: server, - timeout: defaultServerTimeout, clock: clockwork.NewRealClock(), } + + dmh := &deprecatedMachinesHandler{ + clusterInfo: server.Cluster, + } + mux := http.NewServeMux() - mux.HandleFunc(keysPrefix, sh.serveKeys) - mux.HandleFunc(keysPrefix+"/", sh.serveKeys) - mux.HandleFunc(statsPrefix+"/store", sh.serveStoreStats) - mux.HandleFunc(statsPrefix+"/self", sh.serveSelfStats) - mux.HandleFunc(statsPrefix+"/leader", sh.serveLeaderStats) - mux.HandleFunc(deprecatedMachinesPrefix, sh.serveMachines) - mux.HandleFunc(adminMembersPrefix, sh.serveAdminMembers) - mux.HandleFunc(versionPrefix, sh.serveVersion) mux.HandleFunc("/", http.NotFound) + mux.HandleFunc(versionPrefix, serveVersion) + mux.Handle(keysPrefix, kh) + mux.Handle(keysPrefix+"/", kh) + mux.HandleFunc(statsPrefix+"/store", sh.serveStore) + mux.HandleFunc(statsPrefix+"/self", sh.serveSelf) + mux.HandleFunc(statsPrefix+"/leader", sh.serveLeader) + mux.Handle(adminMembersPrefix, amh) + mux.Handle(deprecatedMachinesPrefix, dmh) return mux } -func (h serverHandler) serveKeys(w http.ResponseWriter, r *http.Request) { +type keysHandler struct { + server etcdserver.Server + timer etcdserver.RaftTimer + timeout time.Duration +} + +func (h *keysHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET", "PUT", "POST", "DELETE") { return } @@ -106,8 +124,11 @@ func (h serverHandler) serveKeys(w http.ResponseWriter, r *http.Request) { } } -// serveMachines responds address list in the format '0.0.0.0, 1.1.1.1'. -func (h serverHandler) serveMachines(w http.ResponseWriter, r *http.Request) { +type deprecatedMachinesHandler struct { + clusterInfo etcdserver.ClusterInfo +} + +func (h *deprecatedMachinesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET", "HEAD") { return } @@ -115,7 +136,13 @@ func (h serverHandler) serveMachines(w http.ResponseWriter, r *http.Request) { w.Write([]byte(strings.Join(endpoints, ", "))) } -func (h serverHandler) serveAdminMembers(w http.ResponseWriter, r *http.Request) { +type adminMembersHandler struct { + server etcdserver.Server + clusterInfo etcdserver.ClusterInfo + clock clockwork.Clock +} + +func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET", "POST", "DELETE") { return } @@ -188,7 +215,11 @@ func (h serverHandler) serveAdminMembers(w http.ResponseWriter, r *http.Request) } } -func (h serverHandler) serveStoreStats(w http.ResponseWriter, r *http.Request) { +type statsHandler struct { + stats etcdserver.Stats +} + +func (h *statsHandler) serveStore(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET") { return } @@ -196,7 +227,7 @@ func (h serverHandler) serveStoreStats(w http.ResponseWriter, r *http.Request) { w.Write(h.stats.StoreStats()) } -func (h serverHandler) serveSelfStats(w http.ResponseWriter, r *http.Request) { +func (h *statsHandler) serveSelf(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET") { return } @@ -204,7 +235,7 @@ func (h serverHandler) serveSelfStats(w http.ResponseWriter, r *http.Request) { w.Write(h.stats.SelfStats()) } -func (h serverHandler) serveLeaderStats(w http.ResponseWriter, r *http.Request) { +func (h *statsHandler) serveLeader(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET") { return } @@ -212,7 +243,7 @@ func (h serverHandler) serveLeaderStats(w http.ResponseWriter, r *http.Request) w.Write(h.stats.LeaderStats()) } -func (h serverHandler) serveVersion(w http.ResponseWriter, r *http.Request) { +func serveVersion(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET") { return } diff --git a/etcdserver/etcdhttp/client_test.go b/etcdserver/etcdhttp/client_test.go index 4845f614d..8be9762a4 100644 --- a/etcdserver/etcdhttp/client_test.go +++ b/etcdserver/etcdhttp/client_test.go @@ -553,7 +553,7 @@ func TestServeAdminMembers(t *testing.T) { cluster := &fakeCluster{ members: map[uint64]*etcdserver.Member{1: &memb1, 2: &memb2}, } - h := &serverHandler{ + h := &adminMembersHandler{ server: &serverRecorder{}, clock: clockwork.NewFakeClock(), clusterInfo: cluster, @@ -588,7 +588,7 @@ func TestServeAdminMembers(t *testing.T) { t.Fatal(err) } rw := httptest.NewRecorder() - h.serveAdminMembers(rw, req) + h.ServeHTTP(rw, req) if rw.Code != tt.wcode { t.Errorf("#%d: code=%d, want %d", i, rw.Code, tt.wcode) @@ -616,13 +616,13 @@ func TestServeAdminMembersPut(t *testing.T) { } req.Header.Set("Content-Type", "application/json") s := &serverRecorder{} - h := &serverHandler{ + h := &adminMembersHandler{ server: s, clock: clockwork.NewFakeClock(), } rw := httptest.NewRecorder() - h.serveAdminMembers(rw, req) + h.ServeHTTP(rw, req) wcode := http.StatusCreated if rw.Code != wcode { @@ -658,12 +658,12 @@ func TestServeAdminMembersDelete(t *testing.T) { URL: mustNewURL(t, path.Join(adminMembersPrefix, "BEEF")), } s := &serverRecorder{} - h := &serverHandler{ + h := &adminMembersHandler{ server: s, } rw := httptest.NewRecorder() - h.serveAdminMembers(rw, req) + h.ServeHTTP(rw, req) wcode := http.StatusNoContent if rw.Code != wcode { @@ -767,12 +767,12 @@ func TestServeAdminMembersFail(t *testing.T) { }, } for i, tt := range tests { - h := &serverHandler{ + h := &adminMembersHandler{ server: tt.server, clock: clockwork.NewFakeClock(), } rw := httptest.NewRecorder() - h.serveAdminMembers(rw, tt.req) + h.ServeHTTP(rw, tt.req) if rw.Code != tt.wcode { t.Errorf("#%d: code=%d, want %d", i, rw.Code, tt.wcode) } @@ -884,8 +884,8 @@ func TestServeMachines(t *testing.T) { if err != nil { t.Fatal(err) } - h := &serverHandler{clusterInfo: cluster} - h.serveMachines(writer, req) + h := &deprecatedMachinesHandler{clusterInfo: cluster} + h.ServeHTTP(writer, req) w := "http://localhost:8080, http://localhost:8081, http://localhost:8082" if g := writer.Body.String(); g != w { t.Errorf("body = %s, want %s", g, w) @@ -907,11 +907,11 @@ func (ds *dummyStats) UpdateRecvApp(_ uint64, _ int64) {} func TestServeSelfStats(t *testing.T) { wb := []byte("some statistics") w := string(wb) - sh := &serverHandler{ + sh := &statsHandler{ stats: &dummyStats{data: wb}, } rw := httptest.NewRecorder() - sh.serveSelfStats(rw, &http.Request{Method: "GET"}) + sh.serveSelf(rw, &http.Request{Method: "GET"}) if rw.Code != http.StatusOK { t.Errorf("code = %d, want %d", rw.Code, http.StatusOK) } @@ -926,9 +926,9 @@ func TestServeSelfStats(t *testing.T) { func TestSelfServeStatsBad(t *testing.T) { for _, m := range []string{"PUT", "POST", "DELETE"} { - sh := &serverHandler{} + sh := &statsHandler{} rw := httptest.NewRecorder() - sh.serveSelfStats( + sh.serveSelf( rw, &http.Request{ Method: m, @@ -942,9 +942,9 @@ func TestSelfServeStatsBad(t *testing.T) { func TestLeaderServeStatsBad(t *testing.T) { for _, m := range []string{"PUT", "POST", "DELETE"} { - sh := &serverHandler{} + sh := &statsHandler{} rw := httptest.NewRecorder() - sh.serveLeaderStats( + sh.serveLeader( rw, &http.Request{ Method: m, @@ -959,11 +959,11 @@ func TestLeaderServeStatsBad(t *testing.T) { func TestServeLeaderStats(t *testing.T) { wb := []byte("some statistics") w := string(wb) - sh := &serverHandler{ + sh := &statsHandler{ stats: &dummyStats{data: wb}, } rw := httptest.NewRecorder() - sh.serveLeaderStats(rw, &http.Request{Method: "GET"}) + sh.serveLeader(rw, &http.Request{Method: "GET"}) if rw.Code != http.StatusOK { t.Errorf("code = %d, want %d", rw.Code, http.StatusOK) } @@ -979,11 +979,11 @@ func TestServeLeaderStats(t *testing.T) { func TestServeStoreStats(t *testing.T) { wb := []byte("some statistics") w := string(wb) - sh := &serverHandler{ + sh := &statsHandler{ stats: &dummyStats{data: wb}, } rw := httptest.NewRecorder() - sh.serveStoreStats(rw, &http.Request{Method: "GET"}) + sh.serveStore(rw, &http.Request{Method: "GET"}) if rw.Code != http.StatusOK { t.Errorf("code = %d, want %d", rw.Code, http.StatusOK) } @@ -1002,9 +1002,8 @@ func TestServeVersion(t *testing.T) { if err != nil { t.Fatalf("error creating request: %v", err) } - h := &serverHandler{} rw := httptest.NewRecorder() - h.serveVersion(rw, req) + serveVersion(rw, req) if rw.Code != http.StatusOK { t.Errorf("code=%d, want %d", rw.Code, http.StatusOK) } @@ -1022,9 +1021,8 @@ func TestServeVersionFails(t *testing.T) { if err != nil { t.Fatalf("error creating request: %v", err) } - h := &serverHandler{} rw := httptest.NewRecorder() - h.serveVersion(rw, req) + serveVersion(rw, req) if rw.Code != http.StatusMethodNotAllowed { t.Errorf("method %s: code=%d, want %d", m, rw.Code, http.StatusMethodNotAllowed) } @@ -1102,12 +1100,12 @@ func TestBadServeKeys(t *testing.T) { }, } for i, tt := range testBadCases { - h := &serverHandler{ + h := &keysHandler{ timeout: 0, // context times out immediately server: tt.server, } rw := httptest.NewRecorder() - h.serveKeys(rw, tt.req) + h.ServeHTTP(rw, tt.req) if rw.Code != tt.wcode { t.Errorf("#%d: got code=%d, want %d", i, rw.Code, tt.wcode) } @@ -1127,14 +1125,14 @@ func TestServeKeysEvent(t *testing.T) { }, }, } - h := &serverHandler{ + h := &keysHandler{ timeout: time.Hour, server: server, timer: &dummyRaftTimer{}, } rw := httptest.NewRecorder() - h.serveKeys(rw, req) + h.ServeHTTP(rw, req) wcode := http.StatusOK wbody := mustMarshalEvent( @@ -1165,7 +1163,7 @@ func TestServeKeysWatch(t *testing.T) { Watcher: dw, }, } - h := &serverHandler{ + h := &keysHandler{ timeout: time.Hour, server: server, timer: &dummyRaftTimer{}, @@ -1178,7 +1176,7 @@ func TestServeKeysWatch(t *testing.T) { }() rw := httptest.NewRecorder() - h.serveKeys(rw, req) + h.ServeHTTP(rw, req) wcode := http.StatusOK wbody := mustMarshalEvent( diff --git a/etcdserver/etcdhttp/http.go b/etcdserver/etcdhttp/http.go index 5794fb82d..8350e9a51 100644 --- a/etcdserver/etcdhttp/http.go +++ b/etcdserver/etcdhttp/http.go @@ -23,9 +23,7 @@ import ( "strings" "time" - "github.com/coreos/etcd/Godeps/_workspace/src/github.com/jonboulle/clockwork" etcdErr "github.com/coreos/etcd/error" - "github.com/coreos/etcd/etcdserver" ) const ( @@ -38,16 +36,6 @@ const ( var errClosed = errors.New("etcdhttp: client closed connection") -// serverHandler provides http.Handlers for etcd client and raft communication. -type serverHandler struct { - timeout time.Duration - server etcdserver.Server - stats etcdserver.Stats - timer etcdserver.RaftTimer - clusterInfo etcdserver.ClusterInfo - clock clockwork.Clock -} - // writeError logs and writes the given Error to the ResponseWriter // If Error is an etcdErr, it is rendered to the ResponseWriter // Otherwise, it is assumed to be an InternalServerError diff --git a/etcdserver/etcdhttp/peer.go b/etcdserver/etcdhttp/peer.go index d2f056f12..b6272a72e 100644 --- a/etcdserver/etcdhttp/peer.go +++ b/etcdserver/etcdhttp/peer.go @@ -24,32 +24,41 @@ import ( "strconv" "github.com/coreos/etcd/Godeps/_workspace/src/code.google.com/p/go.net/context" - "github.com/coreos/etcd/Godeps/_workspace/src/github.com/jonboulle/clockwork" "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/raft/raftpb" ) const ( - raftPrefix = "/raft" - membersPrefix = "/members" + raftPrefix = "/raft" + peerMembersPrefix = "/members" ) // NewPeerHandler generates an http.Handler to handle etcd peer (raft) requests. func NewPeerHandler(server *etcdserver.EtcdServer) http.Handler { - sh := &serverHandler{ - server: server, + rh := &raftHandler{ stats: server, + server: server, clusterInfo: server.Cluster, - clock: clockwork.NewRealClock(), } + + mh := &peerMembersHandler{ + clusterInfo: server.Cluster, + } + mux := http.NewServeMux() - mux.HandleFunc(raftPrefix, sh.serveRaft) - mux.HandleFunc(membersPrefix, sh.serveMembers) mux.HandleFunc("/", http.NotFound) + mux.Handle(raftPrefix, rh) + mux.Handle(peerMembersPrefix, mh) return mux } -func (h serverHandler) serveRaft(w http.ResponseWriter, r *http.Request) { +type raftHandler struct { + stats etcdserver.Stats + server etcdserver.Server + clusterInfo etcdserver.ClusterInfo +} + +func (h *raftHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "POST") { return } @@ -92,14 +101,18 @@ func (h serverHandler) serveRaft(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } -func (h serverHandler) serveMembers(w http.ResponseWriter, r *http.Request) { +type peerMembersHandler struct { + clusterInfo etcdserver.ClusterInfo +} + +func (h *peerMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if !allowMethod(w, r.Method, "GET") { return } cid := strconv.FormatUint(h.clusterInfo.ID(), 16) w.Header().Set("X-Etcd-Cluster-ID", cid) - if r.URL.Path != membersPrefix { + if r.URL.Path != peerMembersPrefix { http.Error(w, "bad path", http.StatusBadRequest) return } diff --git a/etcdserver/etcdhttp/peer_test.go b/etcdserver/etcdhttp/peer_test.go index 7f6138c70..7724b0c78 100644 --- a/etcdserver/etcdhttp/peer_test.go +++ b/etcdserver/etcdhttp/peer_test.go @@ -27,7 +27,6 @@ import ( "strconv" "strings" "testing" - "time" "github.com/coreos/etcd/etcdserver" "github.com/coreos/etcd/raft/raftpb" @@ -169,13 +168,9 @@ func TestServeRaft(t *testing.T) { t.Fatalf("#%d: could not create request: %#v", i, err) } req.Header.Set("X-Etcd-Cluster-ID", tt.clusterID) - h := &serverHandler{ - timeout: time.Hour, - server: &errServer{tt.serverErr}, - clusterInfo: &fakeCluster{id: 0}, - } rw := httptest.NewRecorder() - h.serveRaft(rw, req) + h := &raftHandler{stats: nil, server: &errServer{tt.serverErr}, clusterInfo: &fakeCluster{id: 0}} + h.ServeHTTP(rw, req) if rw.Code != tt.wcode { t.Errorf("#%d: got code=%d, want %d", i, rw.Code, tt.wcode) } @@ -201,9 +196,9 @@ func TestServeMembersFails(t *testing.T) { }, } for i, tt := range tests { - h := &serverHandler{} rw := httptest.NewRecorder() - h.serveMembers(rw, &http.Request{Method: tt.method}) + h := &peerMembersHandler{clusterInfo: nil} + h.ServeHTTP(rw, &http.Request{Method: tt.method}) if rw.Code != tt.wcode { t.Errorf("#%d: code=%d, want %d", i, rw.Code, tt.wcode) } @@ -217,11 +212,7 @@ func TestServeMembersGet(t *testing.T) { id: 1, members: map[uint64]*etcdserver.Member{1: &memb1, 2: &memb2}, } - h := &serverHandler{ - server: &serverRecorder{}, - clusterInfo: cluster, - } - + h := &peerMembersHandler{clusterInfo: cluster} msb, err := json.Marshal([]etcdserver.Member{memb1, memb2}) if err != nil { t.Fatal(err) @@ -234,8 +225,8 @@ func TestServeMembersGet(t *testing.T) { wct string wbody string }{ - {membersPrefix, http.StatusOK, "application/json", wms}, - {path.Join(membersPrefix, "bad"), http.StatusBadRequest, "text/plain; charset=utf-8", "bad path\n"}, + {peerMembersPrefix, http.StatusOK, "application/json", wms}, + {path.Join(peerMembersPrefix, "bad"), http.StatusBadRequest, "text/plain; charset=utf-8", "bad path\n"}, } for i, tt := range tests { @@ -244,7 +235,7 @@ func TestServeMembersGet(t *testing.T) { t.Fatal(err) } rw := httptest.NewRecorder() - h.serveMembers(rw, req) + h.ServeHTTP(rw, req) if rw.Code != tt.wcode { t.Errorf("#%d: code=%d, want %d", i, rw.Code, tt.wcode)