Skip to content

Commit

Permalink
cli/demo: sanitize the creation of secondary tenants
Browse files Browse the repository at this point in the history 
Prior to this patch, `cockroach demo --multitenant=true --nodes N` was
creating N different secondary tenants, with one SQL server per
tenant.

This was totally besides the point -- to show multi-node scalability,
we want N different SQL servers _for the same tenant_.

This commit fixes that.
Before:
```
       system tenant
         (webui)    http://127.0.0.1:8083/demologin?password=demo36514&username=demo
         (sql)      postgresql://demo:demo36514@127.0.0.1:26260/defaultdb?sslmode=require&sslrootcert=%2Ftmp%2Fdemo1584871889%2Fca.crt
         (sql/jdbc) jdbc:postgresql://127.0.0.1:26260/defaultdb?password=demo36514&sslmode=require&sslrootcert=%2Ftmp%2Fdemo1584871889%2Fca.crt&user=demo
         (sql/unix) postgresql://demo:demo36514@/defaultdb?host=%2Ftmp%2Fdemo1584871889&port=26260

       tenant 1:
         (webui)    https://127.0.0.1:8080/demologin?password=demo36514&username=demo
         (sql)      postgresql://demo:demo36514@127.0.0.1:26257/movr?sslmode=require&sslrootcert=%2Ftmp%2Fdemo1584871889%2Fca-client-tenant.crt
         (sql/jdbc) jdbc:postgresql://127.0.0.1:26257/movr?password=demo36514&sslmode=require&sslrootcert=%2Ftmp%2Fdemo1584871889%2Fca-client-tenant.crt&user=demo

       tenant 2:
         (webui)    https://127.0.0.1:8081/demologin?password=demo36514&username=demo
         (sql)      postgresql://demo:demo36514@127.0.0.1:26258/movr?sslmode=require&sslrootcert=%2Ftmp%2Fdemo1584871889%2Fca-client-tenant.crt
         (sql/jdbc) jdbc:postgresql://127.0.0.1:26258/movr?password=demo36514&sslmode=require&sslrootcert=%2Ftmp%2Fdemo1584871889%2Fca-client-tenant.crt&user=demo

       tenant 3:
...
```

After:
```
     Application tenant:
      (webui)    https://127.0.0.1:8080/demologin?password=demo41322&username=demo
      (sql)      postgresql://demo:demo41322@127.0.0.1:26257/movr?sslmode=require&sslrootcert=%2Ftmp%2Fdemo3364057791%2Fca-client-tenant.crt
      (sql/jdbc) jdbc:postgresql://127.0.0.1:26257/movr?password=demo41322&sslmode=require&sslrootcert=%2Ftmp%2Fdemo3364057791%2Fca-client-tenant.crt&user=demo

     System tenant:
      (webui)    http://127.0.0.1:8083/demologin?password=demo41322&username=demo
      (sql)      postgresql://demo:demo41322@127.0.0.1:26260/defaultdb?sslmode=require&sslrootcert=%2Ftmp%2Fdemo3364057791%2Fca.crt
      (sql/jdbc) jdbc:postgresql://127.0.0.1:26260/defaultdb?password=demo41322&sslmode=require&sslrootcert=%2Ftmp%2Fdemo3364057791%2Fca.crt&user=demo
      (sql/unix) postgresql://demo:demo41322@/defaultdb?host=%2Ftmp%2Fdemo3364057791&port=26260
```

