Skip to content

Commit 49f2bc9

Browse files
committed
return ResourceExhausted code in CreateVolume
If Openstack quotas for volumes are exceeded, cinder returns with a HTTP 413 response code: https://github.com/openstack/cinder/blob/c4f61c11ef9b590606a2a276bdf256622516181f/cinder/quota_utils.py#L130-L140 CSI spec defines that on a quota limit error, the driver should return a ResourceExhausted grpc error code: https://github.com/container-storage-interface/spec/blob/master/spec.md#createvolume-errors Signed-off-by: Lukas Hoehl <[email protected]>
1 parent 53a4505 commit 49f2bc9

File tree

5 files changed

+73
-16
lines changed

5 files changed

+73
-16
lines changed

pkg/csi/cinder/controllerserver.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package cinder
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"slices"
2324
"sort"
@@ -233,6 +234,9 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
233234
vol, err := cloud.CreateVolume(ctx, opts, schedulerHints)
234235
if err != nil {
235236
klog.Errorf("Failed to CreateVolume: %v", err)
237+
if errors.Is(err, cpoerrors.ErrQuotaExceeded) {
238+
return nil, status.Errorf(codes.ResourceExhausted, "CreateVolume failed due to exceeded quota %v", err)
239+
}
236240
return nil, status.Errorf(codes.Internal, "CreateVolume failed with error %v", err)
237241
}
238242

@@ -661,7 +665,6 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
661665
ReadyToUse: true,
662666
},
663667
}, nil
664-
665668
}
666669

667670
func (cs *controllerServer) createSnapshot(ctx context.Context, cloud openstack.IOpenStack, name string, volumeID string, parameters map[string]string) (snap *snapshots.Snapshot, err error) {
@@ -863,7 +866,6 @@ func (cs *controllerServer) ListSnapshots(ctx context.Context, req *csi.ListSnap
863866
Entries: sentries,
864867
NextToken: nextPageToken,
865868
}, nil
866-
867869
}
868870

869871
// ControllerGetCapabilities implements the default GRPC callout.

pkg/csi/cinder/controllerserver_test.go

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,20 @@ import (
2323
"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes"
2424
"github.com/stretchr/testify/assert"
2525
"github.com/stretchr/testify/mock"
26+
"google.golang.org/grpc/codes"
27+
"google.golang.org/grpc/status"
2628

2729
sharedcsi "k8s.io/cloud-provider-openstack/pkg/csi"
2830
"k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack"
31+
cpoerrors "k8s.io/cloud-provider-openstack/pkg/util/errors"
2932
)
3033

31-
var fakeCs *controllerServer
32-
var fakeCsMultipleClouds *controllerServer
33-
var osmock *openstack.OpenStackMock
34-
var osmockRegionX *openstack.OpenStackMock
34+
var (
35+
fakeCs *controllerServer
36+
fakeCsMultipleClouds *controllerServer
37+
osmock *openstack.OpenStackMock
38+
osmockRegionX *openstack.OpenStackMock
39+
)
3540

