etcdserver: Add MemberAddAsLearner

Made changes to api/membership:

- Added MemberAddAsLearner
- Reverted changes to MemberAdd - removed input parameter isLearner

etcdserver/api/membership/cluster.go

@@ -64,7 +64,7 @@ type RaftCluster struct {
 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, false)
+		m := NewMember(name, urls, token, nil)
 		if _, ok := c.members[m.ID]; ok {
 			return nil, fmt.Errorf("member exists with identical ID %v", m)
 		}

etcdserver/api/membership/member.go

@@ -53,7 +53,17 @@ 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, isLearner bool) *Member {
+func NewMember(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member {
+	return newMember(name, peerURLs, clusterName, now, false)
+}
+
+// NewMemberAsLearner creates a learner Member without an ID and generates one based on the
+// cluster name, peer URLs, and time. This is used for adding new learner member.
+func NewMemberAsLearner(name string, peerURLs types.URLs, clusterName string, now *time.Time) *Member {
+	return newMember(name, peerURLs, clusterName, now, true)
+}
+
+func newMember(name string, peerURLs types.URLs, clusterName string, now *time.Time, isLearner bool) *Member {
 	m := &Member{
 		RaftAttributes: RaftAttributes{
 			PeerURLs:  peerURLs.StringSlice(),

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, false), 14544069596553697298},
+		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
 		// Same ID, different name (names shouldn't matter)
-		{NewMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil, false), 14544069596553697298},
+		{NewMember("memfoo", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", nil), 14544069596553697298},
 		// Same ID, different Time
-		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z"), false), 2448790162483548276},
+		{NewMember("mem1", []url.URL{{Scheme: "http", Host: "10.0.0.8:2379"}}, "", timeParse("1984-12-23T15:04:05Z")), 2448790162483548276},
 		// Different cluster name
-		{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},
+		{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},
 		// 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, 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},
+		{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},
 	}
 	for i, tt := range tests {
 		if tt.mem.ID != tt.id {

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, false) // does not support adding learner via v2http
+		m := membership.NewMember("", req.PeerURLs, "", &now)
 		_, err := h.server.AddMember(ctx, *m)
 		switch {
 		case err == membership.ErrIDExists || err == membership.ErrPeerURLexists:

etcdserver/api/v3rpc/member.go

@@ -46,7 +46,12 @@ func (cs *ClusterServer) MemberAdd(ctx context.Context, r *pb.MemberAddRequest)
 	}
 
 	now := time.Now()
-	m := membership.NewMember("", urls, "", &now, r.IsLearner)
+	var m *membership.Member
+	if r.IsLearner {
+		m = membership.NewMemberAsLearner("", urls, "", &now)
+	} else {
+		m = membership.NewMember("", urls, "", &now)
+	}
 	membs, merr := cs.server.AddMember(ctx, *m)
 	if merr != nil {
 		return nil, togRPCError(merr)

etcdserver/server_test.go

@@ -634,7 +634,7 @@ func TestApplyConfigChangeUpdatesConsistIndex(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	m := membership.NewMember("", urls, "", &now, false)
+	m := membership.NewMember("", urls, "", &now)
 	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, false),
+				membership.NewMember("1", types.MustNewURLs([]string{"http://10.0.0.1:1"}), "a", nil),
 			},
 			[]string{},
 		},
 		{
 			[]*membership.Member{
-				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),
+				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),
 			},
 			[]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, 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),
+				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),
 			},
 			[]string{"http://10.0.0.2:2", "http://10.0.0.3:3"},
 		},

proxy/grpcproxy/cluster.go

@@ -16,6 +16,7 @@ package grpcproxy
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"sync"
@@ -25,7 +26,6 @@ import (
 	"go.etcd.io/etcd/v3/etcdserver/api/v3rpc/rpctypes"
 	pb "go.etcd.io/etcd/v3/etcdserver/etcdserverpb"
 
-	"errors"
 	"golang.org/x/time/rate"
 	gnaming "google.golang.org/grpc/naming"
 )