diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index f6051abd8..b63fb03e4 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -38,6 +38,7 @@ const ( AccessDeniedException = "AccessDeniedException" AccessPointAlreadyExists = "AccessPointAlreadyExists" PvcNameTagKey = "pvcName" + AccessPointPerFsLimit = 1000 ) var ( @@ -266,7 +267,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(1000), + MaxResults: aws.Int64(AccessPointPerFsLimit), } res, err := c.efs.DescribeAccessPointsWithContext(ctx, describeAPInput) if err != nil { @@ -296,6 +297,7 @@ 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 { diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index e4755cbab..82a07a120 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -215,8 +215,8 @@ func TestCreateVolume(t *testing.T) { AccessPointId: apId, FileSystemId: fsId, PosixUser: &cloud.PosixUser{ - Gid: 1003, - Uid: 1003, + Gid: 1001, + Uid: 1001, }, }, { @@ -229,7 +229,7 @@ func TestCreateVolume(t *testing.T) { }, } - var expectedGid int64 = 1001 //1003 and 1002 are taken, next available is 1001 + var expectedGid int64 = 1003 //1001 and 1002 are taken, next available is 1003 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). @@ -429,7 +429,7 @@ func TestCreateVolume(t *testing.T) { } accessPoints := []*cloud.AccessPoint{} - for i := 0; i < ACCESS_POINT_PER_FS_LIMIT; i++ { + for i := 0; i < cloud.AccessPointPerFsLimit; i++ { gidMin, err := strconv.Atoi(req.Parameters[GidMin]) if err != nil { t.Fatalf("Failed to convert GidMax Parameter to int.") @@ -488,6 +488,69 @@ 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) { diff --git a/pkg/driver/gid_allocator.go b/pkg/driver/gid_allocator.go index d820d0684..71460fbf4 100644 --- a/pkg/driver/gid_allocator.go +++ b/pkg/driver/gid_allocator.go @@ -12,8 +12,6 @@ import ( "k8s.io/klog/v2" ) -var ACCESS_POINT_PER_FS_LIMIT int = 1000 - type FilesystemID struct { gidMin int gidMax int @@ -85,9 +83,10 @@ 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 > 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 + 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 } var lookup func(usedGids []int64) @@ -97,7 +96,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.", nextGid) + klog.V(5).Infof("Allocator found GID which is already in use: %v, trying next one.", gid) } return }