diff --git a/etcdserver/cluster.go b/etcdserver/cluster.go index 4f88ea647..c31aca01d 100644 --- a/etcdserver/cluster.go +++ b/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)) + 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, attributesSuffix); n.Nodes[0].Key != w { - return m, fmt.Errorf("key = %v, want %v", n.Nodes[0].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[0].Value), &m.Attributes); err != nil { - return m, fmt.Errorf("unmarshal attributes error: %v", err) - } - 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 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 } diff --git a/etcdserver/cluster_test.go b/etcdserver/cluster_test.go index 08c34a9b5..77323fe33 100644 --- a/etcdserver/cluster_test.go +++ b/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 } diff --git a/etcdserver/server_test.go b/etcdserver/server_test.go index 566e8cc21..14bbed022 100644 --- a/etcdserver/server_test.go +++ b/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{}, diff --git a/integration/v2_http_kv_test.go b/integration/v2_http_kv_test.go index 6047187b7..55addb7cf 100644 --- a/integration/v2_http_kv_test.go +++ b/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", }