etcdserver: not record attributes when add member

There is no need to set attributes value when adding member because new
member will publish the information whenever it starts.

etcdserver/cluster.go

@@ -327,7 +327,8 @@ func (c *Cluster) ValidateConfigurationChange(cc raftpb.ConfChange) error {
 	return nil
 }
 
-// AddMember puts a new Member into the store.
+// AddMember adds a new Member into the cluster, and saves the given member's
+// raftAttributes into the store. The given member should have empty attributes.
 // A Member with a matching id must not exist.
 func (c *Cluster) AddMember(m *Member) {
 	c.Lock()
@@ -340,14 +341,6 @@ func (c *Cluster) AddMember(m *Member) {
 	if _, err := c.store.Create(p, false, string(b), false, store.Permanent); err != nil {
 		log.Panicf("create raftAttributes should never fail: %v", err)
 	}
-	b, err = json.Marshal(m.Attributes)
-	if err != nil {
-		log.Panicf("marshal attributes should never fail: %v", err)
-	}
-	p = path.Join(memberStoreKey(m.ID), attributesSuffix)
-	if _, err := c.store.Create(p, false, string(b), false, store.Permanent); err != nil {
-		log.Panicf("create attributes should never fail: %v", err)
-	}
 	c.members[m.ID] = m
 }
 
@@ -390,20 +383,26 @@ func (c *Cluster) UpdateMember(nm *Member) {
 // the child nodes of the given node should be sorted by key.
 func nodeToMember(n *store.NodeExtern) (*Member, error) {
 	m := &Member{ID: mustParseMemberIDFromKey(n.Key)}
-	if len(n.Nodes) != 2 {
-		return m, fmt.Errorf("len(nodes) = %d, want 2", len(n.Nodes))
-	}
-	if w := path.Join(n.Key, attributesSuffix); n.Nodes[0].Key != w {
-		return m, fmt.Errorf("key = %v, want %v", n.Nodes[0].Key, w)
-	}
-	if err := json.Unmarshal([]byte(*n.Nodes[0].Value), &m.Attributes); err != nil {
-		return m, fmt.Errorf("unmarshal attributes error: %v", err)
+	attrs := make(map[string][]byte)
+	raftAttrKey := path.Join(n.Key, raftAttributesSuffix)
+	attrKey := path.Join(n.Key, attributesSuffix)
+	for _, nn := range n.Nodes {
+		if nn.Key != raftAttrKey && nn.Key != attrKey {
+			return nil, fmt.Errorf("unknown key %q", nn.Key)
+		}
+		attrs[nn.Key] = []byte(*nn.Value)
 	}
-	if w := path.Join(n.Key, raftAttributesSuffix); n.Nodes[1].Key != w {
-		return m, fmt.Errorf("key = %v, want %v", n.Nodes[1].Key, w)
+	if data := attrs[raftAttrKey]; data != nil {
+		if err := json.Unmarshal(data, &m.RaftAttributes); err != nil {
+			return nil, fmt.Errorf("unmarshal raftAttributes error: %v", err)
+		}
+	} else {
+		return nil, fmt.Errorf("raftAttributes key doesn't exist")
 	}
-	if err := json.Unmarshal([]byte(*n.Nodes[1].Value), &m.RaftAttributes); err != nil {
-		return m, fmt.Errorf("unmarshal raftAttributes error: %v", err)
+	if data := attrs[attrKey]; data != nil {
+		if err := json.Unmarshal(data, &m.Attributes); err != nil {
+			return m, fmt.Errorf("unmarshal attributes error: %v", err)
+		}
 	}
 	return m, nil
 }

etcdserver/cluster_test.go

@@ -82,20 +82,21 @@ func TestClusterFromStore(t *testing.T) {
 		mems []*Member
 	}{
 		{
-			[]*Member{newTestMember(1, nil, "node1", nil)},
+			[]*Member{newTestMember(1, nil, "", nil)},
 		},
 		{
 			nil,
 		},
 		{
 			[]*Member{
-				newTestMember(1, nil, "node1", nil),
-				newTestMember(2, nil, "node2", nil),
+				newTestMember(1, nil, "", nil),
+				newTestMember(2, nil, "", nil),
 			},
 		},
 	}
 	for i, tt := range tests {
 		hc := newTestCluster(nil)
+		hc.SetStore(store.New())
 		for _, m := range tt.mems {
 			hc.AddMember(m)
 		}
@@ -500,22 +501,22 @@ func TestNodeToMemberBad(t *testing.T) {
 			{Key: "/1234/strange"},
 		}},
 		{Key: "/1234", Nodes: []*store.NodeExtern{
-			{Key: "/1234/dynamic", Value: stringp("garbage")},
+			{Key: "/1234/raftAttributes", Value: stringp("garbage")},
 		}},
 		{Key: "/1234", Nodes: []*store.NodeExtern{
-			{Key: "/1234/dynamic", Value: stringp(`{"peerURLs":null}`)},
+			{Key: "/1234/attributes", Value: stringp(`{"name":"node1","clientURLs":null}`)},
 		}},
 		{Key: "/1234", Nodes: []*store.NodeExtern{
-			{Key: "/1234/dynamic", Value: stringp(`{"peerURLs":null}`)},
+			{Key: "/1234/raftAttributes", Value: stringp(`{"peerURLs":null}`)},
 			{Key: "/1234/strange"},
 		}},
 		{Key: "/1234", Nodes: []*store.NodeExtern{
-			{Key: "/1234/dynamic", Value: stringp(`{"peerURLs":null}`)},
-			{Key: "/1234/static", Value: stringp("garbage")},
+			{Key: "/1234/raftAttributes", Value: stringp(`{"peerURLs":null}`)},
+			{Key: "/1234/attributes", 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/raftAttributes", Value: stringp(`{"peerURLs":null}`)},
+			{Key: "/1234/attributes", Value: stringp(`{"name":"node1","clientURLs":null}`)},
 			{Key: "/1234/strange"},
 		}},
 	}
@@ -543,16 +544,6 @@ func TestClusterAddMember(t *testing.T) {
 				store.Permanent,
 			},
 		},
-		{
-			name: "Create",
-			params: []interface{}{
-				path.Join(storeMembersPrefix, "1", "attributes"),
-				false,
-				`{"name":"node1"}`,
-				false,
-				store.Permanent,
-			},
-		},
 	}
 	if g := st.Action(); !reflect.DeepEqual(g, wactions) {
 		t.Errorf("actions = %v, want %v", g, wactions)
@@ -656,9 +647,8 @@ func TestNodeToMember(t *testing.T) {
 
 func newTestCluster(membs []*Member) *Cluster {
 	c := &Cluster{members: make(map[types.ID]*Member), removed: make(map[types.ID]bool)}
-	c.store = store.New()
-	for i := range membs {
-		c.AddMember(membs[i])
+	for _, m := range membs {
+		c.members[m.ID] = m
 	}
 	return c
 }

etcdserver/server_test.go

@@ -554,8 +554,8 @@ func testServer(t *testing.T, ns uint64) {
 
 		g, w := resp.Event.Node, &store.NodeExtern{
 			Key:           "/foo",
-			ModifiedIndex: uint64(i) + 2*ns,
-			CreatedIndex:  uint64(i) + 2*ns,
+			ModifiedIndex: uint64(i) + ns,
+			CreatedIndex:  uint64(i) + ns,
 			Value:         stringp("bar"),
 		}
 
@@ -993,7 +993,9 @@ func TestRemoveMember(t *testing.T) {
 			Nodes:     []uint64{1234, 2345, 3456},
 		},
 	}
-	cl := newTestCluster([]*Member{{ID: 1234}})
+	cl := newTestCluster(nil)
+	cl.SetStore(store.New())
+	cl.AddMember(&Member{ID: 1234})
 	s := &EtcdServer{
 		node:    n,
 		store:   &storeRecorder{},
@@ -1027,7 +1029,9 @@ func TestUpdateMember(t *testing.T) {
 			Nodes:     []uint64{1234, 2345, 3456},
 		},
 	}
-	cl := newTestCluster([]*Member{{ID: 1234}})
+	cl := newTestCluster(nil)
+	cl.SetStore(store.New())
+	cl.AddMember(&Member{ID: 1234})
 	s := &EtcdServer{
 		node:    n,
 		store:   &storeRecorder{},

integration/v2_http_kv_test.go

@@ -55,19 +55,19 @@ func TestV2Set(t *testing.T) {
 			"/v2/keys/foo/bar",
 			v,
 			http.StatusCreated,
-			`{"action":"set","node":{"key":"/foo/bar","value":"bar","modifiedIndex":4,"createdIndex":4}}`,
+			`{"action":"set","node":{"key":"/foo/bar","value":"bar","modifiedIndex":3,"createdIndex":3}}`,
 		},
 		{
 			"/v2/keys/foodir?dir=true",
 			url.Values{},
 			http.StatusCreated,
-			`{"action":"set","node":{"key":"/foodir","dir":true,"modifiedIndex":5,"createdIndex":5}}`,
+			`{"action":"set","node":{"key":"/foodir","dir":true,"modifiedIndex":4,"createdIndex":4}}`,
 		},
 		{
 			"/v2/keys/fooempty",
 			url.Values(map[string][]string{"value": {""}}),
 			http.StatusCreated,
-			`{"action":"set","node":{"key":"/fooempty","value":"","modifiedIndex":6,"createdIndex":6}}`,
+			`{"action":"set","node":{"key":"/fooempty","value":"","modifiedIndex":5,"createdIndex":5}}`,
 		},
 	}
 
@@ -216,12 +216,12 @@ func TestV2CAS(t *testing.T) {
 		},
 		{
 			"/v2/keys/cas/foo",
-			url.Values(map[string][]string{"value": {"YYY"}, "prevIndex": {"4"}}),
+			url.Values(map[string][]string{"value": {"YYY"}, "prevIndex": {"3"}}),
 			http.StatusOK,
 			map[string]interface{}{
 				"node": map[string]interface{}{
 					"value":         "YYY",
-					"modifiedIndex": float64(5),
+					"modifiedIndex": float64(4),
 				},
 				"action": "compareAndSwap",
 			},
@@ -233,8 +233,8 @@ func TestV2CAS(t *testing.T) {
 			map[string]interface{}{
 				"errorCode": float64(101),
 				"message":   "Compare failed",
-				"cause":     "[10 != 5]",
-				"index":     float64(5),
+				"cause":     "[10 != 4]",
+				"index":     float64(4),
 			},
 		},
 		{
@@ -283,7 +283,7 @@ func TestV2CAS(t *testing.T) {
 			map[string]interface{}{
 				"errorCode": float64(101),
 				"message":   "Compare failed",
-				"cause":     "[bad_value != ZZZ] [100 != 6]",
+				"cause":     "[bad_value != ZZZ] [100 != 5]",
 			},
 		},
 		{
@@ -293,12 +293,12 @@ func TestV2CAS(t *testing.T) {
 			map[string]interface{}{
 				"errorCode": float64(101),
 				"message":   "Compare failed",
-				"cause":     "[100 != 6]",
+				"cause":     "[100 != 5]",
 			},
 		},
 		{
 			"/v2/keys/cas/foo",
-			url.Values(map[string][]string{"value": {"XXX"}, "prevValue": {"bad_value"}, "prevIndex": {"6"}}),
+			url.Values(map[string][]string{"value": {"XXX"}, "prevValue": {"bad_value"}, "prevIndex": {"5"}}),
 			http.StatusPreconditionFailed,
 			map[string]interface{}{
 				"errorCode": float64(101),
@@ -448,7 +448,7 @@ func TestV2CAD(t *testing.T) {
 			map[string]interface{}{
 				"errorCode": float64(101),
 				"message":   "Compare failed",
-				"cause":     "[100 != 4]",
+				"cause":     "[100 != 3]",
 			},
 		},
 		{
@@ -460,12 +460,12 @@ func TestV2CAD(t *testing.T) {
 			},
 		},
 		{
-			"/v2/keys/foo?prevIndex=4",
+			"/v2/keys/foo?prevIndex=3",
 			http.StatusOK,
 			map[string]interface{}{
 				"node": map[string]interface{}{
 					"key":           "/foo",
-					"modifiedIndex": float64(6),
+					"modifiedIndex": float64(5),
 				},
 				"action": "compareAndDelete",
 			},
@@ -493,7 +493,7 @@ func TestV2CAD(t *testing.T) {
 			map[string]interface{}{
 				"node": map[string]interface{}{
 					"key":           "/foovalue",
-					"modifiedIndex": float64(7),
+					"modifiedIndex": float64(6),
 				},
 				"action": "compareAndDelete",
 			},
@@ -531,7 +531,7 @@ func TestV2Unique(t *testing.T) {
 			http.StatusCreated,
 			map[string]interface{}{
 				"node": map[string]interface{}{
-					"key":   "/foo/4",
+					"key":   "/foo/3",
 					"value": "XXX",
 				},
 				"action": "create",
@@ -543,7 +543,7 @@ func TestV2Unique(t *testing.T) {
 			http.StatusCreated,
 			map[string]interface{}{
 				"node": map[string]interface{}{
-					"key":   "/foo/5",
+					"key":   "/foo/4",
 					"value": "XXX",
 				},
 				"action": "create",
@@ -555,7 +555,7 @@ func TestV2Unique(t *testing.T) {
 			http.StatusCreated,
 			map[string]interface{}{
 				"node": map[string]interface{}{
-					"key":   "/bar/6",
+					"key":   "/bar/5",
 					"value": "XXX",
 				},
 				"action": "create",
@@ -617,8 +617,8 @@ func TestV2Get(t *testing.T) {
 						map[string]interface{}{
 							"key":           "/foo/bar",
 							"dir":           true,
-							"createdIndex":  float64(4),
-							"modifiedIndex": float64(4),
+							"createdIndex":  float64(3),
+							"modifiedIndex": float64(3),
 						},
 					},
 				},
@@ -636,14 +636,14 @@ func TestV2Get(t *testing.T) {
 						map[string]interface{}{
 							"key":           "/foo/bar",
 							"dir":           true,
-							"createdIndex":  float64(4),
-							"modifiedIndex": float64(4),
+							"createdIndex":  float64(3),
+							"modifiedIndex": float64(3),
 							"nodes": []interface{}{
 								map[string]interface{}{
 									"key":           "/foo/bar/zar",
 									"value":         "XXX",
-									"createdIndex":  float64(4),
-									"modifiedIndex": float64(4),
+									"createdIndex":  float64(3),
+									"modifiedIndex": float64(3),
 								},
 							},
 						},
@@ -711,8 +711,8 @@ func TestV2QuorumGet(t *testing.T) {
 						map[string]interface{}{
 							"key":           "/foo/bar",
 							"dir":           true,
-							"createdIndex":  float64(4),
-							"modifiedIndex": float64(4),
+							"createdIndex":  float64(3),
+							"modifiedIndex": float64(3),
 						},
 					},
 				},
@@ -730,14 +730,14 @@ func TestV2QuorumGet(t *testing.T) {
 						map[string]interface{}{
 							"key":           "/foo/bar",
 							"dir":           true,
-							"createdIndex":  float64(4),
-							"modifiedIndex": float64(4),
+							"createdIndex":  float64(3),
+							"modifiedIndex": float64(3),
 							"nodes": []interface{}{
 								map[string]interface{}{
 									"key":           "/foo/bar/zar",
 									"value":         "XXX",
-									"createdIndex":  float64(4),
-									"modifiedIndex": float64(4),
+									"createdIndex":  float64(3),
+									"modifiedIndex": float64(3),
 								},
 							},
 						},
@@ -797,7 +797,7 @@ func TestV2Watch(t *testing.T) {
 		"node": map[string]interface{}{
 			"key":           "/foo/bar",
 			"value":         "XXX",
-			"modifiedIndex": float64(4),
+			"modifiedIndex": float64(3),
 		},
 		"action": "set",
 	}
@@ -818,7 +818,7 @@ func TestV2WatchWithIndex(t *testing.T) {
 	var body map[string]interface{}
 	c := make(chan bool, 1)
 	go func() {
-		resp, _ := tc.Get(fmt.Sprintf("%s%s", u, "/v2/keys/foo/bar?wait=true&waitIndex=5"))
+		resp, _ := tc.Get(fmt.Sprintf("%s%s", u, "/v2/keys/foo/bar?wait=true&waitIndex=4"))
 		body = tc.ReadBodyJSON(resp)
 		c <- true
 	}()
@@ -855,7 +855,7 @@ func TestV2WatchWithIndex(t *testing.T) {
 		"node": map[string]interface{}{
 			"key":           "/foo/bar",
 			"value":         "XXX",
-			"modifiedIndex": float64(5),
+			"modifiedIndex": float64(4),
 		},
 		"action": "set",
 	}