Note: this output will be further simplified once we use a single SQL
listener for multiple tenants; see cockroachdb#92580; and also when we support
using a shared HTTP listener for the demo servers (i.e. when the work
from PR cockroachdb#91744 is extended to `cockroach demo`).

(no release note since this functionality is not yet exposed to end-users)

Release note: None
  • Loading branch information
@knz
knz committed Dec 28, 2022
1 parent 0a3248b commit 4a7b593
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 71 deletions.
1 change: 1 addition & 0 deletions pkg/cli/democluster/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ go_test(
"//pkg/security/securityassets",
"//pkg/security/securitytest",
"//pkg/server",
"//pkg/testutils",
"//pkg/testutils/serverutils/regionlatency",
"//pkg/testutils/skip",
"//pkg/util/leaktest",
Expand Down
136 changes: 69 additions & 67 deletions pkg/cli/democluster/demo_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ type transientCluster struct {
// be connected.
const maxNodeInitTime = 30 * time.Second

// secondaryTenantID is the ID of the secondary tenant to use when
// --multitenant=true.
const secondaryTenantID = 2

// demoOrg is the organization to use to request an evaluation
// license.
const demoOrg = "Cockroach Demo"
Expand Down Expand Up @@ -175,9 +179,12 @@ func NewDemoCluster(
c.httpFirstPort = c.demoCtx.HTTPPort
c.sqlFirstPort = c.demoCtx.SQLPort
if c.demoCtx.Multitenant {
// This allows the first demo tenant to get the desired ports (i.e., those
// configured by --http-port or --sql-port, or the default) without
// conflicting with the system tenant.
// This allows the first secondary tenant server to get the
// desired ports (i.e., those configured by --http-port or
// --sql-port, or the default) without conflicting with the system
// tenant.
// Note: this logic can be removed once we use a single
// listener for HTTP and SQL.
c.httpFirstPort += c.demoCtx.NumNodes
c.sqlFirstPort += c.demoCtx.NumNodes
}
Expand Down Expand Up @@ -374,23 +381,25 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {

c.tenantServers = make([]serverutils.TestTenantInterface, c.demoCtx.NumNodes)
for i := 0; i < c.demoCtx.NumNodes; i++ {
createTenant := i == 0

latencyMap := c.servers[i].Cfg.TestingKnobs.Server.(*server.TestingKnobs).
ContextTestingKnobs.InjectedLatencyOracle
c.infoLog(ctx, "starting tenant node %d", i)
tenantStopper := stop.NewStopper()
tenID := uint64(i + 2)
ts, err := c.servers[i].StartTenant(ctx, base.TestTenantArgs{
// We set the tenant ID to i+2, since tenant 0 is not a tenant, and
// tenant 1 is the system tenant. We also subtract 2 for the "starting"
// SQL/HTTP ports so the first tenant ends up with the desired default
// ports.
TenantName: roachpb.TenantName(fmt.Sprintf("demo-tenant-%d", tenID)),
TenantID: roachpb.MustMakeTenantID(tenID),
DisableCreateTenant: !createTenant,
TenantName: roachpb.TenantName(fmt.Sprintf("demo-tenant-%d", secondaryTenantID)),
TenantID: roachpb.MustMakeTenantID(secondaryTenantID),
Stopper: tenantStopper,
ForceInsecure: c.demoCtx.Insecure,
SSLCertsDir: c.demoDir,
StartingRPCAndSQLPort: c.demoCtx.SQLPort - 2,
StartingHTTPPort: c.demoCtx.HTTPPort - 2,
StartingRPCAndSQLPort: c.demoCtx.SQLPort - secondaryTenantID + i,
StartingHTTPPort: c.demoCtx.HTTPPort - secondaryTenantID + i,
Locality: c.demoCtx.Localities[i],
TestingKnobs: base.TestingKnobs{
Server: &server.TestingKnobs{
Expand All @@ -412,14 +421,14 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
return err
}
c.tenantServers[i] = ts
c.infoLog(ctx, "started tenant %d: %s", i, ts.SQLAddr())
c.infoLog(ctx, "started tenant server %d: %s", i, ts.SQLAddr())

// Propagate the tenant server tags to the initialization
// context, so that the initialization messages below are
// properly annotated in traces.
ctx = ts.AnnotateCtx(ctx)

if !c.demoCtx.Insecure {
if i == 0 && !c.demoCtx.Insecure {
// Set up the demo username and password on each tenant.
ie := ts.DistSQLServer().(*distsql.ServerImpl).ServerConfig.Executor
_, err = ie.Exec(ctx, "tenant-password", nil,
Expand Down Expand Up @@ -1091,36 +1100,33 @@ func (demoCtx *Context) generateCerts(certsDir string) (err error) {
return err
}

for i := 0; i < demoCtx.NumNodes; i++ {
// Create a cert for each tenant.
hostAddrs := []string{
"127.0.0.1",
"::1",
"localhost",
"*.local",
}
tenantID := uint64(i + 2)
pair, err := security.CreateTenantPair(
certsDir,
tenantCAKeyPath,
demoCtx.DefaultKeySize,
demoCtx.DefaultCertLifetime,
tenantID,
hostAddrs,
)
if err != nil {
return err
}
if err := security.WriteTenantPair(certsDir, pair, false /* overwrite */); err != nil {
return err
}
if err := security.CreateTenantSigningPair(
certsDir, demoCtx.DefaultCertLifetime, false /* overwrite */, tenantID,
); err != nil {
return err
}
rootUserScope = append(rootUserScope, roachpb.MustMakeTenantID(tenantID))
// Create a cert for the secondary tenant.
hostAddrs := []string{
"127.0.0.1",
"::1",
"localhost",
"*.local",
}
pair, err := security.CreateTenantPair(
certsDir,
tenantCAKeyPath,
demoCtx.DefaultKeySize,
demoCtx.DefaultCertLifetime,
secondaryTenantID,
hostAddrs,
)
if err != nil {
return err
}
if err := security.WriteTenantPair(certsDir, pair, false /* overwrite */); err != nil {
return err
}
if err := security.CreateTenantSigningPair(
certsDir, demoCtx.DefaultCertLifetime, false /* overwrite */, secondaryTenantID,
); err != nil {
return err
}
rootUserScope = append(rootUserScope, roachpb.MustMakeTenantID(secondaryTenantID))
}
// Create a certificate for the root user. This certificate will be scoped to the
// system tenant and all other tenants created as a part of the demo.
Expand Down Expand Up @@ -1441,9 +1447,6 @@ func (c *transientCluster) GetLocality(nodeID int32) string {
func (c *transientCluster) ListDemoNodes(w, ew io.Writer, justOne bool) {
numNodesLive := 0
// First, list system tenant nodes.
if c.demoCtx.Multitenant {
fmt.Fprintln(w, "system tenant")
}
for i, s := range c.servers {
if s == nil {
continue
Expand All @@ -1460,42 +1463,41 @@ func (c *transientCluster) ListDemoNodes(w, ew io.Writer, justOne bool) {
// the demo.
fmt.Fprintf(w, "node %d:\n", nodeID)
}
uiURL := s.Cfg.AdminURL()
sqlURL, err := c.getNetworkURLForServer(context.Background(), i,
false /* includeAppName */, false /* forSecondaryTenant */)
if err != nil {
fmt.Fprintln(ew, errors.Wrap(err, "retrieving network URL"))
} else {
c.printURLs(w, ew, sqlURL, uiURL, c.sockForServer(i))
}
fmt.Fprintln(w)
}
// Print the SQL address of each tenant if in MT mode.
if c.demoCtx.Multitenant {
for i := range c.servers {
fmt.Fprintf(w, "tenant %d:\n", i+1)
uiURLstr := c.tenantServers[i].AdminURL()
uiURL, err := url.Parse(uiURLstr)
if c.demoCtx.Multitenant {
fmt.Fprintln(w, " Application tenant:")
tenantUiURLstr := c.tenantServers[i].AdminURL()
tenantUiURL, err := url.Parse(tenantUiURLstr)
if err != nil {
fmt.Fprintln(ew, errors.Wrap(err, "retrieving tenant network URL"))
fmt.Fprintln(ew, errors.Wrap(err, "retrieving network URL for tenant server"))
} else {
sqlURL, err := c.getNetworkURLForServer(context.Background(), i,
tenantSqlURL, err := c.getNetworkURLForServer(context.Background(), i,
false /* includeAppName */, true /* forSecondaryTenant */)
if err != nil {
fmt.Fprintln(ew, errors.Wrap(err, "retrieving tenant network URL"))
fmt.Fprintln(ew, errors.Wrap(err, "retrieving network URL for tenant server"))
} else {
// The unix socket is currently not defined for secondary
// tenant servers.
//
// NB: it will become defined once we use a single SQL
// listener for all tenants; after which this code can be
// simplified.
socket := unixSocketDetails{}
c.printURLs(w, ew, sqlURL, uiURL, socket)
tenantSocket := unixSocketDetails{}
c.printURLs(w, ew, tenantSqlURL, tenantUiURL, tenantSocket)
}
}
fmt.Fprintln(w)
fmt.Fprintln(w, " System tenant:")
}
// Connection parameters for the system tenant follow.
uiURL := s.Cfg.AdminURL()
sqlURL, err := c.getNetworkURLForServer(context.Background(), i,
false /* includeAppName */, false /* forSecondaryTenant */)
if err != nil {
fmt.Fprintln(ew, errors.Wrap(err, "retrieving network URL"))
} else {
c.printURLs(w, ew, sqlURL, uiURL, c.sockForServer(i))
}
fmt.Fprintln(w)
}

if numNodesLive == 0 {
Expand All @@ -1520,11 +1522,11 @@ func (c *transientCluster) printURLs(
uiURL.Path = server.DemoLoginPath
uiURL.RawQuery = pwauth.Encode()
}
fmt.Fprintln(w, " (webui) ", uiURL)
fmt.Fprintln(w, " (sql) ", sqlURL.ToPQ())
fmt.Fprintln(w, " (sql/jdbc)", sqlURL.ToJDBC())
fmt.Fprintln(w, " (webui) ", uiURL)
fmt.Fprintln(w, " (sql) ", sqlURL.ToPQ())
fmt.Fprintln(w, " (sql/jdbc)", sqlURL.ToJDBC())
if socket.exists() {
fmt.Fprintln(w, " (sql/unix)", socket)
fmt.Fprintln(w, " (sql/unix)", socket)
}
}

Expand Down
9 changes: 5 additions & 4 deletions pkg/cli/democluster/demo_cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/security/securityassets"
"github.com/cockroachdb/cockroach/pkg/security/securitytest"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils/regionlatency"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
Expand Down Expand Up @@ -293,9 +294,9 @@ func TestTransientClusterMultitenant(t *testing.T) {

require.NoError(t, c.Start(ctx))

for i := 0; i < demoCtx.NumNodes; i++ {
url, err := c.getNetworkURLForServer(ctx, i,
true /* includeAppName */, true /* isTenant */)
testutils.RunTrueAndFalse(t, "forSecondaryTenant", func(t *testing.T, forSecondaryTenant bool) {
url, err := c.getNetworkURLForServer(ctx, 0,
true /* includeAppName */, forSecondaryTenant)
require.NoError(t, err)
sqlConnCtx := clisqlclient.Context{}
conn := sqlConnCtx.MakeSQLConn(io.Discard, io.Discard, url.ToPQ().String())
Expand All @@ -307,5 +308,5 @@ func TestTransientClusterMultitenant(t *testing.T) {

// Create a table on each tenant to make sure that the tenants are separate.
require.NoError(t, conn.Exec(context.Background(), "CREATE TABLE a (a int PRIMARY KEY)"))
}
})
}

0 comments on commit 4a7b593

Please sign in to comment.