Skip to content

Commit 1b8504c

Browse files
sjswerdlowtonyzhc
sjswerdlow
authored and
tonyzhc
committed
Changes remove the API version concept from GetDisk and InsertDisk, now just defaults to using the Beta API.
1 parent 110d04c commit 1b8504c

File tree

4 files changed

+51
-90
lines changed

4 files changed

+51
-90
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func (cloud *FakeCloudProvider) ListSnapshots(ctx context.Context, filter string
188188
}
189189

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

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

Lines changed: 34 additions & 73 deletions
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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ func (gceCS *GCEControllerServer) createSingleDisk(ctx context.Context, req *csi
604604
}
605605

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

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

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

794794
if err != nil {
@@ -884,7 +884,7 @@ func (gceCS *GCEControllerServer) deleteMultiZoneDisk(ctx context.Context, req *
884884
Region: volKey.Region,
885885
Zone: zone,
886886
}
887-
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, zonalVolKey, gce.GCEAPIVersionV1)
887+
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, zonalVolKey)
888888
// TODO: Consolidate the parameters here, rather than taking the last.
889889
metrics.UpdateRequestMetadataFromDisk(ctx, disk)
890890
err := gceCS.CloudProvider.DeleteDisk(ctx, project, zonalVolKey)
@@ -917,7 +917,7 @@ func (gceCS *GCEControllerServer) deleteSingleDeviceDisk(ctx context.Context, re
917917
return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, volumeID)
918918
}
919919
defer gceCS.volumeLocks.Release(volumeID)
920-
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
920+
disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
921921
metrics.UpdateRequestMetadataFromDisk(ctx, disk)
922922
err = gceCS.CloudProvider.DeleteDisk(ctx, project, volKey)
923923
if err != nil {
@@ -1086,7 +1086,7 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con
10861086
return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, lockingVolumeID), nil
10871087
}
10881088
defer gceCS.volumeLocks.Release(lockingVolumeID)
1089-
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1089+
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
10901090
if err != nil {
10911091
if gce.IsGCENotFoundError(err) {
10921092
return nil, status.Errorf(codes.NotFound, "Could not find disk %v: %v", volKey.String(), err.Error()), disk
@@ -1232,7 +1232,7 @@ func (gceCS *GCEControllerServer) executeControllerUnpublishVolume(ctx context.C
12321232
return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, lockingVolumeID), nil
12331233
}
12341234
defer gceCS.volumeLocks.Release(lockingVolumeID)
1235-
diskToUnpublish, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1235+
diskToUnpublish, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
12361236
instance, err := gceCS.CloudProvider.GetInstanceOrError(ctx, instanceZone, instanceName)
12371237
if err != nil {
12381238
if gce.IsGCENotFoundError(err) {
@@ -1300,7 +1300,7 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context
13001300
}
13011301
defer gceCS.volumeLocks.Release(volumeID)
13021302

1303-
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1303+
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
13041304
metrics.UpdateRequestMetadataFromDisk(ctx, disk)
13051305
if err != nil {
13061306
if gce.IsGCENotFoundError(err) {
@@ -1541,7 +1541,7 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C
15411541
defer gceCS.volumeLocks.Release(volumeID)
15421542

15431543
// Check if volume exists
1544-
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1544+
disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
15451545
metrics.UpdateRequestMetadataFromDisk(ctx, disk)
15461546
if err != nil {
15471547
if gce.IsGCENotFoundError(err) {
@@ -1890,7 +1890,7 @@ func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, re
18901890
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)
18911891
}
18921892

1893-
sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1)
1893+
sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey)
18941894
metrics.UpdateRequestMetadataFromDisk(ctx, sourceDisk)
18951895
resizedGb, err := gceCS.CloudProvider.ResizeDisk(ctx, project, volKey, reqBytes)
18961896

@@ -2445,7 +2445,7 @@ func createRegionalDisk(ctx context.Context, cloudProvider gce.GCECompute, name
24452445
gceAPIVersion = gce.GCEAPIVersionBeta
24462446
}
24472447
// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
2448-
disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region), gceAPIVersion)
2448+
disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region))
24492449
if err != nil {
24502450
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("failed to get disk after creating regional disk: %w", err))
24512451
}
@@ -2468,7 +2468,7 @@ func createSingleZoneDisk(ctx context.Context, cloudProvider gce.GCECompute, nam
24682468
gceAPIVersion = gce.GCEAPIVersionBeta
24692469
}
24702470
// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
2471-
disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone), gceAPIVersion)
2471+
disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone))
24722472
if err != nil {
24732473
return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("failed to get disk after creating zonal disk: %w", err))
24742474
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1702,7 +1702,7 @@ func TestMultiZoneVolumeCreation(t *testing.T) {
17021702

17031703
for _, zone := range tc.expZones {
17041704
volumeKey := meta.ZonalKey(name, zone)
1705-
disk, err := fcp.GetDisk(context.Background(), project, volumeKey, gce.GCEAPIVersionBeta)
1705+
disk, err := fcp.GetDisk(context.Background(), project, volumeKey)
17061706
if err != nil {
17071707
t.Fatalf("Get Disk failed for created disk with error: %v", err)
17081708
}
@@ -1996,7 +1996,7 @@ func TestCreateVolumeWithVolumeAttributeClassParameters(t *testing.T) {
19961996
t.Fatalf("Failed to convert volume id to key: %v", err)
19971997
}
19981998

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

20012001
if err != nil {
20022002
t.Fatalf("Failed to get disk: %v", err)
@@ -2101,7 +2101,7 @@ func TestVolumeModifyOperation(t *testing.T) {
21012101
}
21022102
}
21032103

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

21062106
if err != nil {
21072107
t.Errorf("Failed to get volume: %v", err)
@@ -5378,7 +5378,7 @@ func TestCreateConfidentialVolume(t *testing.T) {
53785378

53795379
volumeId := resp.GetVolume().VolumeId
53805380
project, volumeKey, err := common.VolumeIDToKey(volumeId)
5381-
createdDisk, err := fcp.GetDisk(context.Background(), project, volumeKey, gce.GCEAPIVersionBeta)
5381+
createdDisk, err := fcp.GetDisk(context.Background(), project, volumeKey)
53825382
if err != nil {
53835383
t.Fatalf("Get Disk failed for created disk with error: %v", err)
53845384
}

0 commit comments

Comments
 (0)