3641
// Init Controller Server
3742
func init() {
@@ -94,7 +99,53 @@ func TestCreateVolume(t *testing.T) {
9499
assert.NotEqual(0, len(actualRes.Volume.VolumeId), "Volume Id is nil")
95100
assert.NotNil(actualRes.Volume.AccessibleTopology)
96101
assert.Equal(FakeAvailability, actualRes.Volume.AccessibleTopology[0].GetSegments()[topologyKey])
102+
}
103+
104+
// Test CreateVolume fails with quota exceeded error
105+
func TestCreateVolumeQuotaError(t *testing.T) {
106+
errorVolume := "errorVolume"
107+
108+
// mock OpenStack
109+
properties := map[string]string{cinderCSIClusterIDKey: FakeCluster}
110+
// CreateVolume(name string, size int, vtype, availability string, snapshotID string, sourceVolID string, sourceBackupID string, tags map[string]string) (string, string, int, error)
111+
osmock.On("CreateVolume", errorVolume, mock.AnythingOfType("int"), FakeVolType, FakeAvailability, "", "", "", properties).Return(&volumes.Volume{}, cpoerrors.ErrQuotaExceeded)
112+
113+
osmock.On("GetVolumesByName", errorVolume).Return(FakeVolListEmpty, nil)
114+
// Init assert
115+
assert := assert.New(t)
116+
117+
// Fake request
118+
fakeReq := &csi.CreateVolumeRequest{
119+
Name: errorVolume,
120+
VolumeCapabilities: []*csi.VolumeCapability{
121+
{
122+
AccessMode: &csi.VolumeCapability_AccessMode{
123+
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
124+
},
125+
},
126+
},
127+
128+
AccessibilityRequirements: &csi.TopologyRequirement{
129+
Requisite: []*csi.Topology{
130+
{
131+
Segments: map[string]string{topologyKey: FakeAvailability},
132+
},
133+
},
134+
},
135+
}
136+
137+
// Invoke CreateVolume
138+
_, err := fakeCs.CreateVolume(FakeCtx, fakeReq)
139+
if err == nil {
140+
t.Errorf("CreateVolume did not return an error")
141+
}
142+
statusErr, ok := status.FromError(err)
143+
if !ok {
144+
t.Errorf("CreateVolume did not return a grpc status as error, got %v", err)
145+
}
97146

147+
// Assert
148+
assert.Equal(statusErr.Code(), codes.ResourceExhausted)
98149
}
99150

100151
// Test CreateVolume with additional param
@@ -146,7 +197,6 @@ func TestCreateVolumeWithParam(t *testing.T) {
146197
assert.NotEqual(0, len(actualRes.Volume.VolumeId), "Volume Id is nil")
147198
assert.NotNil(actualRes.Volume.AccessibleTopology)
148199
assert.Equal(FakeAvailability, actualRes.Volume.AccessibleTopology[0].GetSegments()[topologyKey])
149-
150200
}
151201

152202
func TestCreateVolumeWithExtraMetadata(t *testing.T) {
@@ -192,7 +242,6 @@ func TestCreateVolumeWithExtraMetadata(t *testing.T) {
192242
if err != nil {
193243
t.Errorf("failed to CreateVolume: %v", err)
194244
}
195-
196245
}
197246

198247
func TestCreateVolumeFromSnapshot(t *testing.T) {
@@ -239,7 +288,6 @@ func TestCreateVolumeFromSnapshot(t *testing.T) {
239288
assert.NotEqual(0, len(actualRes.Volume.VolumeId), "Volume Id is nil")
240289

241290
assert.Equal(FakeSnapshotID, actualRes.Volume.ContentSource.GetSnapshot().SnapshotId)
242-
243291
}
244292

245293
func TestCreateVolumeFromSourceVolume(t *testing.T) {
@@ -286,7 +334,6 @@ func TestCreateVolumeFromSourceVolume(t *testing.T) {
286334
assert.NotEqual(0, len(actualRes.Volume.VolumeId), "Volume Id is nil")
287335

288336
assert.Equal(FakeVolID, actualRes.Volume.ContentSource.GetVolume().VolumeId)
289-
290337
}
291338

292339
// Test CreateVolumeDuplicate
@@ -436,6 +483,7 @@ func genFakeVolumeEntry(fakeVol volumes.Volume) *csi.ListVolumesResponse_Entry {
436483
},
437484
}
438485
}
486+
439487
func genFakeVolumeEntries(fakeVolumes []volumes.Volume) []*csi.ListVolumesResponse_Entry {
440488
entries := make([]*csi.ListVolumesResponse_Entry, 0, len(fakeVolumes))
441489
for _, fakeVol := range fakeVolumes {
@@ -800,7 +848,6 @@ func TestGlobalListVolumesMultipleClouds(t *testing.T) {
800848

801849
// Test CreateSnapshot
802850
func TestCreateSnapshot(t *testing.T) {
803-
804851
osmock.On("CreateSnapshot", FakeSnapshotName, FakeVolID, map[string]string{cinderCSIClusterIDKey: "cluster"}).Return(&FakeSnapshotRes, nil)
805852
osmock.On("ListSnapshots", map[string]string{"Name": FakeSnapshotName}).Return(FakeSnapshotListEmpty, "", nil)
806853
osmock.On("WaitSnapshotReady", FakeSnapshotID).Return(FakeSnapshotRes.Status, nil)
@@ -944,7 +991,6 @@ func TestControllerExpandVolume(t *testing.T) {
944991

945992
// Assert
946993
assert.Equal(expectedRes, actualRes)
947-
948994
}
949995

950996
func TestValidateVolumeCapabilities(t *testing.T) {
@@ -1000,13 +1046,11 @@ func TestValidateVolumeCapabilities(t *testing.T) {
10001046
}
10011047

10021048
actualRes2, err := fakeCs.ValidateVolumeCapabilities(FakeCtx, fakereq2)
1003-
10041049
if err != nil {
10051050
t.Errorf("failed to ValidateVolumeCapabilities: %v", err)
10061051
}
10071052

10081053
// assert
10091054
assert.Equal(expectedRes, actualRes)
10101055
assert.Equal(expectedRes2, actualRes2)
1011-
10121056
}

pkg/csi/cinder/openstack/openstack.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,10 @@ func GetConfigFromFiles(configFilePaths []string) (Config, error) {
146146

147147
const defaultMaxVolAttachLimit int64 = 256
148148

149-
var OsInstances map[string]IOpenStack
150-
var configFiles = []string{"/etc/cloud.conf"}
149+
var (
150+
OsInstances map[string]IOpenStack
151+
configFiles = []string{"/etc/cloud.conf"}
152+
)
151153

152154
func InitOpenStackProvider(cfgFiles []string, httpEndpoint string) {
153155
OsInstances = make(map[string]IOpenStack)

pkg/csi/cinder/openstack/openstack_volumes.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@ package openstack
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
23+
"net/http"
2224
"net/url"
2325
"strings"
2426
"time"
2527

28+
"github.com/gophercloud/gophercloud/v2"
2629
"github.com/gophercloud/gophercloud/v2/openstack"
2730
"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes"
2831
"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/volumeattach"
@@ -71,6 +74,9 @@ func (os *OpenStack) CreateVolume(ctx context.Context, opts *volumes.CreateOpts,
7174
opts.Description = volumeDescription
7275
vol, err := volumes.Create(ctx, blockstorageClient, opts, schedulerHints).Extract()
7376
if mc.ObserveRequest(err) != nil {
77+
if gophercloud.ResponseCodeIs(err, http.StatusRequestEntityTooLarge) {
78+
return nil, errors.Join(err, cpoerrors.ErrQuotaExceeded)
79+
}
7480
return nil, err
7581
}
7682

pkg/util/errors/errors.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import (
2323
"github.com/gophercloud/gophercloud/v2"
2424
)
2525

26+
// ErrQuotaExceeded is used when openstack runs in to quota limits
27+
var ErrQuotaExceeded = errors.New("quota exceeded")
28+
2629
// ErrNotFound is used to inform that the object is missing
2730
var ErrNotFound = errors.New("failed to find object")
2831

0 commit comments

Comments
 (0)