Merge pull request #64427 from andyzhangx/azuredisk-rg

Automatic merge from submit-queue (batch tested with PRs 65032, 63471, 64104, 64672, 64427). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

add external resource group support for azure disk

**What this PR does / why we need it**:
add external resource group support for azure disk, 
 - without this PR, user could only create dynamic azure disk in the same resource group as cluster
 - with this PR, user could specify external resource group in PVC:
```
kind: PersistentVolumeClaim
apiVersion: v1
metadata:
  name: pvc-azuredisk
  annotations:
    volume.beta.kubernetes.io/resource-group: "USER-SPECIFIED-RG"  
spec:
  accessModes:
  - ReadWriteOnce
  resources:
    requests:
      storage: 1Gi
  storageClassName: hdd
```

**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 #64388

**Special notes for your reviewer**:
Pls note above config won't change resource group for azure disk forever, next time if user don't specify resource group, only default resource group will be used.

**Release note**:

```
add external resource group support for azure disk
```
/sig azure
/assign @feiskyer @karataliu 
/cc @khenidak

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
+}

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)
+		}
+	}
+}

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) {

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
 		}