From b5e7171e56121e2b229b42be4e5249254e1bbd99 Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Thu, 9 Jul 2020 16:28:20 -0400 Subject: [PATCH] cli: remove auto-init with `cockroach start` without `--join` Fixes #44116. Informs #24118. Reviving #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 #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 | 7 +- pkg/acceptance/cluster/testconfig.pb.go | 80 +++++++++---------- pkg/acceptance/cluster/testconfig.proto | 4 +- pkg/base/config.go | 9 +++ pkg/base/test_server_args.go | 5 ++ pkg/cli/context.go | 1 + pkg/cli/demo_cluster.go | 24 +++--- pkg/cli/demo_test.go | 22 ++--- pkg/cli/error.go | 11 +++ pkg/cli/init.go | 19 +---- .../test_disable_replication.tcl | 5 +- pkg/cli/interactive_tests/test_dump_sig.tcl | 2 +- pkg/cli/interactive_tests/test_flags.tcl | 10 +-- .../interactive_tests/test_init_command.tcl | 6 +- pkg/cli/start.go | 23 +++--- pkg/cmd/roachtest/cluster_init.go | 38 +-------- pkg/cmd/roachtest/gossip.go | 1 + pkg/server/init.go | 2 +- pkg/server/server.go | 34 ++++---- pkg/server/testserver.go | 1 + pkg/testutils/testcluster/testcluster.go | 4 + 21 files changed, 142 insertions(+), 166 deletions(-) diff --git a/pkg/acceptance/cluster/dockercluster.go b/pkg/acceptance/cluster/dockercluster.go index 654491c6dadd..cc66c5900d08 100644 --- a/pkg/acceptance/cluster/dockercluster.go +++ b/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) diff --git a/pkg/acceptance/cluster/testconfig.pb.go b/pkg/acceptance/cluster/testconfig.pb.go index a2eecc82c2e0..9825aff03499 100644 --- a/pkg/acceptance/cluster/testconfig.pb.go +++ b/pkg/acceptance/cluster/testconfig.pb.go @@ -30,9 +30,6 @@ const ( // INIT_COMMAND starts every node with a join flag and issues the // init command. INIT_COMMAND InitMode = 0 - // INIT_BOOTSTRAP_NODE_ZERO uses the legacy protocol of omitting the - // join flag from node zero. - INIT_BOOTSTRAP_NODE_ZERO InitMode = 1 // INIT_NONE starts every node with a join flag and leaves the // cluster uninitialized. INIT_NONE InitMode = 2 @@ -40,13 +37,11 @@ const ( var InitMode_name = map[int32]string{ 0: "INIT_COMMAND", - 1: "INIT_BOOTSTRAP_NODE_ZERO", 2: "INIT_NONE", } var InitMode_value = map[string]int32{ - "INIT_COMMAND": 0, - "INIT_BOOTSTRAP_NODE_ZERO": 1, - "INIT_NONE": 2, + "INIT_COMMAND": 0, + "INIT_NONE": 2, } func (x InitMode) Enum() *InitMode { @@ -66,7 +61,7 @@ func (x *InitMode) UnmarshalJSON(data []byte) error { return nil } func (InitMode) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_testconfig_af28e3620e9a1d6b, []int{0} + return fileDescriptor_testconfig_a82c0aa336d029cb, []int{0} } // StoreConfig holds the configuration of a collection of similar stores. @@ -78,7 +73,7 @@ func (m *StoreConfig) Reset() { *m = StoreConfig{} } func (m *StoreConfig) String() string { return proto.CompactTextString(m) } func (*StoreConfig) ProtoMessage() {} func (*StoreConfig) Descriptor() ([]byte, []int) { - return fileDescriptor_testconfig_af28e3620e9a1d6b, []int{0} + return fileDescriptor_testconfig_a82c0aa336d029cb, []int{0} } func (m *StoreConfig) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -113,7 +108,7 @@ func (m *NodeConfig) Reset() { *m = NodeConfig{} } func (m *NodeConfig) String() string { return proto.CompactTextString(m) } func (*NodeConfig) ProtoMessage() {} func (*NodeConfig) Descriptor() ([]byte, []int) { - return fileDescriptor_testconfig_af28e3620e9a1d6b, []int{1} + return fileDescriptor_testconfig_a82c0aa336d029cb, []int{1} } func (m *NodeConfig) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -154,7 +149,7 @@ func (m *TestConfig) Reset() { *m = TestConfig{} } func (m *TestConfig) String() string { return proto.CompactTextString(m) } func (*TestConfig) ProtoMessage() {} func (*TestConfig) Descriptor() ([]byte, []int) { - return fileDescriptor_testconfig_af28e3620e9a1d6b, []int{2} + return fileDescriptor_testconfig_a82c0aa336d029cb, []int{2} } func (m *TestConfig) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -810,36 +805,35 @@ var ( ) func init() { - proto.RegisterFile("acceptance/cluster/testconfig.proto", fileDescriptor_testconfig_af28e3620e9a1d6b) -} - -var fileDescriptor_testconfig_af28e3620e9a1d6b = []byte{ - // 422 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0xcd, 0x6e, 0xd3, 0x40, - 0x14, 0x85, 0x3d, 0xf9, 0x21, 0xc9, 0x2d, 0x45, 0xd1, 0x08, 0x24, 0xab, 0x2a, 0x53, 0x2b, 0x95, - 0x90, 0xcb, 0xc2, 0x11, 0x79, 0x83, 0xa6, 0x89, 0x90, 0x17, 0xb1, 0x91, 0x6b, 0x09, 0xa9, 0x1b, - 0x6b, 0x34, 0x1e, 0xcc, 0x08, 0x3c, 0x53, 0xd9, 0x13, 0xe8, 0x23, 0xc0, 0x8e, 0x77, 0xe0, 0x65, - 0xb2, 0xec, 0xb2, 0xab, 0x0a, 0x9c, 0xb7, 0x60, 0x85, 0xec, 0x8e, 0x1b, 0xd8, 0x64, 0x67, 0x9f, - 0x7b, 0xce, 0xf9, 0xee, 0x1d, 0x38, 0xa5, 0x8c, 0xf1, 0x6b, 0x4d, 0x25, 0xe3, 0x53, 0xf6, 0x79, - 0x5d, 0x6a, 0x5e, 0x4c, 0x35, 0x2f, 0x35, 0x53, 0xf2, 0x83, 0xc8, 0xbc, 0xeb, 0x42, 0x69, 0x85, - 0x8f, 0x99, 0x62, 0x9f, 0x0a, 0x45, 0xd9, 0x47, 0x6f, 0x67, 0xf7, 0x8c, 0xfd, 0xe8, 0x79, 0xa6, - 0x32, 0xd5, 0x18, 0xa7, 0xf5, 0xd7, 0x43, 0x66, 0x32, 0x83, 0x83, 0x4b, 0xad, 0x0a, 0x7e, 0xd1, - 0x14, 0xe1, 0x53, 0x80, 0x9c, 0xde, 0x24, 0x05, 0x95, 0x19, 0x2f, 0xed, 0x8e, 0x83, 0xdc, 0xfe, - 0xbc, 0xb7, 0xb9, 0x3f, 0xb1, 0xa2, 0x51, 0x4e, 0x6f, 0xa2, 0x46, 0x9e, 0xac, 0x01, 0x02, 0x95, - 0xb6, 0x11, 0x02, 0x83, 0x2f, 0xbc, 0x28, 0x85, 0x92, 0x36, 0x72, 0x90, 0x3b, 0x32, 0xfe, 0x56, - 0xc4, 0x6f, 0xe1, 0x49, 0x59, 0x13, 0xea, 0xba, 0xae, 0x7b, 0x30, 0x3b, 0xf3, 0xf6, 0xad, 0xe9, - 0xfd, 0xb3, 0x8d, 0x69, 0x32, 0xf1, 0xc9, 0xf7, 0x0e, 0x40, 0xcc, 0x4b, 0x6d, 0xb8, 0x36, 0xf4, - 0x24, 0xcd, 0xf9, 0x7f, 0xd0, 0x46, 0xc1, 0x0b, 0xe8, 0x4b, 0x95, 0x3e, 0x02, 0xdd, 0xfd, 0xc0, - 0xdd, 0x29, 0xa6, 0xe4, 0x21, 0x8c, 0xdf, 0xc0, 0x30, 0x5d, 0x17, 0x54, 0xd7, 0x87, 0x75, 0x1d, - 0xe4, 0x76, 0xe7, 0x2f, 0xea, 0xf1, 0x9f, 0xfb, 0x93, 0x43, 0x2d, 0x72, 0xee, 0x2d, 0xcc, 0x30, - 0x7a, 0xb4, 0x61, 0x1f, 0x46, 0x42, 0x0a, 0x9d, 0xe4, 0x2a, 0xe5, 0x76, 0xcf, 0x41, 0xee, 0xb3, - 0xd9, 0xab, 0xfd, 0x70, 0x5f, 0x0a, 0xbd, 0x52, 0x29, 0x37, 0xe8, 0xa1, 0x30, 0xff, 0xf8, 0x25, - 0x0c, 0xa4, 0x4a, 0xbe, 0x52, 0xa1, 0xed, 0xbe, 0x83, 0xdc, 0x61, 0xfb, 0x16, 0x52, 0xbd, 0xa7, - 0x42, 0xbf, 0x0e, 0x61, 0xd8, 0x46, 0xf1, 0x18, 0x9e, 0xfa, 0x81, 0x1f, 0x27, 0x17, 0xe1, 0x6a, - 0x75, 0x1e, 0x2c, 0xc6, 0x16, 0x3e, 0x06, 0xbb, 0x51, 0xe6, 0x61, 0x18, 0x5f, 0xc6, 0xd1, 0xf9, - 0xbb, 0x24, 0x08, 0x17, 0xcb, 0xe4, 0x6a, 0x19, 0x85, 0x63, 0x84, 0x0f, 0x61, 0xd4, 0x4c, 0x83, - 0x30, 0x58, 0x8e, 0x3b, 0x47, 0xbd, 0x6f, 0x3f, 0x89, 0x35, 0x3f, 0xdb, 0xfc, 0x26, 0xd6, 0xa6, - 0x22, 0xe8, 0xb6, 0x22, 0xe8, 0xae, 0x22, 0xe8, 0x57, 0x45, 0xd0, 0x8f, 0x2d, 0xb1, 0x6e, 0xb7, - 0xc4, 0xba, 0xdb, 0x12, 0xeb, 0x6a, 0x60, 0x76, 0xfe, 0x1b, 0x00, 0x00, 0xff, 0xff, 0x5a, 0x33, - 0x2b, 0x9d, 0x8c, 0x02, 0x00, 0x00, + proto.RegisterFile("acceptance/cluster/testconfig.proto", fileDescriptor_testconfig_a82c0aa336d029cb) +} + +var fileDescriptor_testconfig_a82c0aa336d029cb = []byte{ + // 405 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x90, 0xc1, 0x6e, 0xd3, 0x40, + 0x14, 0x45, 0x3d, 0x89, 0x43, 0xec, 0x57, 0x8a, 0xac, 0x11, 0x48, 0x56, 0x05, 0x53, 0xcb, 0x95, + 0x90, 0xcb, 0xc2, 0x11, 0xd9, 0xb0, 0x26, 0x0d, 0x42, 0x5e, 0xc4, 0x95, 0x42, 0x25, 0x24, 0x36, + 0xd6, 0x68, 0x3c, 0x98, 0x11, 0x78, 0xa6, 0xb2, 0x27, 0xd0, 0x4f, 0x80, 0x1d, 0xff, 0xc0, 0xcf, + 0x64, 0xd9, 0x65, 0x57, 0x15, 0x38, 0x7f, 0xc1, 0x0a, 0xd9, 0x1d, 0x27, 0x74, 0x93, 0x9d, 0x7d, + 0xdf, 0xbd, 0xf7, 0xbc, 0x37, 0x70, 0x42, 0x19, 0xe3, 0x97, 0x9a, 0x4a, 0xc6, 0x27, 0xec, 0xcb, + 0xaa, 0xd6, 0xbc, 0x9a, 0x68, 0x5e, 0x6b, 0xa6, 0xe4, 0x47, 0x51, 0xc4, 0x97, 0x95, 0xd2, 0x0a, + 0x3f, 0x65, 0x8a, 0x7d, 0xae, 0x14, 0x65, 0x9f, 0xe2, 0x9d, 0x3d, 0x36, 0xf6, 0xa3, 0xc7, 0x85, + 0x2a, 0x54, 0x67, 0x9c, 0xb4, 0x5f, 0x77, 0x99, 0x70, 0x0a, 0x07, 0xef, 0xb4, 0xaa, 0xf8, 0x59, + 0x57, 0x84, 0x4f, 0x00, 0x4a, 0x7a, 0x95, 0x55, 0x54, 0x16, 0xbc, 0xf6, 0x07, 0x01, 0x8a, 0x46, + 0x33, 0x7b, 0x7d, 0x7b, 0x6c, 0x2d, 0xdd, 0x92, 0x5e, 0x2d, 0x3b, 0x39, 0x5c, 0x01, 0xa4, 0x2a, + 0xef, 0x23, 0x04, 0xc6, 0x5f, 0x79, 0x55, 0x0b, 0x25, 0x7d, 0x14, 0xa0, 0xc8, 0x35, 0xfe, 0x5e, + 0xc4, 0x6f, 0xe1, 0x41, 0xdd, 0x12, 0xda, 0xba, 0x61, 0x74, 0x30, 0x3d, 0x8d, 0xf7, 0xad, 0x19, + 0xff, 0xb7, 0x8d, 0x69, 0x32, 0xf1, 0xf0, 0xc7, 0x00, 0xe0, 0x82, 0xd7, 0xda, 0x70, 0x7d, 0xb0, + 0x25, 0x2d, 0xf9, 0x3d, 0x68, 0xa7, 0xe0, 0x39, 0x8c, 0xa4, 0xca, 0xb7, 0xc0, 0x68, 0x3f, 0x70, + 0x77, 0x8a, 0x29, 0xb9, 0x0b, 0xe3, 0x97, 0xe0, 0xe4, 0xab, 0x8a, 0xea, 0xf6, 0xb0, 0x61, 0x80, + 0xa2, 0xe1, 0xec, 0x49, 0x3b, 0xfe, 0x7b, 0x7b, 0x7c, 0xa8, 0x45, 0xc9, 0xe3, 0xb9, 0x19, 0x2e, + 0xb7, 0x36, 0x9c, 0x80, 0x2b, 0xa4, 0xd0, 0x59, 0xa9, 0x72, 0xee, 0xdb, 0x01, 0x8a, 0x1e, 0x4d, + 0x9f, 0xef, 0x87, 0x27, 0x52, 0xe8, 0x85, 0xca, 0xb9, 0x41, 0x3b, 0xc2, 0xfc, 0xe3, 0x67, 0x30, + 0x96, 0x2a, 0xfb, 0x46, 0x85, 0xf6, 0x47, 0x01, 0x8a, 0x9c, 0xfe, 0x2d, 0xa4, 0x7a, 0x4f, 0x85, + 0x7e, 0xf1, 0x0a, 0x9c, 0x3e, 0x8a, 0x3d, 0x78, 0x98, 0xa4, 0xc9, 0x45, 0x76, 0x76, 0xbe, 0x58, + 0xbc, 0x4e, 0xe7, 0x9e, 0x85, 0x0f, 0xc1, 0xed, 0x94, 0xf4, 0x3c, 0x7d, 0xe3, 0x0d, 0x8e, 0xec, + 0xef, 0xbf, 0x88, 0x15, 0xda, 0x0e, 0xf2, 0xd0, 0xec, 0x74, 0xfd, 0x87, 0x58, 0xeb, 0x86, 0xa0, + 0xeb, 0x86, 0xa0, 0x9b, 0x86, 0xa0, 0xdf, 0x0d, 0x41, 0x3f, 0x37, 0xc4, 0xba, 0xde, 0x10, 0xeb, + 0x66, 0x43, 0xac, 0x0f, 0x63, 0xb3, 0xdb, 0xbf, 0x00, 0x00, 0x00, 0xff, 0xff, 0x09, 0x22, 0x2a, + 0xa7, 0x74, 0x02, 0x00, 0x00, } diff --git a/pkg/acceptance/cluster/testconfig.proto b/pkg/acceptance/cluster/testconfig.proto index d727cec1458f..cc09659e660e 100644 --- a/pkg/acceptance/cluster/testconfig.proto +++ b/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. diff --git a/pkg/base/config.go b/pkg/base/config.go index 120fb5dd2bd6..6433c727deae 100644 --- a/pkg/base/config.go +++ b/pkg/base/config.go @@ -221,6 +221,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 diff --git a/pkg/base/test_server_args.go b/pkg/base/test_server_args.go index 8a7152010798..7564e120a8f3 100644 --- a/pkg/base/test_server_args.go +++ b/pkg/base/test_server_args.go @@ -91,6 +91,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. diff --git a/pkg/cli/context.go b/pkg/cli/context.go index 761ae48aac94..28d563606557 100644 --- a/pkg/cli/context.go +++ b/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 diff --git a/pkg/cli/demo_cluster.go b/pkg/cli/demo_cluster.go index bd4c1f757f4a..e4c79cffb7b3 100644 --- a/pkg/cli/demo_cluster.go +++ b/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 { diff --git a/pkg/cli/demo_test.go b/pkg/cli/demo_test.go index a9d65c79ab22..faa1379773d2 100644 --- a/pkg/cli/demo_test.go +++ b/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, }, }, } diff --git a/pkg/cli/error.go b/pkg/cli/error.go index f2f436d65d38..30a255eea8a5 100644 --- a/pkg/cli/error.go +++ b/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 } diff --git a/pkg/cli/init.go b/pkg/cli/init.go index 92bbc3a7c4e8..d8e34c5626c2 100644 --- a/pkg/cli/init.go +++ b/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 } diff --git a/pkg/cli/interactive_tests/test_disable_replication.tcl b/pkg/cli/interactive_tests/test_disable_replication.tcl index f2c73f3f4e8e..87f8e6fe00b8 100644 --- a/pkg/cli/interactive_tests/test_disable_replication.tcl +++ b/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 diff --git a/pkg/cli/interactive_tests/test_dump_sig.tcl b/pkg/cli/interactive_tests/test_dump_sig.tcl index 34cd3c658a90..a2c644e4550d 100644 --- a/pkg/cli/interactive_tests/test_dump_sig.tcl +++ b/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 --store=path=logs/db --logtostderr\r" +send "$argv start-single-node --insecure --pid-file=server_pid --store=path=logs/db --logtostderr\r" eexpect "CockroachDB node starting" system "kill -QUIT `cat server_pid`" diff --git a/pkg/cli/interactive_tests/test_flags.tcl b/pkg/cli/interactive_tests/test_flags.tcl index 899c0b2d3e0d..b2b3ff6e4082 100644 --- a/pkg/cli/interactive_tests/test_flags.tcl +++ b/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 "HINT: Consider using 'cockroach init' or 'cockroach start-single-node' instead" +eexpect {Failed running "start"} end_test - start_server $argv start_test "Check that a client can connect using the URL env var" diff --git a/pkg/cli/interactive_tests/test_init_command.tcl b/pkg/cli/interactive_tests/test_init_command.tcl index c955f1a623f6..090131b52e43 100644 --- a/pkg/cli/interactive_tests/test_init_command.tcl +++ b/pkg/cli/interactive_tests/test_init_command.tcl @@ -72,8 +72,7 @@ eexpect ":/# " start_test "Check that init on an already started server immediately complains the server is already initialized" send "$argv init --insecure --host=localhost\r" -eexpect "error" -eexpect "cluster has already been initialized" +eexpect "ERROR: cluster has already been initialized" eexpect ":/# " end_test @@ -82,8 +81,7 @@ start_server $argv start_test "Check that init after server restart still properly complains the server has been initialized" send "$argv init --insecure --host=localhost\r" -eexpect "error" -eexpect "cluster has already been initialized" +eexpect "ERROR: cluster has already been initialized" eexpect ":/# " end_test diff --git a/pkg/cli/start.go b/pkg/cli/start.go index 1384169f9444..391f75905c02 100644 --- a/pkg/cli/start.go +++ b/pkg/cli/start.go @@ -78,9 +78,7 @@ 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. +To initialize the cluster, use 'cockroach init'. `, Example: ` cockroach start --insecure --store=attrs=ssd,path=/mnt/ssd1 --join=host:port,[host:port]`, Args: cobra.NoArgs, @@ -446,8 +444,13 @@ func runStartSingleNode(cmd *cobra.Command, args []string) error { return errCannotUseJoin } // Now actually set the flag as changed so that the start code - // doesn't warn that it was not set. + // doesn't warn that it was not set. This is all to let `start-single-node` + // get by without the use of --join flags. joinFlag.Changed = true + + // Make the node auto-init the cluster if not done already. + serverCfg.AutoInitializeCluster = true + return runStart(cmd, args, true /*disableReplication*/) } @@ -460,9 +463,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() @@ -554,9 +556,10 @@ func runStart(cmd *cobra.Command, args []string, disableReplication bool) error // 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.") + err := errors.WithHint( + errors.New("no --join flags provided to 'cockroach start'"), + "Consider using 'cockroach init' or 'cockroach start-single-node' instead") + return err } // Now perform additional configuration tweaks specific to the start diff --git a/pkg/cmd/roachtest/cluster_init.go b/pkg/cmd/roachtest/cluster_init.go index 66b709f8bc76..dad8194ee5d2 100644 --- a/pkg/cmd/roachtest/cluster_init.go +++ b/pkg/cmd/roachtest/cluster_init.go @@ -38,42 +38,8 @@ func runClusterInit(ctx context.Context, t *test, c *cluster) { t.Fatal("no address for first node") } - // Legacy-style init where we start node 1 without a join flag and then point - // the other nodes at it. - func() { - var g errgroup.Group - g.Go(func() error { - return c.RunE(ctx, c.Node(1), - `mkdir -p {log-dir} && `+ - `./cockroach start --insecure --background --store={store-dir} `+ - `--log-dir={log-dir} --cache=10% --max-sql-memory=10% `+ - `--listen-addr=:{pgport:1} --http-port=$[{pgport:1}+1] `+ - `> {log-dir}/cockroach.stdout 2> {log-dir}/cockroach.stderr`) - }) - for i := 2; i <= c.spec.NodeCount; i++ { - i := i - g.Go(func() error { - return c.RunE(ctx, c.Node(i), - fmt.Sprintf( - `mkdir -p {log-dir} && `+ - `./cockroach start --insecure --background --store={store-dir} `+ - `--log-dir={log-dir} --cache=10%% --max-sql-memory=10%% `+ - `--listen-addr=:{pgport:%[1]d} --http-port=$[{pgport:%[1]d}+1] `+ - `--join=`+addrs[0]+ - `> {log-dir}/cockroach.stdout 2> {log-dir}/cockroach.stderr`, i)) - }) - } - if err := g.Wait(); err != nil { - t.Fatal(err) - } - - db := c.Conn(ctx, 1) - defer db.Close() - waitForFullReplication(t, db) - }() - - // New-style init where we start all nodes with the same join flags and then - // issue an "init" command to one of the nodes. + // We start all nodes with the same join flags and then issue an "init" + // command to one of the nodes. for _, initNode := range []int{1, 2} { c.Wipe(ctx) diff --git a/pkg/cmd/roachtest/gossip.go b/pkg/cmd/roachtest/gossip.go index 18e3dc2b21ef..c2b05f47850f 100644 --- a/pkg/cmd/roachtest/gossip.go +++ b/pkg/cmd/roachtest/gossip.go @@ -397,6 +397,7 @@ SELECT count(replicas) `./cockroach start --insecure --background --store={store-dir} `+ `--log-dir={log-dir} --cache=10% --max-sql-memory=10% `+ `--listen-addr=:$[{pgport:1}+10000] --http-port=$[{pgport:1}+1] `+ + `--join={pghost:1}:{pgport:1}`+ `> {log-dir}/cockroach.stdout 2> {log-dir}/cockroach.stderr`) if err != nil { t.Fatal(err) diff --git a/pkg/server/init.go b/pkg/server/init.go index 2c5662270236..0e93ca3d25b5 100644 --- a/pkg/server/init.go +++ b/pkg/server/init.go @@ -29,7 +29,7 @@ import ( "github.com/cockroachdb/errors" ) -// ErrClusterInitialized is reported when the Boostrap RPC is run on +// ErrClusterInitialized is reported when the Bootstrap RPC is run on // a node that is already part of an initialized cluster. var ErrClusterInitialized = fmt.Errorf("cluster has already been initialized") diff --git a/pkg/server/server.go b/pkg/server/server.go index 2ae0c0aebfab..e1f2a638b4de 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -1303,27 +1303,23 @@ func (s *Server) Start(ctx context.Context) error { defer time.AfterFunc(30*time.Second, s.cfg.DelayedBootstrapFn).Stop() } - // Set up calling s.cfg.ReadyFn at the right time. Essentially, this call - // determines when `./cockroach [...] --background` returns. For any initialized - // nodes (i.e. already part of a cluster) this is when this method returns - // (assuming there's no error). For nodes that need to join a cluster, we - // return once the initServer is ready to accept requests. - var onSuccessfulReturnFn, onInitServerReady func() - selfBootstrap := initServer.NeedsInit() && len(s.cfg.GossipBootstrapResolvers) == 0 + // We self bootstrap for when we're configured to do so, which should only + // happen during tests and for `cockroach start-single-node`. + selfBootstrap := s.cfg.AutoInitializeCluster && initServer.NeedsInit() if selfBootstrap { - // If a new node is started without join flags, self-bootstrap. - // - // Note: this is behavior slated for removal, see: - // https://github.com/cockroachdb/cockroach/pull/44112 - _, err := initServer.Bootstrap(ctx, &serverpb.BootstrapRequest{}) - switch { - case err == nil: - log.Infof(ctx, "**** add additional nodes by specifying --join=%s", s.cfg.AdvertiseAddr) - case errors.Is(err, ErrClusterInitialized): - default: - // Process is shutting down. + if _, err := initServer.Bootstrap(ctx, &serverpb.BootstrapRequest{}); err != nil { + return err } + } else { + log.Info(ctx, "awaiting init command or join with an already initialized node.") } + + // Set up calling s.cfg.ReadyFn at the right time. Essentially, this call + // determines when `./cockroach [...] --background` returns. For any + // initialized nodes (i.e. already part of a cluster) this is when this + // method returns (assuming there's no error). For nodes that need to join a + // cluster, we return once the initServer is ready to accept requests. + var onSuccessfulReturnFn, onInitServerReady func() { readyFn := func(bool) {} if s.cfg.ReadyFn != nil { @@ -1339,7 +1335,7 @@ func (s *Server) Start(ctx context.Context) error { } // This opens the main listener. When the listener is open, we can call - // initServerReadyFn since any request initated to the initServer at that + // initServerReadyFn since any request initiated to the initServer at that // point will reach it once ServeAndWait starts handling the queue of incoming // connections. startRPCServer(workersCtx) diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 5bbcdf594eab..c31a2d269989 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -150,6 +150,7 @@ func makeTestConfigFromParams(params base.TestServerArgs) Config { } cfg.ClusterName = params.ClusterName cfg.Insecure = params.Insecure + cfg.AutoInitializeCluster = !params.NoAutoInitializeCluster cfg.SocketFile = params.SocketFile cfg.RetryOptions = params.RetryOptions cfg.Locality = params.Locality diff --git a/pkg/testutils/testcluster/testcluster.go b/pkg/testutils/testcluster/testcluster.go index e4bbceac6290..87c358448b05 100644 --- a/pkg/testutils/testcluster/testcluster.go +++ b/pkg/testutils/testcluster/testcluster.go @@ -200,6 +200,7 @@ func StartTestCluster(t testing.TB, nodes int, args base.TestClusterArgs) *TestC serverArgs.Addr = firstListener.Addr().String() } else { serverArgs.JoinAddr = firstListener.Addr().String() + serverArgs.NoAutoInitializeCluster = true } // Disable LBS if any server has a very low scan interval. @@ -321,6 +322,9 @@ func (tc *TestCluster) AddServer(t testing.TB, serverArgs base.TestServerArgs) { func (tc *TestCluster) doAddServer(t testing.TB, serverArgs base.TestServerArgs) error { serverArgs.PartOfCluster = true + if serverArgs.JoinAddr != "" { + serverArgs.NoAutoInitializeCluster = true + } // Check args even though they might have been checked in StartTestCluster; // this method might be called for servers being added after the cluster was // started, in which case the check has not been performed.