Skip to content

Commit

Permalink
validate conf change against v3store instead of v2store
Browse files Browse the repository at this point in the history
Signed-off-by: Benjamin Wang <[email protected]>
  • Loading branch information
ahrtr committed Nov 25, 2023
1 parent d940fec commit 12614cc
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
10 changes: 7 additions & 3 deletions server/etcdserver/api/membership/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion server/etcdserver/api/membership/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion server/etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 12614cc

Please sign in to comment.