roachtest: move validate-after-version-upgrade to new framework

pkg/cmd/roachtest/tests/acceptance.go

@@ -76,6 +76,14 @@ func registerAcceptance(r registry.Registry) {
 				numNodes: 3,
 			},
 		},
+		registry.OwnerSQLFoundations: {
+			{
+				name:          "validate-system-schema-after-version-upgrade",
+				fn:            runValidateSystemSchemaAfterVersionUpgrade,
+				timeout:       30 * time.Minute,
+				defaultLeases: true,
+			},
+		},
 	}
 	for owner, tests := range testCases {
 		for _, tc := range tests {

pkg/cmd/roachtest/tests/registry.go

@@ -147,7 +147,6 @@ func RegisterTests(r registry.Registry) {
 	registerTPCHVec(r)
 	registerTypeORM(r)
 	registerUnoptimizedQueryOracle(r)
-	registerValidateSystemSchemaAfterVersionUpgrade(r)
 	registerYCSB(r)
 	registerDeclarativeSchemaChangerJobCompatibilityInMixedVersion(r)
 }

pkg/cmd/roachtest/tests/validate_system_schema_after_version_upgrade.go

@@ -12,123 +12,98 @@ package tests
 
 import (
 	"context"
+	"math/rand"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
-	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/clusterupgrade"
+	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/mixedversion"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
-	"github.com/cockroachdb/cockroach/pkg/testutils/release"
+	"github.com/cockroachdb/cockroach/pkg/roachprod/install"
+	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
+	"github.com/cockroachdb/errors"
 	"github.com/pmezard/go-difflib/difflib"
 )
 
-func registerValidateSystemSchemaAfterVersionUpgrade(r registry.Registry) {
-	// This test tests that, after bootstrapping a cluster from a previous
-	// release's binary and upgrading it to the latest version, the `system`
-	// database "contains the expected tables".
-	// Specifically, we do the check with `USE system; SHOW CREATE ALL TABLES;`
-	// and assert that the output matches the expected output content.
-	r.Add(registry.TestSpec{
-		Name:             "systemschema/validate-after-version-upgrade",
-		Owner:            registry.OwnerSQLFoundations,
-		Cluster:          r.MakeClusterSpec(1),
-		CompatibleClouds: registry.AllExceptAWS,
-		Suites:           registry.Suites(registry.Nightly),
-		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
-			predecessorVersionStr, err := release.LatestPredecessor(t.BuildVersion())
-			if err != nil {
-				t.Fatal(err)
+// This test tests that, after bootstrapping a cluster from a previous
+// release's binary and upgrading it to the latest version, the `system`
+// database "contains the expected tables".
+// Specifically, we do the check with `USE system; SHOW CREATE ALL TABLES;`
+// and assert that the output matches the expected output content.
+func runValidateSystemSchemaAfterVersionUpgrade(
+	ctx context.Context, t test.Test, c cluster.Cluster,
+) {
+	// Obtain system table definitions with `SHOW CREATE ALL TABLES` in the SYSTEM db.
+	obtainSystemSchema := func(ctx context.Context, l *logger.Logger, c cluster.Cluster, node int) string {
+		// Create a connection to the database cluster.
+		db := c.Conn(ctx, l, node)
+		sqlRunner := sqlutils.MakeSQLRunner(db)
+
+		// Prepare the SQL query.
+		sql := `USE SYSTEM; SHOW CREATE ALL TABLES;`
+
+		// Execute the SQL query.
+		rows := sqlRunner.QueryStr(t, sql)
+
+		// Extract return.
+		var sb strings.Builder
+		for _, row := range rows {
+			sb.WriteString(row[0])
+			sb.WriteString("\n")
+		}
+
+		return sb.String()
+	}
+
+	// expected and actual output of `SHOW CREATE ALL TABLES;`.
+	var expected, actual string
+
+	// Start a cluster with the latest binary and get the system schema from the
+	// cluster.
+	if err := clusterupgrade.StartWithSettings(
+		ctx, t.L(), c, c.All(), option.DefaultStartOpts(), install.BinaryOption(test.DefaultCockroachPath),
+	); err != nil {
+		t.Fatal(err)
+	}
+	expected = obtainSystemSchema(ctx, t.L(), c, 1)
+	c.Wipe(ctx, false /* preserveCerts */, c.All())
+
+	mvt := mixedversion.NewTest(ctx, t, t.L(), c, c.All(),
+		// Fixtures are generated on a version that's too old for this test.
+		mixedversion.NeverUseFixtures,
+		// We limit the number of upgrades since the test is not expected to work
+		// on versions older than 22.2.
+		mixedversion.MaxUpgrades(2),
+	)
+	mvt.AfterUpgradeFinalized(
+		"obtain system schema from the upgraded cluster",
+		func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper) error {
+			if !h.Context().ToVersion.IsCurrent() {
+				// Only validate the system schema if we're upgrading to the version
+				// under test.
+				return nil
 			}
-			predecessorVersion := clusterupgrade.MustParseVersion(predecessorVersionStr)
 
-			// Obtain system table definitions with `SHOW CREATE ALL TABLES` in the SYSTEM db.
-			obtainSystemSchema := func(ctx context.Context, t test.Test, u *versionUpgradeTest, node int) string {
-				// Create a connection to the database cluster.
-				db := u.conn(ctx, t, node)
-				sqlRunner := sqlutils.MakeSQLRunner(db)
-
-				// Prepare the SQL query.
-				sql := `USE SYSTEM; SHOW CREATE ALL TABLES;`
-
-				// Execute the SQL query.
-				rows := sqlRunner.QueryStr(t, sql)
-
-				// Extract return.
-				var sb strings.Builder
-				for _, row := range rows {
-					sb.WriteString(row[0])
-					sb.WriteString("\n")
-				}
-
-				return sb.String()
-			}
-
-			// expected and actual output of `SHOW CREATE ALL TABLES;`.
-			var expected, actual string
-
-			// Query node `SHOW CREATE ALL TABLES` and store return in output.
-			obtainSystemSchemaStep := func(node int, output *string) versionStep {
-				return func(ctx context.Context, t test.Test, u *versionUpgradeTest) {
-					*output = obtainSystemSchema(ctx, t, u, node)
-				}
-			}
-
-			// Wipe nodes in this test's cluster.
-			wipeClusterStep := func(nodes option.NodeListOption) versionStep {
-				return func(ctx context.Context, t test.Test, u *versionUpgradeTest) {
-					u.c.Wipe(ctx, false /* preserveCerts */, nodes)
+			// Compare whether the two schemas are equal
+			actual = obtainSystemSchema(ctx, l, c, 1)
+			if expected != actual {
+				diff, diffErr := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{
+					A:       difflib.SplitLines(expected),
+					B:       difflib.SplitLines(actual),
+					Context: 5,
+				})
+				if diffErr != nil {
+					return errors.Wrap(diffErr, "failed to produce diff")
 				}
+				return errors.Newf("After upgrading, `USE system; SHOW CREATE ALL TABLES;` "+
+					"does not match expected output after version upgrade."+
+					"\nDiff:\n%s", diff)
 			}
-
-			// Compare whether two strings are equal -- used to compare expected and actual.
-			validateEquivalenceStep := func(str1, str2 *string) versionStep {
-				return func(ctx context.Context, t test.Test, u *versionUpgradeTest) {
-					if *str1 != *str2 {
-						diff, diffErr := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{
-							A:       difflib.SplitLines(*str1),
-							B:       difflib.SplitLines(*str2),
-							Context: 5,
-						})
-						if diffErr != nil {
-							diff = diffErr.Error()
-							t.Errorf("failed to produce diff: %v", diffErr)
-						}
-						t.Fatalf("After upgrading, `USE system; SHOW CREATE ALL TABLES;` "+
-							"does not match expected output after version upgrade."+
-							"\nDiff:\n%s", diff)
-					}
-					t.L().Printf("validating succeeded:\n%v", *str1)
-				}
-			}
-
-			u := newVersionUpgradeTest(c,
-				// Start the node with the latest binary version.
-				uploadAndStart(c.Node(1), clusterupgrade.CurrentVersion()),
-
-				// Obtain expected output from the node.
-				obtainSystemSchemaStep(1, &expected),
-
-				// Wipe the node.
-				wipeClusterStep(c.Node(1)),
-
-				// Restart the node with a previous binary version.
-				uploadAndStart(c.Node(1), predecessorVersion),
-
-				// Upgrade the node version.
-				binaryUpgradeStep(c.Node(1), clusterupgrade.CurrentVersion()),
-
-				// Wait for the cluster version to also bump up to make sure the migration logic is run.
-				waitForUpgradeStep(c.Node(1)),
-
-				// Obtain the actual output on the upgraded version.
-				obtainSystemSchemaStep(1, &actual),
-
-				// Compare the results.
-				validateEquivalenceStep(&expected, &actual),
-			)
-			u.run(ctx, t)
+			l.Printf("validating succeeded:\n%v", expected)
+			return nil
 		},
-	})
+	)
+	mvt.Run()
 }