From 604bc04f707a144d4b5fe078970acc01776f792b Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Sun, 17 Mar 2019 00:33:28 -0700 Subject: [PATCH] etcdserver: support MemberAdd for learner Added IsLearner field to etcdserver internal Member type. Routed learner MemberAdd request from server API to raft. Apply learner MemberAdd result to server after the request is passed through Raft. --- etcdserver/api/membership/cluster.go | 6 ++++-- etcdserver/api/membership/cluster_test.go | 23 +++++++++++++++++++++++ etcdserver/api/membership/member.go | 14 +++++++++++--- etcdserver/api/membership/member_test.go | 22 +++++++++++++++------- etcdserver/api/v2http/client.go | 2 +- etcdserver/api/v3rpc/member.go | 11 ++++++++--- etcdserver/server.go | 8 +++++++- etcdserver/server_test.go | 16 ++++++++-------- 8 files changed, 77 insertions(+), 25 deletions(-) diff --git a/etcdserver/api/membership/cluster.go b/etcdserver/api/membership/cluster.go index 5553f2890..43d0ec008 100644 --- a/etcdserver/api/membership/cluster.go +++ b/etcdserver/api/membership/cluster.go @@ -59,10 +59,12 @@ type RaftCluster struct { removed map[types.ID]bool } +// NewClusterFromURLsMap creates a new raft cluster using provided urls map. Currently, it does not support creating +// cluster with raft learner member. func NewClusterFromURLsMap(lg *zap.Logger, token string, urlsmap types.URLsMap) (*RaftCluster, error) { c := NewCluster(lg, token) for name, urls := range urlsmap { - m := NewMember(name, urls, token, nil) + m := NewMember(name, urls, token, nil, false) if _, ok := c.members[m.ID]; ok { return nil, fmt.Errorf("member exists with identical ID %v", m) } @@ -259,7 +261,7 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { return ErrIDRemoved } switch cc.Type { - case raftpb.ConfChangeAddNode: + case raftpb.ConfChangeAddNode, raftpb.ConfChangeAddLearnerNode: if members[id] != nil { return ErrIDExists } diff --git a/etcdserver/api/membership/cluster_test.go b/etcdserver/api/membership/cluster_test.go index 04939b49a..e6a23e35c 100644 --- a/etcdserver/api/membership/cluster_test.go +++ b/etcdserver/api/membership/cluster_test.go @@ -472,6 +472,29 @@ func TestClusterAddMember(t *testing.T) { } } +func TestClusterAddMemberAsLearner(t *testing.T) { + st := mockstore.NewRecorder() + c := newTestCluster(nil) + c.SetStore(st) + c.AddMember(newTestMemberAsLearner(1, nil, "node1", nil)) + + wactions := []testutil.Action{ + { + Name: "Create", + Params: []interface{}{ + path.Join(StoreMembersPrefix, "1", "raftAttributes"), + false, + `{"peerURLs":null,"isLearner":true}`, + false, + v2store.TTLOptionSet{ExpireTime: v2store.Permanent}, + }, + }, + } + if g := st.Action(); !reflect.DeepEqual(g, wactions) { + t.Errorf("actions = %v, want %v", g, wactions) + } +} + func TestClusterMembers(t *testing.T) { cls := &RaftCluster{ members: map[types.ID]*Member{ diff --git a/etcdserver/api/membership/member.go b/etcdserver/api/membership/member.go index 3b360e662..ca751a1cd 100644 --- a/etcdserver/api/membership/member.go +++ b/etcdserver/api/membership/member.go @@ -35,6 +35,8 @@ type RaftAttributes struct { // PeerURLs is the list of peers in the raft cluster. // TODO(philips): ensure these are URLs PeerURLs []string `json:"peerURLs"` + // IsLearner indicates if the member is raft learner. + IsLearner bool `json:"isLearner,omitempty"` } // Attributes represents all the non-raft related attributes of an etcd member. @@ -51,10 +53,13 @@ type Member struct { // NewMember creates a Member without an ID and generates one based on the // cluster name, peer URLs, and time. This is used for bootstrapping/adding new member. -func NewMember(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member { +func NewMember(name string, peerURLs types.URLs, clusterName string, now *time.Time, isLearner bool) *Member { m := &Member{ - RaftAttributes: RaftAttributes{PeerURLs: peerURLs.StringSlice()}, - Attributes: Attributes{Name: name}, + RaftAttributes: RaftAttributes{ + PeerURLs: peerURLs.StringSlice(), + IsLearner: isLearner, + }, + Attributes: Attributes{Name: name}, } var b []byte @@ -88,6 +93,9 @@ func (m *Member) Clone() *Member { } mm := &Member{ ID: m.ID, + RaftAttributes: RaftAttributes{ + IsLearner: m.IsLearner, + }, Attributes: Attributes{ Name: m.Name, }, diff --git a/etcdserver/api/membership/member_test.go b/etcdserver/api/membership/member_test.go index bdecfbed9..415946ef0 100644 --- a/etcdserver/api/membership/member_test.go +++ b/etcdserver/api/membership/member_test.go @@ -36,17 +36,17 @@ func TestMemberTime(t *testing.T) { mem *Member id types.ID }{ - {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298}, + {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil, false), 14544069596553697298}, // Same ID, different name (names shouldn't matter) - {NewMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298}, + {NewMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil, false), 14544069596553697298}, // Same ID, different Time - {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 2448790162483548276}, + {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z"), false), 2448790162483548276}, // Different cluster name - {NewMember("mcm1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "etcd", timeParse("1984-12-23T15:04:05Z")), 6973882743191604649}, - {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 1466075294948436910}, + {NewMember("mcm1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "etcd", timeParse("1984-12-23T15:04:05Z"), false), 6973882743191604649}, + {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}}, "", timeParse("1984-12-23T15:04:05Z"), false), 1466075294948436910}, // Order shouldn't matter - {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "10.0.0.2:2379"}}, "", nil), 16552244735972308939}, - {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.2:2379"}, {Scheme: "http", Host: "10.0.0.1:2379"}}, "", nil), 16552244735972308939}, + {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.1:2379"}, {Scheme: "http", Host: "10.0.0.2:2379"}}, "", nil, false), 16552244735972308939}, + {NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.2:2379"}, {Scheme: "http", Host: "10.0.0.1:2379"}}, "", nil, false), 16552244735972308939}, } for i, tt := range tests { if tt.mem.ID != tt.id { @@ -113,3 +113,11 @@ func newTestMember(id uint64, peerURLs []string, name string, clientURLs []strin Attributes: Attributes{Name: name, ClientURLs: clientURLs}, } } + +func newTestMemberAsLearner(id uint64, peerURLs []string, name string, clientURLs []string) *Member { + return &Member{ + ID: types.ID(id), + RaftAttributes: RaftAttributes{PeerURLs: peerURLs, IsLearner: true}, + Attributes: Attributes{Name: name, ClientURLs: clientURLs}, + } +} diff --git a/etcdserver/api/v2http/client.go b/etcdserver/api/v2http/client.go index 4b6fa9f86..58ee32546 100644 --- a/etcdserver/api/v2http/client.go +++ b/etcdserver/api/v2http/client.go @@ -238,7 +238,7 @@ func (h *membersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } now := h.clock.Now() - m := membership.NewMember("", req.PeerURLs, "", &now) + m := membership.NewMember("", req.PeerURLs, "", &now, false) // does not support adding learner via v2http _, err := h.server.AddMember(ctx, *m) switch { case err == membership.ErrIDExists || err == membership.ErrPeerURLexists: diff --git a/etcdserver/api/v3rpc/member.go b/etcdserver/api/v3rpc/member.go index d1bd00b0c..96ebdbf55 100644 --- a/etcdserver/api/v3rpc/member.go +++ b/etcdserver/api/v3rpc/member.go @@ -46,15 +46,19 @@ func (cs *ClusterServer) MemberAdd(ctx context.Context, r *pb.MemberAddRequest) } now := time.Now() - m := membership.NewMember("", urls, "", &now) + m := membership.NewMember("", urls, "", &now, r.IsLearner) membs, merr := cs.server.AddMember(ctx, *m) if merr != nil { return nil, togRPCError(merr) } return &pb.MemberAddResponse{ - Header: cs.header(), - Member: &pb.Member{ID: uint64(m.ID), PeerURLs: m.PeerURLs}, + Header: cs.header(), + Member: &pb.Member{ + ID: uint64(m.ID), + PeerURLs: m.PeerURLs, + IsLearner: m.IsLearner, + }, Members: membersToProtoMembers(membs), }, nil } @@ -101,6 +105,7 @@ func membersToProtoMembers(membs []*membership.Member) []*pb.Member { ID: uint64(membs[i].ID), PeerURLs: membs[i].PeerURLs, ClientURLs: membs[i].ClientURLs, + IsLearner: membs[i].IsLearner, } } return protoMembs diff --git a/etcdserver/server.go b/etcdserver/server.go index d12c94770..ed0941fd2 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -1544,6 +1544,7 @@ func (s *EtcdServer) AddMember(ctx context.Context, memb membership.Member) ([]* return nil, err } + // TODO: might switch to less strict check when adding raft learner if s.Cfg.StrictReconfigCheck { // by default StrictReconfigCheck is enabled; reject new members if unhealthy if !s.cluster.IsReadyToAddNewMember() { @@ -1585,6 +1586,11 @@ func (s *EtcdServer) AddMember(ctx context.Context, memb membership.Member) ([]* NodeID: uint64(memb.ID), Context: b, } + + if memb.IsLearner { + cc.Type = raftpb.ConfChangeAddLearnerNode + } + return s.configure(ctx, cc) } @@ -2054,7 +2060,7 @@ func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, confState *raftpb.Con lg := s.getLogger() *confState = *s.r.ApplyConfChange(cc) switch cc.Type { - case raftpb.ConfChangeAddNode: + case raftpb.ConfChangeAddNode, raftpb.ConfChangeAddLearnerNode: m := new(membership.Member) if err := json.Unmarshal(cc.Context, m); err != nil { if lg != nil { diff --git a/etcdserver/server_test.go b/etcdserver/server_test.go index a8dbbb333..3ba0237cb 100644 --- a/etcdserver/server_test.go +++ b/etcdserver/server_test.go @@ -634,7 +634,7 @@ func TestApplyConfigChangeUpdatesConsistIndex(t *testing.T) { if err != nil { t.Fatal(err) } - m := membership.NewMember("", urls, "", &now) + m := membership.NewMember("", urls, "", &now, false) m.ID = types.ID(2) b, err := json.Marshal(m) if err != nil { @@ -1564,23 +1564,23 @@ func TestGetOtherPeerURLs(t *testing.T) { }{ { []*membership.Member{ - membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil), + membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil, false), }, []string{}, }, { []*membership.Member{ - membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil), - membership.NewMember("2", types.MustNewURLs([]string{"http://10.0.0.2:2"}), "a", nil), - membership.NewMember("3", types.MustNewURLs([]string{"http://10.0.0.3:3"}), "a", nil), + membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil, false), + membership.NewMember("2", types.MustNewURLs([]string{"http://10.0.0.2:2"}), "a", nil, false), + membership.NewMember("3", types.MustNewURLs([]string{"http://10.0.0.3:3"}), "a", nil, false), }, []string{"http://10.0.0.2:2", "http://10.0.0.3:3"}, }, { []*membership.Member{ - membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil), - membership.NewMember("3", types.MustNewURLs([]string{"http://10.0.0.3:3"}), "a", nil), - membership.NewMember("2", types.MustNewURLs([]string{"http://10.0.0.2:2"}), "a", nil), + membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil, false), + membership.NewMember("3", types.MustNewURLs([]string{"http://10.0.0.3:3"}), "a", nil, false), + membership.NewMember("2", types.MustNewURLs([]string{"http://10.0.0.2:2"}), "a", nil, false), }, []string{"http://10.0.0.2:2", "http://10.0.0.3:3"}, },