From 93f7cec5b3f1e891fabbab3637d7637c959ebb18 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Mon, 7 Oct 2019 14:08:24 -0700 Subject: [PATCH] Add RepairVolumeHandle to the csi translation struct --- .../csi-translation-lib/plugins/aws_ebs.go | 6 +- .../csi-translation-lib/plugins/azure_disk.go | 6 +- .../csi-translation-lib/plugins/azure_file.go | 6 +- .../csi-translation-lib/plugins/gce_pd.go | 60 ++++++++++++-- .../plugins/gce_pd_test.go | 81 ++++++++++++++++++- .../plugins/in_tree_volume.go | 5 +- .../plugins/openstack_cinder.go | 6 +- .../k8s.io/csi-translation-lib/translate.go | 8 ++ 8 files changed, 166 insertions(+), 12 deletions(-) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go b/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go index 573214032dd..e058a0161d4 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/aws_ebs.go @@ -23,7 +23,7 @@ import ( "strconv" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -161,6 +161,10 @@ func (t *awsElasticBlockStoreCSITranslator) GetCSIPluginName() string { return AWSEBSDriverName } +func (t *awsElasticBlockStoreCSITranslator) RepairVolumeHandle(volumeHandle, nodeID string) (string, error) { + return volumeHandle, nil +} + // awsVolumeRegMatch represents Regex Match for AWS volume. var awsVolumeRegMatch = regexp.MustCompile("^vol-[^/]*$") diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk.go b/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk.go index c98b80fdeb9..93013ba357c 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/azure_disk.go @@ -21,7 +21,7 @@ import ( "regexp" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -209,6 +209,10 @@ func (t *azureDiskCSITranslator) GetCSIPluginName() string { return AzureDiskDriverName } +func (t *azureDiskCSITranslator) RepairVolumeHandle(volumeHandle, nodeID string) (string, error) { + return volumeHandle, nil +} + func isManagedDisk(diskURI string) bool { if len(diskURI) > 4 && strings.ToLower(diskURI[:4]) == "http" { return false diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go index b78a4f83fda..c449c25599d 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/azure_file.go @@ -20,7 +20,7 @@ import ( "fmt" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -176,6 +176,10 @@ func (t *azureFileCSITranslator) GetCSIPluginName() string { return AzureFileDriverName } +func (t *azureFileCSITranslator) RepairVolumeHandle(volumeHandle, nodeID string) (string, error) { + return volumeHandle, nil +} + // get file share info according to volume id, e.g. // input: "rg#f5713de20cde511e8ba4900#pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41" // output: rg, f5713de20cde511e8ba4900, pvc-file-dynamic-17e43f84-f474-11e8-acd0-000d3a00df41 diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go index ed9d079b5eb..6593c459aa9 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go @@ -21,7 +21,7 @@ import ( "strconv" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -41,9 +41,14 @@ const ( // "projects/{projectName}/zones/{zoneName}/disks/{diskName}" volIDZonalFmt = "projects/%s/zones/%s/disks/%s" // "projects/{projectName}/regions/{regionName}/disks/{diskName}" - volIDRegionalFmt = "projects/%s/regions/%s/disks/%s" - volIDDiskNameValue = 5 - volIDTotalElements = 6 + volIDRegionalFmt = "projects/%s/regions/%s/disks/%s" + volIDProjectValue = 1 + volIDRegionalityValue = 2 + volIDZoneValue = 3 + volIDDiskNameValue = 5 + volIDTotalElements = 6 + + nodeIDFmt = "projects/%s/zones/%s/instances/%s" // UnspecifiedValue is used for an unknown zone string UnspecifiedValue = "UNSPECIFIED" @@ -328,10 +333,53 @@ func (g *gcePersistentDiskCSITranslator) GetCSIPluginName() string { return GCEPDDriverName } +// RepairVolumeHandle returns a fully specified volume handle by inferring +// project, zone/region from the node ID if the volume handle has UNSPECIFIED +// sections +func (g *gcePersistentDiskCSITranslator) RepairVolumeHandle(volumeHandle, nodeID string) (string, error) { + var err error + tok := strings.Split(volumeHandle, "/") + if len(tok) < volIDTotalElements { + return "", fmt.Errorf("volume handle has wrong number of elements; got %v, wanted %v or more", len(tok), volIDTotalElements) + } + if tok[volIDProjectValue] != UnspecifiedValue { + return volumeHandle, nil + } + + nodeTok := strings.Split(nodeID, "/") + if len(nodeTok) < volIDTotalElements { + return "", fmt.Errorf("node handle has wrong number of elements; got %v, wanted %v or more", len(nodeTok), volIDTotalElements) + } + + switch tok[volIDRegionalityValue] { + case "zones": + zone := "" + if tok[volIDZoneValue] == UnspecifiedValue { + zone = nodeTok[volIDZoneValue] + } else { + zone = tok[volIDZoneValue] + } + return fmt.Sprintf(volIDZonalFmt, nodeTok[volIDProjectValue], zone, tok[volIDDiskNameValue]), nil + case "regions": + region := "" + if tok[volIDZoneValue] == UnspecifiedValue { + region, err = getRegionFromZones([]string{nodeTok[volIDZoneValue]}) + if err != nil { + return "", fmt.Errorf("failed to get region from zone %s: %v", nodeTok[volIDZoneValue], err) + } + } else { + region = tok[volIDZoneValue] + } + return fmt.Sprintf(volIDRegionalFmt, nodeTok[volIDProjectValue], region, tok[volIDDiskNameValue]), nil + default: + return "", fmt.Errorf("expected volume handle to have zones or regions regionality value, got: %s", tok[volIDRegionalityValue]) + } +} + func pdNameFromVolumeID(id string) (string, error) { splitID := strings.Split(id, "/") - if len(splitID) != volIDTotalElements { - return "", fmt.Errorf("failed to get id components. Expected projects/{project}/zones/{zone}/disks/{name}. Got: %s", id) + if len(splitID) < volIDTotalElements { + return "", fmt.Errorf("failed to get id components.Got: %v, wanted %v components or more. ", len(splitID), volIDTotalElements) } return splitID[volIDDiskNameValue], nil } diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go index 3df9b0242af..cfb973ef512 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd_test.go @@ -17,10 +17,11 @@ limitations under the License. package plugins import ( + "fmt" "reflect" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" ) @@ -204,6 +205,84 @@ func TestTranslateAllowedTopologies(t *testing.T) { } } +func TestRepairVolumeHandle(t *testing.T) { + testCases := []struct { + name string + volumeHandle string + nodeID string + expectedVolumeHandle string + expectedErr bool + }{ + { + name: "fully specified", + volumeHandle: fmt.Sprintf(volIDZonalFmt, "foo", "bar", "baz"), + nodeID: fmt.Sprintf(nodeIDFmt, "bing", "bada", "boom"), + expectedVolumeHandle: fmt.Sprintf(volIDZonalFmt, "foo", "bar", "baz"), + }, + { + name: "fully specified (regional)", + volumeHandle: fmt.Sprintf(volIDRegionalFmt, "foo", "us-central1-c", "baz"), + nodeID: fmt.Sprintf(nodeIDFmt, "bing", "bada", "boom"), + expectedVolumeHandle: fmt.Sprintf(volIDRegionalFmt, "foo", "us-central1-c", "baz"), + }, + { + name: "no project", + volumeHandle: fmt.Sprintf(volIDZonalFmt, UnspecifiedValue, "bar", "baz"), + nodeID: fmt.Sprintf(nodeIDFmt, "bing", "bada", "boom"), + expectedVolumeHandle: fmt.Sprintf(volIDZonalFmt, "bing", "bar", "baz"), + }, + { + name: "no project or zone", + volumeHandle: fmt.Sprintf(volIDZonalFmt, UnspecifiedValue, UnspecifiedValue, "baz"), + nodeID: fmt.Sprintf(nodeIDFmt, "bing", "bada", "boom"), + expectedVolumeHandle: fmt.Sprintf(volIDZonalFmt, "bing", "bada", "baz"), + }, + { + name: "no project or region", + volumeHandle: fmt.Sprintf(volIDRegionalFmt, UnspecifiedValue, UnspecifiedValue, "baz"), + nodeID: fmt.Sprintf(nodeIDFmt, "bing", "us-central1-c", "boom"), + expectedVolumeHandle: fmt.Sprintf(volIDRegionalFmt, "bing", "us-central1", "baz"), + }, + { + name: "no project (regional)", + volumeHandle: fmt.Sprintf(volIDRegionalFmt, UnspecifiedValue, "us-west1", "baz"), + nodeID: fmt.Sprintf(nodeIDFmt, "bing", "us-central1-c", "boom"), + expectedVolumeHandle: fmt.Sprintf(volIDRegionalFmt, "bing", "us-west1", "baz"), + }, + { + name: "invalid handle", + volumeHandle: "foo", + nodeID: fmt.Sprintf(nodeIDFmt, "bing", "us-central1-c", "boom"), + expectedErr: true, + }, + { + name: "invalid node ID", + volumeHandle: fmt.Sprintf(volIDRegionalFmt, UnspecifiedValue, "us-west1", "baz"), + nodeID: "foo", + expectedErr: true, + }, + } + g := NewGCEPersistentDiskCSITranslator() + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gotVolumeHandle, err := g.RepairVolumeHandle(tc.volumeHandle, tc.nodeID) + if err != nil && !tc.expectedErr { + if !tc.expectedErr { + t.Fatalf("Got error: %v, but expected none", err) + } + return + } + if err == nil && tc.expectedErr { + t.Fatal("Got no error, but expected one") + } + + if gotVolumeHandle != tc.expectedVolumeHandle { + t.Fatalf("Got volume handle %s, but expected %s", gotVolumeHandle, tc.expectedVolumeHandle) + } + }) + } +} + func TestBackwardCompatibleAccessModes(t *testing.T) { testCases := []struct { name string diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go b/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go index 2095084d82b..1415cbc7036 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/in_tree_volume.go @@ -17,7 +17,7 @@ limitations under the License. package plugins import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" ) @@ -55,4 +55,7 @@ type InTreePlugin interface { // GetCSIPluginName returns the name of the CSI plugin that supersedes the in-tree plugin GetCSIPluginName() string + + // RepairVolumeHandle generates a correct volume handle based on node ID information. + RepairVolumeHandle(volumeHandle, nodeID string) (string, error) } diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go b/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go index 9965eacf596..abf578fb4a5 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/openstack_cinder.go @@ -19,7 +19,7 @@ package plugins import ( "fmt" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -140,3 +140,7 @@ func (t *osCinderCSITranslator) GetInTreePluginName() string { func (t *osCinderCSITranslator) GetCSIPluginName() string { return CinderDriverName } + +func (t *osCinderCSITranslator) RepairVolumeHandle(volumeHandle, nodeID string) (string, error) { + return volumeHandle, nil +} diff --git a/staging/src/k8s.io/csi-translation-lib/translate.go b/staging/src/k8s.io/csi-translation-lib/translate.go index e3d4966df38..3b3e0c632ec 100644 --- a/staging/src/k8s.io/csi-translation-lib/translate.go +++ b/staging/src/k8s.io/csi-translation-lib/translate.go @@ -186,3 +186,11 @@ func (CSITranslator) IsInlineMigratable(vol *v1.Volume) bool { } return false } + +// RepairVolumeHandle generates a correct volume handle based on node ID information. +func (CSITranslator) RepairVolumeHandle(driverName, volumeHandle, nodeID string) (string, error) { + if plugin, ok := inTreePlugins[driverName]; ok { + return plugin.RepairVolumeHandle(volumeHandle, nodeID) + } + return "", fmt.Errorf("could not find In-Tree driver name for CSI plugin %v", driverName) +}