Merge pull request #1443 from bcwaldon/member-ID

Serialize member ID in API

etcdserver/cluster.go

@@ -113,7 +113,7 @@ func NewClusterFromStore(name string, st store.Store) *Cluster {
 		log.Panicf("get storeRemovedMembers should never fail: %v", err)
 	}
 	for _, n := range e.Node.Nodes {
-		c.removed[parseMemberID(n.Key)] = true
+		c.removed[mustParseMemberIDFromKey(n.Key)] = true
 	}
 
 	return c
@@ -303,7 +303,7 @@ func (c *Cluster) RemoveMember(id uint64) {
 // nodeToMember builds member through a store node.
 // the child nodes of the given node should be sorted by key.
 func nodeToMember(n *store.NodeExtern) (*Member, error) {
-	m := &Member{ID: parseMemberID(n.Key)}
+	m := &Member{ID: mustParseMemberIDFromKey(n.Key)}
 	if len(n.Nodes) != 2 {
 		return m, fmt.Errorf("len(nodes) = %d, want 2", len(n.Nodes))
 	}

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)),

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"}`},
 	}

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"`

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",

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 parseMemberID(key string) uint64 {
-	id, err := strconv.ParseUint(path.Base(key), 16, 64)
+func mustParseMemberIDFromKey(key string) uint64 {
+	id, err := IDFromHex(path.Base(key))
 	if err != nil {
 		log.Panicf("unexpected parse member id error: %v", err)
 	}
@@ -94,7 +93,7 @@ func parseMemberID(key string) uint64 {
 }
 
 func removedMemberStoreKey(id uint64) string {
-	return path.Join(storeRemovedMembersPrefix, idAsHex(id))
+	return path.Join(storeRemovedMembersPrefix, IDAsHex(id))
 }
 
 type SortableMemberSliceByPeerURLs []*Member

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()

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 {
@@ -592,7 +592,7 @@ func (s *EtcdServer) applyRequest(r pb.Request) Response {
 			return f(s.store.CompareAndSwap(r.Path, r.PrevValue, r.PrevIndex, r.Val, expr))
 		default:
 			if storeMemberAttributeRegexp.MatchString(r.Path) {
-				id := parseMemberID(path.Dir(r.Path))
+				id := mustParseMemberIDFromKey(path.Dir(r.Path))
 				m := s.Cluster.Member(id)
 				if m == nil {
 					log.Fatalf("fetch member %x should never fail", id)
@@ -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)
+}

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)
+		}
+	}
+}