From 72fc0afeba46a16734070b2878d2f641c38348b5 Mon Sep 17 00:00:00 2001 From: Renato Costa Date: Thu, 22 Sep 2022 17:18:55 -0400 Subject: [PATCH] roachprod: fix `roachprod init`. When a cluster is started with the `--skip-init` option, the caller can run `roachprod init` at any time to initialize the cluster. Unfortunately, the code used to initialize the cluster was duplicated: one copy existed in the `start` path, and another in the `init` path. Since the latter is used far less frequently, it had a bug that went unnoticed: it hardcoded the first node index as `0`, when node indices start at 1. This commit fixes the issue by updating the constant and sharing code between `init `and `start`. Fixes #88226. Release note: None --- pkg/roachprod/install/cluster_synced.go | 5 ++--- pkg/roachprod/install/cockroach.go | 22 ++-------------------- 2 files changed, 4 insertions(+), 23 deletions(-) diff --git a/pkg/roachprod/install/cluster_synced.go b/pkg/roachprod/install/cluster_synced.go index df0bac7e9590..7e18ea595f96 100644 --- a/pkg/roachprod/install/cluster_synced.go +++ b/pkg/roachprod/install/cluster_synced.go @@ -2290,12 +2290,11 @@ func (c *SyncedCluster) ParallelE( // Init initializes the cluster. It does it through node 1 (as per TargetNodes) // to maintain parity with auto-init behavior of `roachprod start` (when -// --skip-init) is not specified. The implementation should be kept in -// sync with Start(). +// --skip-init) is not specified. func (c *SyncedCluster) Init(ctx context.Context, l *logger.Logger) error { // See Start(). We reserve a few special operations for the first node, so we // strive to maintain the same here for interoperability. - const firstNodeIdx = 0 + const firstNodeIdx = 1 l.Printf("%s: initializing cluster\n", c.Name) initOut, err := c.initializeCluster(ctx, firstNodeIdx) diff --git a/pkg/roachprod/install/cockroach.go b/pkg/roachprod/install/cockroach.go index 9ae9591baaa8..43bb284d1ca4 100644 --- a/pkg/roachprod/install/cockroach.go +++ b/pkg/roachprod/install/cockroach.go @@ -184,27 +184,9 @@ func (c *SyncedCluster) Start(ctx context.Context, l *logger.Logger, startOpts S shouldInit := !c.useStartSingleNode() if shouldInit { - l.Printf("%s: initializing cluster", c.Name) - initOut, err := c.initializeCluster(ctx, node) - if err != nil { - return nil, errors.WithDetail(err, "unable to initialize cluster") + if err := c.Init(ctx, l); err != nil { + return nil, errors.Wrap(err, "failed to initialize cluster") } - - if initOut != "" { - l.Printf(initOut) - } - } - - // We're sure to set cluster settings after having initialized the - // cluster. - - l.Printf("%s: setting cluster settings", c.Name) - clusterSettingsOut, err := c.setClusterSettings(ctx, l, node) - if err != nil { - return nil, errors.Wrap(err, "unable to set cluster settings") - } - if clusterSettingsOut != "" { - l.Printf(clusterSettingsOut) } return nil, nil })