Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #68117: support cross resource group for azure file #68182

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -79,7 +79,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 @@ -107,7 +107,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