CON-9080 Remove checkLimit validation in favor of forbidden response …

…when creating a volume (#481)

CHANGELOG.md

@@ -1,7 +1,9 @@
 ## unreleased
 
+* Validate CreateVolume limits with forbidden error response
+  [[GH-481]](https://github.com/digitalocean/csi-digitalocean/pull/481)
 * CSI driver can accidentally format existing volume
-  [[GH-469]](https://github.com/digitalocean/csi-digitalocean/pull/478)
+  [[GH-478]](https://github.com/digitalocean/csi-digitalocean/pull/478)
 
 ## v4.5.0 - 2023.01.11
 * Update CSI driver for Kubernetes 1.26

driver/controller.go

@@ -181,20 +181,13 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		volumeReq.SnapshotID = snapshotID
 	}
 
-	log.Info("checking volume limit")
-	details, err := d.checkLimit(ctx)
-	if err != nil {
-		return nil, status.Errorf(codes.Internal, "failed to check volume limit: %s", err)
-	}
-	if details != nil {
-		return nil, status.Errorf(codes.ResourceExhausted,
-			"volume limit (%d) has been reached. Current number of volumes: %d. Please contact support.",
-			details.limit, details.numVolumes)
-	}
-
 	log.WithField("volume_req", volumeReq).Info("creating volume")
-	vol, _, err := d.storage.CreateVolume(ctx, volumeReq)
+	vol, cvResp, err := d.storage.CreateVolume(ctx, volumeReq)
 	if err != nil {
+		if cvResp != nil && cvResp.StatusCode == http.StatusForbidden && strings.Contains(err.Error(), "capacity limit exceeded") {
+			return nil, status.Errorf(codes.ResourceExhausted, "volume limit has been reached. Please contact support")
+		}
+
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 

driver/controller_test.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 	"testing"
@@ -284,10 +285,16 @@ func TestControllerExpandVolume(t *testing.T) {
 }
 
 func TestCreateVolume(t *testing.T) {
+	snapshotId := "snapshotId"
+
 	tests := []struct {
-		name           string
-		listVolumesErr error
-		getSnapshotErr error
+		name                    string
+		listVolumesErr          error
+		getSnapshotErr          error
+		snapshots               map[string]*godo.Snapshot
+		wantErr                 error
+		createVolumeErr         error
+		createVolumeResponseErr *godo.Response
 	}{
 		{
 			name:           "listing volumes failing",
@@ -297,24 +304,75 @@ func TestCreateVolume(t *testing.T) {
 			name:           "fetching snapshot failing",
 			getSnapshotErr: errors.New("failed to get snapshot"),
 		},
+		{
+			name: "volume limit has been reached",
+			snapshots: map[string]*godo.Snapshot{
+				snapshotId: {
+					ID: snapshotId,
+				},
+			},
+			createVolumeErr: &godo.ErrorResponse{
+				Response: &http.Response{
+					Request: &http.Request{
+						Method: http.MethodPost,
+						URL:    &url.URL{},
+					},
+					StatusCode: http.StatusForbidden,
+				},
+				Message: "failed to create volume: volume/snapshot capacity limit exceeded",
+			},
+			createVolumeResponseErr: &godo.Response{
+				Response: &http.Response{
+					StatusCode: http.StatusForbidden,
+				},
+			},
+			wantErr: errors.New("volume limit has been reached. Please contact support"),
+		},
+		{
+			name: "error occurred when creating a volume",
+			snapshots: map[string]*godo.Snapshot{
+				snapshotId: {
+					ID: snapshotId,
+				},
+			},
+			createVolumeErr: &godo.ErrorResponse{
+				Response: &http.Response{
+					Request: &http.Request{
+						Method: http.MethodPost,
+						URL:    &url.URL{},
+					},
+					StatusCode: http.StatusInternalServerError,
+				},
+				Message: "internal server error",
+			},
+			createVolumeResponseErr: &godo.Response{
+				Response: &http.Response{
+					StatusCode: http.StatusInternalServerError,
+				},
+			},
+			wantErr: errors.New("internal server error"),
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			d := &Driver{
 				storage: &fakeStorageDriver{
-					listVolumesErr: test.listVolumesErr,
+					listVolumesErr:          test.listVolumesErr,
+					createVolumeErr:         test.createVolumeErr,
+					createVolumeErrResponse: test.createVolumeResponseErr,
 				},
 				snapshots: &fakeSnapshotsDriver{
 					getSnapshotErr: test.getSnapshotErr,
+					snapshots:      test.snapshots,
 				},
-				log: logrus.New().WithField("test_enabed", true),
+				log: logrus.New().WithField("test_enabled", true),
 			}
 
 			_, err := d.CreateVolume(context.Background(), &csi.CreateVolumeRequest{
 				Name: "name",
 				VolumeCapabilities: []*csi.VolumeCapability{
-					&csi.VolumeCapability{
+					{
 						AccessType: &csi.VolumeCapability_Mount{
 							Mount: &csi.VolumeCapability_MountVolume{},
 						},
@@ -338,6 +396,8 @@ func TestCreateVolume(t *testing.T) {
 				wantErr = test.listVolumesErr
 			case test.getSnapshotErr != nil:
 				wantErr = test.getSnapshotErr
+			case test.createVolumeErr != nil:
+				wantErr = test.wantErr
 			}
 
 			if wantErr == nil && err != nil {

driver/driver_test.go

@@ -155,9 +155,11 @@ func (f *fakeAccountDriver) Get(context.Context) (*godo.Account, *godo.Response,
 }
 
 type fakeStorageDriver struct {
-	volumes        map[string]*godo.Volume
-	snapshots      map[string]*godo.Snapshot
-	listVolumesErr error
+	volumes                 map[string]*godo.Volume
+	snapshots               map[string]*godo.Snapshot
+	listVolumesErr          error
+	createVolumeErr         error
+	createVolumeErrResponse *godo.Response
 }
 
 func (f *fakeStorageDriver) ListVolumes(ctx context.Context, param *godo.ListVolumeParams) ([]godo.Volume, *godo.Response, error) {
@@ -213,6 +215,10 @@ func (f *fakeStorageDriver) GetVolume(ctx context.Context, id string) (*godo.Vol
 }
 
 func (f *fakeStorageDriver) CreateVolume(ctx context.Context, req *godo.VolumeCreateRequest) (*godo.Volume, *godo.Response, error) {
+	if f.createVolumeErr != nil || f.createVolumeErrResponse != nil {
+		return nil, f.createVolumeErrResponse, f.createVolumeErr
+	}
+
 	id := randString(10)
 	vol := &godo.Volume{
 		ID:            id,