cli: remove auto-init with cockroach start without --join

Fixes cockroachdb#44116. Informs cockroachdb#24118. Reviving cockroachdb#44112.

We remove the deprecated behavior of crdb where we previously
auto-initialized the cluster when `cockroach start` was invoked without
a corresponding `--join` flag. We update a few tests along the way that
made use of this behavior by changing them to either explicitly init, or
now lean on `roachprod` changes (from cockroachdb#51329) that transparently manage
initialization.

Release note (backward-incompatible change): A CockroachDB node
started with cockroach start without the --join flag no
longer automatically initializes the cluster. The `cockroach init`
command is now mandatory. The auto-initialization behavior had
been deprecated in version 19.2.

Release note (backward-incompatible change): `cockroach start` without
`--join` is no longer supported. It was deprecated in a previous
release.

pkg/acceptance/cluster/dockercluster.go

@@ -480,10 +480,9 @@ func (l *DockerCluster) startNode(ctx context.Context, node *testNode) {
  }
  cmd = append(cmd, fmt.Sprintf("--store=%s", storeSpec))
  }
- // Append --join flag (for all nodes except first in bootstrap-node-zero mode)
- if node.index > 0 || l.config.InitMode != INIT_BOOTSTRAP_NODE_ZERO {
- cmd = append(cmd, "--join="+net.JoinHostPort(l.Nodes[0].nodeStr, base.DefaultPort))
- }
+ // Append --join flag for all nodes.
+ firstNodeAddr := l.Nodes[0].nodeStr
+ cmd = append(cmd, "--join="+net.JoinHostPort(firstNodeAddr, base.DefaultPort))
 
  dockerLogDir := "/logs/" + node.nodeStr
  localLogDir := filepath.Join(l.volumesDir, "logs", node.nodeStr)

pkg/acceptance/cluster/testconfig.proto

