Skip to content

Commit a062e89

Browse files
sjswerdlowsjswerdlow
authored
Changes remove the API version concept from GetDisk and InsertDisk, now just defaults to using the Beta API. (#5)
1 parent 81ce104 commit a062e89

File tree

4 files changed

+51
-90
lines changed

4 files changed

+51
-90
lines changed

‎pkg/gce-cloud-provider/compute/fake-gce.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ func (cloud *FakeCloudProvider) ListSnapshots(ctx context.Context, filter string
184184
}
185185

186186
// Disk Methods
187-
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, project string, volKey *meta.Key, api GCEAPIVersion) (*CloudDisk, error) {
187+
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, project string, volKey *meta.Key) (*CloudDisk, error) {
188188
disk, ok := cloud.disks[volKey.String()]
189189
if !ok {
190190
return nil, notFoundError()

‎pkg/gce-cloud-provider/compute/gce-compute.go

+34-73
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ type GCECompute interface {
9999
GetDefaultProject() string
100100
GetDefaultZone() string
101101
// Disk Methods
102-
GetDisk(ctx context.Context, project string, volumeKey *meta.Key, gceAPIVersion GCEAPIVersion) (*CloudDisk, error)
102+
GetDisk(ctx context.Context, project string, volumeKey *meta.Key) (*CloudDisk, error)
103103
RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (string, *meta.Key, error)
104104
ValidateExistingDisk(ctx context.Context, disk *CloudDisk, params common.DiskParameters, reqBytes, limBytes int64, multiWriter bool) error
105105
InsertDisk(ctx context.Context, project string, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, volumeContentSourceVolumeID string, multiWriter bool, accessMode string) error
@@ -321,26 +321,16 @@ func (cloud *CloudProvider) ListSnapshots(ctx context.Context, filter string) ([
321321
return items, "", nil
322322
}
323323

324-
func (cloud *CloudProvider) GetDisk(ctx context.Context, project string, key *meta.Key, gceAPIVersion GCEAPIVersion) (*CloudDisk, error) {
324+
func (cloud *CloudProvider) GetDisk(ctx context.Context, project string, key *meta.Key) (*CloudDisk, error) {
325325
klog.V(5).Infof("Getting disk %v", key)
326326

327327
switch key.Type() {
328328
case meta.Zonal:
329-
if gceAPIVersion == GCEAPIVersionBeta {
330-
disk, err := cloud.getZonalBetaDiskOrError(ctx, project, key.Zone, key.Name)
331-
return CloudDiskFromBeta(disk), err
332-
} else {
333-
disk, err := cloud.getZonalDiskOrError(ctx, project, key.Zone, key.Name)
334-
return CloudDiskFromV1(disk), err
335-
}
329+
disk, err := cloud.getZonalBetaDiskOrError(ctx, project, key.Zone, key.Name)
330+
return CloudDiskFromBeta(disk), err
336331
case meta.Regional:
337-
if gceAPIVersion == GCEAPIVersionBeta {
338-
disk, err := cloud.getRegionalBetaDiskOrError(ctx, project, key.Region, key.Name)
339-
return CloudDiskFromBeta(disk), err
340-
} else {
341-
disk, err := cloud.getRegionalDiskOrError(ctx, project, key.Region, key.Name)
342-
return CloudDiskFromV1(disk), err
343-
}
332+
disk, err := cloud.getRegionalBetaDiskOrError(ctx, project, key.Region, key.Name)
333+
return CloudDiskFromBeta(disk), err
344334
default:
345335
return nil, fmt.Errorf("key was neither zonal nor regional, got: %v", key.String())
346336
}
@@ -633,17 +623,11 @@ func (cloud *CloudProvider) insertRegionalDisk(
633623
description string,
634624
multiWriter bool) error {
635625
var (
636-
err error
637-
opName string
638-
gceAPIVersion = GCEAPIVersionV1
626+
err error
627+
opName string
639628
)
640629

641-
// Use beta API for non-hyperdisk types in multi-writer mode.
642-
if multiWriter && !strings.Contains(params.DiskType, "hyperdisk") {
643-
gceAPIVersion = GCEAPIVersionBeta
644-
}
645-
646-
diskToCreate := &computev1.Disk{
630+
diskToCreate := &computebeta.Disk{
647631
Name: volKey.Name,
648632
SizeGb: common.BytesToGbRoundUp(capBytes),
649633
Description: description,
@@ -672,7 +656,7 @@ func (cloud *CloudProvider) insertRegionalDisk(
672656
diskToCreate.ReplicaZones = replicaZones
673657
}
674658
if params.DiskEncryptionKMSKey != "" {
675-
diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
659+
diskToCreate.DiskEncryptionKey = &computebeta.CustomerEncryptionKey{
676660
KmsKeyName: params.DiskEncryptionKMSKey,
677661
}
678662
}
@@ -682,29 +666,21 @@ func (cloud *CloudProvider) insertRegionalDisk(
682666
}
683667

684668
if len(resourceTags) > 0 {
685-
diskToCreate.Params = &computev1.DiskParams{
669+
diskToCreate.Params = &computebeta.DiskParams{
686670
ResourceManagerTags: resourceTags,
687671
}
688672
}
689673

690-
if gceAPIVersion == GCEAPIVersionBeta {
691-
var insertOp *computebeta.Operation
692-
betaDiskToCreate := convertV1DiskToBetaDisk(diskToCreate)
693-
betaDiskToCreate.MultiWriter = multiWriter
694-
insertOp, err = cloud.betaService.RegionDisks.Insert(project, volKey.Region, betaDiskToCreate).Context(ctx).Do()
695-
if insertOp != nil {
696-
opName = insertOp.Name
697-
}
698-
} else {
699-
var insertOp *computev1.Operation
700-
insertOp, err = cloud.service.RegionDisks.Insert(project, volKey.Region, diskToCreate).Context(ctx).Do()
701-
if insertOp != nil {
702-
opName = insertOp.Name
703-
}
674+
var insertOp *computebeta.Operation
675+
diskToCreate.MultiWriter = multiWriter
676+
insertOp, err = cloud.betaService.RegionDisks.Insert(project, volKey.Region, diskToCreate).Context(ctx).Do()
677+
if insertOp != nil {
678+
opName = insertOp.Name
704679
}
680+
705681
if err != nil {
706682
if IsGCEError(err, "alreadyExists") {
707-
disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
683+
disk, err := cloud.GetDisk(ctx, project, volKey)
708684
if err != nil {
709685
// failed to GetDisk, however the Disk may already exist
710686
// the error code should be non-Final
@@ -730,7 +706,7 @@ func (cloud *CloudProvider) insertRegionalDisk(
730706
// the error code returned should be non-final
731707
if err != nil {
732708
if IsGCEError(err, "alreadyExists") {
733-
disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
709+
disk, err := cloud.GetDisk(ctx, project, volKey)
734710
if err != nil {
735711
return common.NewTemporaryError(codes.Unavailable, fmt.Errorf("error when getting disk: %w", err))
736712
}
@@ -762,17 +738,11 @@ func (cloud *CloudProvider) insertZonalDisk(
762738
multiWriter bool,
763739
accessMode string) error {
764740
var (
765-
err error
766-
opName string
767-
gceAPIVersion = GCEAPIVersionV1
741+
err error
742+
opName string
768743
)
769744

770-
// Use beta API for non-hyperdisk types in multi-writer mode.
771-
if multiWriter && !strings.Contains(params.DiskType, "hyperdisk") {
772-
gceAPIVersion = GCEAPIVersionBeta
773-
}
774-
775-
diskToCreate := &computev1.Disk{
745+
diskToCreate := &computebeta.Disk{
776746
Name: volKey.Name,
777747
SizeGb: common.BytesToGbRoundUp(capBytes),
778748
Description: description,
@@ -814,7 +784,7 @@ func (cloud *CloudProvider) insertZonalDisk(
814784
}
815785

816786
if params.DiskEncryptionKMSKey != "" {
817-
diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
787+
diskToCreate.DiskEncryptionKey = &computebeta.CustomerEncryptionKey{
818788
KmsKeyName: params.DiskEncryptionKMSKey,
819789
}
820790
}
@@ -826,31 +796,22 @@ func (cloud *CloudProvider) insertZonalDisk(
826796
}
827797

828798
if len(resourceTags) > 0 {
829-
diskToCreate.Params = &computev1.DiskParams{
799+
diskToCreate.Params = &computebeta.DiskParams{
830800
ResourceManagerTags: resourceTags,
831801
}
832802
}
833-
diskToCreate.AccessMode = accessMode
834803

835-
if gceAPIVersion == GCEAPIVersionBeta {
836-
var insertOp *computebeta.Operation
837-
betaDiskToCreate := convertV1DiskToBetaDisk(diskToCreate)
838-
betaDiskToCreate.MultiWriter = multiWriter
839-
insertOp, err = cloud.betaService.Disks.Insert(project, volKey.Zone, betaDiskToCreate).Context(ctx).Do()
840-
if insertOp != nil {
841-
opName = insertOp.Name
842-
}
843-
} else {
844-
var insertOp *computev1.Operation
845-
insertOp, err = cloud.service.Disks.Insert(project, volKey.Zone, diskToCreate).Context(ctx).Do()
846-
if insertOp != nil {
847-
opName = insertOp.Name
848-
}
804+
diskToCreate.AccessMode = accessMode
805+
var insertOp *computebeta.Operation
806+
diskToCreate.MultiWriter = multiWriter
807+
insertOp, err = cloud.betaService.Disks.Insert(project, volKey.Zone, diskToCreate).Context(ctx).Do()
808+
if insertOp != nil {
809+
opName = insertOp.Name
849810
}
850811

851812
if err != nil {
852813
if IsGCEError(err, "alreadyExists") {
853-
disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
814+
disk, err := cloud.GetDisk(ctx, project, volKey)
854815
if err != nil {
855816
// failed to GetDisk, however the Disk may already exist
856817
// the error code should be non-Final
@@ -877,7 +838,7 @@ func (cloud *CloudProvider) insertZonalDisk(
877838
// failed to wait for Op to finish, however, the Op possibly is still running as expected
878839
// the error code returned should be non-final
879840
if IsGCEError(err, "alreadyExists") {
880-
disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
841+
disk, err := cloud.GetDisk(ctx, project, volKey)
881842
if err != nil {
882843
return common.NewTemporaryError(codes.Unavailable, fmt.Errorf("error when getting disk: %w", err))
883844
}
@@ -1176,7 +1137,7 @@ func (cloud *CloudProvider) waitForAttachOnDisk(ctx context.Context, project str
11761137
start := time.Now()
11771138
return wait.ExponentialBackoff(AttachDiskBackoff, func() (bool, error) {
11781139
klog.V(6).Infof("Polling disks.get for attach of disk %v to instance %v to complete for %v", volKey.Name, instanceName, time.Since(start))
1179-
disk, err := cloud.GetDisk(ctx, project, volKey, GCEAPIVersionV1)
1140+
disk, err := cloud.GetDisk(ctx, project, volKey)
11801141
if err != nil {
11811142
return false, fmt.Errorf("GetDisk failed to get disk: %w", err)
11821143
}
@@ -1426,7 +1387,7 @@ func (cloud *CloudProvider) DeleteImage(ctx context.Context, project, imageName
14261387
// k8s.io/apimachinery/quantity package for better size handling
14271388
func (cloud *CloudProvider) ResizeDisk(ctx context.Context, project string, volKey *meta.Key, requestBytes int64) (int64, error) {
14281389
klog.V(5).Infof("Resizing disk %v to size %v", volKey, requestBytes)
1429-
cloudDisk, err := cloud.GetDisk(ctx, project, volKey, GCEAPIVersionV1)
1390+
cloudDisk, err := cloud.GetDisk(ctx, project, volKey)
14301391
if err != nil {
14311392
return -1, fmt.Errorf("failed to get disk: %w", err)
14321393
}

‎pkg/gce-pd-csi-driver/controller.go

+12-12
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ func (gceCS *GCEControllerServer) createSingleDisk(ctx context.Context, req *csi
602602
}
603603

604604
// Validate if disk already exists
605-
existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, gceCS.CloudProvider.GetDefaultProject(), volKey, getGCEApiVersion(multiWriter))
605+
existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, gceCS.CloudProvider.GetDefaultProject(), volKey)
606606
if err != nil {
607607
if !gce.IsGCEError(err, "notFound") {
608608
// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
@@ -657,7 +657,7 @@ func (gceCS *GCEControllerServer) createSingleDisk(ctx context.Context, req *csi
657657
}
658658

659659
// Verify that the volume in VolumeContentSource exists.
660-
diskFromSourceVolume, err := gceCS.CloudProvider.GetDisk(ctx, project, sourceVolKey, getGCEApiVersion(multiWriter))
660+
diskFromSourceVolume, err := gceCS.CloudProvider.GetDisk(ctx, project, sourceVolKey)
661661
if err != nil {
662662
if gce.IsGCEError(err, "notFound") {
663663
return nil, status.Errorf(codes.NotFound, "CreateVolume source volume %s does not exist", volumeContentSourceVolumeID)
@@ -787,7 +787,7 @@ func (gceCS *GCEControllerServer) ControllerModifyVolume(ctx context.Context, re
787787
}
788788
klog.V(4).Infof("Modify Volume Parameters for %s: %v", volumeID, volumeModifyParams)
789789

790-
existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionBeta)
790+
existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
791791
metrics.UpdateRequestMetadataFromDisk(ctx, existingDisk)
792792

793793
if err != nil {
@@ -883,7 +883,7 @@ func (gceCS *GCEControllerServer) deleteMultiZoneDisk(ctx context.Context, req *
883883
Region: volKey.Region,
884884
Zone: zone,
885885
}
886-
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, zonalVolKey, gce.GCEAPIVersionV1)
886+
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, zonalVolKey)
887887
// TODO: Consolidate the parameters here, rather than taking the last.
888888
metrics.UpdateRequestMetadataFromDisk(ctx, disk)
889889
err := gceCS.CloudProvider.DeleteDisk(ctx, project, zonalVolKey)
@@ -916,7 +916,7 @@ func (gceCS *GCEControllerServer) deleteSingleDeviceDisk(ctx context.Context, re
916916
return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, volumeID)
917917
}
918918
defer gceCS.volumeLocks.Release(volumeID)
919-
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
919+
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
920920
metrics.UpdateRequestMetadataFromDisk(ctx, disk)
921921
err = gceCS.CloudProvider.DeleteDisk(ctx, project, volKey)
922922
if err != nil {
@@ -1085,7 +1085,7 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con
10851085
return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, lockingVolumeID), nil
10861086
}
10871087
defer gceCS.volumeLocks.Release(lockingVolumeID)
1088-
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1088+
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
10891089
if err != nil {
10901090
if gce.IsGCENotFoundError(err) {
10911091
return nil, status.Errorf(codes.NotFound, "Could not find disk %v: %v", volKey.String(), err.Error()), disk
@@ -1231,7 +1231,7 @@ func (gceCS *GCEControllerServer) executeControllerUnpublishVolume(ctx context.C
12311231
return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, lockingVolumeID), nil
12321232
}
12331233
defer gceCS.volumeLocks.Release(lockingVolumeID)
1234-
diskToUnpublish, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1234+
diskToUnpublish, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
12351235
instance, err := gceCS.CloudProvider.GetInstanceOrError(ctx, instanceZone, instanceName)
12361236
if err != nil {
12371237
if gce.IsGCENotFoundError(err) {
@@ -1298,7 +1298,7 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
12981298
}
12991299
defer gceCS.volumeLocks.Release(volumeID)
13001300

1301-
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1301+
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
13021302
metrics.UpdateRequestMetadataFromDisk(ctx, disk)
13031303
if err != nil {
13041304
if gce.IsGCENotFoundError(err) {
@@ -1539,7 +1539,7 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
15391539
defer gceCS.volumeLocks.Release(volumeID)
15401540

15411541
// Check if volume exists
1542-
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1542+
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
15431543
metrics.UpdateRequestMetadataFromDisk(ctx, disk)
15441544
if err != nil {
15451545
if gce.IsGCENotFoundError(err) {
@@ -1881,7 +1881,7 @@ func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, re
18811881
return nil, status.Errorf(codes.InvalidArgument, "ControllerExpandVolume is not supported with the multi-zone PVC volumeHandle feature. Please re-create the volume %v from source if you want a larger size", volumeID)
18821882
}
18831883

1884-
sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1884+
sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
18851885
metrics.UpdateRequestMetadataFromDisk(ctx, sourceDisk)
18861886
resizedGb, err := gceCS.CloudProvider.ResizeDisk(ctx, project, volKey, reqBytes)
18871887

@@ -2437,7 +2437,7 @@ func createRegionalDisk(ctx context.Context, cloudProvider gce.GCECompute, name
24372437
gceAPIVersion = gce.GCEAPIVersionBeta
24382438
}
24392439
// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
2440-
disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region), gceAPIVersion)
2440+
disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region))
24412441
if err != nil {
24422442
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("failed to get disk after creating regional disk: %w", err))
24432443
}
@@ -2460,7 +2460,7 @@ func createSingleZoneDisk(ctx context.Context, cloudProvider gce.GCECompute, nam
24602460
gceAPIVersion = gce.GCEAPIVersionBeta
24612461
}
24622462
// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
2463-
disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone), gceAPIVersion)
2463+
disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone))
24642464
if err != nil {
24652465
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("failed to get disk after creating zonal disk: %w", err))
24662466
}

‎pkg/gce-pd-csi-driver/controller_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1565,7 +1565,7 @@ func TestMultiZoneVolumeCreation(t *testing.T) {
15651565

15661566
for _, zone := range tc.expZones {
15671567
volumeKey := meta.ZonalKey(name, zone)
1568-
disk, err := fcp.GetDisk(context.Background(), project, volumeKey, gce.GCEAPIVersionBeta)
1568+
disk, err := fcp.GetDisk(context.Background(), project, volumeKey)
15691569
if err != nil {
15701570
t.Fatalf("Get Disk failed for created disk with error: %v", err)
15711571
}
@@ -1859,7 +1859,7 @@ func TestCreateVolumeWithVolumeAttributeClassParameters(t *testing.T) {
18591859
t.Fatalf("Failed to convert volume id to key: %v", err)
18601860
}
18611861

1862-
disk, err := fcp.GetDisk(context.Background(), project, volumeKey, gce.GCEAPIVersionBeta)
1862+
disk, err := fcp.GetDisk(context.Background(), project, volumeKey)
18631863

18641864
if err != nil {
18651865
t.Fatalf("Failed to get disk: %v", err)
@@ -1964,7 +1964,7 @@ func TestVolumeModifyOperation(t *testing.T) {
19641964
}
19651965
}
19661966

1967-
modifiedVol, err := fcp.GetDisk(context.Background(), project, volKey, gce.GCEAPIVersionBeta)
1967+
modifiedVol, err := fcp.GetDisk(context.Background(), project, volKey)
19681968

19691969
if err != nil {
19701970
t.Errorf("Failed to get volume: %v", err)
@@ -5241,7 +5241,7 @@ func TestCreateConfidentialVolume(t *testing.T) {
52415241

52425242
volumeId := resp.GetVolume().VolumeId
52435243
project, volumeKey, err := common.VolumeIDToKey(volumeId)
5244-
createdDisk, err := fcp.GetDisk(context.Background(), project, volumeKey, gce.GCEAPIVersionBeta)
5244+
createdDisk, err := fcp.GetDisk(context.Background(), project, volumeKey)
52455245
if err != nil {
52465246
t.Fatalf("Get Disk failed for created disk with error: %v", err)
52475247
}

0 commit comments

Comments
 (0)