Skip to content

Commit

Permalink
Merge pull request etcd-io#3 from jingyih/add_learner_etcdserver
Browse files Browse the repository at this point in the history
etcdserver: support MemberAdd for learner
  • Loading branch information
jingyih authored Mar 20, 2019
2 parents b99b759 + 97564fb commit f74f2f2
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 25 deletions.
6 changes: 4 additions & 2 deletions etcdserver/api/membership/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down
23 changes: 23 additions & 0 deletions etcdserver/api/membership/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
14 changes: 11 additions & 3 deletions etcdserver/api/membership/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -88,6 +93,9 @@ func (m *Member) Clone() *Member {
}
mm := &Member{
ID: m.ID,
RaftAttributes: RaftAttributes{
IsLearner: m.IsLearner,
},
Attributes: Attributes{
Name: m.Name,
},
Expand Down
22 changes: 15 additions & 7 deletions etcdserver/api/membership/member_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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},
}
}
2 changes: 1 addition & 1 deletion etcdserver/api/v2http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
11 changes: 8 additions & 3 deletions etcdserver/api/v3rpc/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions etcdserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"},
},
Expand Down

0 comments on commit f74f2f2

Please sign in to comment.