roachprod: Make multiple set [provider]-zones always geo-distribute n…

…odes

Before: if multiple zones were set for a provider and --geo wasn't set, all
hosts would be started in just one zone in one region.

Why change? Because if multiple zones are set, the intention is that they be
used.

Now, --geo and --[provider]-zones work as follows for gcloud, aws and azure:

1. when geo and zones are not set, nodes are all placed in one of the
   default zones
2. when geo is set but zones aren't, nodes are spread evenly across the
   default zones
3. when zones are set, nodes are spread evenly across the specified zones

Fixes #38542.

Release note: None

pkg/cmd/roachprod/README.md

@@ -87,6 +87,29 @@ marc-foo: 23h59m42s remaining
 Syncing...
 ```
 
+#### Choosing a Provider
+
+Use the `--clouds` flag to set which cloud provider(s) to use. Ex:
+
+```
+$ roachprod create foo --clouds gce,aws
+```
+
+#### Node Distribution Options
+
+There are a couple flags that interact to create nodes in one zone or in
+geographically distributed zones:
+
+- `--geo`
+- the `--[provider]-zones` flags (`--gce-zones`, `--aws-zones`, `--azure-locations`)
+
+Here's what to expect when the options are combined:
+
+- _If neither are set_: nodes are all placed within one of the the provider's default zones
+- _`--geo` only_: nodes are spread across the provider's default zones
+- _`--[provider]-zones` or `--geo --[provider]-zones`_: nodes are spread across
+  all the specified zones
+
 ### Interact using crl-prod tools
 
 `roachprod` populates hosts files in `~/.roachprod/hosts`. These are used by

pkg/cmd/roachprod/vm/aws/aws.go

@@ -77,8 +77,9 @@ type providerOpts struct {
 	EBSProvisionedIOPs int
 
 	// CreateZones stores the list of zones for used cluster creation.
-	// When specifying the geo flag, nodes will be placed over these zones.
-	// See defaultGeoZones.
+	// When > 1 zone specified, geo is automatically used, otherwise, geo depends
+	// on the geo flag being set. If no zones specified, defaultCreateZones are
+	// used. See defaultCreateZones.
 	CreateZones []string
 }
 
@@ -96,8 +97,8 @@ var defaultConfig = func() (cfg *awsConfig) {
 }()
 
 // defaultCreateZones is the list of availability zones used by default for
-// cluster creation. If the geo flag is specified, one zone from each region
-// is randomly chosen.
+// cluster creation. If the geo flag is specified, nodes are distributed between
+// zones.
 var defaultCreateZones = []string{
 	"us-east-2b",
 	"us-west-2b",
@@ -135,10 +136,11 @@ func (o *providerOpts) ConfigureCreateFlags(flags *pflag.FlagSet) {
 		1000, "Number of IOPs to provision, only used if "+ProviderName+
 			"-ebs-volume-type=io1")
 
-	flags.StringSliceVar(&o.CreateZones, ProviderName+"-zones", defaultCreateZones,
-		"aws availability zones to use for cluster creation, the cluster "+
-			" will be spread out evenly by zone (if geo). If zones are formatted as "+
-			"AZ:N where N is an integer, the zone will be repeated N times")
+	flags.StringSliceVar(&o.CreateZones, ProviderName+"-zones", nil,
+		fmt.Sprintf("aws availability zones to use for cluster creation. If zones are formatted\n"+
+			"as AZ:N where N is an integer, the zone will be repeated N times. If > 1\n"+
+			"zone specified, the cluster will be spread out evenly by zone regardless\n"+
+			"of geo (default [%s])", strings.Join(defaultCreateZones, ",")))
 }
 
 func (o *providerOpts) ConfigureClusterFlags(flags *pflag.FlagSet, _ vm.MultipleProjectsOption) {
@@ -209,10 +211,17 @@ func (p *Provider) Create(names []string, opts vm.CreateOpts) error {
 	if err := p.ConfigSSH(); err != nil {
 		return err
 	}
+
 	expandedZones, err := vm.ExpandZonesFlag(p.opts.CreateZones)
 	if err != nil {
 		return err
 	}
+
+	useDefaultZones := len(expandedZones) == 0
+	if useDefaultZones {
+		expandedZones = defaultCreateZones
+	}
+
 	regions, err := p.allRegions(expandedZones)
 	if err != nil {
 		return err
@@ -222,7 +231,7 @@ func (p *Provider) Create(names []string, opts vm.CreateOpts) error {
 	}
 
 	var zones []string // contains an az corresponding to each entry in names
-	if !opts.GeoDistributed {
+	if !opts.GeoDistributed && (useDefaultZones || len(expandedZones) == 1) {
 		// Only use one zone in the region if we're not creating a geo cluster.
 		regionZones, err := p.regionZones(regions[0], expandedZones)
 		if err != nil {

pkg/cmd/roachprod/vm/azure/azure.go

@@ -119,8 +119,12 @@ func (p *Provider) Create(names []string, opts vm.CreateOpts) error {
 	ctx, cancel := context.WithTimeout(context.Background(), p.opts.operationTimeout)
 	defer cancel()
 
-	if !opts.GeoDistributed {
-		p.opts.locations = []string{p.opts.locations[0]}
+	if len(p.opts.locations) == 0 {
+		if opts.GeoDistributed {
+			p.opts.locations = defaultLocations
+		} else {
+			p.opts.locations = []string{defaultLocations[0]}
+		}
 	}
 
 	if _, err := p.createVNets(ctx, p.opts.locations); err != nil {

pkg/cmd/roachprod/vm/azure/flags.go

@@ -11,6 +11,8 @@
 package azure
 
 import (
+	"fmt"
+	"strings"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute"
@@ -26,6 +28,12 @@ type providerOpts struct {
 	vnetName         string
 }
 
+var defaultLocations = []string{
+	"eastus2",
+	"westus",
+	"westeurope",
+}
+
 // ConfigureCreateFlags implements vm.ProviderFlags.
 func (o *providerOpts) ConfigureCreateFlags(flags *pflag.FlagSet) {
 	flags.DurationVar(&o.operationTimeout, ProviderName+"-timeout", 10*time.Minute,
@@ -35,9 +43,9 @@ func (o *providerOpts) ConfigureCreateFlags(flags *pflag.FlagSet) {
 	flags.StringVar(&o.machineType, ProviderName+"-machine-type",
 		string(compute.VirtualMachineSizeTypesStandardD4V3),
 		"Machine type (see https://azure.microsoft.com/en-us/pricing/details/virtual-machines/linux/)")
-	flags.StringSliceVar(&o.locations, ProviderName+"-locations",
-		[]string{"eastus2", "westus", "westeurope"},
-		"Locations for cluster (see `az account list-locations`)")
+	flags.StringSliceVar(&o.locations, ProviderName+"-locations", nil,
+		fmt.Sprintf("Locations for cluster (see `az account list-locations`) (default\n[%s])",
+			strings.Join(defaultLocations, ",")))
 	flags.StringVar(&o.vnetName, ProviderName+"-vnet-name", "common",
 		"The name of the VNet to use")
 }

pkg/cmd/roachprod/vm/gce/gcloud.go

@@ -194,6 +194,14 @@ type projectsVal struct {
 	opts                   *providerOpts
 }
 
+// defaultZones is the list of  zones used by default for cluster creation.
+// If the geo flag is specified, nodes are distributed between zones.
+var defaultZones = []string{
+	"us-east1-b",
+	"us-west1-b",
+	"europe-west2-b",
+}
+
 // Set is part of the pflag.Value interface.
 func (v projectsVal) Set(projects string) error {
 	if projects == "" {
@@ -252,17 +260,18 @@ func (p *Provider) GetProjects() []string {
 func (o *providerOpts) ConfigureCreateFlags(flags *pflag.FlagSet) {
 	flags.StringVar(&o.MachineType, "machine-type", "n1-standard-4", "DEPRECATED")
 	_ = flags.MarkDeprecated("machine-type", "use "+ProviderName+"-machine-type instead")
-	flags.StringSliceVar(&o.Zones, "zones", []string{"us-east1-b", "us-west1-b", "europe-west2-b"}, "DEPRECATED")
+	flags.StringSliceVar(&o.Zones, "zones", nil, "DEPRECATED")
 	_ = flags.MarkDeprecated("zones", "use "+ProviderName+"-zones instead")
 
 	flags.StringVar(&o.ServiceAccount, ProviderName+"-service-account",
 		os.Getenv("GCE_SERVICE_ACCOUNT"), "Service account to use")
 	flags.StringVar(&o.MachineType, ProviderName+"-machine-type", "n1-standard-4",
 		"Machine type (see https://cloud.google.com/compute/docs/machine-types)")
-	flags.StringSliceVar(&o.Zones, ProviderName+"-zones",
-		[]string{"us-east1-b", "us-west1-b", "europe-west2-b"},
-		"Zones for cluster; If zones are formatted as "+
-			"AZ:N where N is an integer, the zone will be repeated N times")
+	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"+
+			"will be repeated N times. If > 1 zone specified, nodes will be geo-distributed\n"+
+			"regardless of geo (default [%s])",
+			strings.Join(defaultZones, ",")))
 	flags.StringVar(&o.Image, ProviderName+"-image", "ubuntu-1604-xenial-v20190122a",
 		"Image to use to create the vm, ubuntu-1904-disco-v20191008 is a more modern image")
 	flags.IntVar(&o.SSDCount, ProviderName+"-local-ssd-count", 1,
@@ -347,8 +356,12 @@ func (p *Provider) Create(names []string, opts vm.CreateOpts) error {
 	if err != nil {
 		return err
 	}
-	if !opts.GeoDistributed {
-		zones = []string{zones[0]}
+	if len(zones) == 0 {
+		if opts.GeoDistributed {
+			zones = defaultZones
+		} else {
+			zones = []string{defaultZones[0]}
+		}
 	}
 
 	// Fixed args.