Merge #63515

63515: cli: add notice about --global being experimental on demo startup r=knz a=otan

Refs: #63469

See individual commits for details. Intended for backport for v21.1.1.

Co-authored-by: Oliver Tan <[email protected]>

pkg/cli/demo.go

@@ -293,12 +293,19 @@ func runDemo(cmd *cobra.Command, gen workload.Generator) (err error) {
 	checkInteractive(cmdIn)
 
 	if cliCtx.isInteractive {
-		fmt.Printf(`#
+		printfUnlessEmbedded(`#
 # Welcome to the CockroachDB demo database!
 #
 # You are connected to a temporary, in-memory CockroachDB cluster of %d node%s.
 `, demoCtx.nodes, util.Pluralize(int64(demoCtx.nodes)))
 
+		if demoCtx.simulateLatency {
+			printfUnlessEmbedded(
+				"#\n# WARNING: the use of --%s is experimental. Some features may not work as expected.\n",
+				cliflags.Global.Name,
+			)
+		}
+
 		// Only print details about the telemetry configuration if the
 		// user has control over it.
 		if demoCtx.disableTelemetry {

pkg/cli/demo_cluster.go

@@ -627,6 +627,9 @@ func (c *transientCluster) cleanup(ctx context.Context) {
 // DrainAndShutdown will gracefully attempt to drain a node in the cluster, and
 // then shut it down.
 func (c *transientCluster) DrainAndShutdown(nodeID roachpb.NodeID) error {
+	if demoCtx.simulateLatency {
+		return errors.Errorf("shutting down nodes is not supported in --%s configurations", cliflags.Global.Name)
+	}
 	nodeIndex := int(nodeID - 1)
 
 	if nodeIndex < 0 || nodeIndex >= len(c.servers) {
@@ -748,6 +751,9 @@ func (c *transientCluster) RestartNode(nodeID roachpb.NodeID) error {
 	// TODO(#42243): re-compute the latency mapping.
 	// TODO(...): the RPC address of the first server may not be available
 	// if the first server was shut down.
+	if demoCtx.simulateLatency {
+		return errors.Errorf("restarting nodes is not supported in --%s configurations", cliflags.Global.Name)
+	}
 	args := testServerArgsForTransientCluster(c.sockForServer(nodeID), nodeID,
 		c.firstServer.ServingRPCAddr(), c.demoDir,
 		c.sqlFirstPort, c.httpFirstPort, c.stickyEngineRegistry)
@@ -783,6 +789,11 @@ func (c *transientCluster) RestartNode(nodeID roachpb.NodeID) error {
 // This function uses RestartNode to perform the actual node
 // starting.
 func (c *transientCluster) AddNode(ctx context.Context, localityString string) error {
+	// TODO(#42243): re-compute the latency mapping for this to work.
+	if demoCtx.simulateLatency {
+		return errors.Errorf("adding nodes is not supported in --%s configurations", cliflags.Global.Name)
+	}
+
 	// '\demo add' accepts both strings that are quoted and not quoted. To properly make use of
 	// quoted strings, strip off the quotes.  Before we do that though, make sure that the quotes match,
 	// or that there aren't any quotes in the string.

pkg/cli/interactive_tests/test_demo_global.tcl

@@ -18,6 +18,14 @@ send "SELECT region, zones FROM \[SHOW REGIONS FROM CLUSTER\] ORDER BY region;\r
 eexpect "  europe-west1 | {b,c,d}"
 eexpect "  us-east1     | {b,c,d}"
 eexpect "  us-west1     | {a,b,c}"
+
+# Test we cannot add or restart nodes.
+send "\\demo add region=europe-west1\r"
+eexpect "adding nodes is not supported in --global configurations"
+eexpect "defaultdb>"
+
+send "\\demo shutdown 3\r"
+eexpect "shutting down nodes is not supported in --global configurations"
 eexpect "defaultdb>"
 
 interrupt

pkg/cli/sql.go

@@ -564,11 +564,6 @@ func (c *cliState) handleDemoAddNode(cmd []string, nextState, errState cliStateE
 		return c.internalServerError(errState, fmt.Errorf("bad call to handleDemoAddNode"))
 	}
 
-	if demoCtx.simulateLatency {
-		fmt.Printf("add command is not supported in --global configurations\n")
-		return nextState
-	}
-
 	if err := demoCtx.transientCluster.AddNode(context.Background(), cmd[1]); err != nil {
 		return c.internalServerError(errState, err)
 	}
@@ -1801,3 +1796,9 @@ func printlnUnlessEmbedded(args ...interface{}) {
 		fmt.Println(args...)
 	}
 }
+
+func printfUnlessEmbedded(f string, args ...interface{}) {
+	if !sqlCtx.embeddedMode {
+		fmt.Printf(f, args...)
+	}
+}