cli: new command 'start-single-node'

Suggested/requested by @bdarnell:

> There are currently two ways to initialize a cluster: Starting the
  first node without a --join flag, or starting all nodes with --join
  flags and running a separate cockroach init command. This redundancy
  is confusing (our docs use both methods without explaining the
  difference) and it's easy to accidentally initialize a new cluster
  by restarting your first node with the wrong flags. (This could have
  catastrophic consequences in 1.1 because data from the two clusters
  could get mixed. We've added safeguards against this in 2.0 but it's
  still an easy way to break things.)
> I think that we should deprecate this implicit initialization mode
  and make the --join flag mandatory when starting a node (and
  therefore the init command would be mandatory too). To avoid adding
  too much complexity to single-node clusters, I propose a new
  `cockroach start-single-node` command to perform the start and init
  combination that is today implicit in the absence of a --join flag.
> This would have a side benefit that we would know when the user
  intends to keep the cluster as a single node instead of adding more
  later. The `start-single-node` command could therefore set the
  replication factor to 1 to disable the "underreplicated ranges"
  warning.

This patch implements the new `start-single-node` command as follows:

- the new command does not accept `--join`.
- it attempts to set the replication factor to 1 upon starting up.

Meanwhile `cockroach start` is changed to produce a deprecation
warning if `--join` is not specified.

Release note (cli change): a new command `cockroach start-single-node`
is introduced to start single-node clusters with replication disabled.

Release note (cli change): using `cockroach start` without `--join` is
now deprecated and this mode of execution will be removed in a later
version of CockroachDB. Consider using `cockroach start-single-node`
instead or combine `cockroach start` with `cockroach init`.

pkg/ccl/cliccl/start.go

