Skip to content

Commit a30896a

Browse files
DarrylWongDarrylWong
committed
roachprod: use PGURLOptions in PGUrl related functions.
The majority of calls to InternalPGUrl and ExternalPGUrl did not specify a tenant or SQLInstance, leaving them as "" and 0 respectively. This change refactors those functions to instead use PGURLOptions which contains the above two options. An auth option is also added which specifies the mode of authentication to be used, defaulting to the previous behavior of root authentication if not specified. Release note: None
1 parent 7a764e5 commit a30896a

File tree

16 files changed

+100
-61
lines changed

16 files changed

+100
-61
lines changed

‎pkg/cmd/roachtest/cluster.go

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2469,19 +2469,10 @@ func (c *clusterImpl) loggerForCmd(
24692469
// internal IPs and communication from a test driver to nodes in a cluster
24702470
// should use external IPs.
24712471
func (c *clusterImpl) pgURLErr(
2472-
ctx context.Context,
2473-
l *logger.Logger,
2474-
nodes option.NodeListOption,
2475-
external bool,
2476-
tenant string,
2477-
sqlInstance int,
2472+
ctx context.Context, l *logger.Logger, nodes option.NodeListOption, opts roachprod.PGURLOptions,
24782473
) ([]string, error) {
2479-
urls, err := roachprod.PgURL(ctx, l, c.MakeNodes(nodes), c.localCertsDir, roachprod.PGURLOptions{
2480-
External: external,
2481-
Secure: c.localCertsDir != "",
2482-
VirtualClusterName: tenant,
2483-
SQLInstance: sqlInstance,
2484-
})
2474+
opts.Secure = c.IsSecure()
2475+
urls, err := roachprod.PgURL(ctx, l, c.MakeNodes(nodes), c.localCertsDir, opts)
24852476
if err != nil {
24862477
return nil, err
24872478
}
@@ -2493,27 +2484,20 @@ func (c *clusterImpl) pgURLErr(
24932484

24942485
// InternalPGUrl returns the internal Postgres endpoint for the specified nodes.
24952486
func (c *clusterImpl) InternalPGUrl(
2496-
ctx context.Context,
2497-
l *logger.Logger,
2498-
nodes option.NodeListOption,
2499-
tenant string,
2500-
sqlInstance int,
2487+
ctx context.Context, l *logger.Logger, nodes option.NodeListOption, opts roachprod.PGURLOptions,
25012488
) ([]string, error) {
2502-
return c.pgURLErr(ctx, l, nodes, false, tenant, sqlInstance)
2489+
return c.pgURLErr(ctx, l, nodes, opts)
25032490
}
25042491

25052492
// Silence unused warning.
25062493
var _ = (&clusterImpl{}).InternalPGUrl
25072494

25082495
// ExternalPGUrl returns the external Postgres endpoint for the specified nodes.
25092496
func (c *clusterImpl) ExternalPGUrl(
2510-
ctx context.Context,
2511-
l *logger.Logger,
2512-
nodes option.NodeListOption,
2513-
tenant string,
2514-
sqlInstance int,
2497+
ctx context.Context, l *logger.Logger, nodes option.NodeListOption, opts roachprod.PGURLOptions,
25152498
) ([]string, error) {
2516-
return c.pgURLErr(ctx, l, nodes, true, tenant, sqlInstance)
2499+
opts.External = true
2500+
return c.pgURLErr(ctx, l, nodes, opts)
25172501
}
25182502

25192503
func addrToAdminUIAddr(addr string) (string, error) {
@@ -2642,7 +2626,7 @@ func (c *clusterImpl) addr(
26422626
ctx context.Context, l *logger.Logger, nodes option.NodeListOption, external bool,
26432627
) ([]string, error) {
26442628
var addrs []string
2645-
urls, err := c.pgURLErr(ctx, l, nodes, external, "" /* tenant */, 0 /* sqlInstance */)
2629+
urls, err := c.pgURLErr(ctx, l, nodes, roachprod.PGURLOptions{External: external})
26462630
if err != nil {
26472631
return nil, err
26482632
}
@@ -2700,7 +2684,10 @@ func (c *clusterImpl) ConnE(
27002684
for _, opt := range opts {
27012685
opt(connOptions)
27022686
}
2703-
urls, err := c.ExternalPGUrl(ctx, l, c.Node(node), connOptions.TenantName, connOptions.SQLInstance)
2687+
urls, err := c.ExternalPGUrl(ctx, l, c.Node(node), roachprod.PGURLOptions{
2688+
VirtualClusterName: connOptions.TenantName,
2689+
SQLInstance: connOptions.SQLInstance,
2690+
})
27042691
if err != nil {
27052692
return nil, err
27062693
}

‎pkg/cmd/roachtest/cluster/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ go_library(
1212
deps = [
1313
"//pkg/cmd/roachtest/option",
1414
"//pkg/cmd/roachtest/spec",
15+
"//pkg/roachprod",
1516
"//pkg/roachprod/install",
1617
"//pkg/roachprod/logger",
1718
"//pkg/roachprod/prometheus",

‎pkg/cmd/roachtest/cluster/cluster_interface.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
1919
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
20+
"github.com/cockroachdb/cockroach/pkg/roachprod"
2021
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
2122
"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
2223
"github.com/cockroachdb/cockroach/pkg/roachprod/prometheus"
@@ -79,8 +80,8 @@ type Cluster interface {
7980

8081
// SQL connection strings.
8182

82-
InternalPGUrl(ctx context.Context, l *logger.Logger, node option.NodeListOption, tenant string, sqlInstance int) ([]string, error)
83-
ExternalPGUrl(ctx context.Context, l *logger.Logger, node option.NodeListOption, tenant string, sqlInstance int) ([]string, error)
83+
InternalPGUrl(ctx context.Context, l *logger.Logger, node option.NodeListOption, opts roachprod.PGURLOptions) ([]string, error)
84+
ExternalPGUrl(ctx context.Context, l *logger.Logger, node option.NodeListOption, opts roachprod.PGURLOptions) ([]string, error)
8485

8586
// SQL clients to nodes.
8687

‎pkg/cmd/roachtest/roachtestutil/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ go_library(
1717
"//pkg/cmd/roachtest/option",
1818
"//pkg/cmd/roachtest/test",
1919
"//pkg/kv/kvpb",
20+
"//pkg/roachprod",
2021
"//pkg/roachprod/config",
2122
"//pkg/roachprod/install",
2223
"//pkg/roachprod/logger",

‎pkg/cmd/roachtest/roachtestutil/utils.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
1717
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
18+
"github.com/cockroachdb/cockroach/pkg/roachprod"
1819
"github.com/cockroachdb/cockroach/pkg/roachprod/config"
1920
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
2021
"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
@@ -26,14 +27,13 @@ func SystemInterfaceSystemdUnitName() string {
2627
return install.VirtualClusterLabel(install.SystemInterfaceName, 0)
2728
}
2829

29-
// DefaultPGUrl is a wrapper over ExternalPGUrl that calls it with the arguments
30+
// DefaultPGUrl is a wrapper over roachprod.PgUrl that calls it with the arguments
3031
// that *almost* all roachtests want: single tenant and only a single node.
31-
// This wrapper will also make fixing #63145 in the future easier as we can
32-
// add "password authenticated" to the above.
3332
func DefaultPGUrl(
3433
ctx context.Context, c cluster.Cluster, l *logger.Logger, node option.NodeListOption,
3534
) (string, error) {
36-
pgurl, err := c.ExternalPGUrl(ctx, l, node, "", 0)
35+
opts := roachprod.PGURLOptions{Secure: c.IsSecure()}
36+
pgurl, err := roachprod.PgURL(ctx, l, c.MakeNodes(node), "certs", opts)
3737
if err != nil {
3838
return "", err
3939
}

‎pkg/cmd/roachtest/tests/cluster_to_cluster.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"github.com/cockroachdb/cockroach/pkg/keys"
3838
"github.com/cockroachdb/cockroach/pkg/multitenant/mtinfopb"
3939
"github.com/cockroachdb/cockroach/pkg/roachpb"
40+
"github.com/cockroachdb/cockroach/pkg/roachprod"
4041
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
4142
"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
4243
"github.com/cockroachdb/cockroach/pkg/roachprod/prometheus"
@@ -514,7 +515,7 @@ func (rd *replicationDriver) setupC2C(
514515
srcNode := srcCluster.SeededRandNode(rd.rng)
515516
destNode := dstCluster.SeededRandNode(rd.rng)
516517

517-
addr, err := c.ExternalPGUrl(ctx, t.L(), srcNode, "" /* tenant */, 0 /* sqlInstance */)
518+
addr, err := c.ExternalPGUrl(ctx, t.L(), srcNode, roachprod.PGURLOptions{})
518519
t.L().Printf("Randomly chosen src node %d for gateway with address %s", srcNode, addr)
519520
t.L().Printf("Randomly chosen dst node %d for gateway", destNode)
520521

‎pkg/cmd/roachtest/tests/copyfrom.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
2323
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
2424
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
25+
"github.com/cockroachdb/cockroach/pkg/roachprod"
2526
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
2627
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
2728
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
@@ -154,7 +155,7 @@ func runCopyFromCRDB(ctx context.Context, t test.Test, c cluster.Cluster, sf int
154155
t.Fatal(err)
155156
}
156157
}
157-
urls, err := c.InternalPGUrl(ctx, t.L(), c.Node(1), "" /* tenant */, 0 /* sqlInstance */)
158+
urls, err := c.InternalPGUrl(ctx, t.L(), c.Node(1), roachprod.PGURLOptions{})
158159
require.NoError(t, err)
159160
m := c.NewMonitor(ctx, c.All())
160161
m.Go(func(ctx context.Context) error {

‎pkg/cmd/roachtest/tests/drain.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
2727
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil"
2828
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
29+
"github.com/cockroachdb/cockroach/pkg/roachprod"
2930
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
3031
"github.com/cockroachdb/cockroach/pkg/testutils"
3132
"github.com/cockroachdb/cockroach/pkg/util/httputil"
@@ -248,7 +249,7 @@ func runWarningForConnWait(ctx context.Context, t test.Test, c cluster.Cluster)
248249

249250
prepareCluster(ctx, t, c, drainWaitDuration, connectionWaitDuration, queryWaitDuration)
250251

251-
pgURL, err := c.ExternalPGUrl(ctx, t.L(), c.Node(nodeToDrain), "" /* tenant */, 0 /* sqlInstance */)
252+
pgURL, err := c.ExternalPGUrl(ctx, t.L(), c.Node(nodeToDrain), roachprod.PGURLOptions{})
252253
require.NoError(t, err)
253254
connNoTxn, err := pgx.Connect(ctx, pgURL[0])
254255
require.NoError(t, err)

‎pkg/cmd/roachtest/tests/gossip.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
2828
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
2929
"github.com/cockroachdb/cockroach/pkg/gossip"
30+
"github.com/cockroachdb/cockroach/pkg/roachprod"
3031
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
3132
"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
3233
"github.com/cockroachdb/cockroach/pkg/util"
@@ -460,7 +461,7 @@ SELECT count(replicas)
460461
if i != 1 {
461462
return c.Conn(ctx, l, i)
462463
}
463-
urls, err := c.ExternalPGUrl(ctx, l, c.Node(1), "" /* tenant */, 0 /* sqlInstance */)
464+
urls, err := c.ExternalPGUrl(ctx, l, c.Node(1), roachprod.PGURLOptions{})
464465
if err != nil {
465466
t.Fatal(err)
466467
}

‎pkg/cmd/roachtest/tests/multitenant_distsql.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
2525
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
2626
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
27+
"github.com/cockroachdb/cockroach/pkg/roachprod"
2728
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
2829
"github.com/cockroachdb/cockroach/pkg/util/intsets"
2930
"github.com/stretchr/testify/require"
@@ -198,7 +199,7 @@ func runMultiTenantDistSQL(
198199
if bundle {
199200
// Open bundle and verify its contents
200201
sqlConnCtx := clisqlclient.Context{}
201-
pgURL, err := c.ExternalPGUrl(ctx, t.L(), c.Node(1), tenantName, 0)
202+
pgURL, err := c.ExternalPGUrl(ctx, t.L(), c.Node(1), roachprod.PGURLOptions{VirtualClusterName: tenantName})
202203
require.NoError(t, err)
203204
conn := sqlConnCtx.MakeSQLConn(io.Discard, io.Discard, pgURL[0])
204205
bundles, err := clisqlclient.StmtDiagListBundles(ctx, conn)

‎pkg/cmd/roachtest/tests/multitenant_utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func (tn *tenantNode) start(ctx context.Context, t test.Test, c cluster.Cluster,
201201
extraArgs...,
202202
)
203203

204-
externalUrls, err := c.ExternalPGUrl(ctx, t.L(), c.Node(tn.node), "" /* tenant */, 0 /* sqlInstance */)
204+
externalUrls, err := c.ExternalPGUrl(ctx, t.L(), c.Node(tn.node), roachprod.PGURLOptions{})
205205
require.NoError(t, err)
206206
u, err := url.Parse(externalUrls[0])
207207
require.NoError(t, err)

‎pkg/cmd/roachtest/tests/pg_regress.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
2323
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
2424
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
25+
"github.com/cockroachdb/cockroach/pkg/roachprod"
2526
rperrors "github.com/cockroachdb/cockroach/pkg/roachprod/errors"
2627
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
2728
"github.com/cockroachdb/errors"
@@ -112,7 +113,7 @@ func initPGRegress(ctx context.Context, t test.Test, c cluster.Cluster) {
112113
}
113114
}
114115

115-
urls, err := c.InternalPGUrl(ctx, t.L(), node, "" /* tenant */, 0 /* sqlInstance */)
116+
urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{})
116117
if err != nil {
117118
t.Fatal(err)
118119
}
@@ -149,7 +150,7 @@ func runPGRegress(ctx context.Context, t test.Test, c cluster.Cluster) {
149150
initPGRegress(ctx, t, c)
150151

151152
node := c.Node(1)
152-
urls, err := c.InternalPGUrl(ctx, t.L(), node, "" /* tenant */, 0 /* sqlInstance */)
153+
urls, err := c.InternalPGUrl(ctx, t.L(), node, roachprod.PGURLOptions{})
153154
if err != nil {
154155
t.Fatal(err)
155156
}

‎pkg/cmd/roachtest/tests/tpcdsvec.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
2222
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
2323
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
24+
"github.com/cockroachdb/cockroach/pkg/roachprod"
2425
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
2526
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
2627
"github.com/cockroachdb/cockroach/pkg/workload/tpcds"
@@ -94,7 +95,7 @@ WITH unsafe_restore_incompatible_version;
9495
// We additionally open fresh connections for each query.
9596
setStmtTimeout := fmt.Sprintf("SET statement_timeout='%s';", timeout)
9697
firstNode := c.Node(1)
97-
urls, err := c.ExternalPGUrl(ctx, t.L(), firstNode, "" /* tenant */, 0 /* sqlInstance */)
98+
urls, err := c.ExternalPGUrl(ctx, t.L(), firstNode, roachprod.PGURLOptions{})
9899
if err != nil {
99100
t.Fatal(err)
100101
}

‎pkg/roachprod/install/cluster_synced.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2577,7 +2577,7 @@ func (c *SyncedCluster) pgurls(
25772577
if err != nil {
25782578
return nil, err
25792579
}
2580-
m[node] = c.NodeURL(host, desc.Port, virtualClusterName, desc.ServiceMode)
2580+
m[node] = c.NodeURL(host, desc.Port, virtualClusterName, desc.ServiceMode, AuthRootCert)
25812581
}
25822582
return m, nil
25832583
}

0 commit comments

Comments
 (0)