Merge #96991 #98398

96991: roachtest: update mixed-version backup to use new framework r=srosenberg a=renatolabs


This updates the `backup/mixed-version` roachtest to use the recently
introduced mixed-version roachtest framework (`mixedversion` package).

The main behavior exercised remains the same: backups are taken in
mixed-binary state, and those backups are restored and verified at the
end of the test. However, this commit also improves the coverage of
mixed-version backup testing in a few ways:

* **Randomization**. By virtue of using the new framework, most runs
will be different from one another since the order of actions taken by
the test will be different. Previously, backups would always be taken
with 2 nodes in the old version and 2 nodes in the new version. Now,
backups can be taken when an arbitrary number of nodes is running the
new version. As a consequence, it's also possible that some executions
will attempt backups when all nodes are running a new binary version,
but the cluster version itself has not been updated. Other points of
new randomization include the choice of the node's external dir where
backups are stored, which node to connect to when running certain
statements, and how much to wait between backups.

* **Backup Options**. Backups will randomly be created with
`revision_history` enabled, or with an `encryption_passphrase`.

* **Downgrades**. The cluster is also downgraded in mixed-version
tests. No downgrades happened in that test before this commit.

* **Workload**. Instead of using fixed call to `generate_series` to
generate data between backups, the test now runs the `bank` workload
continuously during the test. A random wait between backups allows the
workload to make changes to the underlying table during the test and
for the backups to be taken while writes are taking place.

* **Finalization**: the test _may_ attempt to create a backup as the
upgrade is finalizing (i.e., migrations are running and cluster
version is advancing).

In addition, this test will also see improved coverage as we make more
improvements to test plans generated by the `mixedversion` package.
These changes will create more backup scenarios in the future without
requiring any code changes to this test.

This test has already helped us uncover one backup bug (#97953).

Epic: CRDB-19321

Release note: None

98398: statusccl: stop serving /_status/nodes to tenants r=matthewtodd a=matthewtodd

Fixes #98057.

This reverts the work of #93268, which is no longer necessary now that we are eagerly capturing region information at execution time in #95449.

Release note: None

Co-authored-by: Renato Costa <[email protected]>
Co-authored-by: Matthew Todd <[email protected]>

pkg/ccl/serverccl/statusccl/tenant_status_test.go

@@ -66,21 +66,7 @@ func TestTenantStatusAPI(t *testing.T) {
 		StoreDisableCoalesceAdjacent: true,
 	}
 
-	tenantLocalities := []roachpb.Locality{
-		{Tiers: []roachpb.Tier{{Key: "region", Value: "gcp-us-west1"}}},
-		{Tiers: []roachpb.Tier{{Key: "region", Value: "gcp-us-central1"}}},
-		{Tiers: []roachpb.Tier{{Key: "region", Value: "gcp-us-east1"}}},
-	}
-
-	testHelper := serverccl.NewTestTenantHelperWithTenantArgs(
-		t,
-		3, /* tenantClusterSize */
-		knobs,
-		func(tenantIdx int, tenantArgs *base.TestTenantArgs) {
-			tenantArgs.Locality = tenantLocalities[tenantIdx]
-		},
-	)
-
+	testHelper := serverccl.NewTestTenantHelper(t, 3 /* tenantClusterSize */, knobs)
 	defer testHelper.Cleanup(ctx, t)
 
 	// Speed up propagation of tenant capability changes.
@@ -141,10 +127,6 @@ func TestTenantStatusAPI(t *testing.T) {
 		testTenantLogs(ctx, t, testHelper)
 	})
 
-	t.Run("tenant_nodes", func(t *testing.T) {
-		testTenantNodes(ctx, t, testHelper, tenantLocalities)
-	})
-
 	t.Run("tenant_hot_ranges", func(t *testing.T) {
 		testTenantHotRanges(ctx, t, testHelper)
 	})
@@ -1356,37 +1338,6 @@ func testTenantAuthOnStatements(
 	require.NoError(t, err)
 }
 
