From b898c79656a08bb5a5bff1f8edf7aa6f86c95433 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Wed, 18 Dec 2019 14:40:52 +0000 Subject: [PATCH] chore: port azure disk csi code to upstream fix golint error --- .../azure/azure_controller_common.go | 50 +++++++ .../azure/azure_controller_common_test.go | 127 ++++++++++++++++++ .../azure/azure_managedDiskController.go | 11 +- 3 files changed, 187 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go index 81b967b8a8a..d973d2a9797 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go @@ -22,6 +22,7 @@ import ( "context" "fmt" "path" + "regexp" "strings" "sync" "time" @@ -46,6 +47,14 @@ const ( errLeaseIDMissing = "LeaseIdMissing" errContainerNotFound = "ContainerNotFound" errDiskBlobNotFound = "DiskBlobNotFound" + sourceSnapshot = "snapshot" + sourceVolume = "volume" + + // see https://docs.microsoft.com/en-us/rest/api/compute/disks/createorupdate#create-a-managed-disk-by-copying-a-snapshot. + diskSnapshotPath = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/snapshots/%s" + + // see https://docs.microsoft.com/en-us/rest/api/compute/disks/createorupdate#create-a-managed-disk-from-an-existing-managed-disk-in-the-same-or-different-subscription. + managedDiskPath = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/%s" ) var defaultBackOff = kwait.Backoff{ @@ -55,6 +64,11 @@ var defaultBackOff = kwait.Backoff{ Jitter: 0.0, } +var ( + managedDiskPathRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(?:.*)/providers/Microsoft.Compute/disks/(.+)`) + diskSnapshotPathRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(?:.*)/providers/Microsoft.Compute/snapshots/(.+)`) +) + type controllerCommon struct { subscriptionID string location string @@ -312,3 +326,39 @@ func filterDetachingDisks(unfilteredDisks []compute.DataDisk) []compute.DataDisk } return filteredDisks } + +func getValidCreationData(subscriptionID, resourceGroup, sourceResourceID, sourceType string) (compute.CreationData, error) { + if sourceResourceID == "" { + return compute.CreationData{ + CreateOption: compute.Empty, + }, nil + } + + switch sourceType { + case sourceSnapshot: + if match := diskSnapshotPathRE.FindString(sourceResourceID); match == "" { + sourceResourceID = fmt.Sprintf(diskSnapshotPath, subscriptionID, resourceGroup, sourceResourceID) + } + + case sourceVolume: + if match := managedDiskPathRE.FindString(sourceResourceID); match == "" { + sourceResourceID = fmt.Sprintf(managedDiskPath, subscriptionID, resourceGroup, sourceResourceID) + } + default: + return compute.CreationData{ + CreateOption: compute.Empty, + }, nil + } + + splits := strings.Split(sourceResourceID, "/") + if len(splits) > 9 { + if sourceType == sourceSnapshot { + return compute.CreationData{}, fmt.Errorf("sourceResourceID(%s) is invalid, correct format: %s", sourceResourceID, diskSnapshotPathRE) + } + return compute.CreationData{}, fmt.Errorf("sourceResourceID(%s) is invalid, correct format: %s", sourceResourceID, managedDiskPathRE) + } + return compute.CreationData{ + CreateOption: compute.Copy, + SourceResourceID: &sourceResourceID, + }, nil +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go index 5db74cb5a6a..873d1a75146 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go @@ -20,6 +20,7 @@ package azure import ( "fmt" + "reflect" "testing" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute" @@ -284,3 +285,129 @@ func TestFilteredDetatchingDisks(t *testing.T) { filteredDisks = filterDetachingDisks(disks) assert.Equal(t, 0, len(filteredDisks)) } + +func TestGetValidCreationData(t *testing.T) { + sourceResourceSnapshotID := "/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/snapshots/xxx" + sourceResourceVolumeID := "/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/disks/xxx" + + tests := []struct { + subscriptionID string + resourceGroup string + sourceResourceID string + sourceType string + expected1 compute.CreationData + expected2 error + }{ + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "", + sourceType: "", + expected1: compute.CreationData{ + CreateOption: compute.Empty, + }, + expected2: nil, + }, + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/snapshots/xxx", + sourceType: sourceSnapshot, + expected1: compute.CreationData{ + CreateOption: compute.Copy, + SourceResourceID: &sourceResourceSnapshotID, + }, + expected2: nil, + }, + { + subscriptionID: "xxx", + resourceGroup: "xxx", + sourceResourceID: "xxx", + sourceType: sourceSnapshot, + expected1: compute.CreationData{ + CreateOption: compute.Copy, + SourceResourceID: &sourceResourceSnapshotID, + }, + expected2: nil, + }, + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "/subscriptions/23/providers/Microsoft.Compute/disks/name", + sourceType: sourceSnapshot, + expected1: compute.CreationData{}, + expected2: fmt.Errorf("sourceResourceID(%s) is invalid, correct format: %s", "/subscriptions//resourceGroups//providers/Microsoft.Compute/snapshots//subscriptions/23/providers/Microsoft.Compute/disks/name", diskSnapshotPathRE), + }, + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "http://test.com/vhds/name", + sourceType: sourceSnapshot, + expected1: compute.CreationData{}, + expected2: fmt.Errorf("sourceResourceID(%s) is invalid, correct format: %s", "/subscriptions//resourceGroups//providers/Microsoft.Compute/snapshots/http://test.com/vhds/name", diskSnapshotPathRE), + }, + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "/subscriptions/xxx/snapshots/xxx", + sourceType: sourceSnapshot, + expected1: compute.CreationData{}, + expected2: fmt.Errorf("sourceResourceID(%s) is invalid, correct format: %s", "/subscriptions//resourceGroups//providers/Microsoft.Compute/snapshots//subscriptions/xxx/snapshots/xxx", diskSnapshotPathRE), + }, + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/snapshots/xxx/snapshots/xxx/snapshots/xxx", + sourceType: sourceSnapshot, + expected1: compute.CreationData{}, + expected2: fmt.Errorf("sourceResourceID(%s) is invalid, correct format: %s", "/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/snapshots/xxx/snapshots/xxx/snapshots/xxx", diskSnapshotPathRE), + }, + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "xxx", + sourceType: "", + expected1: compute.CreationData{ + CreateOption: compute.Empty, + }, + expected2: nil, + }, + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/disks/xxx", + sourceType: sourceVolume, + expected1: compute.CreationData{ + CreateOption: compute.Copy, + SourceResourceID: &sourceResourceVolumeID, + }, + expected2: nil, + }, + { + subscriptionID: "xxx", + resourceGroup: "xxx", + sourceResourceID: "xxx", + sourceType: sourceVolume, + expected1: compute.CreationData{ + CreateOption: compute.Copy, + SourceResourceID: &sourceResourceVolumeID, + }, + expected2: nil, + }, + { + subscriptionID: "", + resourceGroup: "", + sourceResourceID: "/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/snapshots/xxx", + sourceType: sourceVolume, + expected1: compute.CreationData{}, + expected2: fmt.Errorf("sourceResourceID(%s) is invalid, correct format: %s", "/subscriptions//resourceGroups//providers/Microsoft.Compute/disks//subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/snapshots/xxx", managedDiskPathRE), + }, + } + + for _, test := range tests { + result, err := getValidCreationData(test.subscriptionID, test.resourceGroup, test.sourceResourceID, test.sourceType) + if !reflect.DeepEqual(result, test.expected1) || !reflect.DeepEqual(err, test.expected2) { + t.Errorf("input sourceResourceID: %v, sourceType: %v, getValidCreationData result: %v, expected1 : %v, err: %v, expected2: %v", test.sourceResourceID, test.sourceType, result, test.expected1, err, test.expected2) + } + } +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go index 3a48df20b50..86d9b85af31 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_managedDiskController.go @@ -69,6 +69,10 @@ type ManagedDiskOptions struct { DiskIOPSReadWrite string // Throughput Cap (MBps) for UltraSSD disk DiskMBpsReadWrite string + // if SourceResourceID is not empty, then it's a disk copy operation(for snapshot) + SourceResourceID string + // The type of source + SourceType string // ResourceId of the disk encryption set to use for enabling encryption at rest. DiskEncryptionSetID string } @@ -99,9 +103,14 @@ func (c *ManagedDiskController) CreateManagedDisk(options *ManagedDiskOptions) ( diskSizeGB := int32(options.SizeGB) diskSku := compute.DiskStorageAccountTypes(options.StorageAccountType) + + creationData, err := getValidCreationData(c.common.subscriptionID, options.ResourceGroup, options.SourceResourceID, options.SourceType) + if err != nil { + return "", err + } diskProperties := compute.DiskProperties{ DiskSizeGB: &diskSizeGB, - CreationData: &compute.CreationData{CreateOption: compute.Empty}, + CreationData: &creationData, } if diskSku == compute.UltraSSDLRS {