Merge pull request #56557 from andyzhangx/azurefile-createaccount

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

fix the create azure file pvc failure if there is no storage account in current resource group

**What this PR does / why we need it**:
When create an azure file PVC, there will be error if there is no storage account in current resource group.
With this PR, a storage account will be created if there is no storage account in current resource group.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #56556

**Special notes for your reviewer**:
1. rephrase the code logic of `CreateFileShare` func.
```
if accountName is empty, then 
    find a storage account that matches accountType
    if no storage account found, then
        create a new account
else
    we only use user specified storage account

create a file share according to found storage account
```
2. Use func `getStorageAccountName` to get a unique storage account name by UUID, a storage account for azure file would be like `f0b2b0bd40c010112e897fa`. And in next PR, I will use this function to create storage account for azure disk, the storage account for azure disk would be like `d8f3ad8ad92000f1e1e88bd`.

**Release note**:

```
fix the create azure file pvc failure if there is no storage account in current resource group
```
/sig azure
/assign @rootfs

pkg/cloudprovider/providers/azure/BUILD

@@ -56,6 +56,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//vendor/k8s.io/client-go/tools/cache:go_default_library",
         "//vendor/k8s.io/client-go/util/flowcontrol:go_default_library",

pkg/cloudprovider/providers/azure/azure_blobDiskController.go

@@ -91,7 +91,7 @@ func (c *BlobDiskController) CreateVolume(name, storageAccount, storageAccountTy
 		accounts = append(accounts, accountWithLocation{Name: storageAccount})
 	} else {
 		// find a storage account
-		accounts, err = c.common.cloud.getStorageAccounts()
+		accounts, err = c.common.cloud.getStorageAccounts(storageAccountType, location)
 		if err != nil {
 			// TODO: create a storage account and container
 			return "", "", 0, err

pkg/cloudprovider/providers/azure/azure_standard.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/uuid"
 )
 
 const (
@@ -52,6 +53,8 @@ const (
 
 	// nodeLabelRole specifies the role of a node
 	nodeLabelRole = "kubernetes.io/role"
+
+	storageAccountNameMaxLength = 24
 )
 
 var errNotInVMSet = errors.New("vm is not in the vmset")
@@ -658,3 +661,13 @@ func (as *availabilitySet) EnsureBackendPoolDeleted(poolID, vmSetName string) er
 	// Do nothing for availability set.
 	return nil
 }
+
+// get a storage account by UUID
+func generateStorageAccountName(accountNamePrefix string) string {
+	uniqueID := strings.Replace(string(uuid.NewUUID()), "-", "", -1)
+	accountName := strings.ToLower(accountNamePrefix + uniqueID)
+	if len(accountName) > storageAccountNameMaxLength {
+		return accountName[:storageAccountNameMaxLength-1]
+	}
+	return accountName
+}

pkg/cloudprovider/providers/azure/azure_standard_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 The Kubernetes Authors.
+Copyright 2018 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -90,3 +90,33 @@ func TestGetLastSegment(t *testing.T) {
 		}
 	}
 }
+
+func TestGenerateStorageAccountName(t *testing.T) {
+	tests := []struct {
+		prefix string
+	}{
+		{
+			prefix: "",
+		},
+		{
+			prefix: "pvc",
+		},
+		{
+			prefix: "1234512345123451234512345",
+		},
+	}
+
+	for _, test := range tests {
+		accountName := generateStorageAccountName(test.prefix)
+		if len(accountName) > storageAccountNameMaxLength || len(accountName) < 3 {
+			t.Errorf("input prefix: %s, output account name: %s, length not in [3,%d]", test.prefix, accountName, storageAccountNameMaxLength)
+		}
+
+		for _, char := range accountName {
+			if (char < 'a' || char > 'z') && (char < '0' || char > '9') {
+				t.Errorf("input prefix: %s, output account name: %s, there is non-digit or non-letter(%q)", test.prefix, accountName, char)
+				break
+			}
+		}
+	}
+}

pkg/cloudprovider/providers/azure/azure_storage.go

@@ -19,54 +19,75 @@ package azure
 import (
 	"fmt"
 
+	"github.com/Azure/azure-sdk-for-go/arm/storage"
+	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/golang/glog"
 )
 
+const (
+	defaultStorageAccountType  = string(storage.StandardLRS)
+	fileShareAccountNamePrefix = "f"
+)
+
 // CreateFileShare creates a file share, using a matching storage account
