diff --git a/pkg/csi/cinder/controllerserver.go b/pkg/csi/cinder/controllerserver.go index f1d4905665..5ba13bb0b7 100644 --- a/pkg/csi/cinder/controllerserver.go +++ b/pkg/csi/cinder/controllerserver.go @@ -18,6 +18,7 @@ package cinder import ( "context" + "errors" "fmt" "slices" "sort" @@ -233,6 +234,9 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol vol, err := cloud.CreateVolume(ctx, opts, schedulerHints) if err != nil { klog.Errorf("Failed to CreateVolume: %v", err) + if errors.Is(err, cpoerrors.ErrQuotaExceeded) { + return nil, status.Errorf(codes.ResourceExhausted, "CreateVolume failed due to exceeded quota %v", err) + } return nil, status.Errorf(codes.Internal, "CreateVolume failed with error %v", err) } @@ -661,7 +665,6 @@ func (cs *controllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS ReadyToUse: true, }, }, nil - } 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 Entries: sentries, NextToken: nextPageToken, }, nil - } // ControllerGetCapabilities implements the default GRPC callout. diff --git a/pkg/csi/cinder/controllerserver_test.go b/pkg/csi/cinder/controllerserver_test.go index 5563c79fdb..fc5adb227b 100644 --- a/pkg/csi/cinder/controllerserver_test.go +++ b/pkg/csi/cinder/controllerserver_test.go @@ -23,15 +23,20 @@ import ( "github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" sharedcsi "k8s.io/cloud-provider-openstack/pkg/csi" "k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack" + cpoerrors "k8s.io/cloud-provider-openstack/pkg/util/errors" ) -var fakeCs *controllerServer -var fakeCsMultipleClouds *controllerServer -var osmock *openstack.OpenStackMock -var osmockRegionX *openstack.OpenStackMock +var ( + fakeCs *controllerServer + fakeCsMultipleClouds *controllerServer + osmock *openstack.OpenStackMock + osmockRegionX *openstack.OpenStackMock +) // Init Controller Server func init() { @@ -94,7 +99,53 @@ func TestCreateVolume(t *testing.T) { assert.NotEqual(0, len(actualRes.Volume.VolumeId), "Volume Id is nil") assert.NotNil(actualRes.Volume.AccessibleTopology) assert.Equal(FakeAvailability, actualRes.Volume.AccessibleTopology[0].GetSegments()[topologyKey]) +} + +// Test CreateVolume fails with quota exceeded error +func TestCreateVolumeQuotaError(t *testing.T) { + errorVolume := "errorVolume" + + // mock OpenStack + properties := map[string]string{cinderCSIClusterIDKey: FakeCluster} + // CreateVolume(name string, size int, vtype, availability string, snapshotID string, sourceVolID string, sourceBackupID string, tags map[string]string) (string, string, int, error) + osmock.On("CreateVolume", errorVolume, mock.AnythingOfType("int"), FakeVolType, FakeAvailability, "", "", "", properties).Return(&volumes.Volume{}, cpoerrors.ErrQuotaExceeded) + + osmock.On("GetVolumesByName", errorVolume).Return(FakeVolListEmpty, nil) + // Init assert + assert := assert.New(t) + + // Fake request + fakeReq := &csi.CreateVolumeRequest{ + Name: errorVolume, + VolumeCapabilities: []*csi.VolumeCapability{ + { + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + }, + + AccessibilityRequirements: &csi.TopologyRequirement{ + Requisite: []*csi.Topology{ + { + Segments: map[string]string{topologyKey: FakeAvailability}, + }, + }, + }, + } + + // Invoke CreateVolume + _, err := fakeCs.CreateVolume(FakeCtx, fakeReq) + if err == nil { + t.Errorf("CreateVolume did not return an error") + } + statusErr, ok := status.FromError(err) + if !ok { + t.Errorf("CreateVolume did not return a grpc status as error, got %v", err) + } + // Assert + assert.Equal(statusErr.Code(), codes.ResourceExhausted) } // Test CreateVolume with additional param @@ -146,7 +197,6 @@ func TestCreateVolumeWithParam(t *testing.T) { assert.NotEqual(0, len(actualRes.Volume.VolumeId), "Volume Id is nil") assert.NotNil(actualRes.Volume.AccessibleTopology) assert.Equal(FakeAvailability, actualRes.Volume.AccessibleTopology[0].GetSegments()[topologyKey]) - } func TestCreateVolumeWithExtraMetadata(t *testing.T) { @@ -192,7 +242,6 @@ func TestCreateVolumeWithExtraMetadata(t *testing.T) { if err != nil { t.Errorf("failed to CreateVolume: %v", err) } - } func TestCreateVolumeFromSnapshot(t *testing.T) { @@ -239,7 +288,6 @@ func TestCreateVolumeFromSnapshot(t *testing.T) { assert.NotEqual(0, len(actualRes.Volume.VolumeId), "Volume Id is nil") assert.Equal(FakeSnapshotID, actualRes.Volume.ContentSource.GetSnapshot().SnapshotId) - } func TestCreateVolumeFromSourceVolume(t *testing.T) { @@ -286,7 +334,6 @@ func TestCreateVolumeFromSourceVolume(t *testing.T) { assert.NotEqual(0, len(actualRes.Volume.VolumeId), "Volume Id is nil") assert.Equal(FakeVolID, actualRes.Volume.ContentSource.GetVolume().VolumeId) - } // Test CreateVolumeDuplicate @@ -436,6 +483,7 @@ func genFakeVolumeEntry(fakeVol volumes.Volume) *csi.ListVolumesResponse_Entry { }, } } + func genFakeVolumeEntries(fakeVolumes []volumes.Volume) []*csi.ListVolumesResponse_Entry { entries := make([]*csi.ListVolumesResponse_Entry, 0, len(fakeVolumes)) for _, fakeVol := range fakeVolumes { @@ -800,7 +848,6 @@ func TestGlobalListVolumesMultipleClouds(t *testing.T) { // Test CreateSnapshot func TestCreateSnapshot(t *testing.T) { - osmock.On("CreateSnapshot", FakeSnapshotName, FakeVolID, map[string]string{cinderCSIClusterIDKey: "cluster"}).Return(&FakeSnapshotRes, nil) osmock.On("ListSnapshots", map[string]string{"Name": FakeSnapshotName}).Return(FakeSnapshotListEmpty, "", nil) osmock.On("WaitSnapshotReady", FakeSnapshotID).Return(FakeSnapshotRes.Status, nil) @@ -944,7 +991,6 @@ func TestControllerExpandVolume(t *testing.T) { // Assert assert.Equal(expectedRes, actualRes) - } func TestValidateVolumeCapabilities(t *testing.T) { @@ -1000,7 +1046,6 @@ func TestValidateVolumeCapabilities(t *testing.T) { } actualRes2, err := fakeCs.ValidateVolumeCapabilities(FakeCtx, fakereq2) - if err != nil { t.Errorf("failed to ValidateVolumeCapabilities: %v", err) } @@ -1008,5 +1053,4 @@ func TestValidateVolumeCapabilities(t *testing.T) { // assert assert.Equal(expectedRes, actualRes) assert.Equal(expectedRes2, actualRes2) - } diff --git a/pkg/csi/cinder/openstack/openstack.go b/pkg/csi/cinder/openstack/openstack.go index fbc386edf3..7402a61fbb 100644 --- a/pkg/csi/cinder/openstack/openstack.go +++ b/pkg/csi/cinder/openstack/openstack.go @@ -146,8 +146,10 @@ func GetConfigFromFiles(configFilePaths []string) (Config, error) { const defaultMaxVolAttachLimit int64 = 256 -var OsInstances map[string]IOpenStack -var configFiles = []string{"/etc/cloud.conf"} +var ( + OsInstances map[string]IOpenStack + configFiles = []string{"/etc/cloud.conf"} +) func InitOpenStackProvider(cfgFiles []string, httpEndpoint string) { OsInstances = make(map[string]IOpenStack) diff --git a/pkg/csi/cinder/openstack/openstack_volumes.go b/pkg/csi/cinder/openstack/openstack_volumes.go index c57b1443d1..6f517747c7 100644 --- a/pkg/csi/cinder/openstack/openstack_volumes.go +++ b/pkg/csi/cinder/openstack/openstack_volumes.go @@ -18,11 +18,14 @@ package openstack import ( "context" + "errors" "fmt" + "net/http" "net/url" "strings" "time" + "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack" "github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/volumeattach" @@ -71,6 +74,9 @@ func (os *OpenStack) CreateVolume(ctx context.Context, opts *volumes.CreateOpts, opts.Description = volumeDescription vol, err := volumes.Create(ctx, blockstorageClient, opts, schedulerHints).Extract() if mc.ObserveRequest(err) != nil { + if gophercloud.ResponseCodeIs(err, http.StatusRequestEntityTooLarge) { + return nil, errors.Join(err, cpoerrors.ErrQuotaExceeded) + } return nil, err } diff --git a/pkg/util/errors/errors.go b/pkg/util/errors/errors.go index 28d4ba29d5..7f874ebda6 100644 --- a/pkg/util/errors/errors.go +++ b/pkg/util/errors/errors.go @@ -23,6 +23,9 @@ import ( "github.com/gophercloud/gophercloud/v2" ) +// ErrQuotaExceeded is used when openstack runs in to quota limits +var ErrQuotaExceeded = errors.New("quota exceeded") + // ErrNotFound is used to inform that the object is missing var ErrNotFound = errors.New("failed to find object")