From 97564fbb962f1986eb45c221465cc14b5b90bf67 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 65ea46edc54..c3a76b0371a 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 7aed5aec244..9c6b46b9cd5 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 6a3e79305f2..74c5fec7723 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 bbbb88986c3..8e2d487307f 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 1d1e592b25d..61de6abac1e 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 9d00a986469..74c4122e901 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 5a97b8341ef..d965f80aefd 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 46a5363f815..f92461fe7db 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"}, },