From a70386a1a41e41316ce41ebf652de44805913517 Mon Sep 17 00:00:00 2001 From: Piotr Tabor Date: Fri, 2 Apr 2021 17:22:19 +0200 Subject: [PATCH] Simplify membership interface: Does not pass the 'unused' token. --- etcdctl/ctlv3/command/migrate_command.go | 2 +- server/etcdserver/api/membership/cluster.go | 10 +++---- .../etcdserver/api/membership/cluster_test.go | 2 +- server/etcdserver/cluster_util.go | 2 +- server/etcdserver/raft.go | 4 +-- server/etcdserver/raft_test.go | 2 +- server/etcdserver/server_test.go | 29 ++++++++++--------- 7 files changed, 25 insertions(+), 26 deletions(-) diff --git a/etcdctl/ctlv3/command/migrate_command.go b/etcdctl/ctlv3/command/migrate_command.go index 74245483c..33e87c09e 100644 --- a/etcdctl/ctlv3/command/migrate_command.go +++ b/etcdctl/ctlv3/command/migrate_command.go @@ -128,7 +128,7 @@ func prepareBackend() backend.Backend { func rebuildStoreV2() (v2store.Store, uint64) { var index uint64 - cl := membership.NewCluster(zap.NewExample(), "") + cl := membership.NewCluster(zap.NewExample()) waldir := migrateWALdir if len(waldir) == 0 { diff --git a/server/etcdserver/api/membership/cluster.go b/server/etcdserver/api/membership/cluster.go index bd5389212..7697b3d42 100644 --- a/server/etcdserver/api/membership/cluster.go +++ b/server/etcdserver/api/membership/cluster.go @@ -48,7 +48,6 @@ type RaftCluster struct { localID types.ID cid types.ID - token string v2store v2store.Store be backend.Backend @@ -82,7 +81,7 @@ const ( // 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) + c := NewCluster(lg) for name, urls := range urlsmap { m := NewMember(name, urls, token, nil) if _, ok := c.members[m.ID]; ok { @@ -97,8 +96,8 @@ func NewClusterFromURLsMap(lg *zap.Logger, token string, urlsmap types.URLsMap) return c, nil } -func NewClusterFromMembers(lg *zap.Logger, token string, id types.ID, membs []*Member) *RaftCluster { - c := NewCluster(lg, token) +func NewClusterFromMembers(lg *zap.Logger, id types.ID, membs []*Member) *RaftCluster { + c := NewCluster(lg) c.cid = id for _, m := range membs { c.members[m.ID] = m @@ -106,13 +105,12 @@ func NewClusterFromMembers(lg *zap.Logger, token string, id types.ID, membs []*M return c } -func NewCluster(lg *zap.Logger, token string) *RaftCluster { +func NewCluster(lg *zap.Logger) *RaftCluster { if lg == nil { lg = zap.NewNop() } return &RaftCluster{ lg: lg, - token: token, members: make(map[types.ID]*Member), removed: make(map[types.ID]bool), downgradeInfo: &DowngradeInfo{Enabled: false}, diff --git a/server/etcdserver/api/membership/cluster_test.go b/server/etcdserver/api/membership/cluster_test.go index c3975df25..23d81fec1 100644 --- a/server/etcdserver/api/membership/cluster_test.go +++ b/server/etcdserver/api/membership/cluster_test.go @@ -279,7 +279,7 @@ func TestClusterValidateAndAssignIDs(t *testing.T) { } func TestClusterValidateConfigurationChange(t *testing.T) { - cl := NewCluster(zap.NewExample(), "") + cl := NewCluster(zaptest.NewLogger(t)) cl.SetStore(v2store.New()) for i := 1; i <= 4; i++ { attr := RaftAttributes{PeerURLs: []string{fmt.Sprintf("http://127.0.0.1:%d", i)}} diff --git a/server/etcdserver/cluster_util.go b/server/etcdserver/cluster_util.go index 8dd8f205d..595586e20 100644 --- a/server/etcdserver/cluster_util.go +++ b/server/etcdserver/cluster_util.go @@ -113,7 +113,7 @@ func getClusterFromRemotePeers(lg *zap.Logger, urls []string, timeout time.Durat // if membership members are not present then the raft cluster formed will be // an invalid empty cluster hence return failed to get raft cluster member(s) from the given urls error if len(membs) > 0 { - return membership.NewClusterFromMembers(lg, "", id, membs), nil + return membership.NewClusterFromMembers(lg, id, membs), nil } return nil, fmt.Errorf("failed to get raft cluster member(s) from the given URLs") } diff --git a/server/etcdserver/raft.go b/server/etcdserver/raft.go index 664894cbc..bb248eb36 100644 --- a/server/etcdserver/raft.go +++ b/server/etcdserver/raft.go @@ -487,7 +487,7 @@ func restartNode(cfg config.ServerConfig, snapshot *raftpb.Snapshot) (types.ID, zap.String("local-member-id", id.String()), zap.Uint64("commit-index", st.Commit), ) - cl := membership.NewCluster(cfg.Logger, "") + cl := membership.NewCluster(cfg.Logger) cl.SetID(id, cid) s := raft.NewMemoryStorage() if snapshot != nil { @@ -565,7 +565,7 @@ func restartAsStandaloneNode(cfg config.ServerConfig, snapshot *raftpb.Snapshot) zap.Uint64("commit-index", st.Commit), ) - cl := membership.NewCluster(cfg.Logger, "") + cl := membership.NewCluster(cfg.Logger) cl.SetID(id, cid) s := raft.NewMemoryStorage() if snapshot != nil { diff --git a/server/etcdserver/raft_test.go b/server/etcdserver/raft_test.go index 871b15978..3eb5345dc 100644 --- a/server/etcdserver/raft_test.go +++ b/server/etcdserver/raft_test.go @@ -230,7 +230,7 @@ func TestConfigChangeBlocksApply(t *testing.T) { func TestProcessDuplicatedAppRespMessage(t *testing.T) { n := newNopReadyNode() - cl := membership.NewCluster(zap.NewExample(), "abc") + cl := membership.NewCluster(zap.NewExample()) rs := raft.NewMemoryStorage() p := mockstorage.NewStorageRecorder("") diff --git a/server/etcdserver/server_test.go b/server/etcdserver/server_test.go index b50b03625..50e8533a9 100644 --- a/server/etcdserver/server_test.go +++ b/server/etcdserver/server_test.go @@ -178,7 +178,7 @@ func TestApplyRepeat(t *testing.T) { n.readyc <- raft.Ready{ SoftState: &raft.SoftState{RaftState: raft.StateLeader}, } - cl := newTestCluster(nil) + cl := newTestCluster(t, nil) st := v2store.New() cl.SetStore(v2store.New()) cl.AddMember(&membership.Member{ID: 1234}, true) @@ -483,7 +483,7 @@ func TestApplyRequest(t *testing.T) { } func TestApplyRequestOnAdminMemberAttributes(t *testing.T) { - cl := newTestCluster([]*membership.Member{{ID: 1}}) + cl := newTestCluster(t, []*membership.Member{{ID: 1}}) srv := &EtcdServer{ lgMu: new(sync.RWMutex), lg: zap.NewExample(), @@ -506,7 +506,7 @@ func TestApplyRequestOnAdminMemberAttributes(t *testing.T) { } func TestApplyConfChangeError(t *testing.T) { - cl := membership.NewCluster(zap.NewExample(), "") + cl := membership.NewCluster(zaptest.NewLogger(t)) cl.SetStore(v2store.New()) for i := 1; i <= 4; i++ { cl.AddMember(&membership.Member{ID: types.ID(i)}, true) @@ -594,7 +594,7 @@ func TestApplyConfChangeError(t *testing.T) { } func TestApplyConfChangeShouldStop(t *testing.T) { - cl := membership.NewCluster(zap.NewExample(), "") + cl := membership.NewCluster(zaptest.NewLogger(t)) cl.SetStore(v2store.New()) for i := 1; i <= 3; i++ { cl.AddMember(&membership.Member{ID: types.ID(i)}, true) @@ -638,7 +638,7 @@ func TestApplyConfChangeShouldStop(t *testing.T) { // TestApplyConfigChangeUpdatesConsistIndex ensures a config change also updates the consistIndex // where consistIndex equals to applied index. func TestApplyConfigChangeUpdatesConsistIndex(t *testing.T) { - cl := membership.NewCluster(zap.NewExample(), "") + cl := membership.NewCluster(zaptest.NewLogger(t)) cl.SetStore(v2store.New()) cl.AddMember(&membership.Member{ID: types.ID(1)}, true) r := newRaftNode(raftNodeConfig{ @@ -685,7 +685,7 @@ func TestApplyConfigChangeUpdatesConsistIndex(t *testing.T) { // TestApplyMultiConfChangeShouldStop ensures that apply will return shouldStop // if the local member is removed along with other conf updates. func TestApplyMultiConfChangeShouldStop(t *testing.T) { - cl := membership.NewCluster(zap.NewExample(), "") + cl := membership.NewCluster(zaptest.NewLogger(t)) cl.SetStore(v2store.New()) for i := 1; i <= 5; i++ { cl.AddMember(&membership.Member{ID: types.ID(i)}, true) @@ -1038,7 +1038,7 @@ func TestSnapshot(t *testing.T) { func TestSnapshotOrdering(t *testing.T) { n := newNopReadyNode() st := v2store.New() - cl := membership.NewCluster(zap.NewExample(), "abc") + cl := membership.NewCluster(zaptest.NewLogger(t)) cl.SetStore(st) testdir, err := ioutil.TempDir(os.TempDir(), "testsnapdir") @@ -1192,7 +1192,7 @@ func TestTriggerSnap(t *testing.T) { func TestConcurrentApplyAndSnapshotV3(t *testing.T) { n := newNopReadyNode() st := v2store.New() - cl := membership.NewCluster(zap.NewExample(), "abc") + cl := membership.NewCluster(zaptest.NewLogger(t)) cl.SetStore(st) testdir, err := ioutil.TempDir(os.TempDir(), "testsnapdir") @@ -1292,7 +1292,7 @@ func TestAddMember(t *testing.T) { n.readyc <- raft.Ready{ SoftState: &raft.SoftState{RaftState: raft.StateLeader}, } - cl := newTestCluster(nil) + cl := newTestCluster(t, nil) st := v2store.New() cl.SetStore(st) r := newRaftNode(raftNodeConfig{ @@ -1336,7 +1336,7 @@ func TestRemoveMember(t *testing.T) { n.readyc <- raft.Ready{ SoftState: &raft.SoftState{RaftState: raft.StateLeader}, } - cl := newTestCluster(nil) + cl := newTestCluster(t, nil) st := v2store.New() cl.SetStore(v2store.New()) cl.AddMember(&membership.Member{ID: 1234}, true) @@ -1380,7 +1380,7 @@ func TestUpdateMember(t *testing.T) { n.readyc <- raft.Ready{ SoftState: &raft.SoftState{RaftState: raft.StateLeader}, } - cl := newTestCluster(nil) + cl := newTestCluster(t, nil) st := v2store.New() cl.SetStore(st) cl.AddMember(&membership.Member{ID: 1234}, true) @@ -1689,6 +1689,7 @@ func TestStopNotify(t *testing.T) { } func TestGetOtherPeerURLs(t *testing.T) { + lg := zaptest.NewLogger(t) tests := []struct { membs []*membership.Member wurls []string @@ -1717,7 +1718,7 @@ func TestGetOtherPeerURLs(t *testing.T) { }, } for i, tt := range tests { - cl := membership.NewClusterFromMembers(zap.NewExample(), "", types.ID(0), tt.membs) + cl := membership.NewClusterFromMembers(lg, types.ID(0), tt.membs) self := "1" urls := getRemotePeerURLs(cl, self) if !reflect.DeepEqual(urls, tt.wurls) { @@ -1868,8 +1869,8 @@ func (n *nodeCommitter) Propose(ctx context.Context, data []byte) error { return nil } -func newTestCluster(membs []*membership.Member) *membership.RaftCluster { - c := membership.NewCluster(zap.NewExample(), "") +func newTestCluster(t testing.TB, membs []*membership.Member) *membership.RaftCluster { + c := membership.NewCluster(zaptest.NewLogger(t)) for _, m := range membs { c.AddMember(m, true) }