From e2fdc2fdd1f8ea46e0c71650e918828e36d0be12 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 | 30 ++++++++++--------- 7 files changed, 26 insertions(+), 26 deletions(-) diff --git a/etcdctl/ctlv3/command/migrate_command.go b/etcdctl/ctlv3/command/migrate_command.go index a7a7b6dcabf0..f9cfe161d5ca 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 fef6b0f27048..7c071d4841c4 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 @@ -75,7 +74,7 @@ type ConfigChangeContext struct { // 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 { @@ -90,8 +89,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 @@ -99,13 +98,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 e4351d15352f..5e6152e24006 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 197d93a0845f..ef9af5d2b7ee 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 4bac644964e6..9053673de43e 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 d3b91f99ebc6..4e946ca336fd 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 60ae694677c1..2cb3f68fbb39 100644 --- a/server/etcdserver/server_test.go +++ b/server/etcdserver/server_test.go @@ -49,6 +49,7 @@ import ( "go.etcd.io/etcd/server/v3/mvcc" "go.etcd.io/etcd/server/v3/mvcc/backend" "go.uber.org/zap" + "go.uber.org/zap/zaptest" ) // TestDoLocalAction tests requests which do not need to go through raft to be applied, @@ -174,7 +175,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}) @@ -479,7 +480,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(), @@ -502,7 +503,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)}) @@ -590,7 +591,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)}) @@ -634,7 +635,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)}) r := newRaftNode(raftNodeConfig{ @@ -681,7 +682,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)}) @@ -1037,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") @@ -1191,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") @@ -1291,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{ @@ -1335,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}) @@ -1379,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}) @@ -1567,6 +1568,7 @@ func TestStopNotify(t *testing.T) { } func TestGetOtherPeerURLs(t *testing.T) { + lg := zaptest.NewLogger(t) tests := []struct { membs []*membership.Member wurls []string @@ -1595,7 +1597,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) { @@ -1746,8 +1748,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) }