@@ -22,9 +22,7 @@ enum InitMode {
  // init command.
  INIT_COMMAND = 0;
 
- // INIT_BOOTSTRAP_NODE_ZERO uses the legacy protocol of omitting the
- // join flag from node zero.
- INIT_BOOTSTRAP_NODE_ZERO = 1;
+ reserved 1;
 
  // INIT_NONE starts every node with a join flag and leaves the
  // cluster uninitialized.

pkg/base/config.go

@@ -208,6 +208,15 @@ type Config struct {
  // Enables the use of an PTP hardware clock user space API for HLC current time.
  // This contains the path to the device to be used (i.e. /dev/ptp0)
  ClockDevicePath string
+
+ // AutoInitializeCluster, if set, causes the server to bootstrap the
+ // cluster. Note that if two nodes are started with this flag set
+ // and also configured to join each other, each node will bootstrap
+ // its own unique cluster and the join will fail.
+ //
+ // The flag exists mostly for the benefit of tests, and for
+ // `cockroach start-single-node`.
+ AutoInitializeCluster bool
 }
 
 // HistogramWindowInterval is used to determine the approximate length of time

pkg/base/test_server_args.go

@@ -89,6 +89,11 @@ type TestServerArgs struct {
  SQLMemoryPoolSize int64
  CacheSize int64
 
+ // By default, test servers have AutoInitializeCluster=true set in
+ // their config. If NoAutoInitializeCluster is set, that behavior is disabled
+ // and the test becomes responsible for initializing the cluster.
+ NoAutoInitializeCluster bool
+
  // If set, this will be appended to the Postgres URL by functions that
  // automatically open a connection to the server. That's equivalent to running
  // SET DATABASE=foo, which works even if the database doesn't (yet) exist.

pkg/cli/context.go

@@ -92,6 +92,7 @@ func setServerContextDefaults() {
 
  serverCfg.KVConfig.GoroutineDumpDirName = ""
  serverCfg.KVConfig.HeapProfileDirName = ""
+ serverCfg.AutoInitializeCluster = false
  serverCfg.KVConfig.ReadyFn = nil
  serverCfg.KVConfig.DelayedBootstrapFn = nil
  serverCfg.KVConfig.JoinList = nil

pkg/cli/demo_cluster.go

@@ -121,11 +121,15 @@ func setupTransientCluster(
  for i := 0; i < demoCtx.nodes; i++ {
  // All the nodes connect to the address of the first server created.
  var joinAddr string
- if c.s != nil {
+ if i != 0 {
  joinAddr = c.s.ServingRPCAddr()
  }
  nodeID := roachpb.NodeID(i + 1)
  args := testServerArgsForTransientCluster(c.sockForServer(nodeID), nodeID, joinAddr, c.demoDir)
+ if i == 0 {
+ // The first node also auto-inits the cluster.
+ args.NoAutoInitializeCluster = false
+ }
 
  // servRPCReadyCh is used if latency simulation is requested to notify that a test server has
  // successfully computed its RPC address.
@@ -144,7 +148,6 @@ func setupTransientCluster(
  }
 
  serv := serverFactory.New(args).(*server.TestServer)
-
  if i == 0 {
  c.s = serv
  }
@@ -294,14 +297,15 @@ func testServerArgsForTransientCluster(
  storeSpec.StickyInMemoryEngineID = fmt.Sprintf("demo-node%d", nodeID)
 
  args := base.TestServerArgs{
- SocketFile: sock.filename(),
- PartOfCluster: true,
- Stopper: stop.NewStopper(),
- JoinAddr: joinAddr,
- DisableTLSForHTTP: true,
- StoreSpecs: []base.StoreSpec{storeSpec},
- SQLMemoryPoolSize: demoCtx.sqlPoolMemorySize,
- CacheSize: demoCtx.cacheSize,
+ SocketFile: sock.filename(),
+ PartOfCluster: true,
+ Stopper: stop.NewStopper(),
+ JoinAddr: joinAddr,
+ DisableTLSForHTTP: true,
+ StoreSpecs: []base.StoreSpec{storeSpec},
+ SQLMemoryPoolSize: demoCtx.sqlPoolMemorySize,
+ CacheSize: demoCtx.cacheSize,
+ NoAutoInitializeCluster: true,
  }
 
  if demoCtx.localities != nil {

pkg/cli/demo_test.go

@@ -39,11 +39,12 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
  sqlPoolMemorySize: 2 << 10,
  cacheSize: 1 << 10,
  expected: base.TestServerArgs{
- PartOfCluster: true,
- JoinAddr: "127.0.0.1",
- DisableTLSForHTTP: true,
- SQLMemoryPoolSize: 2 << 10,
- CacheSize: 1 << 10,
+ PartOfCluster: true,
+ JoinAddr: "127.0.0.1",
+ DisableTLSForHTTP: true,
+ SQLMemoryPoolSize: 2 << 10,
+ CacheSize: 1 << 10,
+ NoAutoInitializeCluster: true,
  },
  },
  {
@@ -52,11 +53,12 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
  sqlPoolMemorySize: 4 << 10,
  cacheSize: 4 << 10,
  expected: base.TestServerArgs{
- PartOfCluster: true,
- JoinAddr: "127.0.0.1",
- DisableTLSForHTTP: true,
- SQLMemoryPoolSize: 4 << 10,
- CacheSize: 4 << 10,
+ PartOfCluster: true,
+ JoinAddr: "127.0.0.1",
+ DisableTLSForHTTP: true,
+ SQLMemoryPoolSize: 4 << 10,
+ CacheSize: 4 << 10,
+ NoAutoInitializeCluster: true,
  },
  },
  }

pkg/cli/error.go

@@ -21,6 +21,7 @@ import (
  "strings"
 
  "github.com/cockroachdb/cockroach/pkg/security"
+ "github.com/cockroachdb/cockroach/pkg/server"
  "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
  "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
  "github.com/cockroachdb/cockroach/pkg/util/grpcutil"
@@ -348,6 +349,16 @@ func MaybeDecorateGRPCError(
  "The CockroachDB CLI does not support GSSAPI authentication; use 'psql' instead")
  }
 
+ // Are we trying to re-initialize an initialized cluster?
+ if strings.Contains(err.Error(), server.ErrClusterInitialized.Error()) {
+ // We really want to use errors.Is() here but this would require
+ // error serialization support in gRPC.
+ // This is not yet performed in CockroachDB even though the error
+ // library now has infrastructure to do so, see:
+ // https://github.com/cockroachdb/errors/pull/14
+ return server.ErrClusterInitialized
+ }
+
  // Nothing we can special case, just return what we have.
  return err
  }

pkg/cli/init.go

@@ -14,10 +14,8 @@ import (
  "context"
  "fmt"
  "os"
- "strings"
  "time"
 
- "github.com/cockroachdb/cockroach/pkg/server"
  "github.com/cockroachdb/cockroach/pkg/server/serverpb"
  "github.com/cockroachdb/cockroach/pkg/util/contextutil"
  "github.com/cockroachdb/cockroach/pkg/util/retry"
@@ -34,14 +32,10 @@ Perform one-time-only initialization of a CockroachDB cluster.
 
 After starting one or more nodes with --join flags, run the init
 command on one node (passing the same --host and certificate flags
-you would use for the sql command). The target of the init command
-must appear in the --join flags of other nodes.
-
-A node started without the --join flag initializes itself as a
-single-node cluster, so the init command is not used in that case.
+you would use for the sql command).
 `,
  Args: cobra.NoArgs,
- RunE: maybeShoutError(MaybeDecorateGRPCError(runInit)),
+ RunE: MaybeDecorateGRPCError(runInit),
 }
 
 func runInit(cmd *cobra.Command, args []string) error {
@@ -59,15 +53,6 @@ func runInit(cmd *cobra.Command, args []string) error {
  c := serverpb.NewInitClient(conn)
 
  if _, err = c.Bootstrap(ctx, &serverpb.BootstrapRequest{}); err != nil {
- if strings.Contains(err.Error(), server.ErrClusterInitialized.Error()) {
- // We really want to use errors.Is() here but this would require
- // error serialization support in gRPC.
- // This is not yet performed in CockroachDB even though
- // the error library now has infrastructure to do so, see:
- // https://github.com/cockroachdb/errors/pull/14
- return errors.WithHint(err,
- "Please ensure all your start commands are using --join.")
- }
  return err
  }
 

pkg/cli/interactive_tests/test_disable_replication.tcl

@@ -36,8 +36,9 @@ stop_server $argv
 start_test "Check that start-single-node on a regular cluster does not reset the replication factor"
 # make a fresh server but using the regular 'start'
 system "rm -rf logs/db"
-system "$argv start --insecure --pid-file=server_pid --background -s=path=logs/db >>logs/expect-cmd.log 2>&1;
- $argv sql -e 'select 1'"
+system "$argv start --insecure --pid-file=server_pid --background -s=path=logs/db --join=:26257 >>logs/expect-cmd.log 2>&1"
+system "$argv init --insecure"
+system "$argv sql -e 'select 1'"
 # restart with start-single-node
 stop_server $argv
 start_server $argv

pkg/cli/interactive_tests/test_dump_sig.tcl

@@ -8,7 +8,7 @@ send "PS1='\\h:''/# '\r"
 eexpect ":/# "
 
 start_test "Check that the server emits a goroutine dump upon receiving signal"
-send "$argv start --insecure --pid-file=server_pid --log-dir=logs --logtostderr\r"
+send "$argv start-single-node --insecure --pid-file=server_pid --log-dir=logs --logtostderr\r"
 eexpect "CockroachDB node starting"
 
 system "kill -QUIT `cat server_pid`"

pkg/cli/interactive_tests/test_flags.tcl

@@ -72,15 +72,13 @@ eexpect {Failed running "cockroach"}
 eexpect ":/# "
 end_test
 
-start_test "Check that start without --join reports a deprecation warning"
+start_test "Check that start without --join errors out"
 send "$argv start --insecure\r"
-eexpect "running 'cockroach start' without --join is deprecated."
-eexpect "node starting"
-interrupt
-eexpect ":/# "
+eexpect "ERROR: no --join flags provided to 'cockroach start'"
+eexpect "Consider using 'cockroach start-single-node' or 'cockroach init' instead"
+eexpect {Failed running "start"}
 end_test
 
-
 start_server $argv
 
 start_test "Check that a client can connect using the URL env var"