Merge pull request #19276 from ahrtr/downgrad_test_20250126

[Solution 2] Add `DowngradeVersionTestRequest` for Downgrade or migration test only

api/etcdserverpb/raft_internal.proto

@@ -71,6 +71,8 @@ message InternalRaftRequest {
   membershippb.ClusterVersionSetRequest cluster_version_set = 1300 [(versionpb.etcd_version_field) = "3.5"];
   membershippb.ClusterMemberAttrSetRequest cluster_member_attr_set = 1301 [(versionpb.etcd_version_field) = "3.5"];
   membershippb.DowngradeInfoSetRequest  downgrade_info_set = 1302 [(versionpb.etcd_version_field) = "3.5"];
+
+  DowngradeVersionTestRequest downgrade_version_test = 9900 [(versionpb.etcd_version_field) = "3.6"];
 }
 
 message EmptyResponse {

api/etcdserverpb/rpc.proto

@@ -1158,6 +1158,16 @@ message DowngradeResponse {
   string version = 2;
 }
 
+// DowngradeVersionTestRequest is used for test only. The version in
+// this request will be read as the WAL record version.If the downgrade
+// target version is less than this version, then the downgrade(online)
+// or migration(offline) isn't safe, so shouldn't be allowed.
+message DowngradeVersionTestRequest {
+  option (versionpb.etcd_version_msg) = "3.6";
+
+  string ver = 1;
+}
+
 message StatusRequest {
   option (versionpb.etcd_version_msg) = "3.0";
 }

scripts/etcd_version_annotations.txt

@@ -160,6 +160,8 @@ etcdserverpb.DowngradeRequest.version: ""
 etcdserverpb.DowngradeResponse: "3.5"
 etcdserverpb.DowngradeResponse.header: ""
 etcdserverpb.DowngradeResponse.version: ""
+etcdserverpb.DowngradeVersionTestRequest: "3.6"
+etcdserverpb.DowngradeVersionTestRequest.ver: ""
 etcdserverpb.EmptyResponse: ""
 etcdserverpb.HashKVRequest: "3.3"
 etcdserverpb.HashKVRequest.revision: ""
@@ -201,6 +203,7 @@ etcdserverpb.InternalRaftRequest.cluster_version_set: "3.5"
 etcdserverpb.InternalRaftRequest.compaction: ""
 etcdserverpb.InternalRaftRequest.delete_range: ""
 etcdserverpb.InternalRaftRequest.downgrade_info_set: "3.5"
+etcdserverpb.InternalRaftRequest.downgrade_version_test: "3.6"
 etcdserverpb.InternalRaftRequest.header: ""
 etcdserverpb.InternalRaftRequest.lease_checkpoint: "3.4"
 etcdserverpb.InternalRaftRequest.lease_grant: ""

server/etcdserver/server.go

@@ -2006,7 +2006,7 @@ func (s *EtcdServer) applyEntryNormal(e *raftpb.Entry, shouldApplyV3 membership.
 }
 
 func (s *EtcdServer) applyInternalRaftRequest(r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3) *apply.Result {
-	if r.ClusterVersionSet == nil && r.ClusterMemberAttrSet == nil && r.DowngradeInfoSet == nil {
+	if r.ClusterVersionSet == nil && r.ClusterMemberAttrSet == nil && r.DowngradeInfoSet == nil && r.DowngradeVersionTest == nil {
 		if !shouldApplyV3 {
 			return nil
 		}
@@ -2029,6 +2029,11 @@ func (s *EtcdServer) applyInternalRaftRequest(r *pb.InternalRaftRequest, shouldA
 	case r.DowngradeInfoSet != nil:
 		op = "DowngradeInfoSet" // Implemented in 3.5.x
 		membershipApplier.DowngradeInfoSet(r.DowngradeInfoSet, shouldApplyV3)
+	case r.DowngradeVersionTest != nil:
+		op = "DowngradeVersionTest" // Implemented in 3.6 for test only
+		// do nothing, we are just to ensure etcdserver don't panic in case
+		// users(test cases) intentionally inject DowngradeVersionTestRequest
+		// into the WAL files.
 	default:
 		s.lg.Panic("not implemented apply", zap.Stringer("raft-request", r))
 		return nil

server/storage/schema/schema_test.go

@@ -180,17 +180,17 @@ func TestMigrate(t *testing.T) {
 			},
 			expectVersion: nil,
 		},
-		/* TODO: add a dedicated request for testing only to cover such case
 		{
-			name:          "Downgrading v3.6 to v3.5 works even if there is ClusterVersionSetRequest WAL entries with 3.6 clusterVersion",
+			name:          "Downgrading v3.6 to v3.5 fails if there are newer WAL entries",
 			version:       version.V3_6,
 			targetVersion: version.V3_5,
 			walEntries: []etcdserverpb.InternalRaftRequest{
-				{ClusterVersionSet: &membershippb.ClusterVersionSetRequest{Ver: "3.6.0"}},
+				{DowngradeVersionTest: &etcdserverpb.DowngradeVersionTestRequest{Ver: "3.6.0"}},
 			},
-			expectVersion: nil, // 3.5 doesn't have field `storageVersion`, so it should be nil.
-			expectError:   false,
-		},*/
+			expectVersion:  &version.V3_6,
+			expectError:    true,
+			expectErrorMsg: "cannot downgrade storage, WAL contains newer entries",
+		},
 		{
 			name:           "Downgrading v3.5 to v3.4 is not supported as schema was introduced in v3.6",
 			version:        version.V3_5,

server/storage/wal/version.go

@@ -112,6 +112,16 @@ func visitEntryData(entryType raftpb.EntryType, data []byte, visitor Visitor) er
 			break
 		}
 		msg = proto.MessageReflect(&raftReq)
+		if raftReq.DowngradeVersionTest != nil {
+			ver, err := semver.NewVersion(raftReq.DowngradeVersionTest.Ver)
+			if err != nil {
+				return err
+			}
+			err = visitor(msg.Descriptor().FullName(), ver)
+			if err != nil {
+				return err
+			}
+		}
 	case raftpb.EntryConfChange:
 		var confChange raftpb.ConfChange
 		err := pbutil.Unmarshaler(&confChange).Unmarshal(data)

server/storage/wal/version_test.go

@@ -35,6 +35,12 @@ func TestEtcdVersionFromEntry(t *testing.T) {
 	raftReq := etcdserverpb.InternalRaftRequest{Header: &etcdserverpb.RequestHeader{AuthRevision: 1}}
 	normalRequestData := pbutil.MustMarshal(&raftReq)
 
+	downgradeVersionTestV3_6Req := etcdserverpb.InternalRaftRequest{DowngradeVersionTest: &etcdserverpb.DowngradeVersionTestRequest{Ver: "3.6.0"}}
+	downgradeVersionTestV3_6Data := pbutil.MustMarshal(&downgradeVersionTestV3_6Req)
+
+	downgradeVersionTestV3_7Req := etcdserverpb.InternalRaftRequest{DowngradeVersionTest: &etcdserverpb.DowngradeVersionTestRequest{Ver: "3.7.0"}}
+	downgradeVersionTestV3_7Data := pbutil.MustMarshal(&downgradeVersionTestV3_7Req)
+
 	confChange := raftpb.ConfChange{Type: raftpb.ConfChangeAddLearnerNode}
 	confChangeData := pbutil.MustMarshal(&confChange)
 
@@ -56,6 +62,26 @@ func TestEtcdVersionFromEntry(t *testing.T) {
 			},
 			expect: &version.V3_1,
 		},
+		{
+			name: "Setting downgradeTest version to 3.6 implies version within WAL",
+			input: raftpb.Entry{
+				Term:  1,
+				Index: 2,
+				Type:  raftpb.EntryNormal,
+				Data:  downgradeVersionTestV3_6Data,
+			},
+			expect: &version.V3_6,
+		},
+		{
+			name: "Setting downgradeTest version to 3.7 implies version within WAL",
+			input: raftpb.Entry{
+				Term:  1,
+				Index: 2,
+				Type:  raftpb.EntryNormal,
+				Data:  downgradeVersionTestV3_7Data,
+			},
+			expect: &version.V3_7,
+		},
 		{
 			name: "Using ConfigChange implies v3.0",
 			input: raftpb.Entry{