diff --git a/pkg/cloudprovider/providers/azure/azure_managedDiskController.go b/pkg/cloudprovider/providers/azure/azure_managedDiskController.go index fac548f1d2..4cb8aa2672 100644 --- a/pkg/cloudprovider/providers/azure/azure_managedDiskController.go +++ b/pkg/cloudprovider/providers/azure/azure_managedDiskController.go @@ -40,7 +40,8 @@ func newManagedDiskController(common *controllerCommon) (*ManagedDiskController, } //CreateManagedDisk : create managed disk -func (c *ManagedDiskController) CreateManagedDisk(diskName string, storageAccountType storage.SkuName, sizeGB int, tags map[string]string) (string, error) { +func (c *ManagedDiskController) CreateManagedDisk(diskName string, storageAccountType storage.SkuName, resourceGroup string, + sizeGB int, tags map[string]string) (string, error) { glog.V(4).Infof("azureDisk - creating new managed Name:%s StorageAccountType:%s Size:%v", diskName, storageAccountType, sizeGB) newTags := make(map[string]*string) @@ -68,9 +69,14 @@ func (c *ManagedDiskController) CreateManagedDisk(diskName string, storageAccoun DiskSizeGB: &diskSizeGB, CreationData: &compute.CreationData{CreateOption: compute.Empty}, }} + + if resourceGroup == "" { + resourceGroup = c.common.resourceGroup + } + ctx, cancel := getContextWithCancel() defer cancel() - _, err := c.common.cloud.DisksClient.CreateOrUpdate(ctx, c.common.resourceGroup, diskName, model) + _, err := c.common.cloud.DisksClient.CreateOrUpdate(ctx, resourceGroup, diskName, model) if err != nil { return "", err } @@ -104,10 +110,15 @@ func (c *ManagedDiskController) CreateManagedDisk(diskName string, storageAccoun //DeleteManagedDisk : delete managed disk func (c *ManagedDiskController) DeleteManagedDisk(diskURI string) error { diskName := path.Base(diskURI) + resourceGroup, err := getResourceGroupFromDiskURI(diskURI) + if err != nil { + return err + } + ctx, cancel := getContextWithCancel() defer cancel() - _, err := c.common.cloud.DisksClient.Delete(ctx, c.common.resourceGroup, diskName) + _, err = c.common.cloud.DisksClient.Delete(ctx, resourceGroup, diskName) if err != nil { return err } @@ -137,11 +148,17 @@ func (c *ManagedDiskController) getDisk(diskName string) (string, string, error) } // ResizeDisk Expand the disk to new size -func (c *ManagedDiskController) ResizeDisk(diskName string, oldSize resource.Quantity, newSize resource.Quantity) (resource.Quantity, error) { +func (c *ManagedDiskController) ResizeDisk(diskURI string, oldSize resource.Quantity, newSize resource.Quantity) (resource.Quantity, error) { ctx, cancel := getContextWithCancel() defer cancel() - result, err := c.common.cloud.DisksClient.Get(ctx, c.common.resourceGroup, diskName) + diskName := path.Base(diskURI) + resourceGroup, err := getResourceGroupFromDiskURI(diskURI) + if err != nil { + return oldSize, err + } + + result, err := c.common.cloud.DisksClient.Get(ctx, resourceGroup, diskName) if err != nil { return oldSize, err } @@ -173,3 +190,14 @@ func (c *ManagedDiskController) ResizeDisk(diskName string, oldSize resource.Qua return newSizeQuant, nil } + +// get resource group name from a managed disk URI, e.g. return {group-name} according to +// /subscriptions/{sub-id}/resourcegroups/{group-name}/providers/microsoft.compute/disks/{disk-id} +// according to https://docs.microsoft.com/en-us/rest/api/compute/disks/get +func getResourceGroupFromDiskURI(diskURI string) (string, error) { + fields := strings.Split(diskURI, "/") + if len(fields) != 9 || fields[3] != "resourceGroups" { + return "", fmt.Errorf("invalid disk URI: %s", diskURI) + } + return fields[4], nil +} diff --git a/pkg/cloudprovider/providers/azure/azure_test.go b/pkg/cloudprovider/providers/azure/azure_test.go index dfcf2ec92a..c8b0eb7be1 100644 --- a/pkg/cloudprovider/providers/azure/azure_test.go +++ b/pkg/cloudprovider/providers/azure/azure_test.go @@ -2699,3 +2699,39 @@ func TestCanCombineSharedAndPrivateRulesInSameGroup(t *testing.T) { // func TestIfServiceIsEditedFromSharedRuleToOwnRuleThenItIsRemovedFromSharedRuleAndOwnRuleIsCreated(t *testing.T) { // t.Error() // } + +func TestGetResourceGroupFromDiskURI(t *testing.T) { + tests := []struct { + diskURL string + expectedResult string + expectError bool + }{ + { + diskURL: "/subscriptions/4be8920b-2978-43d7-axyz-04d8549c1d05/resourceGroups/azure-k8s1102/providers/Microsoft.Compute/disks/andy-mghyb1102-dynamic-pvc-f7f014c9-49f4-11e8-ab5c-000d3af7b38e", + expectedResult: "azure-k8s1102", + expectError: false, + }, + { + diskURL: "/4be8920b-2978-43d7-axyz-04d8549c1d05/resourceGroups/azure-k8s1102/providers/Microsoft.Compute/disks/andy-mghyb1102-dynamic-pvc-f7f014c9-49f4-11e8-ab5c-000d3af7b38e", + expectedResult: "", + expectError: true, + }, + { + diskURL: "", + expectedResult: "", + expectError: true, + }, + } + + for _, test := range tests { + result, err := getResourceGroupFromDiskURI(test.diskURL) + assert.Equal(t, result, test.expectedResult, "Expect result not equal with getResourceGroupFromDiskURI(%s) return: %q, expected: %q", + test.diskURL, result, test.expectedResult) + + if test.expectError { + assert.NotNil(t, err, "Expect error during getResourceGroupFromDiskURI(%s)", test.diskURL) + } else { + assert.Nil(t, err, "Expect error is nil during getResourceGroupFromDiskURI(%s)", test.diskURL) + } + } +} diff --git a/pkg/volume/azure_dd/azure_dd.go b/pkg/volume/azure_dd/azure_dd.go index 8310e0ab00..91745c39fc 100644 --- a/pkg/volume/azure_dd/azure_dd.go +++ b/pkg/volume/azure_dd/azure_dd.go @@ -36,7 +36,7 @@ type DiskController interface { CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int) (string, error) DeleteBlobDisk(diskUri string) error - CreateManagedDisk(diskName string, storageAccountType storage.SkuName, sizeGB int, tags map[string]string) (string, error) + CreateManagedDisk(diskName string, storageAccountType storage.SkuName, resourceGroup string, sizeGB int, tags map[string]string) (string, error) DeleteManagedDisk(diskURI string) error // Attaches the disk to the host machine. @@ -58,7 +58,7 @@ type DiskController interface { DeleteVolume(diskURI string) error // Expand the disk to new size - ResizeDisk(diskName string, oldSize resource.Quantity, newSize resource.Quantity) (resource.Quantity, error) + ResizeDisk(diskURI string, oldSize resource.Quantity, newSize resource.Quantity) (resource.Quantity, error) } type azureDataDiskPlugin struct { @@ -242,7 +242,7 @@ func (plugin *azureDataDiskPlugin) ExpandVolumeDevice( return oldSize, err } - return diskController.ResizeDisk(spec.PersistentVolume.Spec.AzureDisk.DiskName, oldSize, newSize) + return diskController.ResizeDisk(spec.PersistentVolume.Spec.AzureDisk.DataDiskURI, oldSize, newSize) } func (plugin *azureDataDiskPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { diff --git a/pkg/volume/azure_dd/azure_provision.go b/pkg/volume/azure_dd/azure_provision.go index ee7aa78f66..20cd6f6423 100644 --- a/pkg/volume/azure_dd/azure_provision.go +++ b/pkg/volume/azure_dd/azure_provision.go @@ -43,6 +43,10 @@ type azureDiskDeleter struct { var _ volume.Provisioner = &azureDiskProvisioner{} var _ volume.Deleter = &azureDiskDeleter{} +// PVCAnnotationResourceGroup is the annotation used on the PVC +// to specify the resource group of azure managed disk that are not in the same resource group as the cluster. +const PVCAnnotationResourceGroup = "volume.beta.kubernetes.io/resource-group" + func (d *azureDiskDeleter) GetPath() string { return getPath(d.podUID, d.dataDisk.diskName, d.plugin.host) } @@ -145,7 +149,15 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie // create disk diskURI := "" if kind == v1.AzureManagedDisk { - diskURI, err = diskController.CreateManagedDisk(name, skuName, requestGB, *(p.options.CloudTags)) + resourceGroup := "" + if rg, found := p.options.PVC.Annotations[PVCAnnotationResourceGroup]; found { + resourceGroup = rg + } + tags := make(map[string]string) + if p.options.CloudTags != nil { + tags = *(p.options.CloudTags) + } + diskURI, err = diskController.CreateManagedDisk(name, skuName, resourceGroup, requestGB, tags) if err != nil { return nil, err }