roachprod: implement --dry-run and --verbose

pkg/BUILD.bazel

@@ -312,6 +312,7 @@ ALL_TESTS = [
     "//pkg/roachprod/prometheus:prometheus_test",
     "//pkg/roachprod/promhelperclient:promhelperclient_test",
     "//pkg/roachprod/ssh:ssh_test",
+    "//pkg/roachprod/vm/cli:cli_test",
     "//pkg/roachprod/vm/gce:gce_test",
     "//pkg/roachprod/vm/local:local_test",
     "//pkg/roachprod/vm:vm_test",
@@ -1634,6 +1635,8 @@ GO_TARGETS = [
     "//pkg/roachprod/vm/aws/terraformgen:terraformgen_lib",
     "//pkg/roachprod/vm/aws:aws",
     "//pkg/roachprod/vm/azure:azure",
+    "//pkg/roachprod/vm/cli:cli",
+    "//pkg/roachprod/vm/cli:cli_test",
     "//pkg/roachprod/vm/flagstub:flagstub",
     "//pkg/roachprod/vm/gce/testutils:testutils",
     "//pkg/roachprod/vm/gce:gce",

pkg/cmd/roachprod/cli/commands.go

@@ -1117,7 +1117,7 @@ func buildSSHKeysListCmd() *cobra.Command {
 		Use:   "list",
 		Short: "list every SSH public key installed on clusters managed by roachprod",
 		Run: wrap(func(cmd *cobra.Command, args []string) error {
-			authorizedKeys, err := gce.GetUserAuthorizedKeys()
+			authorizedKeys, err := gce.GetUserAuthorizedKeys(config.Logger)
 			if err != nil {
 				return err
 			}
@@ -1151,7 +1151,7 @@ func buildSSHKeysAddCmd() *cobra.Command {
 			}
 
 			fmt.Printf("Adding new public key for user %s...\n", ak.User)
-			return gce.AddUserAuthorizedKey(ak)
+			return gce.AddUserAuthorizedKey(config.Logger, ak)
 		}),
 	}
 	sshKeysAddCmd.Flags().StringVar(&sshKeyUser, "user", config.OSUser.Username,
@@ -1168,7 +1168,7 @@ func buildSSHKeysRemoveCmd() *cobra.Command {
 		Run: wrap(func(cmd *cobra.Command, args []string) error {
 			user := args[0]
 
-			existingKeys, err := gce.GetUserAuthorizedKeys()
+			existingKeys, err := gce.GetUserAuthorizedKeys(config.Logger)
 			if err != nil {
 				return fmt.Errorf("failed to fetch existing keys: %w", err)
 			}

pkg/cmd/roachprod/cli/flags.go

@@ -121,6 +121,10 @@ func initRootCmdFlags(rootCmd *cobra.Command) {
 		"use-shared-user", true,
 		fmt.Sprintf("use the shared user %q for ssh rather than your user %q",
 			config.SharedUser, config.OSUser.Username))
+	rootCmd.PersistentFlags().BoolVarP(&config.DryRun, "dry-run", "d",
+		false, "dry-run mode (log all commands & execute read-only ones; i.e., no infra changes)")
+	rootCmd.PersistentFlags().BoolVarP(&config.Verbose, "verbose", "v",
+		false, "verbose mode (log all executed commands)")
 }
 
 func initCreateCmdFlags(createCmd *cobra.Command) {
@@ -201,7 +205,7 @@ func initListCmdFlags(listCmd *cobra.Command) {
 		"Show cost estimates",
 	)
 	listCmd.Flags().BoolVarP(&listDetails,
-		"details", "d", false, "Show cluster details")
+		"details", "", false, "Show cluster details")
 	listCmd.Flags().BoolVar(&listJSON,
 		"json", false, "Show cluster specs in a json format")
 	listCmd.Flags().BoolVarP(&listMine,

pkg/cmd/roachprod/cli/util.go

@@ -12,6 +12,7 @@ import (
 	"text/tabwriter"
 	"time"
 
+	"github.com/cockroachdb/cockroach/pkg/roachprod/config"
 	rperrors "github.com/cockroachdb/cockroach/pkg/roachprod/errors"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm/gce"
@@ -202,7 +203,7 @@ Node specification
 func ValidateAndConfigure(cmd *cobra.Command, args []string) {
 	// Skip validation for commands that are self-sufficient.
 	switch cmd.Name() {
-	case "help", "version", "list":
+	case "help", "version":
 		return
 	}
 
@@ -237,4 +238,10 @@ func ValidateAndConfigure(cmd *cobra.Command, args []string) {
 		}
 		providersSet[p] = struct{}{}
 	}
+	if config.DryRun {
+		if config.Verbose {
+			printErrAndExit(fmt.Errorf("--verbose and --dry-run are mutually exclusive"))
+		}
+		config.Logger.Printf("Enabling [***Experimental***] --dry-run mode. No infra changes will be made!")
+	}
 }

pkg/roachprod/BUILD.bazel

@@ -27,6 +27,7 @@ go_library(
         "//pkg/roachprod/vm",
         "//pkg/roachprod/vm/aws",
         "//pkg/roachprod/vm/azure",
+        "//pkg/roachprod/vm/cli",
         "//pkg/roachprod/vm/gce",
         "//pkg/roachprod/vm/local",
         "//pkg/server/debug/replay",

pkg/roachprod/cloud/cluster_cloud.go

@@ -433,7 +433,7 @@ func DestroyCluster(l *logger.Logger, c *Cluster) error {
 	// and clean-up entries prematurely.
 	stopSpinner = ui.NewDefaultSpinner(l, "Destroying DNS entries").Start()
 	dnsErr := vm.FanOutDNS(c.VMs, func(p vm.DNSProvider, vms vm.List) error {
-		return p.DeleteRecordsBySubdomain(context.Background(), c.Name)
+		return p.DeleteRecordsBySubdomain(context.Background(), l, c.Name)
 	})
 	stopSpinner()
 

pkg/roachprod/cloud/gc.go

@@ -518,7 +518,7 @@ func GCDNS(l *logger.Logger, cloud *Cloud, dryrun bool) error {
 		if !ok {
 			continue
 		}
-		records, err := p.ListRecords(ctx)
+		records, err := p.ListRecords(ctx, l)
 		if err != nil {
 			return err
 		}
@@ -541,7 +541,7 @@ func GCDNS(l *logger.Logger, cloud *Cloud, dryrun bool) error {
 		sort.Strings(recordNames)
 
 		if err := destroyResource(dryrun, func() error {
-			return p.DeleteRecordsByName(ctx, recordNames...)
+			return p.DeleteRecordsByName(ctx, l, recordNames...)
 		}); err != nil {
 			return err
 		}

pkg/roachprod/clusters_cache.go

@@ -86,10 +86,20 @@ func saveCluster(l *logger.Logger, c *cloud.Cluster) error {
 	err = errors.CombineErrors(err, tmpFile.Sync())
 	err = errors.CombineErrors(err, tmpFile.Close())
 	if err == nil {
-		err = os.Rename(tmpFile.Name(), filename)
+		if config.DryRun || config.Verbose {
+			l.Printf("exec: mv %s %s", tmpFile.Name(), filename)
+		}
+		if !config.DryRun {
+			err = os.Rename(tmpFile.Name(), filename)
+		}
 	}
 	if err != nil {
-		_ = os.Remove(tmpFile.Name())
+		if config.DryRun || config.Verbose {
+			l.Printf("exec: rm %s", tmpFile.Name())
+		}
+		if !config.DryRun {
+			_ = os.Remove(tmpFile.Name())
+		}
 		return err
 	}
 	return nil
@@ -258,5 +268,12 @@ func (localVMStorage) SaveCluster(l *logger.Logger, cluster *cloud.Cluster) erro
 // DeleteCluster is part of the local.VMStorage interface.
 func (localVMStorage) DeleteCluster(l *logger.Logger, name string) error {
 	path := clusterFilename(name)
+	if config.DryRun || config.Verbose {
+		l.Printf("exec: rm %s", path)
+	}
+	if config.DryRun {
+		return nil
+	}
+
 	return os.Remove(path)
 }

pkg/roachprod/config/config.go

@@ -31,6 +31,10 @@ var (
 	OSUser *user.User
 	// Quiet is used to disable fancy progress output.
 	Quiet = false
+	// DryRun disables executing commands which would otherwise cause infra. changes.
+	DryRun = false
+	// Verbose enables logging all executed commands. What gets logged is a superset of DryRun.
+	Verbose = false
 	// The default roachprod logger.
 	// N.B. When roachprod is used via CLI, this logger is used for all output.
 	//	When roachprod is used via API (e.g. from roachtest), this logger is used only in the few cases,

pkg/roachprod/install/BUILD.bazel

@@ -34,6 +34,7 @@ go_library(
         "//pkg/roachprod/ui",
         "//pkg/roachprod/vm",
         "//pkg/roachprod/vm/aws",
+        "//pkg/roachprod/vm/cli",
         "//pkg/roachprod/vm/gce",
         "//pkg/roachprod/vm/local",
         "//pkg/testutils",

pkg/roachprod/install/cluster_synced.go

@@ -32,6 +32,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachprod/ui"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm/aws"
+	"github.com/cockroachdb/cockroach/pkg/roachprod/vm/cli"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm/local"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
@@ -438,7 +439,7 @@ func (c *SyncedCluster) Stop(
 			return err
 		}
 
-		services, err := c.DiscoverServices(ctx, name, ServiceTypeSQL)
+		services, err := c.DiscoverServices(ctx, l, name, ServiceTypeSQL)
 		if err != nil {
 			return err
 		}
@@ -2339,6 +2340,8 @@ func (c *SyncedCluster) Logs(
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = &stderrBuf
 
+		cli.MaybeLogCmd(context.Background(), l, cmd)
+
 		if err := cmd.Run(); err != nil {
 			if ctx.Err() != nil {
 				return nil
@@ -2379,6 +2382,9 @@ func (c *SyncedCluster) Logs(
 		cmd.Stdout = out
 		var errBuf bytes.Buffer
 		cmd.Stderr = &errBuf
+
+		cli.MaybeLogCmd(ctx, l, cmd)
+
 		if err := cmd.Run(); err != nil && ctx.Err() == nil {
 			return errors.Wrapf(err, "failed to run cockroach debug merge-logs:\n%v", errBuf.String())
 		}
@@ -2632,7 +2638,7 @@ func (c *SyncedCluster) pgurls(
 	}
 	m := make(map[Node]string, len(hosts))
 	for node, host := range hosts {
-		desc, err := c.DiscoverService(ctx, node, virtualClusterName, ServiceTypeSQL, sqlInstance)
+		desc, err := c.DiscoverService(ctx, l, node, virtualClusterName, ServiceTypeSQL, sqlInstance)
 		if err != nil {
 			return nil, err
 		}
@@ -2667,7 +2673,7 @@ func (c *SyncedCluster) loadBalancerURL(
 	sqlInstance int,
 	auth PGAuthMode,
 ) (string, error) {
-	services, err := c.DiscoverServices(ctx, virtualClusterName, ServiceTypeSQL)
+	services, err := c.DiscoverServices(ctx, l, virtualClusterName, ServiceTypeSQL)
 	if err != nil {
 		return "", err
 	}
@@ -2802,6 +2808,8 @@ func scp(ctx context.Context, l *logger.Logger, src, dest string) (*RunResultDet
 	cmd := exec.CommandContext(ctx, args[0], args[1:]...)
 	cmd.WaitDelay = time.Second // make sure the call below returns when the context is canceled
 
+	cli.MaybeLogCmd(context.Background(), l, cmd)
+
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		err = rperrors.NewSSHError(errors.Wrapf(err, "~ %s\n%s", strings.Join(args, " "), out))
@@ -2993,10 +3001,10 @@ func (c *SyncedCluster) Init(ctx context.Context, l *logger.Logger, node Node) e
 
 // allPublicAddrs returns a string that can be used when starting cockroach to
 // indicate the location of all nodes in the cluster.
-func (c *SyncedCluster) allPublicAddrs(ctx context.Context) (string, error) {
+func (c *SyncedCluster) allPublicAddrs(ctx context.Context, l *logger.Logger) (string, error) {
 	var addrs []string
 	for _, node := range c.Nodes {
-		port, err := c.NodePort(ctx, node, "" /* virtualClusterName */, 0 /* sqlInstance */)
+		port, err := c.NodePort(ctx, l, node, "" /* virtualClusterName */, 0 /* sqlInstance */)
 		if err != nil {
 			return "", err
 		}