From 9c0b17d39248412e17176bacfdc8f765e1e40dba Mon Sep 17 00:00:00 2001
From: Renato Costa <renato@cockroachlabs.com>
Date: Mon, 26 Sep 2022 10:32:04 -0400
Subject: [PATCH] roachprod: set default cluster settings when starting

In #88514, the cluster start logic was refactored to reuse the same
code across `init` and `start`, fixing a bug in the former. However,
the refactoring overlooked the fact that we previously always set the
default cluster settings when there's more than one node in the
cluster.

This fixes that by setting the default cluster settings in that case;
one particularly important cluster setting is the license key,
necessary for some roachtests.

Fixes #88660.
Fixes #88665.
Fixes #88666.
Fixes #88710.

Release note: None
---
 pkg/roachprod/install/cluster_synced.go | 17 +++----------
 pkg/roachprod/install/cockroach.go      | 33 ++++++++++++++++---------
 2 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/pkg/roachprod/install/cluster_synced.go b/pkg/roachprod/install/cluster_synced.go
index 4b93d19d07de..def0a1e99932 100644
--- a/pkg/roachprod/install/cluster_synced.go
+++ b/pkg/roachprod/install/cluster_synced.go
@@ -2208,26 +2208,17 @@ func (c *SyncedCluster) ParallelE(
 // to maintain parity with auto-init behavior of `roachprod start` (when
 // --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
+	// We reserve a few special operations for the first node, so we
 	// strive to maintain the same here for interoperability.
 	const firstNodeIdx = 1
 
-	l.Printf("%s: initializing cluster\n", c.Name)
-	initOut, err := c.initializeCluster(ctx, firstNodeIdx)
-	if err != nil {
+	if err := c.initializeCluster(ctx, l, firstNodeIdx); err != nil {
 		return errors.WithDetail(err, "install.Init() failed: unable to initialize cluster.")
 	}
-	if initOut != "" {
-		l.Printf(initOut)
-	}
 
-	l.Printf("%s: setting cluster settings", c.Name)
-	clusterSettingsOut, err := c.setClusterSettings(ctx, l, firstNodeIdx)
-	if err != nil {
+	if err := c.setClusterSettings(ctx, l, firstNodeIdx); err != nil {
 		return errors.WithDetail(err, "install.Init() failed: unable to set cluster settings.")
 	}
-	if clusterSettingsOut != "" {
-		l.Printf(clusterSettingsOut)
-	}
+
 	return nil
 }
diff --git a/pkg/roachprod/install/cockroach.go b/pkg/roachprod/install/cockroach.go
index a1a2cc7761a7..17fdcb85660d 100644
--- a/pkg/roachprod/install/cockroach.go
+++ b/pkg/roachprod/install/cockroach.go
@@ -208,10 +208,14 @@ func (c *SyncedCluster) Start(ctx context.Context, l *logger.Logger, startOpts S
 
 		shouldInit := !c.useStartSingleNode()
 		if shouldInit {
-			if err := c.Init(ctx, l); err != nil {
+			if err := c.initializeCluster(ctx, l, node); err != nil {
 				return nil, errors.Wrap(err, "failed to initialize cluster")
 			}
 		}
+
+		if err := c.setClusterSettings(ctx, l, node); err != nil {
+			return nil, errors.Wrap(err, "failed to set cluster settings")
+		}
 		return nil, nil
 	})
 }
@@ -619,38 +623,45 @@ func (c *SyncedCluster) maybeScaleMem(val int) int {
 	return val
 }
 
-func (c *SyncedCluster) initializeCluster(ctx context.Context, node Node) (string, error) {
+func (c *SyncedCluster) initializeCluster(ctx context.Context, l *logger.Logger, node Node) error {
+	l.Printf("%s: initializing cluster\n", c.Name)
 	initCmd := c.generateInitCmd(node)
 
 	sess, err := c.newSession(node)
 	if err != nil {
-		return "", err
+		return err
 	}
 	defer sess.Close()
 
 	out, err := sess.CombinedOutput(ctx, initCmd)
 	if err != nil {
-		return "", errors.Wrapf(err, "~ %s\n%s", initCmd, out)
+		return errors.Wrapf(err, "~ %s\n%s", initCmd, out)
 	}
-	return strings.TrimSpace(string(out)), nil
+
+	if out := strings.TrimSpace(string(out)); out != "" {
+		l.Printf(out)
+	}
+	return nil
 }
 
-func (c *SyncedCluster) setClusterSettings(
-	ctx context.Context, l *logger.Logger, node Node,
-) (string, error) {
+func (c *SyncedCluster) setClusterSettings(ctx context.Context, l *logger.Logger, node Node) error {
+	l.Printf("%s: setting cluster settings", c.Name)
 	clusterSettingCmd := c.generateClusterSettingCmd(l, node)
 
 	sess, err := c.newSession(node)
 	if err != nil {
-		return "", err
+		return err
 	}
 	defer sess.Close()
 
 	out, err := sess.CombinedOutput(ctx, clusterSettingCmd)
 	if err != nil {
-		return "", errors.Wrapf(err, "~ %s\n%s", clusterSettingCmd, out)
+		return errors.Wrapf(err, "~ %s\n%s", clusterSettingCmd, out)
 	}
-	return strings.TrimSpace(string(out)), nil
+	if out := strings.TrimSpace(string(out)); out != "" {
+		l.Printf(out)
+	}
+	return nil
 }
 
 func (c *SyncedCluster) generateClusterSettingCmd(l *logger.Logger, node Node) string {