From 2472953939a0d0ae6626dd90d686e59a5d3193b9 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Mon, 27 Oct 2014 15:00:13 -0700 Subject: [PATCH] etcdhttp: hex-encode member ID --- etcdserver/etcdhttp/client.go | 2 +- etcdserver/etcdhttp/client_test.go | 14 ++--- etcdserver/etcdhttp/httptypes/member.go | 2 +- etcdserver/etcdhttp/httptypes/member_test.go | 12 ++-- etcdserver/member.go | 7 +-- etcdserver/sender.go | 2 +- etcdserver/server.go | 12 ++-- etcdserver/server_test.go | 65 ++++++++++++++++++++ 8 files changed, 90 insertions(+), 26 deletions(-) diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index ae29d4fd3..addd6b349 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -534,7 +534,7 @@ func newMemberCollection(ms []*etcdserver.Member) httptypes.MemberCollection { for i, m := range ms { tm := httptypes.Member{ - ID: m.ID, + ID: etcdserver.IDAsHex(m.ID), Name: m.Name, PeerURLs: make([]string, len(m.PeerURLs)), ClientURLs: make([]string, len(m.ClientURLs)), diff --git a/etcdserver/etcdhttp/client_test.go b/etcdserver/etcdhttp/client_test.go index e0abf5749..04506a1df 100644 --- a/etcdserver/etcdhttp/client_test.go +++ b/etcdserver/etcdhttp/client_test.go @@ -549,8 +549,8 @@ func TestGoodParseRequest(t *testing.T) { } func TestServeAdminMembers(t *testing.T) { - memb1 := etcdserver.Member{ID: 1, Attributes: etcdserver.Attributes{ClientURLs: []string{"http://localhost:8080"}}} - memb2 := etcdserver.Member{ID: 2, Attributes: etcdserver.Attributes{ClientURLs: []string{"http://localhost:8081"}}} + memb1 := etcdserver.Member{ID: 12, Attributes: etcdserver.Attributes{ClientURLs: []string{"http://localhost:8080"}}} + memb2 := etcdserver.Member{ID: 13, Attributes: etcdserver.Attributes{ClientURLs: []string{"http://localhost:8081"}}} cluster := &fakeCluster{ id: 1, members: map[uint64]*etcdserver.Member{1: &memb1, 2: &memb2}, @@ -561,11 +561,7 @@ func TestServeAdminMembers(t *testing.T) { clusterInfo: cluster, } - mcb, err := json.Marshal(newMemberCollection([]*etcdserver.Member{&memb1, &memb2})) - if err != nil { - t.Fatal(err) - } - wmc := string(mcb) + "\n" + wmc := string(`[{"id":"c","name":"","peerURLs":[],"clientURLs":[""]},{"id":"d","name":"","peerURLs":[],"clientURLs":[""]}]`) tests := []struct { path string @@ -573,8 +569,8 @@ func TestServeAdminMembers(t *testing.T) { wct string wbody string }{ - {adminMembersPrefix, http.StatusOK, "application/json", wmc}, - {adminMembersPrefix + "/", http.StatusOK, "application/json", wmc}, + {adminMembersPrefix, http.StatusOK, "application/json", wmc + "\n"}, + {adminMembersPrefix + "/", http.StatusOK, "application/json", wmc + "\n"}, {path.Join(adminMembersPrefix, "100"), http.StatusNotFound, "application/json", `{"message":"Not found"}`}, {path.Join(adminMembersPrefix, "foobar"), http.StatusNotFound, "application/json", `{"message":"Not found"}`}, } diff --git a/etcdserver/etcdhttp/httptypes/member.go b/etcdserver/etcdhttp/httptypes/member.go index eb52cacb2..7d972daf0 100644 --- a/etcdserver/etcdhttp/httptypes/member.go +++ b/etcdserver/etcdhttp/httptypes/member.go @@ -21,7 +21,7 @@ import ( ) type Member struct { - ID uint64 `json:"id"` + ID string `json:"id"` Name string `json:"name"` PeerURLs []string `json:"peerURLs"` ClientURLs []string `json:"clientURLs"` diff --git a/etcdserver/etcdhttp/httptypes/member_test.go b/etcdserver/etcdhttp/httptypes/member_test.go index 4ac6b3ed1..6c0b3cdf5 100644 --- a/etcdserver/etcdhttp/httptypes/member_test.go +++ b/etcdserver/etcdhttp/httptypes/member_test.go @@ -30,8 +30,8 @@ func TestMemberUnmarshal(t *testing.T) { }{ // no URLs, just check ID & Name { - body: []byte(`{"id": 1, "name": "dungarees"}`), - wantMember: Member{ID: 1, Name: "dungarees", PeerURLs: nil, ClientURLs: nil}, + body: []byte(`{"id": "c", "name": "dungarees"}`), + wantMember: Member{ID: "c", Name: "dungarees", PeerURLs: nil, ClientURLs: nil}, }, // both client and peer URLs @@ -102,11 +102,11 @@ func TestMemberCollectionUnmarshal(t *testing.T) { want: MemberCollection([]Member{}), }, { - body: []byte(`{"members":[{"id":176869799018424574,"peerURLs":["http://127.0.0.1:7003"],"name":"node3","clientURLs":["http://127.0.0.1:4003"]},{"id":297577273835923749,"peerURLs":["http://127.0.0.1:2380","http://127.0.0.1:7001"],"name":"node1","clientURLs":["http://127.0.0.1:2379","http://127.0.0.1:4001"]},{"id":10666918107976480891,"peerURLs":["http://127.0.0.1:7002"],"name":"node2","clientURLs":["http://127.0.0.1:4002"]}]}`), + body: []byte(`{"members":[{"id":"2745e2525fce8fe","peerURLs":["http://127.0.0.1:7003"],"name":"node3","clientURLs":["http://127.0.0.1:4003"]},{"id":"42134f434382925","peerURLs":["http://127.0.0.1:2380","http://127.0.0.1:7001"],"name":"node1","clientURLs":["http://127.0.0.1:2379","http://127.0.0.1:4001"]},{"id":"94088180e21eb87b","peerURLs":["http://127.0.0.1:7002"],"name":"node2","clientURLs":["http://127.0.0.1:4002"]}]}`), want: MemberCollection( []Member{ { - ID: 176869799018424574, + ID: "2745e2525fce8fe", Name: "node3", PeerURLs: []string{ "http://127.0.0.1:7003", @@ -116,7 +116,7 @@ func TestMemberCollectionUnmarshal(t *testing.T) { }, }, { - ID: 297577273835923749, + ID: "42134f434382925", Name: "node1", PeerURLs: []string{ "http://127.0.0.1:2380", @@ -128,7 +128,7 @@ func TestMemberCollectionUnmarshal(t *testing.T) { }, }, { - ID: 10666918107976480891, + ID: "94088180e21eb87b", Name: "node2", PeerURLs: []string{ "http://127.0.0.1:7002", diff --git a/etcdserver/member.go b/etcdserver/member.go index 322e2a491..cb0edab6d 100644 --- a/etcdserver/member.go +++ b/etcdserver/member.go @@ -24,7 +24,6 @@ import ( "math/rand" "path" "sort" - "strconv" "time" "github.com/coreos/etcd/pkg/types" @@ -82,11 +81,11 @@ func (m *Member) PickPeerURL() string { } func memberStoreKey(id uint64) string { - return path.Join(storeMembersPrefix, idAsHex(id)) + return path.Join(storeMembersPrefix, IDAsHex(id)) } func mustParseMemberIDFromKey(key string) uint64 { - id, err := strconv.ParseUint(path.Base(key), 16, 64) + id, err := IDFromHex(path.Base(key)) if err != nil { log.Panicf("unexpected parse member id error: %v", err) } @@ -94,7 +93,7 @@ func mustParseMemberIDFromKey(key string) uint64 { } func removedMemberStoreKey(id uint64) string { - return path.Join(storeRemovedMembersPrefix, idAsHex(id)) + return path.Join(storeRemovedMembersPrefix, IDAsHex(id)) } type SortableMemberSliceByPeerURLs []*Member diff --git a/etcdserver/sender.go b/etcdserver/sender.go index 655608662..60aad66a5 100644 --- a/etcdserver/sender.go +++ b/etcdserver/sender.go @@ -72,7 +72,7 @@ func send(c *http.Client, cl *Cluster, m raftpb.Message, ss *stats.ServerStats, if m.Type == raftpb.MsgApp { ss.SendAppendReq(len(data)) } - to := idAsHex(m.To) + to := IDAsHex(m.To) fs := ls.Follower(to) start := time.Now() diff --git a/etcdserver/server.go b/etcdserver/server.go index da9bb7582..35006452f 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -239,9 +239,9 @@ func NewServer(cfg *ServerConfig) *EtcdServer { sstats := &stats.ServerStats{ Name: cfg.Name, - ID: idAsHex(id), + ID: IDAsHex(id), } - lstats := stats.NewLeaderStats(idAsHex(id)) + lstats := stats.NewLeaderStats(IDAsHex(id)) s := &EtcdServer{ store: st, @@ -423,7 +423,7 @@ func (s *EtcdServer) StoreStats() []byte { } func (s *EtcdServer) UpdateRecvApp(from uint64, length int64) { - s.stats.RecvAppendReq(idAsHex(from), int(length)) + s.stats.RecvAppendReq(IDAsHex(from), int(length)) } func (s *EtcdServer) AddMember(ctx context.Context, memb Member) error { @@ -781,6 +781,10 @@ func containsUint64(a []uint64, x uint64) bool { return false } -func idAsHex(id uint64) string { +func IDAsHex(id uint64) string { return strconv.FormatUint(id, 16) } + +func IDFromHex(s string) (uint64, error) { + return strconv.ParseUint(s, 16, 64) +} diff --git a/etcdserver/server_test.go b/etcdserver/server_test.go index 6693d4f4e..ca9cdb0f0 100644 --- a/etcdserver/server_test.go +++ b/etcdserver/server_test.go @@ -1390,3 +1390,68 @@ func mustMakePeerSlice(t *testing.T, ids ...uint64) []raft.Peer { } return peers } + +func TestIDAsHex(t *testing.T) { + tests := []struct { + input uint64 + want string + }{ + { + input: uint64(12), + want: "c", + }, + { + input: uint64(4918257920282737594), + want: "444129853c343bba", + }, + } + + for i, tt := range tests { + got := IDAsHex(tt.input) + if tt.want != got { + t.Errorf("#%d: IDAsHex failure: want=%v, got=%v", i, tt.want, got) + } + } +} + +func TestIDFromHex(t *testing.T) { + tests := []struct { + input string + want uint64 + }{ + { + input: "17", + want: uint64(23), + }, + { + input: "612840dae127353", + want: uint64(437557308098245459), + }, + } + + for i, tt := range tests { + got, err := IDFromHex(tt.input) + if err != nil { + t.Errorf("#%d: IDFromHex failure: err=%v", i, err) + continue + } + if tt.want != got { + t.Errorf("#%d: IDFromHex failure: want=%v, got=%v", i, tt.want, got) + } + } +} + +func TestIDFromHexFail(t *testing.T) { + tests := []string{ + "", + "XXX", + "612840dae127353612840dae127353", + } + + for i, tt := range tests { + _, err := IDFromHex(tt) + if err == nil { + t.Fatalf("#%d: IDFromHex expected error, but err=nil", i) + } + } +}