Merge pull request #19703 from k8s-infra-cherrypick-robot/cherry-pick-19694-to-release-3.6

[release-3.6] Switch to validating v3 when v2 and v3 are synchronized

Author: ahrtr

server/etcdserver/api/membership/cluster.go

@@ -313,9 +313,16 @@ 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 {
+	var membersMap map[types.ID]*Member
+	var removedMap map[types.ID]bool
+
+	if shouldApplyV3 {
+		membersMap, removedMap = c.be.MustReadMembersFromBackend()
+	} else {
+		membersMap, removedMap = membersFromStore(c.lg, c.v2store)
+	}
+
 	id := types.ID(cc.NodeID)
 	if removedMap[id] {
 		return ErrIDRemoved

server/etcdserver/api/membership/cluster_test.go

@@ -278,7 +278,15 @@ func TestClusterValidateAndAssignIDs(t *testing.T) {
 	}
 }
 
+func TestClusterValidateConfigurationChangeV3(t *testing.T) {
+	testClusterValidateConfigurationChange(t, true)
+}
+
 func TestClusterValidateConfigurationChangeV2(t *testing.T) {
+	testClusterValidateConfigurationChange(t, false)
+}
+
+func testClusterValidateConfigurationChange(t *testing.T, shouldApplyV3 ShouldApplyV3) {
 	cl := NewCluster(zaptest.NewLogger(t), WithMaxLearners(1))
 	be := newMembershipBackend()
 	cl.SetBackend(be)
@@ -459,7 +467,7 @@ func TestClusterValidateConfigurationChangeV2(t *testing.T) {
 		},
 	}
 	for i, tt := range tests {
-		err := cl.ValidateConfigurationChange(tt.cc)
+		err := cl.ValidateConfigurationChange(tt.cc, shouldApplyV3)
 		if !errors.Is(err, tt.werr) {
 			t.Errorf("#%d: validateConfigurationChange error = %v, want %v", i, err, tt.werr)
 		}

server/etcdserver/server.go

@@ -2069,7 +2069,7 @@ func removeNeedlessRangeReqs(txn *pb.TxnRequest) {
 // 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) {
 	lg := s.Logger()
-	if err := s.cluster.ValidateConfigurationChange(cc); err != nil {
+	if err := s.cluster.ValidateConfigurationChange(cc, shouldApplyV3); err != nil {
 		lg.Error("Validation on configuration change failed", zap.Bool("shouldApplyV3", bool(shouldApplyV3)), zap.Error(err))
 		cc.NodeID = raft.None
 		s.r.ApplyConfChange(cc)

tests/e2e/metrics_test.go

@@ -167,7 +167,6 @@ func TestNoMetricsMissing(t *testing.T) {
 			"etcd_debugging_snap_save_marshalling_duration_seconds",
 			"etcd_debugging_snap_save_total_duration_seconds",
 			"etcd_debugging_store_expires_total",
-			"etcd_debugging_store_reads_total",
 			"etcd_debugging_store_watch_requests_total",
 			"etcd_debugging_store_watchers",
 			"etcd_debugging_store_writes_total",