Merge pull request #1381 from jonboulle/members

/v2/admin/members API should use JSON containers in response

Documentation/0.5/admin_api.md

@@ -1,30 +1,7 @@
 ## Admin API
 
-### GET /v2/admin/members/:id
-Returns an HTTP 200 OK response code and a representation of the requested member; returns a 404 status code and an error message if the id does not exist.
-```
-    Example Request: GET 
-                     http://localhost:2379/v2/admin/members/272e204152
-    Response formats: JSON
-    Example Response:
-```
-```json
-    [
-        {
-            "ID":"272e204152",
-            "Name":"node1",
-            "PeerURLs":[
-                "http://10.0.0.10:2379"
-            ],
-            "ClientURLs":[
-                "http://10.0.0.10:2380"
-            ]
-        },
-    ]
-```
-
 ### GET /v2/admin/members/
-Return an HTTP 200 OK response code and a representation of all the members;
+Return an HTTP 200 OK response code and a representation of all members in the etcd cluster:
 ```
     Example Request: GET 
                      http://localhost:2379/v2/admin/members/
@@ -32,28 +9,30 @@ Return an HTTP 200 OK response code and a representation of all the members;
     Example Response:
 ```
 ```json
-    [
-        {
-            "ID":"272e204152",
-            "Name":"node1",
-            "PeerURLs":[
-                "http://10.0.0.10:2379"
-            ],
-            "ClientURLs":[
-                "http://10.0.0.10:2380"
-            ]
-        },
-        {
-            "ID":"2225373f43",
-            "Name":"node2",
-            "PeerURLs":[
-                "http://127.0.0.11:2379"
-            ],
-            "ClientURLs":[
-                "http://127.0.0.11:2380"
-            ]
-        },
-    ]
+    {
+        "members": [
+                {
+                    "id":"272e204152",
+                    "name":"node1",
+                    "peerURLs":[
+                        "http://10.0.0.10:2379"
+                    ],
+                    "clientURLs":[
+                            "http://10.0.0.10:2380"
+                    ]
+                },
+                {
+                    "id":"2225373f43",
+                    "name":"node2",
+                    "peerURLs":[
+                        "http://127.0.0.11:2379"
+                    ],
+                    "clientURLs":[
+                        "http://127.0.0.11:2380"
+                    ]
+            },
+        ]
+    }
 ```
 
 ### POST /v2/admin/members/
@@ -73,7 +52,7 @@ If the POST body is malformed an HTTP 400 will be returned. If the member exists
     [
         {
             "id":"3777296169",
-            "PeerURLs":[
+            "peerURLs":[
                 "http://10.0.0.10:2379"
             ],
         },

etcdserver/cluster_test.go

@@ -381,19 +381,19 @@ func TestNodeToMemberBad(t *testing.T) {
 			{Key: "/1234/dynamic", Value: stringp("garbage")},
 		}},
 		{Key: "/1234", Nodes: []*store.NodeExtern{
-			{Key: "/1234/dynamic", Value: stringp(`{"PeerURLs":null}`)},
+			{Key: "/1234/dynamic", Value: stringp(`{"peerURLs":null}`)},
 		}},
 		{Key: "/1234", Nodes: []*store.NodeExtern{
-			{Key: "/1234/dynamic", Value: stringp(`{"PeerURLs":null}`)},
+			{Key: "/1234/dynamic", Value: stringp(`{"peerURLs":null}`)},
 			{Key: "/1234/strange"},
 		}},
 		{Key: "/1234", Nodes: []*store.NodeExtern{
-			{Key: "/1234/dynamic", Value: stringp(`{"PeerURLs":null}`)},
+			{Key: "/1234/dynamic", Value: stringp(`{"peerURLs":null}`)},
 			{Key: "/1234/static", Value: stringp("garbage")},
 		}},
 		{Key: "/1234", Nodes: []*store.NodeExtern{
-			{Key: "/1234/dynamic", Value: stringp(`{"PeerURLs":null}`)},
-			{Key: "/1234/static", Value: stringp(`{"Name":"node1","ClientURLs":null}`)},
+			{Key: "/1234/dynamic", Value: stringp(`{"peerURLs":null}`)},
+			{Key: "/1234/static", Value: stringp(`{"name":"node1","clientURLs":null}`)},
 			{Key: "/1234/strange"},
 		}},
 	}
