From 2ddb9e08836ff6cdb63a566c50bc1f0b1f98aec0 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Mon, 25 Jul 2022 13:27:53 +0200 Subject: [PATCH] tests: Fix member id in CORRUPT alarm Signed-off-by: Marek Siarkowicz --- server/etcdserver/corrupt.go | 27 +++++++++++++-------------- server/etcdserver/corrupt_test.go | 12 ++++++------ tests/e2e/corrupt_test.go | 12 ++++++++++-- tests/integration/corrupt_test.go | 3 +-- 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/server/etcdserver/corrupt.go b/server/etcdserver/corrupt.go index d7ae377b0..f721b3034 100644 --- a/server/etcdserver/corrupt.go +++ b/server/etcdserver/corrupt.go @@ -56,7 +56,7 @@ type Hasher interface { MemberId() types.ID PeerHashByRev(int64) []*peerHashKVResp LinearizableReadNotify(context.Context) error - TriggerCorruptAlarm(uint64) + TriggerCorruptAlarm(types.ID) } func newCorruptionChecker(lg *zap.Logger, s *EtcdServer, storage mvcc.HashStorage) *corruptionChecker { @@ -83,7 +83,7 @@ func (h hasherAdapter) PeerHashByRev(rev int64) []*peerHashKVResp { return h.EtcdServer.getPeerHashKVs(rev) } -func (h hasherAdapter) TriggerCorruptAlarm(memberID uint64) { +func (h hasherAdapter) TriggerCorruptAlarm(memberID types.ID) { h.EtcdServer.triggerCorruptAlarm(memberID) } @@ -189,7 +189,7 @@ func (cm *corruptionChecker) PeriodicCheck() error { } alarmed := false - mismatch := func(id uint64) { + mismatch := func(id types.ID) { if alarmed { return } @@ -207,7 +207,7 @@ func (cm *corruptionChecker) PeriodicCheck() error { zap.Int64("compact-revision-2", h2.CompactRevision), zap.Uint32("hash-2", h2.Hash), ) - mismatch(uint64(cm.hasher.MemberId())) + mismatch(cm.hasher.MemberId()) } checkedCount := 0 @@ -216,7 +216,6 @@ func (cm *corruptionChecker) PeriodicCheck() error { continue } checkedCount++ - id := p.resp.Header.MemberId // leader expects follower's latest revision less than or equal to leader's if p.resp.Header.Revision > rev2 { @@ -224,9 +223,9 @@ func (cm *corruptionChecker) PeriodicCheck() error { "revision from follower must be less than or equal to leader's", zap.Int64("leader-revision", rev2), zap.Int64("follower-revision", p.resp.Header.Revision), - zap.String("follower-peer-id", types.ID(id).String()), + zap.String("follower-peer-id", p.id.String()), ) - mismatch(id) + mismatch(p.id) } // leader expects follower's latest compact revision less than or equal to leader's @@ -235,9 +234,9 @@ func (cm *corruptionChecker) PeriodicCheck() error { "compact revision from follower must be less than or equal to leader's", zap.Int64("leader-compact-revision", h2.CompactRevision), zap.Int64("follower-compact-revision", p.resp.CompactRevision), - zap.String("follower-peer-id", types.ID(id).String()), + zap.String("follower-peer-id", p.id.String()), ) - mismatch(id) + mismatch(p.id) } // follower's compact revision is leader's old one, then hashes must match @@ -248,9 +247,9 @@ func (cm *corruptionChecker) PeriodicCheck() error { zap.Uint32("leader-hash", h.Hash), zap.Int64("follower-compact-revision", p.resp.CompactRevision), zap.Uint32("follower-hash", p.resp.Hash), - zap.String("follower-peer-id", types.ID(id).String()), + zap.String("follower-peer-id", p.id.String()), ) - mismatch(id) + mismatch(p.id) } } cm.lg.Info("finished peer corruption check", zap.Int("number-of-peers-checked", checkedCount)) @@ -277,7 +276,7 @@ func (cm *corruptionChecker) CompactHashCheck() { // follower's compact revision is leader's old one, then hashes must match if p.resp.Hash != hash.Hash { - cm.hasher.TriggerCorruptAlarm(uint64(p.id)) + cm.hasher.TriggerCorruptAlarm(p.id) cm.lg.Error("failed compaction hash check", zap.Int64("revision", hash.Revision), zap.Int64("leader-compact-revision", hash.CompactRevision), @@ -335,9 +334,9 @@ func (cm *corruptionChecker) uncheckedRevisions() []mvcc.KeyValueHash { return hashes } -func (s *EtcdServer) triggerCorruptAlarm(id uint64) { +func (s *EtcdServer) triggerCorruptAlarm(id types.ID) { a := &pb.AlarmRequest{ - MemberID: id, + MemberID: uint64(id), Action: pb.AlarmRequest_ACTIVATE, Alarm: pb.AlarmType_CORRUPT, } diff --git a/server/etcdserver/corrupt_test.go b/server/etcdserver/corrupt_test.go index 9e4030714..9e8156f2a 100644 --- a/server/etcdserver/corrupt_test.go +++ b/server/etcdserver/corrupt_test.go @@ -161,7 +161,7 @@ func TestPeriodicCheck(t *testing.T) { { name: "Peer with newer revision", hasher: fakeHasher{ - peerHashes: []*peerHashKVResp{{resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 1, MemberId: 42}}}}, + peerHashes: []*peerHashKVResp{{peerInfo: peerInfo{id: 42}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 1}}}}, }, expectActions: []string{"HashByRev(0)", "PeerHashByRev(0)", "ReqTimeout()", "LinearizableReadNotify()", "HashByRev(0)", "TriggerCorruptAlarm(42)"}, expectCorrupt: true, @@ -169,7 +169,7 @@ func TestPeriodicCheck(t *testing.T) { { name: "Peer with newer compact revision", hasher: fakeHasher{ - peerHashes: []*peerHashKVResp{{resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10, MemberId: 88}, CompactRevision: 2}}}, + peerHashes: []*peerHashKVResp{{peerInfo: peerInfo{id: 88}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10}, CompactRevision: 2}}}, }, expectActions: []string{"HashByRev(0)", "PeerHashByRev(0)", "ReqTimeout()", "LinearizableReadNotify()", "HashByRev(0)", "TriggerCorruptAlarm(88)"}, expectCorrupt: true, @@ -186,7 +186,7 @@ func TestPeriodicCheck(t *testing.T) { name: "Peer with different hash and same compact revision as first local", hasher: fakeHasher{ hashByRevResponses: []hashByRev{{hash: mvcc.KeyValueHash{Hash: 1, CompactRevision: 1}, revision: 1}, {hash: mvcc.KeyValueHash{Hash: 2, CompactRevision: 2}, revision: 2}}, - peerHashes: []*peerHashKVResp{{resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 1, MemberId: 666}, CompactRevision: 1, Hash: 2}}}, + peerHashes: []*peerHashKVResp{{peerInfo: peerInfo{id: 666}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 1}, CompactRevision: 1, Hash: 2}}}, }, expectActions: []string{"HashByRev(0)", "PeerHashByRev(1)", "ReqTimeout()", "LinearizableReadNotify()", "HashByRev(0)", "TriggerCorruptAlarm(666)"}, expectCorrupt: true, @@ -195,8 +195,8 @@ func TestPeriodicCheck(t *testing.T) { name: "Multiple corrupted peers trigger one alarm", hasher: fakeHasher{ peerHashes: []*peerHashKVResp{ - {resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10, MemberId: 88}, CompactRevision: 2}}, - {resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10, MemberId: 89}, CompactRevision: 2}}, + {peerInfo: peerInfo{id: 88}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10}, CompactRevision: 2}}, + {peerInfo: peerInfo{id: 89}, resp: &pb.HashKVResponse{Header: &pb.ResponseHeader{Revision: 10}, CompactRevision: 2}}, }, }, expectActions: []string{"HashByRev(0)", "PeerHashByRev(0)", "ReqTimeout()", "LinearizableReadNotify()", "HashByRev(0)", "TriggerCorruptAlarm(88)"}, @@ -371,7 +371,7 @@ func (f *fakeHasher) LinearizableReadNotify(ctx context.Context) error { return f.linearizableReadNotify } -func (f *fakeHasher) TriggerCorruptAlarm(memberId uint64) { +func (f *fakeHasher) TriggerCorruptAlarm(memberId types.ID) { f.actions = append(f.actions, fmt.Sprintf("TriggerCorruptAlarm(%d)", memberId)) f.alarmTriggered = true } diff --git a/tests/e2e/corrupt_test.go b/tests/e2e/corrupt_test.go index 7044d4a06..f21eed590 100644 --- a/tests/e2e/corrupt_test.go +++ b/tests/e2e/corrupt_test.go @@ -119,6 +119,15 @@ func TestPeriodicCheckDetectsCorruption(t *testing.T) { assert.NoError(t, err, "error on put") } + members, err := cc.MemberList() + assert.NoError(t, err, "error on member list") + var memberID uint64 + for _, m := range members.Members { + if m.Name == epc.procs[0].Config().name { + memberID = m.ID + } + } + assert.NotZero(t, memberID, "member not found") epc.procs[0].Stop() err = testutil.CorruptBBolt(datadir.ToBackendFileName(epc.procs[0].Config().dataDirPath)) assert.NoError(t, err) @@ -128,8 +137,7 @@ func TestPeriodicCheckDetectsCorruption(t *testing.T) { time.Sleep(checkTime * 11 / 10) alarmResponse, err := cc.AlarmList() assert.NoError(t, err, "error on alarm list") - // TODO: Investigate why MemberID is 0? - assert.Equal(t, []*etcdserverpb.AlarmMember{{Alarm: etcdserverpb.AlarmType_CORRUPT, MemberID: 0}}, alarmResponse.Alarms) + assert.Equal(t, []*etcdserverpb.AlarmMember{{Alarm: etcdserverpb.AlarmType_CORRUPT, MemberID: memberID}}, alarmResponse.Alarms) } func TestCompactHashCheckDetectCorruption(t *testing.T) { diff --git a/tests/integration/corrupt_test.go b/tests/integration/corrupt_test.go index 80202f974..b04a77e6f 100644 --- a/tests/integration/corrupt_test.go +++ b/tests/integration/corrupt_test.go @@ -96,8 +96,7 @@ func TestPeriodicCheckDetectsCorruption(t *testing.T) { time.Sleep(50 * time.Millisecond) alarmResponse, err := cc.AlarmList(ctx) assert.NoError(t, err, "error on alarm list") - // TODO: Investigate why MemberID is 0? - assert.Equal(t, []*etcdserverpb.AlarmMember{{Alarm: etcdserverpb.AlarmType_CORRUPT, MemberID: 0}}, alarmResponse.Alarms) + assert.Equal(t, []*etcdserverpb.AlarmMember{{Alarm: etcdserverpb.AlarmType_CORRUPT, MemberID: uint64(clus.Members[0].ID())}}, alarmResponse.Alarms) } func TestCompactHashCheck(t *testing.T) {