@@ -21,12 +21,14 @@ import (
 var storeEncryptionSpecs baseccl.StoreEncryptionSpecList
 
 func init() {
- cli.VarFlag(cli.StartCmd.Flags(), &storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption)
+ for _, cmd := range cli.StartCmds {
+ cli.VarFlag(cmd.Flags(), &storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption)
 
- // Add a new pre-run command to match encryption specs to store specs.
- cli.AddPersistentPreRunE(cli.StartCmd, func(cmd *cobra.Command, _ []string) error {
- return populateStoreSpecsEncryption()
- })
+ // Add a new pre-run command to match encryption specs to store specs.
+ cli.AddPersistentPreRunE(cmd, func(cmd *cobra.Command, _ []string) error {
+ return populateStoreSpecsEncryption()
+ })
+ }
 }
 
 // populateStoreSpecsEncryption is a PreRun hook that matches store encryption specs with the

pkg/cli/cli.go

@@ -154,7 +154,8 @@ func init() {
  })
 
  cockroachCmd.AddCommand(
- StartCmd,
+ startCmd,
+ startSingleNodeCmd,
  initCmd,
  certCmd,
  quitCmd,

pkg/cli/cli_test.go

@@ -1766,23 +1766,24 @@ func TestFlagUsage(t *testing.T) {
  cockroach [command]
 
 Available Commands:
- start start a node
- init initialize a cluster
- cert create ca, node, and client certs
- quit drain and shutdown node
-
- sql open a sql shell
- user get, set, list and remove users
- zone get, set, list and remove zones
- node list, inspect or remove nodes
- dump dump sql tables
-
- demo open a demo sql shell
- gen generate auxiliary files
- version output version information
- debug debugging commands
- sqlfmt format SQL statements
- help Help about any command
+ start start a node
+ start-single-node start a single-node cluster
+ init initialize a cluster
+ cert create ca, node, and client certs
+ quit drain and shutdown node
+
+ sql open a sql shell
+ user get, set, list and remove users
+ zone get, set, list and remove zones
+ node list, inspect or remove nodes
+ dump dump sql tables
+
+ demo open a demo sql shell
+ gen generate auxiliary files
+ version output version information
+ debug debugging commands
+ sqlfmt format SQL statements
+ help Help about any command
 
 Flags:
  -h, --help help for cockroach

pkg/cli/flags.go

@@ -207,11 +207,13 @@ func init() {
  return setDefaultStderrVerbosity(cmd, log.Severity_WARNING)
  }
 
- // Add a pre-run command for `start`.
- AddPersistentPreRunE(StartCmd, func(cmd *cobra.Command, _ []string) error {
- extraServerFlagInit()
- return setDefaultStderrVerbosity(cmd, log.Severity_INFO)
- })
+ // Add a pre-run command for `start` and `start-single-node`.
+ for _, cmd := range StartCmds {
+ AddPersistentPreRunE(cmd, func(cmd *cobra.Command, _ []string) error {
+ extraServerFlagInit()
+ return setDefaultStderrVerbosity(cmd, log.Severity_INFO)
+ })
+ }
 
  // Map any flags registered in the standard "flag" package into the
  // top-level cockroach command.
@@ -254,8 +256,8 @@ func init() {
 
  // Security flags.
 
- {
- f := StartCmd.Flags()
+ for _, cmd := range StartCmds {
+ f := cmd.Flags()
 
  // Server flags.
  VarFlag(f, addrSetter{&startCtx.serverListenAddr, &serverListenPort}, cliflags.ListenAddr)
@@ -301,8 +303,15 @@ func init() {
  // variables, but share the same default.
  StringFlag(f, &startCtx.serverSSLCertsDir, cliflags.ServerCertsDir, startCtx.serverSSLCertsDir)
 
- // Cluster joining flags.
+ // Cluster joining flags. We need to enable this both for 'start'
+ // and 'start-single-node' although the latter does not support
+ // --join, because it delegates its logic to that of 'start', and
+ // 'start' will check that the flag is properly defined and set.
  VarFlag(f, &serverCfg.JoinList, cliflags.Join)
+ // However we do hide it from help for 'start-single-node'.
+ if cmd == startSingleNodeCmd {
+ _ = f.MarkHidden(cliflags.Join.Name)
+ }
 
  // Engine flags.
  VarFlag(f, cacheSizeValue, cliflags.Cache)
@@ -354,7 +363,7 @@ func init() {
  genHAProxyCmd,
  quitCmd,
  sqlShellCmd,
- /* StartCmd is covered above */
+ /* StartCmds are covered above */
  }
  clientCmds = append(clientCmds, userCmds...)
  clientCmds = append(clientCmds, zoneCmds...)

pkg/cli/flags_test.go

@@ -63,7 +63,7 @@ func TestCacheFlagValue(t *testing.T) {
  // Avoid leaking configuration changes after the test ends.
  defer initCLIDefaults()
 
- f := StartCmd.Flags()
+ f := startCmd.Flags()
  args := []string{"--cache", "100MB"}
  if err := f.Parse(args); err != nil {
  t.Fatal(err)
@@ -81,7 +81,7 @@ func TestSQLMemoryPoolFlagValue(t *testing.T) {
  // Avoid leaking configuration changes after the test ends.
  defer initCLIDefaults()
 
- f := StartCmd.Flags()
+ f := startCmd.Flags()
 
  // Check absolute values.
  testCases := []struct {
@@ -128,7 +128,7 @@ func TestClockOffsetFlagValue(t *testing.T) {
  // Avoid leaking configuration changes after the tests end.
  defer initCLIDefaults()
 
- f := StartCmd.Flags()
+ f := startCmd.Flags()
  testData := []struct {
  args []string
  expected time.Duration
@@ -155,7 +155,7 @@ func TestServerConnSettings(t *testing.T) {
  // Avoid leaking configuration changes after the tests end.
  defer initCLIDefaults()
 
- f := StartCmd.Flags()
+ f := startCmd.Flags()
  testData := []struct {
  args []string
  expectedAddr string
@@ -279,7 +279,7 @@ func TestHttpHostFlagValue(t *testing.T) {
  // Avoid leaking configuration changes after the tests end.
  defer initCLIDefaults()
 
- f := StartCmd.Flags()
+ f := startCmd.Flags()
  testData := []struct {
  args []string
  expected string

pkg/cli/interactive_tests/common.tcl

@@ -89,7 +89,7 @@ proc send_eof {} {
 proc start_server {argv} {
  report "BEGIN START SERVER"
  system "mkfifo pid_fifo || true;
- $argv start --insecure --pid-file=pid_fifo --background -s=path=logs/db >>logs/expect-cmd.log 2>&1 &
+ $argv start-single-node --insecure --pid-file=pid_fifo --background -s=path=logs/db >>logs/expect-cmd.log 2>&1 &
  cat pid_fifo > server_pid"
  report "START SERVER DONE"
 }

pkg/cli/interactive_tests/test_audit_log.tcl

@@ -62,7 +62,7 @@ stop_server $argv
 
 start_test "Check that audit logging works even with a custom directory"
 # Start a server with a custom log
-system "$argv start --insecure --pid-file=pid_fifo --background -s=path=logs/db --sql-audit-dir=logs/db/audit-new >>logs/expect-cmd.log 2>&1 & cat pid_fifo > server_pid"
+system "$argv start-single-node --insecure --pid-file=pid_fifo --background -s=path=logs/db --sql-audit-dir=logs/db/audit-new >>logs/expect-cmd.log 2>&1 & cat pid_fifo > server_pid"
 
 set logfile logs/db/audit-new/cockroach-sql-audit.log
 

pkg/cli/interactive_tests/test_error_hints.tcl

@@ -43,7 +43,7 @@ end_test
 # Check what happens when attempting to connect securely to an
 # insecure server.
 
-send "$argv start --insecure\r"
+send "$argv start-single-node --insecure\r"
 eexpect "initialized new cluster"
 
 spawn /bin/bash
@@ -73,7 +73,7 @@ interrupt
 interrupt
 eexpect ":/# "
 
-send "$argv start --certs-dir=$certs_dir\r"
+send "$argv start-single-node --listen-addr=localhost --certs-dir=$certs_dir\r"
 eexpect "restarted pre-existing node"
 
 set spawn_id $client_spawn_id

pkg/cli/interactive_tests/test_extern_dir.tcl

@@ -11,14 +11,14 @@ eexpect ":/# "
 
 start_test "Check that non-absolute external-io-dir rejected"
 
-send "$argv start --insecure --store=$storedir --external-io-dir=blah\r"
+send "$argv start-single-node --insecure --store=$storedir --external-io-dir=blah\r"
 eexpect "external-io-dir path must be absolute"
 
 end_test
 
 start_test "Check disabling external IO explicitly"
 
-send "$argv start --insecure --store=$storedir --external-io-dir=disabled\r"
+send "$argv start-single-node --insecure --store=$storedir --external-io-dir=disabled\r"
 eexpect "external I/O path: <disabled>"
 interrupt
 eexpect "shutdown completed"
@@ -27,7 +27,7 @@ end_test
 
 start_test "Check setting external IO explicitly"
 
-send "$argv start --insecure --store=$storedir --external-io-dir=$externdir\r"
+send "$argv start-single-node --insecure --store=$storedir --external-io-dir=$externdir\r"
 eexpect "external I/O path: $externdir"
 interrupt
 eexpect "shutdown completed"
@@ -36,7 +36,7 @@ end_test
 
 start_test "Check implicit external I/O dir under store dir"
 
-send "$argv start --insecure --store=$storedir\r"
+send "$argv start-single-node --insecure --store=$storedir\r"
 eexpect "external I/O path: $env(HOME)/$storedir/extern"
 interrupt
 eexpect "shutdown completed"

pkg/cli/interactive_tests/test_flags.tcl

@@ -7,28 +7,28 @@ send "PS1=':''/# '\r"
 eexpect ":/# "
 
 start_test "Check that --max-disk-temp-storage works."
-send "$argv start --insecure --store=path=mystore --max-disk-temp-storage=10GiB\r"
+send "$argv start-single-node --insecure --store=path=mystore --max-disk-temp-storage=10GiB\r"
 eexpect "node starting"
 interrupt
 eexpect ":/# "
 end_test
 
 start_test "Check that --max-disk-temp-storage can be expressed as a percentage."
-send "$argv start --insecure --store=path=mystore --max-disk-temp-storage=10%\r"
+send "$argv start-single-node --insecure --store=path=mystore --max-disk-temp-storage=10%\r"
 eexpect "node starting"
 interrupt
 eexpect ":/# "
 end_test
 
 start_test "Check that --max-disk-temp-storage percentage works when the store is in-memory."
-send "$argv start --insecure --store=type=mem,size=1GB --max-disk-temp-storage=10%\r"
+send "$argv start-single-node --insecure --store=type=mem,size=1GB --max-disk-temp-storage=10%\r"
 eexpect "node starting"
 interrupt
 eexpect ":/# "
 end_test
 
 start_test "Check that memory max flags do not exceed available RAM."
-send "$argv start --insecure --cache=.40 --max-sql-memory=.40\r"
+send "$argv start-single-node --insecure --cache=.40 --max-sql-memory=.40\r"
 eexpect "WARNING: the sum of --max-sql-memory"
 eexpect "is larger than"
 eexpect "of total RAM"
@@ -39,31 +39,31 @@ eexpect ":/# "
 end_test
 
 start_test "Check that --host causes a deprecation warning."
-send "$argv start --insecure --host=localhost\r"
+send "$argv start-single-node --insecure --host=localhost\r"
 eexpect "host has been deprecated, use --listen-addr/--advertise-addr instead."
 eexpect "node starting"
 interrupt
 eexpect ":/# "
 end_test
 
 start_test "Check that server --port causes a deprecation warning."
-send "$argv start --insecure --port=26257\r"
+send "$argv start-single-node --insecure --port=26257\r"
 eexpect "port has been deprecated, use --listen-addr instead."
 eexpect "node starting"
 interrupt
 eexpect ":/# "
 end_test
 
 start_test "Check that server --advertise-port causes a deprecation warning."
-send "$argv start --insecure --advertise-port=12345\r"
+send "$argv start-single-node --insecure --advertise-port=12345\r"
 eexpect "advertise-port has been deprecated, use --advertise-addr"
 eexpect "node starting"
 interrupt
 eexpect ":/# "
 end_test
 
 start_test "Check that not using --host nor --advertise causes a user warning."
-send "$argv start --insecure\r"
+send "$argv start-single-node --insecure\r"
 eexpect "WARNING: neither --listen-addr nor --advertise-addr was specified"
 eexpect "node starting"
 interrupt
@@ -73,16 +73,35 @@ end_test
 start_test {Check that the "failed running SUBCOMMAND" message does not consider a flag the subcommand}
 send "$argv --verbosity 2 start --garbage\r"
 eexpect {Failed running "start"}
+eexpect ":/# "
 end_test
 
 start_test {Check that the "failed running SUBCOMMAND" message handles nested subcommands}
 send "$argv --verbosity 2 debug zip --garbage\r"
 eexpect {Failed running "debug zip"}
+eexpect ":/# "
 end_test
 
 start_test {Check that the "failed running SUBCOMMAND" message handles missing subcommands}
 send "$argv --verbosity 2 --garbage\r"
 eexpect {Failed running "cockroach"}
+eexpect ":/# "
+end_test
+
+start_test "Check that start without --join reports a deprecation warning"
+send "$argv start --insecure\r"
+eexpect "running 'cockroach start' without --join is deprecated."
+eexpect "node starting"
+interrupt
+eexpect ":/# "
+end_test
+
+start_test "Check that start-single-node disable replication properly"
+start_server $argv
+send "$argv zone get .default\r"
+eexpect "num_replicas: 1"
+eexpect ":/# "
+stop_server $argv
 end_test
 
 send "exit 0\r"

pkg/cli/interactive_tests/test_high_verbosity.tcl

@@ -2,7 +2,7 @@
 
 source [file join [file dirname $argv0] common.tcl]
 
-system "mkfifo pid_fifo || true; $argv start --insecure --verbosity 3 --pid-file=pid_fifo -s=path=logs/db & cat pid_fifo > server_pid"
+system "mkfifo pid_fifo || true; $argv start-single-node --insecure --verbosity 3 --pid-file=pid_fifo -s=path=logs/db & cat pid_fifo > server_pid"
 
 spawn /bin/bash
 send "PS1=':''/# '\r"

pkg/cli/interactive_tests/test_log_config_msg.tcl

@@ -14,7 +14,7 @@ end_test
 
 
 # Make a server with a tiny log buffer so as to force frequent log rotation.
-system "mkfifo pid_fifo || true; $argv start --insecure --pid-file=pid_fifo --background -s=path=logs/db --log-file-max-size=2k >>logs/expect-cmd.log 2>&1 & cat pid_fifo > server_pid"
+system "mkfifo pid_fifo || true; $argv start-single-node --insecure --pid-file=pid_fifo --background -s=path=logs/db --log-file-max-size=2k >>logs/expect-cmd.log 2>&1 & cat pid_fifo > server_pid"
 stop_server $argv
 
 start_test "Check that the cluster ID is reported at the start of new log files."