@@ -416,7 +416,7 @@ func TestClusterAddMember(t *testing.T) {
 			params: []interface{}{
 				path.Join(storeMembersPrefix, "1", "raftAttributes"),
 				false,
-				`{"PeerURLs":null}`,
+				`{"peerURLs":null}`,
 				false,
 				store.Permanent,
 			},
@@ -426,7 +426,7 @@ func TestClusterAddMember(t *testing.T) {
 			params: []interface{}{
 				path.Join(storeMembersPrefix, "1", "attributes"),
 				false,
-				`{"Name":"node1"}`,
+				`{"name":"node1"}`,
 				false,
 				store.Permanent,
 			},
@@ -519,8 +519,8 @@ func TestClusterRemoveMember(t *testing.T) {
 
 func TestNodeToMember(t *testing.T) {
 	n := &store.NodeExtern{Key: "/1234", Nodes: []*store.NodeExtern{
-		{Key: "/1234/attributes", Value: stringp(`{"Name":"node1","ClientURLs":null}`)},
-		{Key: "/1234/raftAttributes", Value: stringp(`{"PeerURLs":null}`)},
+		{Key: "/1234/attributes", Value: stringp(`{"name":"node1","clientURLs":null}`)},
+		{Key: "/1234/raftAttributes", Value: stringp(`{"peerURLs":null}`)},
 	}}
 	wm := &Member{ID: 0x1234, RaftAttributes: RaftAttributes{}, Attributes: Attributes{Name: "node1"}}
 	m, err := nodeToMember(n)

etcdserver/etcdhttp/http.go

@@ -161,30 +161,19 @@ func (h serverHandler) serveAdminMembers(w http.ResponseWriter, r *http.Request)
 
 	switch r.Method {
 	case "GET":
-		idStr := strings.TrimPrefix(r.URL.Path, adminMembersPrefix)
-		if idStr == "" {
-			ms := h.clusterInfo.Members()
-			w.Header().Set("Content-Type", "application/json")
-			if err := json.NewEncoder(w).Encode(ms); err != nil {
-				log.Printf("etcdhttp: %v", err)
-			}
-			return
-		}
-		id, err := strconv.ParseUint(idStr, 16, 64)
-		if err != nil {
-			http.Error(w, err.Error(), http.StatusBadRequest)
+		if s := strings.TrimPrefix(r.URL.Path, adminMembersPrefix); s != "" {
+			http.NotFound(w, r)
 			return
 		}
-		m := h.clusterInfo.Member(id)
-		if m == nil {
-			http.Error(w, "member not found", http.StatusNotFound)
-			return
+		ms := struct {
+			Members []*etcdserver.Member `json:"members"`
+		}{
+			Members: h.clusterInfo.Members(),
 		}
 		w.Header().Set("Content-Type", "application/json")
-		if err := json.NewEncoder(w).Encode(m); err != nil {
+		if err := json.NewEncoder(w).Encode(ms); err != nil {
 			log.Printf("etcdhttp: %v", err)
 		}
-		return
 	case "POST":
 		ctype := r.Header.Get("Content-Type")
 		if ctype != "application/json" {

etcdserver/etcdhttp/http_test.go

@@ -1563,16 +1563,6 @@ func TestServeAdminMembersFail(t *testing.T) {
 
 			http.StatusInternalServerError,
 		},
-		{
-			// etcdserver.GetMember bad id
-			&http.Request{
-				URL:    mustNewURL(t, path.Join(adminMembersPrefix, "badid")),
-				Method: "GET",
-			},
-			&errServer{},
-
-			http.StatusBadRequest,
-		},
 	}
 	for i, tt := range tests {
 		h := &serverHandler{
@@ -1627,16 +1617,17 @@ func TestServeAdminMembers(t *testing.T) {
 		clusterInfo: cluster,
 	}
 
-	msb, err := json.Marshal([]etcdserver.Member{memb1, memb2})
+	msb, err := json.Marshal(
+		struct {
+			Members []etcdserver.Member `json:"members"`
+		}{
+			Members: []etcdserver.Member{memb1, memb2},
+		},
+	)
 	if err != nil {
 		t.Fatal(err)
 	}
 	wms := string(msb) + "\n"
-	mb, err := json.Marshal(memb1)
-	if err != nil {
-		t.Fatal(err)
-	}
-	wm := string(mb) + "\n"
 
 	tests := []struct {
 		path  string
@@ -1645,8 +1636,8 @@ func TestServeAdminMembers(t *testing.T) {
 		wbody string
 	}{
 		{adminMembersPrefix, http.StatusOK, "application/json", wms},
-		{path.Join(adminMembersPrefix, "1"), http.StatusOK, "application/json", wm},
-		{path.Join(adminMembersPrefix, "100"), http.StatusNotFound, "text/plain; charset=utf-8", "member not found\n"},
+		{path.Join(adminMembersPrefix, "100"), http.StatusNotFound, "text/plain; charset=utf-8", "404 page not found\n"},
+		{path.Join(adminMembersPrefix, "foobar"), http.StatusNotFound, "text/plain; charset=utf-8", "404 page not found\n"},
 	}
 
 	for i, tt := range tests {
@@ -1664,7 +1655,7 @@ func TestServeAdminMembers(t *testing.T) {
 			t.Errorf("#%d: content-type = %s, want %s", i, gct, tt.wct)
 		}
 		if rw.Body.String() != tt.wbody {
-			t.Errorf("#%d: body = %s, want %s", i, rw.Body.String(), tt.wbody)
+			t.Errorf("#%d: body = %q, want %q", i, rw.Body.String(), tt.wbody)
 		}
 	}
 }

etcdserver/member.go

@@ -33,17 +33,17 @@ import (
 // RaftAttributes represents the raft related attributes of an etcd member.
 type RaftAttributes struct {
 	// TODO(philips): ensure these are URLs
-	PeerURLs []string
+	PeerURLs []string `json:"peerURLs"`
 }
 
 // Attributes represents all the non-raft related attributes of an etcd member.
 type Attributes struct {
-	Name       string   `json:",omitempty"`
-	ClientURLs []string `json:",omitempty"`
+	Name       string   `json:"name,omitempty"`
+	ClientURLs []string `json:"clientURLs,omitempty"`
 }
 
 type Member struct {
-	ID uint64
+	ID uint64 `json:"id"`
 	RaftAttributes
 	Attributes
 }