From 12614cc3408b7f62079fec6c57ba79a4db1bed36 Mon Sep 17 00:00:00 2001 From: Benjamin Wang Date: Sat, 25 Nov 2023 15:14:16 +0000 Subject: [PATCH] validate conf change against v3store instead of v2store Signed-off-by: Benjamin Wang --- server/etcdserver/api/membership/cluster.go | 10 +++++++--- server/etcdserver/api/membership/cluster_test.go | 4 +++- server/etcdserver/server.go | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/server/etcdserver/api/membership/cluster.go b/server/etcdserver/api/membership/cluster.go index d28efb097811..87dc170dc703 100644 --- a/server/etcdserver/api/membership/cluster.go +++ b/server/etcdserver/api/membership/cluster.go @@ -300,9 +300,13 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) { // ValidateConfigurationChange takes a proposed ConfChange and // ensures that it is still valid. -func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { - // TODO: this must be switched to backend as well. - membersMap, removedMap := membersFromStore(c.lg, c.v2store) +func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange, shouldApplyV3 ShouldApplyV3) error { + // It makes no sense to validate a confChange if we don't apply it. + if !shouldApplyV3 { + return nil + } + membersMap, removedMap := c.be.MustReadMembersFromBackend() + id := types.ID(cc.NodeID) if removedMap[id] { return ErrIDRemoved diff --git a/server/etcdserver/api/membership/cluster_test.go b/server/etcdserver/api/membership/cluster_test.go index b06d3302475a..62f188e3e74d 100644 --- a/server/etcdserver/api/membership/cluster_test.go +++ b/server/etcdserver/api/membership/cluster_test.go @@ -278,6 +278,8 @@ func TestClusterValidateAndAssignIDs(t *testing.T) { func TestClusterValidateConfigurationChange(t *testing.T) { cl := NewCluster(zaptest.NewLogger(t), WithMaxLearners(1)) + be := newMembershipBackend() + cl.SetBackend(be) cl.SetStore(v2store.New()) for i := 1; i <= 4; i++ { var isLearner bool @@ -455,7 +457,7 @@ func TestClusterValidateConfigurationChange(t *testing.T) { }, } for i, tt := range tests { - err := cl.ValidateConfigurationChange(tt.cc) + err := cl.ValidateConfigurationChange(tt.cc, true) if err != tt.werr { t.Errorf("#%d: validateConfigurationChange error = %v, want %v", i, err, tt.werr) } diff --git a/server/etcdserver/server.go b/server/etcdserver/server.go index 4ad4539c6e2f..a4a8161c2088 100644 --- a/server/etcdserver/server.go +++ b/server/etcdserver/server.go @@ -1935,7 +1935,7 @@ func removeNeedlessRangeReqs(txn *pb.TxnRequest) { // applyConfChange applies a ConfChange to the server. It is only // invoked with a ConfChange that has already passed through Raft func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, confState *raftpb.ConfState, shouldApplyV3 membership.ShouldApplyV3) (bool, error) { - if err := s.cluster.ValidateConfigurationChange(cc); err != nil { + if err := s.cluster.ValidateConfigurationChange(cc, shouldApplyV3); err != nil { cc.NodeID = raft.None s.r.ApplyConfChange(cc)