Skip to content

changefeedccl: enable a couple of tests with test tenant #156306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 28, 2025
+26 −42
Merged

changefeedccl: enable a couple of tests with test tenant #156306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/alter_changefeed_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestAlterChangefeedAddTargetPrivileges(t *testing.T) {
ctx := context.Background()

s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
DefaultTestTenant: base.TODOTestTenantDisabled,
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(142799),
Knobs: base.TestingKnobs{
JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
DistSQL: &execinfra.TestingKnobs{
Expand Down
22 changes: 7 additions & 15 deletions pkg/ccl/changefeedccl/changefeed_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2621,7 +2621,7 @@ func TestChangefeedExternalIODisabled(t *testing.T) {
}
ctx := context.Background()
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
DefaultTestTenant: base.TODOTestTenantDisabled,
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(142799),
ExternalIODirConfig: base.ExternalIODirConfig{
DisableOutbound: true,
},
Expand Down Expand Up @@ -4067,7 +4067,7 @@ func TestChangefeedCreateAuthorizationWithChangefeedPriv(t *testing.T) {

ctx := context.Background()
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
DefaultTestTenant: base.TODOTestTenantDisabled,
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(142799),
Knobs: base.TestingKnobs{
JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
DistSQL: &execinfra.TestingKnobs{
Expand Down Expand Up @@ -8384,9 +8384,7 @@ func TestChangefeedHandlesDrainingNodes(t *testing.T) {

tc := serverutils.StartCluster(t, 4, base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
// Test uses SPLIT AT, which isn't currently supported for
// secondary tenants. Tracked with #76378.
DefaultTestTenant: base.TODOTestTenantDisabled,
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(142799),
UseDatabase: "test",
Knobs: knobs,
ExternalIODir: sinkDir,
Expand Down Expand Up @@ -8549,9 +8547,7 @@ func TestChangefeedHandlesRollingRestart(t *testing.T) {
return perNode
}(),
ServerArgs: base.TestServerArgs{
// Test uses SPLIT AT, which isn't currently supported for
// secondary tenants. Tracked with #76378.
DefaultTestTenant: base.TODOTestTenantDisabled,
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(142799),
Copy link
Contributor

@aerfrei aerfrei Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these comments (that SPLIT AT isn't supported for secondary tenants) no longer true?
Are we now tracking these exclusively in #142799 and not #76378?

Copy link
Member Author

@yuzefovich yuzefovich Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments here are definitely stale, which is why I decided to remove them to avoid possible confusion. (In particular, when this comment was added, we needed to explicitly add tenantcapabilitiespb.CanAdminSplit to the tenant, but now this capability is granted by default, so no need to do anything about it. Perhaps the test will just work, perhaps it'll require some other adjustments, but SPLIT AT should just work.)

I'm working on doing first level triage of all things that reference #76378 with the goal of closing that issue soon. All tests / packages that have test tenant randomization disabled will have a separate, specific issue, and #142799 is the common issue for all CDC tests.

},
})
defer tc.Stopper().Stop(context.Background())
Expand Down Expand Up @@ -8707,9 +8703,7 @@ func TestChangefeedTimelyResolvedTimestampUpdatePostRollingRestart(t *testing.T)
base.TestClusterArgs{
ServerArgsPerNode: perServerKnobs,
ServerArgs: base.TestServerArgs{
// Test uses SPLIT AT, which isn't currently supported for
// secondary tenants. Tracked with #76378.
DefaultTestTenant: base.TODOTestTenantDisabled,
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(142799),
},
ReusableListenerReg: listenerReg,
})
Expand Down Expand Up @@ -8802,9 +8796,7 @@ func TestChangefeedPropagatesTerminalError(t *testing.T) {
ServerArgsPerNode: perServerKnobs,
ReplicationMode: base.ReplicationManual,
ServerArgs: base.TestServerArgs{
// Test uses SPLIT AT, which isn't currently supported for
// secondary tenants. Tracked with #76378.
DefaultTestTenant: base.TODOTestTenantDisabled,
DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(142799),
},
})
defer tc.Stopper().Stop(context.Background())
Expand Down Expand Up @@ -11309,7 +11301,7 @@ func TestChangefeedExecLocality(t *testing.T) {
const nodes = 4
args := base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
DefaultTestTenant: base.TODOTestTenantDisabled, // need nodelocal and splits.
Copy link
Contributor

@aerfrei aerfrei Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment too, is it not accurate for why this test wasn't working for tenants?

Copy link
Member Author

@yuzefovich yuzefovich Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is somewhat stale for sure. CanUseNodelocalStorage capability should now be granted in the tests by default (see testServer.grantDefaultTenantCapabilities).

DefaultTestTenant: base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(142799),
},
ServerArgsPerNode: map[int]base.TestServerArgs{},
}
Expand Down
15 changes: 7 additions & 8 deletions pkg/ccl/changefeedccl/parquet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,16 @@ func TestParquetRows(t *testing.T) {
skip.UnderStress(t)

ctx := context.Background()
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
// TODO(#98816): cdctest.GetHydratedTableDescriptor does not work with tenant dbs.
Copy link
Contributor

@aerfrei aerfrei Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, nice that we have this test back.

// Once it is fixed, this flag can be removed.
DefaultTestTenant: base.TODOTestTenantDisabled,
})
defer s.Stopper().Stop(ctx)
srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
defer srv.Stopper().Stop(ctx)
s := srv.ApplicationLayer()

sysDB := sqlutils.MakeSQLRunner(srv.SystemLayer().SQLConn(t))
sysDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true")

maxRowGroupSize := int64(4)

sqlDB := sqlutils.MakeSQLRunner(db)
sqlDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true")

newDecimal := func(s string) *tree.DDecimal {
d, _, _ := apd.NewFromString(s)
Expand Down Expand Up @@ -264,7 +263,7 @@ func TestParquetRows(t *testing.T) {
}

func makeRangefeedReaderAndDecoder(
t *testing.T, s serverutils.TestServerInterface,
t *testing.T, s serverutils.ApplicationLayerInterface,
) (func(t testing.TB) *kvpb.RangeFeedValue, func(), cdcevent.Decoder) {
tableDesc := cdctest.GetHydratedTableDescriptor(t, s.ExecutorConfig(), "foo")
popRow, cleanup := cdctest.MakeRangeFeedValueReader(t, s.ExecutorConfig(), tableDesc)
Expand Down
29 changes: 11 additions & 18 deletions pkg/ccl/changefeedccl/scheduled_changefeed_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"context"
gosql "database/sql"
"fmt"
"net/url"
"strconv"
"strings"
"testing"
Expand Down Expand Up @@ -304,8 +303,7 @@ func TestCreateChangefeedScheduleChecksPermissionsDuringDryRun(t *testing.T) {
defer log.Scope(t).Close(t)

ctx := context.Background()
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
DefaultTestTenant: base.TODOTestTenantDisabled,
srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{
Knobs: base.TestingKnobs{
JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
DistSQL: &execinfra.TestingKnobs{
Expand All @@ -320,25 +318,20 @@ func TestCreateChangefeedScheduleChecksPermissionsDuringDryRun(t *testing.T) {
},
},
})
defer s.Stopper().Stop(ctx)
rootDB := sqlutils.MakeSQLRunner(db)
rootDB.Exec(t, `SET CLUSTER SETTING kv.rangefeed.enabled = true`)
defer srv.Stopper().Stop(ctx)
s := srv.ApplicationLayer()

sysDB := sqlutils.MakeSQLRunner(srv.SystemLayer().SQLConn(t))
sysDB.Exec(t, "SET CLUSTER SETTING kv.rangefeed.enabled = true")

sqlDB := sqlutils.MakeSQLRunner(db)
enableEnterprise := utilccl.TestingDisableEnterprise()
enableEnterprise()

rootDB.Exec(t, `CREATE TABLE table_a (i int)`)
rootDB.Exec(t, `CREATE USER testuser WITH PASSWORD 'test'`)
sqlDB.Exec(t, `CREATE TABLE table_a (i int)`)
sqlDB.Exec(t, `CREATE USER testuser WITH PASSWORD 'test'`)

pgURL := url.URL{
Scheme: "postgres",
User: url.UserPassword("testuser", "test"),
Host: s.SQLAddr(),
}
db2, err := gosql.Open("postgres", pgURL.String())
if err != nil {
t.Fatal(err)
}
defer db2.Close()
db2 := s.SQLConn(t, serverutils.UserPassword("testuser", "test"))
userDB := sqlutils.MakeSQLRunner(db2)

userDB.ExpectErr(t, `Failed to dry run create changefeed: user "testuser" requires the CHANGEFEED privilege on all target tables to be able to run an enterprise changefeed`,
Expand Down
Loading