Skip to content

Commit

Permalink
Merge pull request #68117 from andyzhangx/azurefile-crsss-rg
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 68051, 68130, 67211, 68065, 68117). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

support cross resource group for azure file

**What this PR does / why we need it**:
support cross resource group for azure file: by `resourceGroup` field, azure cloud provider will create azure file on user specified resource group

```
---
kind: StorageClass
apiVersion: storage.k8s.io/v1
metadata:
  name: azurefile-rg
provisioner: kubernetes.io/azure-file
parameters:
  resourceGroup: RESOURCE_GROUP_NAME
  storageAccount: EXISTING_STORAGE_ACCOUNT
```
**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 #64428

**Special notes for your reviewer**:

**Release note**:

```
resourcegroup parameter is added to AzureFile storage class to support azure file dyanmic provision in cross resource group.
```

/kind bug
/sig azure
/assign @feiskyer 
cd @khenidak
  • Loading branch information
Kubernetes Submit Queue committed Aug 31, 2018
2 parents 3966b8b + 4e3b4a7 commit e1a270d
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 19 deletions.
4 changes: 2 additions & 2 deletions pkg/cloudprovider/providers/azure/azure_blobDiskController.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func newBlobDiskController(common *controllerCommon) (*BlobDiskController, error
// If no storage account is given, search all the storage accounts associated with the resource group and pick one that
// fits storage type and location.
func (c *BlobDiskController) CreateVolume(blobName, accountName, accountType, location string, requestGB int) (string, string, int, error) {
account, key, err := c.common.cloud.ensureStorageAccount(accountName, accountType, location, dedicatedDiskAccountNamePrefix)
account, key, err := c.common.cloud.ensureStorageAccount(accountName, accountType, c.common.resourceGroup, location, dedicatedDiskAccountNamePrefix)
if err != nil {
return "", "", 0, fmt.Errorf("could not get storage key for storage account %s: %v", accountName, err)
}
Expand Down Expand Up @@ -108,7 +108,7 @@ func (c *BlobDiskController) DeleteVolume(diskURI string) error {
if err != nil {
return fmt.Errorf("failed to parse vhd URI %v", err)
}
key, err := c.common.cloud.getStorageAccesskey(accountName)
key, err := c.common.cloud.getStorageAccesskey(accountName, c.common.resourceGroup)
if err != nil {
return fmt.Errorf("no key for storage account %s, err %v", accountName, err)
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/cloudprovider/providers/azure/azure_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ const (
)

// CreateFileShare creates a file share, using a matching storage account
func (az *Cloud) CreateFileShare(shareName, accountName, accountType, location string, requestGiB int) (string, string, error) {
account, key, err := az.ensureStorageAccount(accountName, accountType, location, fileShareAccountNamePrefix)
func (az *Cloud) CreateFileShare(shareName, accountName, accountType, resourceGroup, location string, requestGiB int) (string, string, error) {
if resourceGroup == "" {
resourceGroup = az.resourceGroup
}

account, key, err := az.ensureStorageAccount(accountName, accountType, resourceGroup, location, fileShareAccountNamePrefix)
if err != nil {
return "", "", fmt.Errorf("could not get storage key for storage account %s: %v", accountName, err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloudprovider/providers/azure/azure_storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestCreateFileShare(t *testing.T) {
fake.Keys = test.keys
fake.Err = test.err

account, key, err := cloud.CreateFileShare(test.name, test.acct, test.acctType, test.loc, test.gb)
account, key, err := cloud.CreateFileShare(test.name, test.acct, test.acctType, "rg", test.loc, test.gb)
if test.expectErr && err == nil {
t.Errorf("unexpected non-error")
continue
Expand Down
20 changes: 10 additions & 10 deletions pkg/cloudprovider/providers/azure/azure_storageaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ type accountWithLocation struct {
}

// getStorageAccounts gets name, type, location of all storage accounts in a resource group which matches matchingAccountType, matchingLocation
func (az *Cloud) getStorageAccounts(matchingAccountType, matchingLocation string) ([]accountWithLocation, error) {
func (az *Cloud) getStorageAccounts(matchingAccountType, resourceGroup, matchingLocation string) ([]accountWithLocation, error) {
ctx, cancel := getContextWithCancel()
defer cancel()
result, err := az.StorageAccountClient.ListByResourceGroup(ctx, az.ResourceGroup)
result, err := az.StorageAccountClient.ListByResourceGroup(ctx, resourceGroup)
if err != nil {
return nil, err
}
if result.Value == nil {
return nil, fmt.Errorf("unexpected error when listing storage accounts from resource group %s", az.ResourceGroup)
return nil, fmt.Errorf("unexpected error when listing storage accounts from resource group %s", resourceGroup)
}

accounts := []accountWithLocation{}
Expand All @@ -61,11 +61,11 @@ func (az *Cloud) getStorageAccounts(matchingAccountType, matchingLocation string
}

// getStorageAccesskey gets the storage account access key
func (az *Cloud) getStorageAccesskey(account string) (string, error) {
func (az *Cloud) getStorageAccesskey(account, resourceGroup string) (string, error) {
ctx, cancel := getContextWithCancel()
defer cancel()

result, err := az.StorageAccountClient.ListKeys(ctx, az.ResourceGroup, account)
result, err := az.StorageAccountClient.ListKeys(ctx, resourceGroup, account)
if err != nil {
return "", err
}
Expand All @@ -86,10 +86,10 @@ func (az *Cloud) getStorageAccesskey(account string) (string, error) {
}

// ensureStorageAccount search storage account, create one storage account(with genAccountNamePrefix) if not found, return accountName, accountKey
func (az *Cloud) ensureStorageAccount(accountName, accountType, location, genAccountNamePrefix string) (string, string, error) {
func (az *Cloud) ensureStorageAccount(accountName, accountType, resourceGroup, location, genAccountNamePrefix string) (string, string, error) {
if len(accountName) == 0 {
// find a storage account that matches accountType
accounts, err := az.getStorageAccounts(accountType, location)
accounts, err := az.getStorageAccounts(accountType, resourceGroup, location)
if err != nil {
return "", "", fmt.Errorf("could not list storage accounts for account type %s: %v", accountType, err)
}
Expand All @@ -110,7 +110,7 @@ func (az *Cloud) ensureStorageAccount(accountName, accountType, location, genAcc
}

glog.V(2).Infof("azure - no matching account found, begin to create a new account %s in resource group %s, location: %s, accountType: %s",
accountName, az.ResourceGroup, location, accountType)
accountName, resourceGroup, location, accountType)
cp := storage.AccountCreateParameters{
Sku: &storage.Sku{Name: storage.SkuName(accountType)},
// switch to use StorageV2 as it's recommended according to https://docs.microsoft.com/en-us/azure/storage/common/storage-account-options
Expand All @@ -121,15 +121,15 @@ func (az *Cloud) ensureStorageAccount(accountName, accountType, location, genAcc

ctx, cancel := getContextWithCancel()
defer cancel()
_, err := az.StorageAccountClient.Create(ctx, az.ResourceGroup, accountName, cp)
_, err := az.StorageAccountClient.Create(ctx, resourceGroup, accountName, cp)
if err != nil {
return "", "", fmt.Errorf(fmt.Sprintf("Failed to create storage account %s, error: %s", accountName, err))
}
}
}

// find the access key with this account
accountKey, err := az.getStorageAccesskey(accountName)
accountKey, err := az.getStorageAccesskey(accountName, resourceGroup)
if err != nil {
return "", "", fmt.Errorf("could not get storage key for storage account %s: %v", accountName, err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestGetStorageAccessKeys(t *testing.T) {
expectedKey := test.expectedKey
fake.Keys = test.results
fake.Err = test.err
key, err := cloud.getStorageAccesskey("acct")
key, err := cloud.getStorageAccesskey("acct", "rg")
if test.expectErr && err == nil {
t.Errorf("Unexpected non-error")
continue
Expand Down
8 changes: 5 additions & 3 deletions pkg/volume/azure_file/azure_provision.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ var _ volume.ProvisionableVolumePlugin = &azureFilePlugin{}
// azure cloud provider should implement it
type azureCloudProvider interface {
// create a file share
CreateFileShare(shareName, accountName, accountType, location string, requestGiB int) (string, string, error)
CreateFileShare(shareName, accountName, accountType, resourceGroup, location string, requestGiB int) (string, string, error)
// delete a file share
DeleteFileShare(accountName, accountKey, shareName string) error
// resize a file share
Expand Down Expand Up @@ -139,7 +139,7 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie
return nil, fmt.Errorf("%s does not support block volume provisioning", a.plugin.GetPluginName())
}

var sku, location, account string
var sku, resourceGroup, location, account string

// File share name has a length limit of 63, and it cannot contain two consecutive '-'s.
name := util.GenerateVolumeName(a.options.ClusterName, a.options.PVName, 63)
Expand All @@ -160,6 +160,8 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie
account = v
case "secretnamespace":
secretNamespace = v
case "resourcegroup":
resourceGroup = v
default:
return nil, fmt.Errorf("invalid option %q for volume plugin %s", k, a.plugin.GetPluginName())
}
Expand All @@ -169,7 +171,7 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie
return nil, fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on Azure file")
}

account, key, err := a.azureProvider.CreateFileShare(name, account, sku, location, requestGiB)
account, key, err := a.azureProvider.CreateFileShare(name, account, sku, resourceGroup, location, requestGiB)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit e1a270d

Please sign in to comment.