-func testTenantNodes(
-	ctx context.Context,
-	t *testing.T,
-	helper serverccl.TenantTestHelper,
-	tenantLocalities []roachpb.Locality,
-) {
-	// TODO(todd): Get this test working over HTTP, #93267.
-	//
-	// When I first tried, I ran into errors about the SQL user
-	// `authentic_user_noadmin` already existing, but at first glance it seemed
-	// that the sync.Once mechanism used in its setup had already accounted
-	// for that scenario; that we're properly passing by reference all the way
-	// through; and that these tests aren't being run in parallel.
-	tenantA := helper.TestCluster().TenantStatusSrv(0)
-	resp, err := tenantA.Nodes(ctx, &serverpb.NodesRequest{})
-	require.NoError(t, err)
-
-	expected := map[roachpb.NodeID]roachpb.Locality{
-		1: tenantLocalities[0],
-		2: tenantLocalities[1],
-		3: tenantLocalities[2],
-	}
-
-	actual := make(map[roachpb.NodeID]roachpb.Locality)
-	for _, node := range resp.Nodes {
-		actual[node.Desc.NodeID] = node.Desc.Locality
-	}
-
-	require.Equal(t, expected, actual)
-}
-
 // assertStartKeyInRange compares the pretty printed startKey with the provided
 // tenantPrefix key, ensuring that the startKey starts with the tenantPrefix.
 func assertStartKeyInRange(t *testing.T, startKey string, tenantPrefix roachpb.Key) {

pkg/ccl/serverccl/tenant_test_utils.go

@@ -145,18 +145,6 @@ func NewTestTenantHelper(
 ) TenantTestHelper {
 	t.Helper()
 
-	return NewTestTenantHelperWithTenantArgs(t, tenantClusterSize, knobs, func(int, *base.TestTenantArgs) {})
-}
-
-// NewTestTenantHelperWithTenantArgs constructs a TenantTestHelper instance,
-// offering the caller the opportunity to modify the arguments passed when
-// starting each tenant.
-func NewTestTenantHelperWithTenantArgs(
-	t *testing.T,
-	tenantClusterSize int,
-	knobs base.TestingKnobs,
-	customizeTenantArgs func(tenantIdx int, tenantArgs *base.TestTenantArgs),
-) TenantTestHelper {
 	t.Helper()
 
 	params, _ := tests.CreateTestServerParams()
@@ -176,7 +164,6 @@ func NewTestTenantHelperWithTenantArgs(
 			tenantClusterSize,
 			security.EmbeddedTenantIDs()[0],
 			knobs,
-			customizeTenantArgs,
 		),
 		// Spin up a small tenant cluster under a different tenant ID to test
 		// tenant isolation.
@@ -186,7 +173,6 @@ func NewTestTenantHelperWithTenantArgs(
 			1, /* tenantClusterSize */
 			security.EmbeddedTenantIDs()[1],
 			knobs,
-			func(int, *base.TestTenantArgs) {},
 		),
 	}
 }
@@ -234,15 +220,13 @@ func newTenantClusterHelper(
 	tenantClusterSize int,
 	tenantID uint64,
 	knobs base.TestingKnobs,
-	customizeTenantArgs func(tenantIdx int, tenantArgs *base.TestTenantArgs),
 ) TenantClusterHelper {
 	t.Helper()
 
 	var cluster tenantCluster = make([]TestTenant, tenantClusterSize)
 	for i := 0; i < tenantClusterSize; i++ {
 		args := tests.CreateTestTenantParams(roachpb.MustMakeTenantID(tenantID))
 		args.TestingKnobs = knobs
-		customizeTenantArgs(i, &args)
 		cluster[i] = newTestTenant(t, server, args)
 	}
 

pkg/cmd/roachtest/tests/BUILD.bazel

@@ -88,6 +88,7 @@ go_library(
         "liquibase_blocklist.go",
         "loss_of_quorum_recovery.go",
         "many_splits.go",
+        "mixed_version_backup.go",
         "mixed_version_cdc.go",
         "mixed_version_change_replicas.go",
         "mixed_version_decl_schemachange_compat.go",