fix CreateVolume: search mode for Dedicated kind

pkg/cloudprovider/providers/azure/azure_blobDiskController.go

@@ -80,52 +80,55 @@ func newBlobDiskController(common *controllerCommon) (*BlobDiskController, error
 	return &c, nil
 }
 
-// CreateVolume creates a VHD blob in a given storage account, will create the given storage account if it does not exist in current resource group
+// CreateVolume creates a VHD blob in a storage account that has storageType and location using the given storage account.
+// 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(name, storageAccount string, storageAccountType storage.SkuName, location string, requestGB int) (string, string, int, error) {
-	key, err := c.common.cloud.getStorageAccesskey(storageAccount)
+	var err error
-	if err != nil {
+	accounts := []accountWithLocation{}
-		glog.V(2).Infof("azureDisk - no key found for storage account %s in resource group %s, begin to create a new storage account", storageAccount, c.common.resourceGroup)
+	if len(storageAccount) > 0 {
-
+		accounts = append(accounts, accountWithLocation{Name: storageAccount})
-		cp := storage.AccountCreateParameters{
+	} else {
-			Sku:      &storage.Sku{Name: storageAccountType},
+		// find a storage account
-			Tags:     &map[string]*string{"created-by": to.StringPtr("azure-dd")},
+		accounts, err = c.common.cloud.getStorageAccounts()
-			Location: &location}
-		cancel := make(chan struct{})
-
-		_, errchan := c.common.cloud.StorageAccountClient.Create(c.common.resourceGroup, storageAccount, cp, cancel)
-		err = <-errchan
 		if err != nil {
-			return "", "", 0, fmt.Errorf(fmt.Sprintf("Create Storage Account %s, error: %s", storageAccount, err))
+			// TODO: create a storage account and container
+			return "", "", 0, err
 		}
+	}
+	for _, account := range accounts {
+		glog.V(4).Infof("account %s type %s location %s", account.Name, account.StorageType, account.Location)
+		if (account.StorageType == string(storageAccountType)) && (location == "" || account.Location == location) {
+			// find the access key with this account
+			key, err := c.common.cloud.getStorageAccesskey(account.Name)
+			if err != nil {
+				glog.V(2).Infof("no key found for storage account %s", account.Name)
+				continue
+			}
 
-		key, err = c.common.cloud.getStorageAccesskey(storageAccount)
+			client, err := azstorage.NewBasicClientOnSovereignCloud(account.Name, key, c.common.cloud.Environment)
-		if err != nil {
+			if err != nil {
-			return "", "", 0, fmt.Errorf("no key found for storage account %s even after creating a new storage account", storageAccount)
+				return "", "", 0, err
+			}
+			blobClient := client.GetBlobService()
+
+			container := blobClient.GetContainerReference(vhdContainerName)
+			_, err = container.CreateIfNotExists(&azstorage.CreateContainerOptions{Access: azstorage.ContainerAccessTypePrivate})
+			if err != nil {
+				return "", "", 0, err
+			}
+
+			// create a page blob in this account's vhd container
+			diskName, diskURI, err := c.createVHDBlobDisk(blobClient, account.Name, name, vhdContainerName, int64(requestGB))
+			if err != nil {
+				return "", "", 0, err
+			}
+
+			glog.V(4).Infof("azureDisk - created vhd blob uri: %s", diskURI)
+			return diskName, diskURI, requestGB, err
 		}
-
-		glog.Errorf("no key found for storage account %s in resource group %s", storageAccount, c.common.resourceGroup)
-		return "", "", 0, err
 	}
-
+	return "", "", 0, fmt.Errorf("failed to find a matching storage account")
-	client, err := azstorage.NewBasicClientOnSovereignCloud(storageAccount, key, c.common.cloud.Environment)
-	if err != nil {
-		return "", "", 0, err
-	}
-	blobClient := client.GetBlobService()
-
-	container := blobClient.GetContainerReference(vhdContainerName)
-	_, err = container.CreateIfNotExists(&azstorage.CreateContainerOptions{Access: azstorage.ContainerAccessTypePrivate})
-	if err != nil {
-		return "", "", 0, err
-	}
-
-	diskName, diskURI, err := c.createVHDBlobDisk(blobClient, storageAccount, name, vhdContainerName, int64(requestGB))
-	if err != nil {
-		return "", "", 0, err
-	}
-
-	glog.V(4).Infof("azureDisk - created vhd blob uri: %s", diskURI)
-	return diskName, diskURI, requestGB, err
 }
 
 // DeleteVolume deletes a VHD blob
@@ -236,24 +239,12 @@ func (c *BlobDiskController) deleteVhdBlob(accountName, accountKey, blobName str
 }
 
 //CreateBlobDisk : create a blob disk in a node
-func (c *BlobDiskController) CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int, forceStandAlone bool) (string, error) {
+func (c *BlobDiskController) CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int) (string, error) {
-	glog.V(4).Infof("azureDisk - creating blob data disk named:%s on StorageAccountType:%s StandAlone:%v", dataDiskName, storageAccountType, forceStandAlone)
+	glog.V(4).Infof("azureDisk - creating blob data disk named:%s on StorageAccountType:%s", dataDiskName, storageAccountType)
 
-	var storageAccountName = ""
+	storageAccountName, err := c.findSANameForDisk(storageAccountType)
-	var err error
+	if err != nil {
-
+		return "", err
-	if forceStandAlone {
-		// we have to wait until the storage account is is created
-		storageAccountName = "p" + MakeCRC32(c.common.subscriptionID+c.common.resourceGroup+dataDiskName)
-		err = c.createStorageAccount(storageAccountName, storageAccountType, c.common.location, false)
-		if err != nil {
-			return "", err
-		}
-	} else {
-		storageAccountName, err = c.findSANameForDisk(storageAccountType)
-		if err != nil {
-			return "", err
-		}
 	}
 
 	blobClient, err := c.getBlobSvcClient(storageAccountName)
@@ -266,15 +257,13 @@ func (c *BlobDiskController) CreateBlobDisk(dataDiskName string, storageAccountT
 		return "", err
 	}
 
-	if !forceStandAlone {
+	atomic.AddInt32(&c.accounts[storageAccountName].diskCount, 1)
-		atomic.AddInt32(&c.accounts[storageAccountName].diskCount, 1)
-	}
 
 	return diskURI, nil
 }
 
 //DeleteBlobDisk : delete a blob disk from a node
-func (c *BlobDiskController) DeleteBlobDisk(diskURI string, wasForced bool) error {
+func (c *BlobDiskController) DeleteBlobDisk(diskURI string) error {
 	storageAccountName, vhdName, err := diskNameandSANameFromURI(diskURI)
 	if err != nil {
 		return err
@@ -286,11 +275,6 @@ func (c *BlobDiskController) DeleteBlobDisk(diskURI string, wasForced bool) erro
 		glog.V(4).Infof("azureDisk - deleting volume %s", diskURI)
 		return c.DeleteVolume(diskURI)
 	}
-	// if forced (as in one disk = one storage account)
-	// delete the account completely
-	if wasForced {
-		return c.deleteStorageAccount(storageAccountName)
-	}
 
 	blobSvc, err := c.getBlobSvcClient(storageAccountName)
 	if err != nil {

pkg/volume/azure_dd/azure_dd.go

@@ -28,8 +28,8 @@ import (
 
 // interface exposed by the cloud provider implementing Disk functionlity
 type DiskController interface {
-	CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int, forceStandAlone bool) (string, error)
+	CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int) (string, error)
-	DeleteBlobDisk(diskUri string, wasForced bool) error
+	DeleteBlobDisk(diskUri string) error
 
 	CreateManagedDisk(diskName string, storageAccountType storage.SkuName, sizeGB int, tags map[string]string) (string, error)
 	DeleteManagedDisk(diskURI string) error

pkg/volume/azure_dd/azure_provision.go

@@ -55,14 +55,13 @@ func (d *azureDiskDeleter) Delete() error {
 		return err
 	}
 
-	wasStandAlone := (*volumeSource.Kind != v1.AzureSharedBlobDisk)
 	managed := (*volumeSource.Kind == v1.AzureManagedDisk)
 
 	if managed {
 		return diskController.DeleteManagedDisk(volumeSource.DataDiskURI)
 	}
 
-	return diskController.DeleteBlobDisk(volumeSource.DataDiskURI, wasStandAlone)
+	return diskController.DeleteBlobDisk(volumeSource.DataDiskURI)
 }
 
 func (p *azureDiskProvisioner) Provision() (*v1.PersistentVolume, error) {
@@ -149,26 +148,13 @@ func (p *azureDiskProvisioner) Provision() (*v1.PersistentVolume, error) {
 			return nil, err
 		}
 	} else {
-		forceStandAlone := (kind == v1.AzureDedicatedBlobDisk)
 		if kind == v1.AzureDedicatedBlobDisk {
-			if location != "" && account != "" {
+			_, diskURI, _, err = diskController.CreateVolume(name, account, skuName, location, requestGB)
-				// use dedicated kind (by default) for compatibility
+			if err != nil {
-				_, diskURI, _, err = diskController.CreateVolume(name, account, skuName, location, requestGB)
+				return nil, err
-				if err != nil {
-					return nil, err
-				}
-			} else {
-				if location != "" || account != "" {
-					return nil, fmt.Errorf("AzureDisk - location(%s) and account(%s) must be both empty or specified for dedicated kind, only one value specified is not allowed",
-						location, account)
-				}
-				diskURI, err = diskController.CreateBlobDisk(name, skuName, requestGB, forceStandAlone)
-				if err != nil {
-					return nil, err
-				}
 			}
 		} else {
-			diskURI, err = diskController.CreateBlobDisk(name, skuName, requestGB, forceStandAlone)
+			diskURI, err = diskController.CreateBlobDisk(name, skuName, requestGB)
 			if err != nil {
 				return nil, err
 			}

test/e2e/framework/pv_util.go

@@ -718,16 +718,11 @@ func createPD(zone string) (string, error) {
 			return "", err
 		}
 
-		if azureCloud.BlobDiskController == nil {
+		_, diskURI, _, err := azureCloud.CreateVolume(pdName, "" /* account */, "" /* sku */, "" /* location */, 1 /* sizeGb */)
-			return "", fmt.Errorf("BlobDiskController is nil, it's not expected.")
-		}
-
-		diskUri, err := azureCloud.BlobDiskController.CreateBlobDisk(pdName, "standard_lrs", 1, false)
 		if err != nil {
 			return "", err
 		}
-
+		return diskURI, nil
-		return diskUri, nil
 	} else {
 		return "", fmt.Errorf("provider does not support volume creation")
 	}
@@ -772,11 +767,7 @@ func deletePD(pdName string) error {
 		if err != nil {
 			return err
 		}
-		if azureCloud.BlobDiskController == nil {
+		err = azureCloud.DeleteVolume(pdName)
-			return fmt.Errorf("BlobDiskController is nil, it's not expected.")
-		}
-		diskName := pdName[(strings.LastIndex(pdName, "/") + 1):]
-		err = azureCloud.BlobDiskController.DeleteBlobDisk(diskName, false)
 		if err != nil {
 			Logf("failed to delete Azure volume %q: %v", pdName, err)
 			return err