From aa21bef6778528b63fea863b2a42c627dfcd8e7b Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Wed, 29 Nov 2017 08:11:58 +0000 Subject: [PATCH] create storage account if necessary when create azure file pvc use new storage account name generation method use uuid to generate account name change azure file account prefix use uniqueID to generate a storage account name fix comments fix comments fix comments fix a storage account matching bug only use UUID in getStorageAccountName func use shorter storage account prefix for azure file fix comments fix comments fix comments fix rebase build error rewrite CreateFileShare code logic fix gofmt issue fix test error fix comments fix a location matching bug --- pkg/cloudprovider/providers/azure/BUILD | 1 + .../azure/azure_blobDiskController.go | 2 +- .../providers/azure/azure_standard.go | 13 +++ .../providers/azure/azure_standard_test.go | 32 ++++++- .../providers/azure/azure_storage.go | 83 ++++++++++++------- .../providers/azure/azure_storageaccount.go | 24 +++--- pkg/volume/azure_file/azure_provision.go | 4 +- 7 files changed, 112 insertions(+), 47 deletions(-) diff --git a/pkg/cloudprovider/providers/azure/BUILD b/pkg/cloudprovider/providers/azure/BUILD index 66e9eb2dca2..df2cfa9608a 100644 --- a/pkg/cloudprovider/providers/azure/BUILD +++ b/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", diff --git a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go index 0d22ff84926..9fefe7a4588 100644 --- a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go +++ b/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 diff --git a/pkg/cloudprovider/providers/azure/azure_standard.go b/pkg/cloudprovider/providers/azure/azure_standard.go index 8667bc15d0e..565c14d939b 100644 --- a/pkg/cloudprovider/providers/azure/azure_standard.go +++ b/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 +} diff --git a/pkg/cloudprovider/providers/azure/azure_standard_test.go b/pkg/cloudprovider/providers/azure/azure_standard_test.go index 169f0cea0e6..45a4a945587 100644 --- a/pkg/cloudprovider/providers/azure/azure_standard_test.go +++ b/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 + } + } + } +} diff --git a/pkg/cloudprovider/providers/azure/azure_storage.go b/pkg/cloudprovider/providers/azure/azure_storage.go index ab5f820ff6e..c05316313d9 100644 --- a/pkg/cloudprovider/providers/azure/azure_storage.go +++ b/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 } diff --git a/pkg/cloudprovider/providers/azure/azure_storageaccount.go b/pkg/cloudprovider/providers/azure/azure_storageaccount.go index 4d33fb21e66..5666da1732c 100644 --- a/pkg/cloudprovider/providers/azure/azure_storageaccount.go +++ b/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}) } } diff --git a/pkg/volume/azure_file/azure_provision.go b/pkg/volume/azure_file/azure_provision.go index 76a9db87cea..6386d5354ec 100644 --- a/pkg/volume/azure_file/azure_provision.go +++ b/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 }