diff --git a/pkg/common/utils.go b/pkg/common/utils.go index 0d0ec125f..5885ecc9a 100644 --- a/pkg/common/utils.go +++ b/pkg/common/utils.go @@ -38,6 +38,8 @@ const ( nodeIDZoneValue = 0 nodeIDNameValue = 1 nodeIDTotalElements = 2 + + regionalDeviceNameSuffix = "_regional" ) func BytesToGb(bytes int64) int64 { @@ -90,3 +92,14 @@ func GetRegionFromZones(zones []string) (string, error) { } return regions.UnsortedList()[0], nil } + +func GetDeviceName(volKey *meta.Key) (string, error) { + switch volKey.Type() { + case meta.Zonal: + return volKey.Name, nil + case meta.Regional: + return volKey.Name + regionalDeviceNameSuffix, nil + default: + return "", fmt.Errorf("volume key %v neither zonal nor regional", volKey.Name) + } +} diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go index ab795d173..fab269416 100644 --- a/pkg/gce-cloud-provider/compute/gce-compute.go +++ b/pkg/gce-cloud-provider/compute/gce-compute.go @@ -300,8 +300,12 @@ func (cloud *CloudProvider) deleteRegionalDisk(ctx context.Context, region, name func (cloud *CloudProvider) AttachDisk(ctx context.Context, disk *CloudDisk, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error { source := cloud.GetDiskSourceURI(disk, volKey) + deviceName, err := common.GetDeviceName(volKey) + if err != nil { + return fmt.Errorf("failed to get device name: %v", err) + } attachedDiskV1 := &compute.AttachedDisk{ - DeviceName: disk.GetName(), + DeviceName: deviceName, Kind: disk.GetKind(), Mode: readWrite, Source: source, @@ -320,7 +324,12 @@ func (cloud *CloudProvider) AttachDisk(ctx context.Context, disk *CloudDisk, vol } func (cloud *CloudProvider) DetachDisk(ctx context.Context, volKey *meta.Key, instanceZone, instanceName string) error { - op, err := cloud.service.Instances.DetachDisk(cloud.project, instanceZone, instanceName, volKey.Name).Context(ctx).Do() + deviceName, err := common.GetDeviceName(volKey) + if err != nil { + return err + } + + op, err := cloud.service.Instances.DetachDisk(cloud.project, instanceZone, instanceName, deviceName).Context(ctx).Do() if err != nil { return err } diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index 96336b2f4..0deb76479 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -251,7 +251,12 @@ func (gceCS *GCEControllerServer) ControllerPublishVolume(ctx context.Context, r readWrite = "READ_ONLY" } - attached, err := diskIsAttachedAndCompatible(disk, instance, volumeCapability, readWrite) + deviceName, err := common.GetDeviceName(volKey) + if err != nil { + return nil, status.Error(codes.Internal, fmt.Sprintf("error getting device name: %v", err)) + } + + attached, err := diskIsAttachedAndCompatible(deviceName, instance, volumeCapability, readWrite) if err != nil { return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("Disk %v already published to node %v but incompatbile: %v", volKey.Name, nodeID, err)) } @@ -298,10 +303,6 @@ func (gceCS *GCEControllerServer) ControllerUnpublishVolume(ctx context.Context, return nil, err } - disk, err := gceCS.CloudProvider.GetDisk(ctx, volKey) - if err != nil { - return nil, err - } instanceZone, instanceName, err := common.NodeIDToZoneAndName(nodeID) if err != nil { return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("could not split nodeID: %v", err)) @@ -311,7 +312,12 @@ func (gceCS *GCEControllerServer) ControllerUnpublishVolume(ctx context.Context, return nil, err } - attached := diskIsAttached(disk, instance) + deviceName, err := common.GetDeviceName(volKey) + if err != nil { + return nil, status.Error(codes.Internal, fmt.Sprintf("error getting device name: %v", err)) + } + + attached := diskIsAttached(deviceName, instance) if !attached { // Volume is not attached to node. Success! @@ -473,9 +479,9 @@ func getRequestCapacity(capRange *csi.CapacityRange) (int64, error) { return capBytes, nil } -func diskIsAttached(volume *gce.CloudDisk, instance *compute.Instance) bool { +func diskIsAttached(deviceName string, instance *compute.Instance) bool { for _, disk := range instance.Disks { - if disk.DeviceName == volume.GetName() { + if disk.DeviceName == deviceName { // Disk is attached to node return true } @@ -483,9 +489,9 @@ func diskIsAttached(volume *gce.CloudDisk, instance *compute.Instance) bool { return false } -func diskIsAttachedAndCompatible(volume *gce.CloudDisk, instance *compute.Instance, volumeCapability *csi.VolumeCapability, readWrite string) (bool, error) { +func diskIsAttachedAndCompatible(deviceName string, instance *compute.Instance, volumeCapability *csi.VolumeCapability, readWrite string) (bool, error) { for _, disk := range instance.Disks { - if disk.DeviceName == volume.GetName() { + if disk.DeviceName == deviceName { // Disk is attached to node if disk.Mode != readWrite { return true, fmt.Errorf("disk mode does not match. Got %v. Want %v", disk.Mode, readWrite) diff --git a/pkg/gce-pd-csi-driver/controller_test.go b/pkg/gce-pd-csi-driver/controller_test.go index d9d2cdff6..d9629e073 100644 --- a/pkg/gce-pd-csi-driver/controller_test.go +++ b/pkg/gce-pd-csi-driver/controller_test.go @@ -604,15 +604,13 @@ func TestGetRequestCapacity(t *testing.T) { func TestDiskIsAttached(t *testing.T) { testCases := []struct { name string - disk *compute.Disk + deviceName string instance *compute.Instance expAttached bool }{ { - name: "normal-attached", - disk: &compute.Disk{ - Name: "test-disk", - }, + name: "normal-attached", + deviceName: "test-disk", instance: &compute.Instance{ Disks: []*compute.AttachedDisk{ { @@ -623,10 +621,8 @@ func TestDiskIsAttached(t *testing.T) { expAttached: true, }, { - name: "normal-not-attached", - disk: &compute.Disk{ - Name: "test-disk", - }, + name: "normal-not-attached", + deviceName: "test-disk", instance: &compute.Instance{ Disks: []*compute.AttachedDisk{ { @@ -639,7 +635,7 @@ func TestDiskIsAttached(t *testing.T) { } for _, tc := range testCases { t.Logf("test case: %s", tc.name) - if attached := diskIsAttached(gce.ZonalCloudDisk(tc.disk), tc.instance); attached != tc.expAttached { + if attached := diskIsAttached(tc.deviceName, tc.instance); attached != tc.expAttached { t.Errorf("Expected disk attached to be %v, but got %v", tc.expAttached, attached) } } @@ -648,17 +644,15 @@ func TestDiskIsAttached(t *testing.T) { func TestDiskIsAttachedAndCompatible(t *testing.T) { testCases := []struct { name string - disk *compute.Disk + deviceName string instance *compute.Instance mode string expAttached bool expErr bool }{ { - name: "normal-attached", - disk: &compute.Disk{ - Name: "test-disk", - }, + name: "normal-attached", + deviceName: "test-disk", instance: &compute.Instance{ Disks: []*compute.AttachedDisk{ { @@ -671,10 +665,8 @@ func TestDiskIsAttachedAndCompatible(t *testing.T) { expAttached: true, }, { - name: "normal-not-attached", - disk: &compute.Disk{ - Name: "test-disk", - }, + name: "normal-not-attached", + deviceName: "test-disk", instance: &compute.Instance{ Disks: []*compute.AttachedDisk{ { @@ -687,10 +679,8 @@ func TestDiskIsAttachedAndCompatible(t *testing.T) { expAttached: false, }, { - name: "incompatible mode", - disk: &compute.Disk{ - Name: "test-disk", - }, + name: "incompatible mode", + deviceName: "test-disk", instance: &compute.Instance{ Disks: []*compute.AttachedDisk{ { @@ -706,7 +696,7 @@ func TestDiskIsAttachedAndCompatible(t *testing.T) { } for _, tc := range testCases { t.Logf("test case: %s", tc.name) - attached, err := diskIsAttachedAndCompatible(gce.ZonalCloudDisk(tc.disk), tc.instance, nil, tc.mode) + attached, err := diskIsAttachedAndCompatible(tc.deviceName, tc.instance, nil, tc.mode) if err != nil && !tc.expErr { t.Errorf("Did not expect error but got: %v", err) } diff --git a/pkg/gce-pd-csi-driver/node.go b/pkg/gce-pd-csi-driver/node.go index bcdd923bc..728752b34 100644 --- a/pkg/gce-pd-csi-driver/node.go +++ b/pkg/gce-pd-csi-driver/node.go @@ -177,7 +177,12 @@ func (ns *GCENodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStage // TODO(#83): Get real partitions partition := "" - devicePaths := ns.DeviceUtils.GetDiskByIdPaths(volumeKey.Name, partition) + deviceName, err := common.GetDeviceName(volumeKey) + if err != nil { + status.Error(codes.Internal, fmt.Sprintf("error getting device name: %v", err)) + } + + devicePaths := ns.DeviceUtils.GetDiskByIdPaths(deviceName, partition) devicePath, err := ns.DeviceUtils.VerifyDevicePath(devicePaths) if err != nil { diff --git a/pkg/mount-manager/device-utils.go b/pkg/mount-manager/device-utils.go index fd8f44706..fcd67ca49 100644 --- a/pkg/mount-manager/device-utils.go +++ b/pkg/mount-manager/device-utils.go @@ -54,7 +54,7 @@ const ( type DeviceUtils interface { // GetDiskByIdPaths returns a list of all possible paths for a // given Persistent Disk - GetDiskByIdPaths(pdName string, partition string) []string + GetDiskByIdPaths(deviceName string, partition string) []string // VerifyDevicePath returns the first of the list of device paths that // exists on the machine, or an empty string if none exists @@ -71,12 +71,12 @@ func NewDeviceUtils() *deviceUtils { } // Returns list of all /dev/disk/by-id/* paths for given PD. -func (m *deviceUtils) GetDiskByIdPaths(pdName string, partition string) []string { +func (m *deviceUtils) GetDiskByIdPaths(deviceName string, partition string) []string { devicePaths := []string{ - path.Join(diskByIdPath, diskGooglePrefix+pdName), - path.Join(diskByIdPath, diskScsiGooglePrefix+pdName), - path.Join(diskByHostIdPath, diskScsiGooglePrefix+pdName), - path.Join(diskByHostIdPath, diskScsiGooglePrefix+pdName), + path.Join(diskByIdPath, diskGooglePrefix+deviceName), + path.Join(diskByIdPath, diskScsiGooglePrefix+deviceName), + path.Join(diskByHostIdPath, diskScsiGooglePrefix+deviceName), + path.Join(diskByHostIdPath, diskScsiGooglePrefix+deviceName), } if partition != "" { diff --git a/test/run-e2e-local.sh b/test/run-e2e-local.sh index 1ed3546f8..b9ef10e0a 100755 --- a/test/run-e2e-local.sh +++ b/test/run-e2e-local.sh @@ -1,9 +1,8 @@ #!/bin/bash -#set -o nounset +set -o nounset set -o errexit readonly PKGDIR=sigs.k8s.io/gcp-compute-persistent-disk-csi-driver - ginkgo -v "test/e2e/tests" --logtostderr -- --project ${PROJECT} --service-account ${IAM_NAME} \ No newline at end of file