testutils: add GrantTenantCapabilities to test interfaces

[shubhamdhama]

Many tests require granting a capability when using external-process mode. Since this pattern is repeated in multiple places, it makes sense to provide it as a utility.

As part of this cleanup, I've also adjusted when it runs: it now applies only to external-process tenants, as shared-process tenants always have all capabilities.

Informs: #138912
Epic: CRDB-38970
Release note: None

[cockroach-teamcity]

This change is 

[stevendanna]

This looks like a nice improvement. I do wonder if we want to go even further. We could add this to TestServerArgs and then handle them as part of this code:

cockroach/pkg/server/testserver.go

Lines 731 to 780 in 9598211

	func (ts *testServer) grantDefaultTenantCapabilities(
	ctx context.Context, tenantID roachpb.TenantID, skipTenantCheck bool,
	) error {
	ie := ts.InternalExecutor().(*sql.InternalExecutor)
	for _, setting := range []settings.Setting{
	sql.SecondaryTenantScatterEnabled,
	sql.SecondaryTenantSplitAtEnabled,
	sqlclustersettings.SecondaryTenantZoneConfigsEnabled,
	sql.SecondaryTenantsMultiRegionAbstractionsEnabled,
	} {
	// Update the override for this setting. We need to do this
	// instead of calling .Override() on the setting directly: certain
	// tests expect to be able to change the value afterwards using
	// another ALTER VC SET CLUSTER SETTING statement, which is not
	// possible with regular overrides.
	_, err := ie.Exec(ctx, "testserver-alter-tenant-cap", nil,
	fmt.Sprintf("ALTER VIRTUAL CLUSTER [$1] SET CLUSTER SETTING %s = true", setting.Name()), tenantID.ToUint64())
	if err != nil {
	if skipTenantCheck {
	log.Infof(ctx, "ignoring error changing setting because SkipTenantCheck is true: %v", err)
	} else {
	return err
	}
	}
	}
	// Waiting for capabilities can take time. To avoid paying this cost in all
	// cases, we only set the nodelocal storage capability if the caller has
	// configured an ExternalIODir since nodelocal storage only works with that
	// configured.
	shouldGrantNodelocalCap := ts.params.ExternalIODir != ""
	if shouldGrantNodelocalCap {
	_, err := ie.Exec(ctx, "testserver-alter-tenant-cap", nil,
	"ALTER TENANT [$1] GRANT CAPABILITY can_use_nodelocal_storage", tenantID.ToUint64())
	if err != nil {
	if skipTenantCheck {
	log.Infof(ctx, "ignoring error granting capability because SkipTenantCheck is true: %v", err)
	} else {
	return err
	}
	} else {
	if err := ts.WaitForTenantCapabilities(ctx, tenantID, map[tenantcapabilities.ID]string{
	tenantcapabilities.CanUseNodelocalStorage: "true",
	}, ""); err != nil {
	return err
	}
	}
	}
	return nil
	}

To avoid waiting for capabilities longer than we have to. Don't let this suggestion get in the way of this improvement though, I think both functions are useful.

[shubhamdhama]

This looks like a nice improvement. I do wonder if we want to go even further. We could add this to TestServerArgs and then handle them as part of this code:

It's a great suggestion. I'm working on it as a follow-up.

[cthumuluru-crdb]

Except for one or two minor comments, this looks great!

[cthumuluru-crdb]

Except for one or two minor comments, this looks great!

[shubhamdhama]

TFTRs!

bors r=stevendanna,cthumuluru-crdb

[craig]

Build succeeded:

• docker_image_amd64
• examples_orms
• local_roachtest
• linux_amd64_fips_build
• lint
• linux_amd64_build
• acceptance
• local_roachtest_fips
• unit_tests
• check_generated_code