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 a2b562c4c38..07c303476af 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 }