-func (az *Cloud) CreateFileShare(name, storageAccount, storageType, location string, requestGiB int) (string, string, error) {
-	var errResult error
-	accounts := []accountWithLocation{}
-	if len(storageAccount) > 0 {
-		accounts = append(accounts, accountWithLocation{Name: storageAccount})
-	} else {
-		// find a storage account
-		accounts, errResult = az.getStorageAccounts()
-		if errResult != nil {
-			// TODO: create a storage account and container
-			return "", "", errResult
+func (az *Cloud) CreateFileShare(shareName, accountName, accountType, location string, requestGiB int) (string, string, error) {
+	if len(accountName) == 0 {
+		// find a storage account that matches accountType
+		accounts, err := az.getStorageAccounts(accountType, location)
+		if err != nil {
+			return "", "", fmt.Errorf("could not list storage accounts for account type %s: %v", accountType, err)
 		}
-	}
-	for _, account := range accounts {
-		glog.V(4).Infof("account %s type %s location %s", account.Name, account.StorageType, account.Location)
-		if ((storageType == "" || account.StorageType == storageType) && (location == "" || account.Location == location)) || len(storageAccount) > 0 {
-			// find the access key with this account
-			key, innerErr := az.getStorageAccesskey(account.Name)
-			if innerErr != nil {
-				errResult = fmt.Errorf("could not get storage key for storage account %s: %v", account.Name, innerErr)
-				continue
+
+		if len(accounts) > 0 {
+			accountName = accounts[0].Name
+			glog.V(4).Infof("found a matching account %s type %s location %s", accounts[0].Name, accounts[0].StorageType, accounts[0].Location)
+		}
+
+		if len(accountName) == 0 {
+			// not found a matching account, now create a new account in current resource group
+			accountName = generateStorageAccountName(fileShareAccountNamePrefix)
+			if location == "" {
+				location = az.Location
+			}
+			if accountType == "" {
+				accountType = defaultStorageAccountType
 			}
 
-			if innerErr = az.createFileShare(account.Name, key, name, requestGiB); innerErr != nil {
-				errResult = fmt.Errorf("failed to create share %s in account %s: %v", name, account.Name, innerErr)
-				continue
+			glog.V(2).Infof("azureFile - no matching account found, begin to create a new account %s in resource group %s, location: %s, accountType: %s",
+				accountName, az.ResourceGroup, location, accountType)
+			cp := storage.AccountCreateParameters{
+				Sku:      &storage.Sku{Name: storage.SkuName(accountType)},
+				Tags:     &map[string]*string{"created-by": to.StringPtr("azure-file")},
+				Location: &location}
+			cancel := make(chan struct{})
+
+			_, errchan := az.StorageAccountClient.Create(az.ResourceGroup, accountName, cp, cancel)
+			err := <-errchan
+			if err != nil {
+				return "", "", fmt.Errorf(fmt.Sprintf("Failed to create storage account %s, error: %s", accountName, err))
 			}
-			glog.V(4).Infof("created share %s in account %s", name, account.Name)
-			return account.Name, key, nil
 		}
 	}
 
-	if errResult == nil {
-		errResult = fmt.Errorf("failed to find a matching storage account")
+	// find the access key with this account
+	accountKey, err := az.getStorageAccesskey(accountName)
+	if err != nil {
+		return "", "", fmt.Errorf("could not get storage key for storage account %s: %v", accountName, err)
 	}
-	return "", "", errResult
+
+	if err := az.createFileShare(accountName, accountKey, shareName, requestGiB); err != nil {
+		return "", "", fmt.Errorf("failed to create share %s in account %s: %v", shareName, accountName, err)
+	}
+	glog.V(4).Infof("created share %s in account %s", shareName, accountName)
+	return accountName, accountKey, nil
 }
 
 // DeleteFileShare deletes a file share using storage account name and key
-func (az *Cloud) DeleteFileShare(accountName, key, name string) error {
-	if err := az.deleteFileShare(accountName, key, name); err != nil {
+func (az *Cloud) DeleteFileShare(accountName, accountKey, shareName string) error {
+	if err := az.deleteFileShare(accountName, accountKey, shareName); err != nil {
 		return err
 	}
-	glog.V(4).Infof("share %s deleted", name)
+	glog.V(4).Infof("share %s deleted", shareName)
 	return nil
 }
 

pkg/cloudprovider/providers/azure/azure_storageaccount.go

@@ -25,29 +25,29 @@ type accountWithLocation struct {
 	Name, StorageType, Location string
 }
 
-// getStorageAccounts gets the storage accounts' name, type, location in a resource group
-func (az *Cloud) getStorageAccounts() ([]accountWithLocation, error) {
+// getStorageAccounts gets name, type, location of all storage accounts in a resource group which matches matchingAccountType
+func (az *Cloud) getStorageAccounts(matchingAccountType, matchingLocation string) ([]accountWithLocation, error) {
 	result, err := az.StorageAccountClient.ListByResourceGroup(az.ResourceGroup)
 	if err != nil {
 		return nil, err
 	}
 	if result.Value == nil {
-		return nil, fmt.Errorf("no storage accounts from resource group %s", az.ResourceGroup)
+		return nil, fmt.Errorf("unexpected error when listing storage accounts from resource group %s", az.ResourceGroup)
 	}
 
 	accounts := []accountWithLocation{}
 	for _, acct := range *result.Value {
-		if acct.Name != nil {
-			name := *acct.Name
-			loc := ""
-			if acct.Location != nil {
-				loc = *acct.Location
+		if acct.Name != nil && acct.Location != nil && acct.Sku != nil {
+			storageType := string((*acct.Sku).Name)
+			if matchingAccountType != "" && !strings.EqualFold(matchingAccountType, storageType) {
+				continue
 			}
-			storageType := ""
-			if acct.Sku != nil {
-				storageType = string((*acct.Sku).Name)
+
+			location := *acct.Location
+			if matchingLocation != "" && !strings.EqualFold(matchingLocation, location) {
+				continue
 			}
-			accounts = append(accounts, accountWithLocation{Name: name, StorageType: storageType, Location: loc})
+			accounts = append(accounts, accountWithLocation{Name: *acct.Name, StorageType: storageType, Location: location})
 		}
 	}
 

pkg/volume/azure_file/azure_provision.go

@@ -38,9 +38,9 @@ var _ volume.ProvisionableVolumePlugin = &azureFilePlugin{}
 // azure cloud provider should implement it
 type azureCloudProvider interface {
 	// create a file share
-	CreateFileShare(name, storageAccount, storageType, location string, requestGiB int) (string, string, error)
+	CreateFileShare(shareName, accountName, accountType, location string, requestGiB int) (string, string, error)
 	// delete a file share
-	DeleteFileShare(accountName, key, name string) error
+	DeleteFileShare(accountName, accountKey, shareName string) error
 	// resize a file share
 	ResizeFileShare(accountName, accountKey, name string, sizeGiB int) error
 }