From e16cc14fa66b08871705e72faf993a0a7e002803 Mon Sep 17 00:00:00 2001
From: Alex Sarkesian <sarkesian@cockroachlabs.com>
Date: Tue, 16 Nov 2021 01:41:53 -0500
Subject: [PATCH] pkg/roachprod: fix mounting on GCP and default to multiple
 stores

Fixes bugs introduced by #72553, which ported support for multiple
stores from AWS to GCP.  This change fixes the bugs that caused
disks mounted to not be written to `/etc/fstab` on GCP instances,
causing mounts to go missing on instance restart.  Additionally, this
change modifies the defaults on GCP and AWS to prefer creating multiple
stores rather than creating a RAID 0 array, thus preserving the existing
behavior used in roachtests such as `kv/multi-store-with-overload`.

Fixes #72635

Release note: None
---
 pkg/roachprod/vm/aws/aws.go     | 4 ++--
 pkg/roachprod/vm/aws/support.go | 4 ++--
 pkg/roachprod/vm/gce/gcloud.go  | 4 ++--
 pkg/roachprod/vm/gce/utils.go   | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/pkg/roachprod/vm/aws/aws.go b/pkg/roachprod/vm/aws/aws.go
index 915e141e8589..46809de6dcd6 100644
--- a/pkg/roachprod/vm/aws/aws.go
+++ b/pkg/roachprod/vm/aws/aws.go
@@ -301,8 +301,8 @@ func (o *ProviderOpts) ConfigureCreateFlags(flags *pflag.FlagSet) {
 	flags.StringVar(&o.ImageAMI, ProviderName+"-image-ami",
 		o.ImageAMI, "Override image AMI to use.  See https://awscli.amazonaws.com/v2/documentation/api/latest/reference/ec2/describe-images.html")
 	flags.BoolVar(&o.UseMultipleDisks, ProviderName+"-enable-multiple-stores",
-		o.UseMultipleDisks, "Enable the use of multiple stores by creating one store directory per disk. "+
-			"Default is to raid0 stripe all disks. "+
+		true, "Enable the use of multiple stores by creating one store directory per disk. "+
+			"Default enabled, disable to raid0 stripe all disks. "+
 			"See repeating --"+ProviderName+"-ebs-volume for adding extra volumes.")
 	flags.Float64Var(&o.CreateRateLimit, ProviderName+"-create-rate-limit", o.CreateRateLimit, "aws"+
 		" rate limit (per second) for instance creation. This is used to avoid hitting the request"+
diff --git a/pkg/roachprod/vm/aws/support.go b/pkg/roachprod/vm/aws/support.go
index 570d72fa552f..99931048b6ac 100644
--- a/pkg/roachprod/vm/aws/support.go
+++ b/pkg/roachprod/vm/aws/support.go
@@ -52,7 +52,7 @@ mount_prefix="/mnt/data"
 for d in $(ls /dev/nvme?n1 /dev/xvdd); do
   if ! mount | grep ${d}; then
     disks+=("${d}")
-    echo "Disk ${d} not mounted, creating..."
+    echo "Disk ${d} not mounted, need to mount..."
   else
     echo "Disk ${d} already mounted, skipping..."
   fi
@@ -70,7 +70,7 @@ elif [ "${#disks[@]}" -eq "1" ] || [ -n "$use_multiple_disks" ]; then
   do
     mountpoint="${mount_prefix}${disknum}"
     disknum=$((disknum + 1 ))
-    echo "Creating ${mountpoint}"
+    echo "Mounting ${disk} at ${mountpoint}"
     mkdir -p ${mountpoint}
     mkfs.ext4 -F ${disk}
     mount -o ${mount_opts} ${disk} ${mountpoint}
diff --git a/pkg/roachprod/vm/gce/gcloud.go b/pkg/roachprod/vm/gce/gcloud.go
index 76d707a571e0..fd950c2bc4cc 100644
--- a/pkg/roachprod/vm/gce/gcloud.go
+++ b/pkg/roachprod/vm/gce/gcloud.go
@@ -303,8 +303,8 @@ func (o *ProviderOpts) ConfigureCreateFlags(flags *pflag.FlagSet) {
 	flags.IntVar(&o.PDVolumeSize, ProviderName+"-pd-volume-size", 500,
 		"Size in GB of persistent disk volume, only used if local-ssd=false")
 	flags.BoolVar(&o.UseMultipleDisks, ProviderName+"-enable-multiple-stores",
-		o.UseMultipleDisks, "Enable the use of multiple stores by creating one store directory per disk. "+
-			"Default is to raid0 stripe all disks.")
+		true, "Enable the use of multiple stores by creating one store directory per disk. "+
+			"Default enabled, disable to raid0 stripe all disks.")
 
 	flags.StringSliceVar(&o.Zones, ProviderName+"-zones", nil,
 		fmt.Sprintf("Zones for cluster. If zones are formatted as AZ:N where N is an integer, the zone\n"+
diff --git a/pkg/roachprod/vm/gce/utils.go b/pkg/roachprod/vm/gce/utils.go
index b80b12b237cf..73f9a5f99152 100644
--- a/pkg/roachprod/vm/gce/utils.go
+++ b/pkg/roachprod/vm/gce/utils.go
@@ -73,7 +73,7 @@ for d in $(ls /dev/disk/by-id/google-local-* /dev/disk/by-id/google-persistent-d
   if ! mount | grep ${d}; then
 {{ end }}
     disks+=("${d}")
-    echo "Disk ${d} not mounted, creating..."
+    echo "Disk ${d} not mounted, need to mount..."
   else
     echo "Disk ${d} already mounted, skipping..."
   fi
@@ -90,7 +90,7 @@ elif [ "${#disks[@]}" -eq "1" ] || [ -n "$use_multiple_disks" ]; then
   do
     mountpoint="${mount_prefix}${disknum}"
     disknum=$((disknum + 1 ))
-    echo "Creating ${mountpoint}"
+    echo "Mounting ${disk} at ${mountpoint}"
     mkdir -p ${mountpoint}
 {{ if .Zfs }}
     zpool create -f $(basename $mountpoint) -m ${mountpoint} ${disk}
@@ -98,7 +98,7 @@ elif [ "${#disks[@]}" -eq "1" ] || [ -n "$use_multiple_disks" ]; then
 {{ else }}
     mkfs.ext4 -q -F ${disk}
     mount -o ${mount_opts} ${disk} ${mountpoint}
-    echo "/dev/md0 ${mountpoint} ext4 ${mount_opts} 1 1" | tee -a /etc/fstab
+    echo "{d} ${mountpoint} ext4 ${mount_opts} 1 1" | tee -a /etc/fstab
 {{ end }}
     chmod 777 ${mountpoint}
   done