Skip to content

Commit

Permalink
roachprod: fix roachprod init.
Browse files Browse the repository at this point in the history
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 cockroachdb#88226.

Release note: None
  • Loading branch information
renatolabs authored and Miral Gadani committed Jan 18, 2023
1 parent fa8af54 commit 2ecd18f
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 23 deletions.
5 changes: 2 additions & 3 deletions pkg/roachprod/install/cluster_synced.go
Original file line number Diff line number Diff line change
Expand Up @@ -2206,12 +2206,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)
Expand Down
22 changes: 2 additions & 20 deletions pkg/roachprod/install/cockroach.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,27 +208,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
})
Expand Down

0 comments on commit 2ecd18f

Please sign in to comment.