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

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

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

pkg/acceptance/cluster/dockercluster.go

@@ -480,10 +480,12 @@ 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 := ""
+	if node.index > 0 {
+		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 WaitForBootstrap is set, that behavior is disabled
+	// and the test becomes responsible for initializing the cluster.
+	WaitForBootstrap 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

@@ -91,6 +91,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

@@ -120,11 +120,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.WaitForBootstrap = false
+		}
 
 		// servRPCReadyCh is used if latency simulation is requested to notify that a test server has
 		// successfully computed its RPC address.
@@ -143,7 +147,6 @@ func setupTransientCluster(
 		}
 
 		serv := serverFactory.New(args).(*server.TestServer)
-
 		if i == 0 {
 			c.s = serv
 		}
@@ -301,6 +304,7 @@ func testServerArgsForTransientCluster(
 		StoreSpecs:        []base.StoreSpec{storeSpec},
 		SQLMemoryPoolSize: demoCtx.sqlPoolMemorySize,
 		CacheSize:         demoCtx.cacheSize,
+		WaitForBootstrap:  true,
 	}
 
 	if demoCtx.localities != nil {

pkg/cli/demo_test.go

@@ -44,6 +44,7 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
 				DisableTLSForHTTP: true,
 				SQLMemoryPoolSize: 2 << 10,
 				CacheSize:         1 << 10,
+				WaitForBootstrap:  true,
 			},
 		},
 		{
@@ -57,6 +58,7 @@ func TestTestServerArgsForTransientCluster(t *testing.T) {
 				DisableTLSForHTTP: true,
 				SQLMemoryPoolSize: 4 << 10,
 				CacheSize:         4 << 10,
+				WaitForBootstrap:  true,
 			},
 		},
 	}

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,11 +32,7 @@ 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)),
@@ -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 >>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,6 @@ 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_server $argv
 
 start_test "Check that a client can connect using the URL env var"

pkg/cli/start.go

@@ -76,10 +76,6 @@ Specify the --join flag to point to another node or nodes that are
 part of the same cluster. The other nodes do not need to be started
 yet, and if the address of the other nodes to be added are not yet
 known it is legal for the first node to join itself.
-
-If --join is not specified, the cluster will also be initialized.
-THIS BEHAVIOR IS DEPRECATED; consider using 'cockroach init' or
-'cockroach start-single-node' instead.
 `,
 	Example: `  cockroach start --insecure --store=attrs=ssd,path=/mnt/ssd1 --join=host:port,[host:port]`,
 	Args:    cobra.NoArgs,
@@ -446,9 +442,10 @@ func runStartSingleNode(cmd *cobra.Command, args []string) error {
 	if joinFlag.Changed {
 		return errCannotUseJoin
 	}
-	// Now actually set the flag as changed so that the start code
-	// doesn't warn that it was not set.
-	joinFlag.Changed = true
+
+	// Make the node auto-init the cluster if not done already.
+	serverCfg.AutoInitializeCluster = true
+
 	return runStart(cmd, args, true /*disableReplication*/)
 }
 
@@ -461,9 +458,8 @@ func runStartJoin(cmd *cobra.Command, args []string) error {
 // of other active nodes used to join this node to the cockroach
 // cluster, if this is its first time connecting.
 //
-// If the argument disableReplication is true and we are starting
-// a fresh cluster, the replication factor will be disabled in
-// all zone configs.
+// If the argument disableReplication is set the replication factor
+// will be set to 1 all zone configs.
 func runStart(cmd *cobra.Command, args []string, disableReplication bool) error {
 	tBegin := timeutil.Now()
 
@@ -537,13 +533,6 @@ func runStart(cmd *cobra.Command, args []string, disableReplication bool) error
 	// but when actually starting a server, we enable them.
 	grpcutil.SetSeverity(log.Severity_WARNING)
 
-	// Check the --join flag.
-	if !flagSetForCmd(cmd).Lookup(cliflags.Join.Name).Changed {
-		log.Shout(ctx, log.Severity_WARNING,
-			"running 'cockroach start' without --join is deprecated.\n"+
-				"Consider using 'cockroach start-single-node' or 'cockroach init' instead.")
-	}
-
 	// Now perform additional configuration tweaks specific to the start
 	// command.