From 9f4c44302532a26b380be9efc2d60cd95acc582c Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Tue, 27 Dec 2022 17:36:46 +0100 Subject: [PATCH] cli/demo: sanitize the creation of secondary tenants 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 #92580; and also when we support using a shared HTTP listener for the demo servers (i.e. when the work from PR #91744 is extended to `cockroach demo`). (no release note since this functionality is not yet exposed to end-users) Release note: None --- pkg/cli/democluster/demo_cluster.go | 136 ++++++++++++++-------------- 1 file changed, 69 insertions(+), 67 deletions(-) diff --git a/pkg/cli/democluster/demo_cluster.go b/pkg/cli/democluster/demo_cluster.go index cd47579f18be..7791f6d5e75d 100644 --- a/pkg/cli/democluster/demo_cluster.go +++ b/pkg/cli/democluster/demo_cluster.go @@ -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" @@ -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 } @@ -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{ @@ -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, @@ -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. @@ -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 @@ -1460,29 +1463,17 @@ 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. @@ -1490,12 +1481,23 @@ func (c *transientCluster) ListDemoNodes(w, ew io.Writer, justOne bool) { // 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 { @@ -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) } }