Merge pull request juju#10375 from anastasiamac/merge-25-26-2506

juju#10375 Bring forward: juju#10355 juju#10363
google38438 · Jun 25, 2019 · 4c98b64 · 4c98b64
2 parents fe82d5e + 7b6e2fd
commit 4c98b64
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 34 deletions.
diff --git a/provider/azure/config.go b/provider/azure/config.go
@@ -6,6 +6,7 @@ package azure
 import (
 	"strings"
 
+	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-08-01/network"
 	"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2018-07-01/storage"
 	"github.com/juju/errors"
 	"github.com/juju/schema"
@@ -15,8 +16,15 @@ import (
 )
 
 const (
+	// configAttrStorageAccountType mirrors the storage SKU name in the Azure SDK
+	//
+	// The term "storage account" has been replaced with "SKU name" in recent
+	// Azure SDK, but we keep it to maintain backwards-compatibility.
 	configAttrStorageAccountType = "storage-account-type"
 
+	// configAttrLoadBalancerSkuName mirrors the LoadBalancerSkuName type in the Azure SDK
+	configAttrLoadBalancerSkuName = "load-balancer-sku-name"
+
 	// The below bits are internal book-keeping things, rather than
 	// configuration. Config is just what we have to work with.
 
@@ -26,11 +34,13 @@ const (
 )
 
 var configFields = schema.Fields{
-	configAttrStorageAccountType: schema.String(),
+	configAttrStorageAccountType:  schema.String(),
+	configAttrLoadBalancerSkuName: schema.String(),
 }
 
 var configDefaults = schema.Defaults{
-	configAttrStorageAccountType: string(storage.StandardLRS),
+	configAttrStorageAccountType:  string(storage.StandardLRS),
+	configAttrLoadBalancerSkuName: string(network.LoadBalancerSkuNameStandard),
 }
 
 var immutableConfigAttributes = []string{
@@ -39,11 +49,26 @@ var immutableConfigAttributes = []string{
 
 type azureModelConfig struct {
 	*config.Config
-	storageAccountType string
+	storageAccountType  string
+	loadBalancerSkuName string
+}
+
+// knownStorageAccountTypes returns a list of valid storage SKU names.
+//
+// The term "account type" is is used in previous versions of the Azure SDK.
+func knownStorageAccountTypes() (accountTypes []string) {
+	for _, name := range storage.PossibleSkuNameValues() {
+		accountTypes = append(accountTypes, string(name))
+	}
+	return accountTypes
 }
 
-var knownStorageAccountTypes = []string{
-	"Standard_LRS", "Standard_GRS", "Standard_RAGRS", "Standard_ZRS", "Premium_LRS",
+// knownLoadBalancerSkuNames returns a list of valid load balancer SKU names.
+func knownLoadBalancerSkuNames() (skus []string) {
+	for _, name := range network.PossibleLoadBalancerSkuNameValues() {
+		skus = append(skus, string(name))
+	}
+	return skus
 }
 
 // Validate ensures that the provided configuration is valid for this
@@ -114,28 +139,57 @@ Please choose a model name of no more than %d characters.`,
 	if !isKnownStorageAccountType(storageAccountType) {
 		return nil, errors.Errorf(
 			"invalid storage account type %q, expected one of: %q",
-			storageAccountType, knownStorageAccountTypes,
+			storageAccountType, knownStorageAccountTypes(),
 		)
 	}
 
+	loadBalancerSkuName, ok := validated[configAttrLoadBalancerSkuName].(string)
+	if ok {
+		loadBalancerSkuNameTitle := strings.Title(loadBalancerSkuName)
+		if loadBalancerSkuName != loadBalancerSkuNameTitle {
+			loadBalancerSkuName = loadBalancerSkuNameTitle
+			logger.Infof("using %q for config parameter %s", loadBalancerSkuName, configAttrLoadBalancerSkuName)
+		}
+		if !isKnownLoadBalancerSkuName(loadBalancerSkuName) {
+			return nil, errors.Errorf(
+				"invalid load balancer SKU name %q, expected one of: %q",
+				loadBalancerSkuName, knownLoadBalancerSkuNames(),
+			)
+		}
+	} else {
+		loadBalancerSkuName = string(network.LoadBalancerSkuNameStandard)
+	}
+
 	azureConfig := &azureModelConfig{
 		newCfg,
 		storageAccountType,
+		loadBalancerSkuName,
 	}
 	return azureConfig, nil
 }
 
 // isKnownStorageAccountType reports whether or not the given string identifies
 // a known storage account type.
 func isKnownStorageAccountType(t string) bool {
-	for _, knownStorageAccountType := range knownStorageAccountTypes {
+	for _, knownStorageAccountType := range knownStorageAccountTypes() {
 		if t == knownStorageAccountType {
 			return true
 		}
 	}
 	return false
 }
 
+// isKnownLoadBalancerSkuName reports whether or not the given string
+// a valid storage SKU within the Azure SDK
+func isKnownLoadBalancerSkuName(n string) bool {
+	for _, skuName := range knownLoadBalancerSkuNames() {
+		if n == skuName {
+			return true
+		}
+	}
+	return false
+}
+
 // canonicalLocation returns the canonicalized location string. This involves
 // stripping whitespace, and lowercasing. The ARM APIs do not support embedded
 // whitespace, whereas the old Service Management APIs used to; we allow the

diff --git a/provider/azure/config_test.go b/provider/azure/config_test.go
@@ -42,7 +42,14 @@ func (s *configSuite) TestValidateNew(c *gc.C) {
 func (s *configSuite) TestValidateInvalidStorageAccountType(c *gc.C) {
 	s.assertConfigInvalid(
 		c, testing.Attrs{"storage-account-type": "savings"},
-		`invalid storage account type "savings", expected one of: \["Standard_LRS" "Standard_GRS" "Standard_RAGRS" "Standard_ZRS" "Premium_LRS"\]`,
+		`invalid storage account type "savings", expected one of: \["Premium_LRS" "Premium_ZRS" "Standard_GRS" "Standard_LRS" "Standard_RAGRS" "Standard_ZRS"\]`,
+	)
+}
+
+func (s *configSuite) TestValidateInvalidLoadBalancerSkuName(c *gc.C) {
+	s.assertConfigInvalid(
+		c, testing.Attrs{"load-balancer-sku-name": "premium"},
+		`invalid load balancer SKU name "Premium", expected one of: \["Basic" "Standard"\]`,
 	)
 }
 
@@ -71,6 +78,19 @@ func (s *configSuite) TestValidateStorageAccountTypeCantChange(c *gc.C) {
 	c.Assert(err, gc.ErrorMatches, `cannot change immutable "storage-account-type" config \(Standard_LRS -> Premium_LRS\)`)
 }
 
+func (s *configSuite) TestValidateLoadBalancerSkuNameCanChange(c *gc.C) {
+	cfgOld := makeTestModelConfig(c, testing.Attrs{"load-balancer-sku-name": "Standard"})
+	_, err := s.provider.Validate(cfgOld, cfgOld)
+	c.Assert(err, jc.ErrorIsNil)
+
+	cfgNew := makeTestModelConfig(c, testing.Attrs{"load-balancer-sku-name": "Basic"})
+	_, err = s.provider.Validate(cfgNew, cfgOld)
+	c.Assert(err, jc.ErrorIsNil)
+
+	_, err = s.provider.Validate(cfgOld, cfgNew)
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 func (s *configSuite) assertConfigValid(c *gc.C, attrs testing.Attrs) {
 	cfg := makeTestModelConfig(c, attrs)
 	_, err := s.provider.Validate(cfg, nil)

diff --git a/provider/azure/environ.go b/provider/azure/environ.go
@@ -495,6 +495,7 @@ func (env *azureEnviron) StartInstance(ctx context.ProviderCallContext, args env
 	}
 	if seriesOS == os.Windows {
 		if instanceSpec.InstanceType.RootDisk < windowsMinRootDiskMB {
+			logger.Infof("root disk size has been increased to 127GiB")
 			instanceSpec.InstanceType.RootDisk = windowsMinRootDiskMB
 		}
 	}
@@ -654,7 +655,7 @@ func (env *azureEnviron) createVirtualMachine(
 		)
 		var (
 			availabilitySetProperties  interface{}
-			availabilityStorageOptions *storage.Sku
+			availabilityStorageOptions armtemplates.Sku
 		)
 		if maybeStorageAccount == nil {
 			// This model uses managed disks; we must create
@@ -669,9 +670,7 @@ func (env *azureEnviron) createVirtualMachine(
 				PlatformFaultDomainCount: to.Int32Ptr(maxFaultDomains(env.location)),
 			}
 			// Availability needs to be 'Aligned' to support managed disks.
-			availabilityStorageOptions = &storage.Sku{
-				Name: "Aligned",
-			}
+			availabilityStorageOptions.Name = "Aligned"
 		}
 		resources = append(resources, armtemplates.Resource{
 			APIVersion: computeAPIVersion,
@@ -680,7 +679,7 @@ func (env *azureEnviron) createVirtualMachine(
 			Location:   env.location,
 			Tags:       envTags,
 			Properties: availabilitySetProperties,
-			StorageSku: availabilityStorageOptions,
+			Sku:        &availabilityStorageOptions,
 		})
 		availabilitySetSubResource = &compute.SubResource{
 			ID: to.StringPtr(availabilitySetId),
@@ -690,16 +689,20 @@ func (env *azureEnviron) createVirtualMachine(
 
 	publicIPAddressName := vmName + "-public-ip"
 	publicIPAddressId := fmt.Sprintf(`[resourceId('Microsoft.Network/publicIPAddresses', '%s')]`, publicIPAddressName)
+	publicIPAddressAllocationMethod := network.Static
+	if env.config.loadBalancerSkuName == string(network.LoadBalancerSkuNameBasic) {
+		publicIPAddressAllocationMethod = network.Dynamic // preserve the settings that were used in Juju 2.4 and earlier
+	}
 	resources = append(resources, armtemplates.Resource{
 		APIVersion: networkAPIVersion,
 		Type:       "Microsoft.Network/publicIPAddresses",
 		Name:       publicIPAddressName,
 		Location:   env.location,
 		Tags:       vmTags,
-		StorageSku: &storage.Sku{Name: "Standard", Tier: "Regional"},
+		Sku:        &armtemplates.Sku{Name: env.config.loadBalancerSkuName},
 		Properties: &network.PublicIPAddressPropertiesFormat{
 			PublicIPAddressVersion:   network.IPv4,
-			PublicIPAllocationMethod: network.Static,
+			PublicIPAllocationMethod: publicIPAddressAllocationMethod,
 		},
 	})
 

diff --git a/provider/azure/environ_test.go b/provider/azure/environ_test.go
@@ -987,9 +987,7 @@ func (s *environSuite) assertStartInstanceRequests(
 				Name:       storageAccountName,
 				Location:   "westus",
 				Tags:       to.StringMap(s.envTags),
-				StorageSku: &storage.Sku{
-					Name: storage.SkuName("Standard_LRS"),
-				},
+				Sku:        &armtemplates.Sku{Name: "Standard_LRS"},
 			})
 			vmDependsOn = append(vmDependsOn,
 				`[resourceId('Microsoft.Storage/storageAccounts', '`+storageAccountName+`')]`,
@@ -1008,15 +1006,13 @@ func (s *environSuite) assertStartInstanceRequests(
 		)
 		var (
 			availabilitySetProperties  interface{}
-			availabilityStorageOptions *storage.Sku
+			availabilityStorageOptions armtemplates.Sku
 		)
 		if !args.unmanagedStorage {
 			availabilitySetProperties = &compute.AvailabilitySetProperties{
 				PlatformFaultDomainCount: to.Int32Ptr(3),
 			}
-			availabilityStorageOptions = &storage.Sku{
-				Name: "Aligned",
-			}
+			availabilityStorageOptions.Name = "Aligned"
 		}
 		templateResources = append(templateResources, armtemplates.Resource{
 			APIVersion: computeAPIVersion,
@@ -1025,7 +1021,7 @@ func (s *environSuite) assertStartInstanceRequests(
 			Location:   "westus",
 			Tags:       to.StringMap(s.envTags),
 			Properties: availabilitySetProperties,
-			StorageSku: availabilityStorageOptions,
+			Sku:        &availabilityStorageOptions,
 		})
 		availabilitySetSubResource = &compute.SubResource{
 			ID: to.StringPtr(availabilitySetId),
@@ -1059,7 +1055,7 @@ func (s *environSuite) assertStartInstanceRequests(
 			PublicIPAllocationMethod: network.Static,
 			PublicIPAddressVersion:   "IPv4",
 		},
-		StorageSku: &storage.Sku{Name: "Standard", Tier: "Regional"},
+		Sku: &armtemplates.Sku{Name: "Standard"},
 	}, {
 		APIVersion: networkAPIVersion,
 		Type:       "Microsoft.Network/networkInterfaces",

diff --git a/provider/azure/environprovider.go b/provider/azure/environprovider.go
@@ -65,6 +65,10 @@ type ProviderConfig struct {
 
 	// AzureCLI is the interface the to Azure CLI (az) command.
 	AzureCLI AzureCLI
+
+	// LoadBalancerSkuName is the load balancer SKU name.
+	// Legal values are determined by the Azure SDK.
+	LoadBalancerSkuName string
 }
 
 // Validate validates the Azure provider configuration.

diff --git a/provider/azure/internal/armtemplates/template.go b/provider/azure/internal/armtemplates/template.go
@@ -3,8 +3,6 @@
 
 package armtemplates
 
-import "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2018-07-01/storage"
-
 const (
 	schema         = "http://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#"
 	contentVersion = "1.0.0.0"
@@ -29,6 +27,13 @@ func (t *Template) Map() (map[string]interface{}, error) {
 	return m, nil
 }
 
+// Sku represents an Azure SKU. Each API (compute/networking/storage)
+// defines its own SKU types, but we use a common type because we
+// don't require many fields.
+type Sku struct {
+	Name string `json:"name,omitempty"`
+}
+
 // Resource describes a template resource. For information on the
 // individual fields, see https://azure.microsoft.com/en-us/documentation/articles/resource-group-authoring-templates/.
 type Resource struct {
@@ -43,5 +48,5 @@ type Resource struct {
 	Resources  []Resource        `json:"resources,omitempty"`
 
 	// Non-uniform attributes.
-	StorageSku *storage.Sku `json:"sku,omitempty"`
+	Sku *Sku `json:"sku,omitempty"`
 }
diff --git a/provider/azure/storage.go b/provider/azure/storage.go
@@ -1001,8 +1001,6 @@ func storageAccountTemplateResource(
 		Name:       accountName,
 		Location:   location,
 		Tags:       envTags,
-		StorageSku: &armstorage.Sku{
-			Name: armstorage.SkuName(accountType),
-		},
+		Sku:        &armtemplates.Sku{Name: accountType},
 	}
 }
diff --git a/provider/azure/upgrades_test.go b/provider/azure/upgrades_test.go
@@ -9,7 +9,6 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute"
 	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-08-01/network"
 	"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources"
-	"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2018-07-01/storage"
 	"github.com/Azure/go-autorest/autorest/mocks"
 	"github.com/Azure/go-autorest/autorest/to"
 	jc "github.com/juju/testing/checkers"
@@ -174,9 +173,7 @@ func (s *environUpgradeSuite) TestEnvironUpgradeOperationCreateCommonDeployment(
 		Type:     "Microsoft.Storage/storageAccounts",
 		Name:     storageAccountName,
 		Location: "westus",
-		StorageSku: &storage.Sku{
-			Name: storage.SkuName("Standard_LRS"),
-		},
+		Sku:      &armtemplates.Sku{Name: "Standard_LRS"},
 	}}
 
 	var actual resources.Deployment