Skip to content

[cinder-csi-plugin]: ResourceExhausted code for CreateVolume #2860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions pkg/csi/cinder/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cinder

import (
"context"
"errors"
"fmt"
"slices"
"sort"
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
68 changes: 56 additions & 12 deletions pkg/csi/cinder/controllerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -192,7 +242,6 @@ func TestCreateVolumeWithExtraMetadata(t *testing.T) {
if err != nil {
t.Errorf("failed to CreateVolume: %v", err)
}

}

func TestCreateVolumeFromSnapshot(t *testing.T) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -944,7 +991,6 @@ func TestControllerExpandVolume(t *testing.T) {

// Assert
assert.Equal(expectedRes, actualRes)

}

func TestValidateVolumeCapabilities(t *testing.T) {
Expand Down Expand Up @@ -1000,13 +1046,11 @@ func TestValidateVolumeCapabilities(t *testing.T) {
}

actualRes2, err := fakeCs.ValidateVolumeCapabilities(FakeCtx, fakereq2)

if err != nil {
t.Errorf("failed to ValidateVolumeCapabilities: %v", err)
}

// assert
assert.Equal(expectedRes, actualRes)
assert.Equal(expectedRes2, actualRes2)

}
6 changes: 4 additions & 2 deletions pkg/csi/cinder/openstack/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions pkg/csi/cinder/openstack/openstack_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/util/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down