Skip to content

Commit 7d12054

Browse files
committed
change GetDisk error reporting to temporary in CreateVolume codepath
1 parent 57fc063 commit 7d12054

File tree

2 files changed

+25
-14
lines changed

2 files changed

+25
-14
lines changed

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

+15-7
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,9 @@ func (cloud *CloudProvider) insertRegionalDisk(
560560
if IsGCEError(err, "alreadyExists") {
561561
disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
562562
if err != nil {
563-
return err
563+
// failed to GetDisk, however the Disk may already exist
564+
// the error code should be non-Final
565+
return status.Error(codes.Unavailable, err.Error())
564566
}
565567
err = cloud.ValidateExistingDisk(ctx, disk, params,
566568
int64(capacityRange.GetRequiredBytes()),
@@ -572,16 +574,18 @@ func (cloud *CloudProvider) insertRegionalDisk(
572574
klog.Warningf("GCE PD %s already exists, reusing", volKey.Name)
573575
return nil
574576
}
575-
return status.Error(codes.Internal, fmt.Sprintf("unknown Insert disk error: %v", err.Error()))
577+
return fmt.Errorf("unknown Insert Regional disk error: %w", err)
576578
}
577579
klog.V(5).Infof("InsertDisk operation %s for disk %s", opName, diskToCreate.Name)
578580

579581
err = cloud.waitForRegionalOp(ctx, project, opName, volKey.Region)
582+
// failed to wait for Op to finish, however, the Op possibly is still running as expected
583+
// the error code returned should be non-final
580584
if err != nil {
581585
if IsGCEError(err, "alreadyExists") {
582586
disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
583587
if err != nil {
584-
return err
588+
return status.Errorf(codes.Unavailable, "error when getting disk: %v", err.Error())
585589
}
586590
err = cloud.ValidateExistingDisk(ctx, disk, params,
587591
int64(capacityRange.GetRequiredBytes()),
@@ -593,7 +597,7 @@ func (cloud *CloudProvider) insertRegionalDisk(
593597
klog.Warningf("GCE PD %s already exists after wait, reusing", volKey.Name)
594598
return nil
595599
}
596-
return fmt.Errorf("unknown Insert disk operation error: %w", err)
600+
return status.Errorf(codes.Unavailable, "unknown error when polling the operation: %v", err.Error())
597601
}
598602
return nil
599603
}
@@ -695,7 +699,9 @@ func (cloud *CloudProvider) insertZonalDisk(
695699
if IsGCEError(err, "alreadyExists") {
696700
disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
697701
if err != nil {
698-
return err
702+
// failed to GetDisk, however the Disk may already exist
703+
// the error code should be non-Final
704+
return status.Error(codes.Unavailable, err.Error())
699705
}
700706
err = cloud.ValidateExistingDisk(ctx, disk, params,
701707
int64(capacityRange.GetRequiredBytes()),
@@ -714,10 +720,12 @@ func (cloud *CloudProvider) insertZonalDisk(
714720
err = cloud.waitForZonalOp(ctx, project, opName, volKey.Zone)
715721

716722
if err != nil {
723+
// failed to wait for Op to finish, however, the Op possibly is still running as expected
724+
// the error code returned should be non-final
717725
if IsGCEError(err, "alreadyExists") {
718726
disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion)
719727
if err != nil {
720-
return err
728+
return status.Errorf(codes.Unavailable, "error when getting disk: %v", err.Error())
721729
}
722730
err = cloud.ValidateExistingDisk(ctx, disk, params,
723731
int64(capacityRange.GetRequiredBytes()),
@@ -729,7 +737,7 @@ func (cloud *CloudProvider) insertZonalDisk(
729737
klog.Warningf("GCE PD %s already exists after wait, reusing", volKey.Name)
730738
return nil
731739
}
732-
return fmt.Errorf("unknown Insert disk operation error: %w", err)
740+
return status.Errorf(codes.Unavailable, "unknown error when polling the operation: %v", err.Error())
733741
}
734742
return nil
735743
}

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

+10-7
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,8 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
324324
existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, gceCS.CloudProvider.GetDefaultProject(), volKey, gceAPIVersion)
325325
if err != nil {
326326
if !gce.IsGCEError(err, "notFound") {
327-
return nil, common.LoggedError("CreateVolume, failed to getDisk when validating: ", err)
327+
// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
328+
return nil, common.LoggedError("CreateVolume, failed to getDisk when validating: ", status.Error(codes.Unavailable, err.Error()))
328329
}
329330
}
330331
if err == nil {
@@ -339,10 +340,10 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
339340

340341
ready, err := isDiskReady(existingDisk)
341342
if err != nil {
342-
return nil, common.LoggedError("CreateVolume disk "+volKey.String()+" had error checking ready status: ", err)
343+
return nil, status.Errorf(codes.Aborted, "CreateVolume disk %q had error checking ready status: %v", volKey.String(), err.Error())
343344
}
344345
if !ready {
345-
return nil, status.Errorf(codes.Internal, "CreateVolume existing disk %v is not ready", volKey)
346+
return nil, status.Errorf(codes.Aborted, "CreateVolume existing disk %v is not ready", volKey)
346347
}
347348

348349
// If there is no validation error, immediately return success
@@ -417,10 +418,10 @@ func (gceCS *GCEControllerServer) CreateVolume(ctx context.Context, req *csi.Cre
417418
// Verify the source disk is ready.
418419
ready, err := isDiskReady(diskFromSourceVolume)
419420
if err != nil {
420-
return nil, common.LoggedError("CreateVolume disk from source volume "+sourceVolKey.String()+" had error checking ready status: ", err)
421+
return nil, status.Errorf(codes.Aborted, "CreateVolume disk from source volume %q had error checking ready status: %v", sourceVolKey.String(), err.Error())
421422
}
422423
if !ready {
423-
return nil, status.Errorf(codes.Internal, "CreateVolume disk from source volume %v is not ready", sourceVolKey)
424+
return nil, status.Errorf(codes.Aborted, "CreateVolume disk from source volume %v is not ready", sourceVolKey)
424425
}
425426
}
426427
} else { // if VolumeContentSource is nil, validate access mode is not read only
@@ -1876,9 +1877,10 @@ func createRegionalDisk(ctx context.Context, cloudProvider gce.GCECompute, name
18761877
if multiWriter {
18771878
gceAPIVersion = gce.GCEAPIVersionBeta
18781879
}
1880+
// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
18791881
disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region), gceAPIVersion)
18801882
if err != nil {
1881-
return nil, fmt.Errorf("failed to get disk after creating regional disk: %w", err)
1883+
return nil, status.Errorf(codes.Unavailable, "failed to get disk after creating regional disk: %v", err.Error())
18821884
}
18831885
return disk, nil
18841886
}
@@ -1898,9 +1900,10 @@ func createSingleZoneDisk(ctx context.Context, cloudProvider gce.GCECompute, nam
18981900
if multiWriter {
18991901
gceAPIVersion = gce.GCEAPIVersionBeta
19001902
}
1903+
// failed to GetDisk, however the Disk may already be created, the error code should be non-Final
19011904
disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone), gceAPIVersion)
19021905
if err != nil {
1903-
return nil, err
1906+
return nil, status.Errorf(codes.Unavailable, "failed to get disk after creating zonal disk: %v", err.Error())
19041907
}
19051908
return disk, nil
19061909
}

0 commit comments

Comments
 (0)