Skip to content

Commit

Permalink
Merge pull request #65689 from andyzhangx/automated-cherry-pick-of-#6…
Browse files Browse the repository at this point in the history 
…4427-upstream-release-1.9

Automatic merge from submit-queue.

Automated cherry pick of #64427: add external resource group support for azure disk

Cherry pick of #64427 on release-1.9.

#64427: add external resource group support for azure disk
  • Loading branch information
Kubernetes Submit Queue committed Aug 10, 2018
2 parents 2f70738 + 79a0914 commit a2c6e6d
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 9 deletions.
34 changes: 27 additions & 7 deletions pkg/cloudprovider/providers/azure/azure_managedDiskController.go
View file
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package azure

import (
"fmt"
"path"
"strings"

Expand All @@ -36,7 +37,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)
Expand All @@ -62,8 +64,11 @@ func (c *ManagedDiskController) CreateManagedDisk(diskName string, storageAccoun
DiskSizeGB: &diskSizeGB,
CreationData: &disk.CreationData{CreateOption: disk.Empty},
}}
if resourceGroup == "" {
resourceGroup = c.common.resourceGroup
}
cancel := make(chan struct{})
respChan, errChan := c.common.cloud.DisksClient.CreateOrUpdate(c.common.resourceGroup, diskName, model, cancel)
respChan, errChan := c.common.cloud.DisksClient.CreateOrUpdate(resourceGroup, diskName, model, cancel)
<-respChan
err := <-errChan
if err != nil {
Expand All @@ -73,7 +78,7 @@ func (c *ManagedDiskController) CreateManagedDisk(diskName string, storageAccoun
diskID := ""

err = kwait.ExponentialBackoff(defaultBackOff, func() (bool, error) {
provisonState, id, err := c.getDisk(diskName)
provisonState, id, err := c.getDisk(resourceGroup, diskName)
diskID = id
// We are waiting for provisioningState==Succeeded
// We don't want to hand-off managed disks to k8s while they are
Expand All @@ -99,10 +104,14 @@ 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
}
cancel := make(chan struct{})
respChan, errChan := c.common.cloud.DisksClient.Delete(c.common.resourceGroup, diskName, cancel)
respChan, errChan := c.common.cloud.DisksClient.Delete(resourceGroup, diskName, cancel)
<-respChan
err := <-errChan
err = <-errChan
if err != nil {
return err
}
Expand All @@ -115,8 +124,8 @@ func (c *ManagedDiskController) DeleteManagedDisk(diskURI string) error {
}

// return: disk provisionState, diskID, error
func (c *ManagedDiskController) getDisk(diskName string) (string, string, error) {
result, err := c.common.cloud.DisksClient.Get(c.common.resourceGroup, diskName)
func (c *ManagedDiskController) getDisk(resourceGroup, diskName string) (string, string, error) {
result, err := c.common.cloud.DisksClient.Get(resourceGroup, diskName)
if err != nil {
return "", "", err
}
Expand All @@ -127,3 +136,14 @@ func (c *ManagedDiskController) getDisk(diskName string) (string, string, error)

return "", "", err
}

// 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
}
37 changes: 37 additions & 0 deletions pkg/cloudprovider/providers/azure/azure_test.go
View file
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/Azure/azure-sdk-for-go/arm/compute"
"github.com/Azure/azure-sdk-for-go/arm/network"
"github.com/Azure/go-autorest/autorest/to"
"github.com/stretchr/testify/assert"
)

var testClusterName = "testCluster"
Expand Down Expand Up @@ -2603,3 +2604,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)
}
}
}
2 changes: 1 addition & 1 deletion pkg/volume/azure_dd/azure_dd.go
View file
Expand Up @@ -31,7 +31,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.
Expand Down
14 changes: 13 additions & 1 deletion pkg/volume/azure_dd/azure_provision.go
View file
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package azure_dd

import (
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -91,6 +92,7 @@ func (p *azureDiskProvisioner) Provision() (*v1.PersistentVolume, error) {
cachingMode v1.AzureDataDiskCachingMode
strKind string
err error
resourceGroup string
)
// maxLength = 79 - (4 for ".vhd") = 75
name := volume.GenerateVolumeName(p.options.ClusterName, p.options.PVName, 75)
Expand All @@ -114,6 +116,8 @@ func (p *azureDiskProvisioner) Provision() (*v1.PersistentVolume, error) {
cachingMode = v1.AzureDataDiskCachingMode(v)
case volume.VolumeParameterFSType:
fsType = strings.ToLower(v)
case "resourcegroup":
resourceGroup = v
default:
return nil, fmt.Errorf("AzureDisk - invalid option %s in storage class", k)
}
Expand All @@ -139,10 +143,18 @@ func (p *azureDiskProvisioner) Provision() (*v1.PersistentVolume, error) {
return nil, err
}

if resourceGroup != "" && kind != v1.AzureManagedDisk {
return nil, errors.New("StorageClass option 'resourceGroup' can be used only for managed disks")
}

// create disk
diskURI := ""
if kind == v1.AzureManagedDisk {
diskURI, err = diskController.CreateManagedDisk(name, skuName, requestGB, *(p.options.CloudTags))
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
}
Expand Down

0 comments on commit a2c6e6d

Please sign in to comment.