Skip to content

Commit

Permalink
Revert "set results count for listing access points"
Browse files Browse the repository at this point in the history
This reverts commit d31495d.
  • Loading branch information
mskanth972 committed Jan 3, 2024
1 parent d31495d commit dd6b287
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 75 deletions.
4 changes: 1 addition & 3 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const (
AccessDeniedException = "AccessDeniedException"
AccessPointAlreadyExists = "AccessPointAlreadyExists"
PvcNameTagKey = "pvcName"
AccessPointPerFsLimit = 1000
)

var (
Expand Down Expand Up @@ -267,7 +266,7 @@ func (c *cloud) findAccessPointByClientToken(ctx context.Context, clientToken st
klog.V(2).Infof("ClientToken to find AP : %s", clientToken)
describeAPInput := &efs.DescribeAccessPointsInput{
FileSystemId: &accessPointOpts.FileSystemId,
MaxResults: aws.Int64(AccessPointPerFsLimit),
MaxResults: aws.Int64(1000),
}
res, err := c.efs.DescribeAccessPointsWithContext(ctx, describeAPInput)
if err != nil {
Expand Down Expand Up @@ -297,7 +296,6 @@ func (c *cloud) findAccessPointByClientToken(ctx context.Context, clientToken st
func (c *cloud) ListAccessPoints(ctx context.Context, fileSystemId string) (accessPoints []*AccessPoint, err error) {
describeAPInput := &efs.DescribeAccessPointsInput{
FileSystemId: &fileSystemId,
MaxResults: aws.Int64(AccessPointPerFsLimit),
}
res, err := c.efs.DescribeAccessPointsWithContext(ctx, describeAPInput)
if err != nil {
Expand Down
71 changes: 4 additions & 67 deletions pkg/driver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ func TestCreateVolume(t *testing.T) {
AccessPointId: apId,
FileSystemId: fsId,
PosixUser: &cloud.PosixUser{
Gid: 1001,
Uid: 1001,
Gid: 1003,
Uid: 1003,
},
},
{
Expand All @@ -229,7 +229,7 @@ func TestCreateVolume(t *testing.T) {
},
}

var expectedGid int64 = 1003 //1001 and 1002 are taken, next available is 1003
var expectedGid int64 = 1001 //1003 and 1002 are taken, next available is 1001
mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil)
mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil)
mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), false).Return(accessPoint, nil).
Expand Down Expand Up @@ -429,7 +429,7 @@ func TestCreateVolume(t *testing.T) {
}

accessPoints := []*cloud.AccessPoint{}
for i := 0; i < cloud.AccessPointPerFsLimit; i++ {
for i := 0; i < ACCESS_POINT_PER_FS_LIMIT; i++ {
gidMin, err := strconv.Atoi(req.Parameters[GidMin])
if err != nil {
t.Fatalf("Failed to convert GidMax Parameter to int.")
Expand Down Expand Up @@ -488,69 +488,6 @@ func TestCreateVolume(t *testing.T) {
mockCtl.Finish()
},
},
{
name: "Success: GID range exceeds EFS access point limit",
testFunc: func(t *testing.T) {
mockCtl := gomock.NewController(t)
mockCloud := mocks.NewMockCloud(mockCtl)

driver := &Driver{
endpoint: endpoint,
cloud: mockCloud,
gidAllocator: NewGidAllocator(),
}

req := &csi.CreateVolumeRequest{
Name: volumeName,
VolumeCapabilities: []*csi.VolumeCapability{
stdVolCap,
},
CapacityRange: &csi.CapacityRange{
RequiredBytes: capacityRange,
},
Parameters: map[string]string{
ProvisioningMode: "efs-ap",
FsId: fsId,
DirectoryPerms: "777",
BasePath: "test",
GidMin: "1000",
GidMax: "1000000",
},
}

ctx := context.Background()
fileSystem := &cloud.FileSystem{
FileSystemId: fsId,
}

accessPoint := &cloud.AccessPoint{
AccessPointId: apId,
FileSystemId: fsId,
}

expectedGid := 1000 // Allocator should pick lowest available GID
mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil)
mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil)
mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), false).Return(accessPoint, nil).
Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) {
if accessPointOpts.Uid != int64(expectedGid) {
t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid)
}
if accessPointOpts.Gid != int64(expectedGid) {
t.Fatalf("Gid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Gid)
}
})

var err error

_, err = driver.CreateVolume(ctx, req)
if err != nil {
t.Fatalf("CreateVolume failed unexpectedly: %v", err)
}

mockCtl.Finish()
},
},
{
name: "Success: Normal flow",
testFunc: func(t *testing.T) {
Expand Down
11 changes: 6 additions & 5 deletions pkg/driver/gid_allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"k8s.io/klog/v2"
)

var ACCESS_POINT_PER_FS_LIMIT int = 1000

type FilesystemID struct {
gidMin int
gidMax int
Expand Down Expand Up @@ -83,10 +85,9 @@ func (g *GidAllocator) getUsedGids(ctx context.Context, localCloud cloud.Cloud,
func getNextUnusedGid(usedGids []int64, gidMin, gidMax int) (nextGid int, err error) {
requestedRange := gidMax - gidMin

if requestedRange > cloud.AccessPointPerFsLimit {
overrideGidMax := gidMin + cloud.AccessPointPerFsLimit
klog.Warningf("Requested GID range (%v:%v) exceeds EFS Access Point limit (%v) per Filesystem. Driver will use limited GID range (%v:%v)", gidMin, gidMax, cloud.AccessPointPerFsLimit, gidMin, overrideGidMax)
gidMax = overrideGidMax
if requestedRange > ACCESS_POINT_PER_FS_LIMIT {
klog.Warningf("Requested GID range (%v:%v) exceeds EFS Access Point limit (%v) per Filesystem. Driver will not allocate GIDs outside of this limit.", gidMin, gidMax, ACCESS_POINT_PER_FS_LIMIT)
gidMin = gidMax - ACCESS_POINT_PER_FS_LIMIT
}

var lookup func(usedGids []int64)
Expand All @@ -96,7 +97,7 @@ func getNextUnusedGid(usedGids []int64, gidMin, gidMax int) (nextGid int, err er
nextGid = gid
return
}
klog.V(5).Infof("Allocator found GID which is already in use: %v, trying next one.", gid)
klog.V(5).Infof("Allocator found GID which is already in use: %v - trying next one.", nextGid)
}
return
}
Expand Down

0 comments on commit dd6b287

Please sign in to comment.