Skip to content

Commit

Permalink
server,demo: sanitize tenant server cfg for demo
Browse files Browse the repository at this point in the history 
Prior to this patch, the port numbering for the servers started by the
server controller inside `cockroach demo` was difficult to grok.

This patch fixes it, and restores the "default" port numbers
for the application tenant.

It also fixes the display of the UI connection strings.

For example:
```
...
   - Connection parameters:
      (webui)    http://127.0.0.1:8080/demologin?password=demo60684&tenant_name=demo-tenant&username=demo
...
```

Release note: None
  • Loading branch information
@knz
knz committed Jan 6, 2023
1 parent f11f231 commit fa71be3
Show file tree
Hide file tree
Showing 9 changed files with 184 additions and 127 deletions.
9 changes: 9 additions & 0 deletions pkg/base/test_server_args.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ type TestServerArgs struct {
// DisableTLSForHTTP if set, disables TLS for the HTTP interface.
DisableTLSForHTTP bool

// SecondaryTenantPortOffset if non-zero forces the network addresses
// generated for servers started by the serverController to be offset
// from the base addressed by the specified amount.
SecondaryTenantPortOffset int

// SecondaryTenantKnobs contains the testing knobs to use
// for tenant servers started by the serverController.
SecondaryTenantKnobs TestingKnobs

// JoinAddr is the address of a node we are joining.
//
// If left empty and the TestServer is being added to a nonempty cluster, this
Expand Down
89 changes: 60 additions & 29 deletions pkg/cli/democluster/demo_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,9 @@ func NewDemoCluster(
// tenant.
// Note: this logic can be removed once we use a single
// listener for HTTP and SQL.
c.httpFirstPort += c.demoCtx.NumNodes
if !c.demoCtx.InProcessTenant {
c.httpFirstPort += c.demoCtx.NumNodes
}
c.sqlFirstPort += c.demoCtx.NumNodes
c.rpcFirstPort += c.demoCtx.NumNodes
}
Expand Down Expand Up @@ -419,23 +421,16 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
latencyMap := c.servers[i].Cfg.TestingKnobs.Server.(*server.TestingKnobs).
ContextTestingKnobs.InjectedLatencyOracle
c.infoLog(ctx, "starting tenant node %d", i)
tenantStopper := stop.NewStopper()
ts, err := c.servers[i].StartTenant(ctx, base.TestTenantArgs{

args := 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.
DisableCreateTenant: !createTenant,
TenantName: roachpb.TenantName(fmt.Sprintf("demo-tenant-%d", secondaryTenantID)),
TenantID: roachpb.MustMakeTenantID(secondaryTenantID),
Stopper: tenantStopper,
ForceInsecure: c.demoCtx.Insecure,
SSLCertsDir: c.demoDir,
DisableTLSForHTTP: true,
EnableDemoLoginEndpoint: true,
StartingRPCAndSQLPort: c.demoCtx.SQLPort - secondaryTenantID + i,
StartingHTTPPort: c.demoCtx.HTTPPort - secondaryTenantID + i,
Locality: c.demoCtx.Localities[i],
DisableCreateTenant: !createTenant,
TenantName: roachpb.TenantName("demo-tenant"),
TenantID: roachpb.MustMakeTenantID(secondaryTenantID),
InProcessTenant: c.demoCtx.InProcessTenant,
TestingKnobs: base.TestingKnobs{
Server: &server.TestingKnobs{
ContextTestingKnobs: rpc.ContextTestingKnobs{
Expand All @@ -444,15 +439,33 @@ func (c *transientCluster) Start(ctx context.Context) (err error) {
},
},
},
InProcessTenant: c.demoCtx.InProcessTenant,
})
c.stopper.AddCloser(stop.CloserFn(func() {
stopCtx := context.Background()
if ts != nil {
stopCtx = ts.AnnotateCtx(stopCtx)
}
tenantStopper.Stop(stopCtx)
}))
}

var tenantStopper *stop.Stopper
if !c.demoCtx.InProcessTenant {
tenantStopper = stop.NewStopper()
args.Stopper = tenantStopper
args.ForceInsecure = c.demoCtx.Insecure
args.SSLCertsDir = c.demoDir
args.DisableTLSForHTTP = true
args.EnableDemoLoginEndpoint = true
args.StartingRPCAndSQLPort = c.demoCtx.SQLPort - secondaryTenantID + i
args.StartingHTTPPort = c.demoCtx.HTTPPort - secondaryTenantID + i
args.Locality = c.demoCtx.Localities[i]
}

ts, err := c.servers[i].StartTenant(ctx, args)
if !c.demoCtx.InProcessTenant {
// InProcessTenant means that the server controller is
// already taking care of shutdown.
c.stopper.AddCloser(stop.CloserFn(func() {
stopCtx := context.Background()
if ts != nil {
stopCtx = ts.AnnotateCtx(stopCtx)
}
tenantStopper.Stop(stopCtx)
}))
}
if err != nil {
return err
}
Expand Down Expand Up @@ -601,10 +614,14 @@ func (c *transientCluster) createAndAddNode(
// The latency map will be populated after all servers have
// started listening on RPC, and before they proceed with their
// startup routine.
serverKnobs.ContextTestingKnobs = rpc.ContextTestingKnobs{
rpcKnobs := rpc.ContextTestingKnobs{
InjectedLatencyOracle: regionlatency.MakeAddrMap(),
InjectedLatencyEnabled: c.latencyEnabled.Get,
}
serverKnobs.ContextTestingKnobs = rpcKnobs
args.SecondaryTenantKnobs.Server = &server.TestingKnobs{
ContextTestingKnobs: rpcKnobs,
}
}

// Create the server instance. This also registers the in-memory store
Expand Down Expand Up @@ -806,6 +823,7 @@ func (demoCtx *Context) testServerArgsForTransientCluster(
// Demo clusters by default will create their own tenants, so we
// don't need to create them here.
DisableDefaultTestTenant: true,

Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
StickyEngineRegistry: stickyEngineRegistry,
Expand Down Expand Up @@ -839,6 +857,15 @@ func (demoCtx *Context) testServerArgsForTransientCluster(
args.Addr = fmt.Sprintf("127.0.0.1:%d", rpcPort)
args.SQLAddr = fmt.Sprintf("127.0.0.1:%d", sqlPort)
args.HTTPAddr = fmt.Sprintf("127.0.0.1:%d", httpPort)

if demoCtx.InProcessTenant {
// The code in NewDemoCluster put the KV ports higher
// so we need to subtract the number of nodes to get
// back to the "good" ports.
// We reduce NumNodes by 1 because the server controller
// uses 1-based indexing for servers.
args.SecondaryTenantPortOffset = -(demoCtx.NumNodes + 1)
}
}

if demoCtx.Localities != nil {
Expand Down Expand Up @@ -1793,6 +1820,11 @@ func (c *transientCluster) ListDemoNodes(w, ew io.Writer, justOne, verbose bool)
// Connection parameters for the system tenant follow.

uiURL := s.Cfg.AdminURL()
if q := uiURL.Query(); c.demoCtx.Multitenant && c.demoCtx.InProcessTenant && !q.Has(server.TenantNameParamInQueryURL) {
q.Add(server.TenantNameParamInQueryURL, catconstants.SystemTenantName)
uiURL.RawQuery = q.Encode()
}

sqlURL, err := c.getNetworkURLForServer(context.Background(), i,
false /* includeAppName */, false /* forSecondaryTenant */)
if err != nil {
Expand Down Expand Up @@ -1824,12 +1856,11 @@ func (c *transientCluster) printURLs(
// Print node ID and web UI URL. Embed the autologin feature inside the URL.
// We avoid printing those when insecure, as the autologin path is not available
// in that case.
pwauth := url.Values{
"username": []string{c.adminUser.Normalized()},
"password": []string{c.adminPassword},
}
q := uiURL.Query()
q.Add("username", c.adminUser.Normalized())
q.Add("password", c.adminPassword)
uiURL.Path = server.DemoLoginPath
uiURL.RawQuery = pwauth.Encode()
uiURL.RawQuery = q.Encode()
}
fmt.Fprintln(w, " (webui) ", uiURL)

Expand Down
47 changes: 25 additions & 22 deletions pkg/cli/democluster/demo_cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,18 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
sqlPoolMemorySize: 2 << 10,
cacheSize: 1 << 10,
expected: base.TestServerArgs{
DisableDefaultTestTenant: true,
PartOfCluster: true,
JoinAddr: "127.0.0.1",
DisableTLSForHTTP: true,
Addr: "127.0.0.1:7890",
SQLAddr: "127.0.0.1:1234",
HTTPAddr: "127.0.0.1:4567",
SQLMemoryPoolSize: 2 << 10,
CacheSize: 1 << 10,
NoAutoInitializeCluster: true,
EnableDemoLoginEndpoint: true,
DisableDefaultTestTenant: true,
PartOfCluster: true,
JoinAddr: "127.0.0.1",
DisableTLSForHTTP: true,
Addr: "127.0.0.1:7890",
SQLAddr: "127.0.0.1:1234",
HTTPAddr: "127.0.0.1:4567",
SecondaryTenantPortOffset: -2,
SQLMemoryPoolSize: 2 << 10,
CacheSize: 1 << 10,
NoAutoInitializeCluster: true,
EnableDemoLoginEndpoint: true,
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
StickyEngineRegistry: stickyEnginesRegistry,
Expand All @@ -91,17 +92,18 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
sqlPoolMemorySize: 4 << 10,
cacheSize: 4 << 10,
expected: base.TestServerArgs{
DisableDefaultTestTenant: true,
PartOfCluster: true,
JoinAddr: "127.0.0.1",
Addr: "127.0.0.1:7892",
SQLAddr: "127.0.0.1:1236",
HTTPAddr: "127.0.0.1:4569",
DisableTLSForHTTP: true,
SQLMemoryPoolSize: 4 << 10,
CacheSize: 4 << 10,
NoAutoInitializeCluster: true,
EnableDemoLoginEndpoint: true,
DisableDefaultTestTenant: true,
PartOfCluster: true,
JoinAddr: "127.0.0.1",
Addr: "127.0.0.1:7892",
SQLAddr: "127.0.0.1:1236",
HTTPAddr: "127.0.0.1:4569",
SecondaryTenantPortOffset: -2,
DisableTLSForHTTP: true,
SQLMemoryPoolSize: 4 << 10,
CacheSize: 4 << 10,
NoAutoInitializeCluster: true,
EnableDemoLoginEndpoint: true,
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
StickyEngineRegistry: stickyEnginesRegistry,
Expand Down Expand Up @@ -257,6 +259,7 @@ func TestTransientClusterMultitenant(t *testing.T) {

// This test is too slow to complete under the race detector, sometimes.
skip.UnderRace(t)
skip.UnderStress(t)

demoCtx := newDemoCtx()
// Set up an empty 3-node cluster with tenants on each node.
Expand Down
10 changes: 5 additions & 5 deletions pkg/cli/interactive_tests/test_demo.tcl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ eexpect "sslmode=disable"
eexpect "System tenant"
eexpect "(webui)"
eexpect "http://"
eexpect ":8081"
eexpect ":8080"
eexpect "(sql)"
eexpect "root@"
eexpect ":26258"
Expand Down Expand Up @@ -133,7 +133,7 @@ eexpect "sslrootcert="

eexpect "System tenant"
eexpect "(webui)"
eexpect "http://127.0.0.1:8081/demologin"
eexpect "http://127.0.0.1:8080/demologin"
eexpect "(sql)"
eexpect "postgresql://demo:"
eexpect ":26258"
Expand Down Expand Up @@ -261,11 +261,11 @@ eexpect "defaultdb>"
# Show the URLs.
send "\\demo ls\r"
eexpect "http://"
eexpect ":8003"
eexpect ":8000"
eexpect "http://"
eexpect ":8004"
eexpect ":8001"
eexpect "http://"
eexpect ":8005"
eexpect ":8002"
eexpect "defaultdb>"

send_eof
Expand Down
8 changes: 4 additions & 4 deletions pkg/cli/interactive_tests/test_demo_cli_integration.tcl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ eexpect ":/# "
# Check that the cookies work.
set pyfile [file join [file dirname $argv0] test_auth_cookie.py]

send "$python $pyfile cookie_system.txt 'http://localhost:8081/_admin/v1/users'\r"
send "$python $pyfile cookie_system.txt 'http://localhost:8080/_admin/v1/users?tenant_name=system'\r"
eexpect "username"
eexpect "demo"
send "$python $pyfile cookie_app.txt 'http://localhost:8080/_admin/v1/users'\r"
send "$python $pyfile cookie_app.txt 'http://localhost:8080/_admin/v1/users?tenant_name=demo-tenant'\r"
eexpect "username"
eexpect "demo"
end_test
Expand All @@ -111,10 +111,10 @@ eexpect "defaultdb>"

set spawn_id $shell_spawn_id

send "$python $pyfile cookie_system.txt 'http://localhost:8081/_admin/v1/users'\r"
send "$python $pyfile cookie_system.txt 'http://localhost:8080/_admin/v1/users?tenant_name=system'\r"
eexpect "username"
eexpect "demo"
send "$python $pyfile cookie_app.txt 'http://localhost:8080/_admin/v1/users'\r"
send "$python $pyfile cookie_app.txt 'http://localhost:8080/_admin/v1/users?tenant_name=demo-tenant'\r"
eexpect "username"
eexpect "demo"
end_test
Expand Down
4 changes: 4 additions & 0 deletions pkg/server/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ type BaseConfig struct {
// TestingKnobs is used for internal test controls only.
TestingKnobs base.TestingKnobs

// SecondaryTenantKnobs contains the testing knobs to use
// for tenant servers started by the serverController.
SecondaryTenantKnobs base.TestingKnobs

// TestingInsecureWebAccess enables uses of the HTTP and UI
// endpoints without a valid authentication token. This should be
// used only in tests what want a secure cluster with RPC
Expand Down
Loading

0 comments on commit fa71be3

Please sign in to comment.