diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 3d702f522..ef01edcce 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -80,12 +80,12 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Eq(volumeName), gomock.Any(), gomock.Eq(false)).Return(accessPoint, nil). - Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) { - if accessPointOpts.Uid != 1000 { - t.Fatalf("Uid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1000) + Do(func(ctx context.Context, clientToken string, accessPointsOptions *cloud.AccessPointOptions, reuseAccessPointName bool) { + if accessPointsOptions.Uid != 1000 { + t.Fatalf("Uid mismatched. Expected: %v, actual: %v", 1000, accessPointsOptions.Uid) } - if accessPointOpts.Gid != 1001 { - t.Fatalf("Gid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1001) + if accessPointsOptions.Gid != 1001 { + t.Fatalf("Gid mismatched. Expected: %v, actual: %v", 1001, accessPointsOptions.Gid) } }) @@ -149,10 +149,10 @@ func TestCreateVolume(t *testing.T) { mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), gomock.Any()).Return(accessPoint, nil). Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) { if accessPointOpts.Uid != 1000 { - t.Fatalf("Uid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1000) + t.Fatalf("Uid mismatched. Expected: %v, actual: %v", 1000, accessPointOpts.Uid) } if accessPointOpts.Gid != 1001 { - t.Fatalf("Gid mimatched. Expected: %v, actual: %v", accessPointOpts.Uid, 1001) + t.Fatalf("Gid mismatched. Expected: %v, actual: %v", 1001, accessPointOpts.Uid) } }) @@ -197,7 +197,7 @@ func TestCreateVolume(t *testing.T) { FsId: fsId, DirectoryPerms: "777", BasePath: "test", - GidMin: "1000", + GidMin: "1001", GidMax: "1003", }, } @@ -283,8 +283,8 @@ func TestCreateVolume(t *testing.T) { FsId: fsId, DirectoryPerms: "777", BasePath: "test", - GidMin: "1000", - GidMax: "1004", + GidMin: "1001", + GidMax: "1005", }, } @@ -326,8 +326,8 @@ func TestCreateVolume(t *testing.T) { } // Let allocator jump over some GIDS. - accessPoints := []*cloud.AccessPoint{ap3, ap4} - var expectedGid int64 = 1002 // 1003 and 1004 is taken. + accessPoints := []*cloud.AccessPoint{ap1, ap2, ap3} + var expectedGid int64 = 1004 // 1001-1003 is taken. mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil) @@ -345,7 +345,7 @@ func TestCreateVolume(t *testing.T) { // 2. Simulate access point removal and verify their GIDs returned to allocator. accessPoints = []*cloud.AccessPoint{} - expectedGid = 1004 // 1003 and 1004 are now free, if no GID return would happen allocator would pick 1001. + expectedGid = 1001 // 1001 is now free and lowest possible, if no GID return would happen allocator would pick 1005. mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil) @@ -360,10 +360,11 @@ func TestCreateVolume(t *testing.T) { }) res, err = driver.CreateVolume(ctx, req) - //// + + // 3. Simulate access point ID gap and verify their GIDs returned to allocator accessPoints = []*cloud.AccessPoint{ap1, ap4} + expectedGid = 1002 // 1001 and 1004 are now taken, lowest available is 1002 - expectedGid = 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(ap2, nil). @@ -428,12 +429,12 @@ func TestCreateVolume(t *testing.T) { } accessPoints := []*cloud.AccessPoint{} - for i := 0; i < ACCESS_POINT_PER_FS_LIMIT-1; i++ { - gidMax, err := strconv.Atoi(req.Parameters[GidMax]) + 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.") } - userGid := gidMax - i + userGid := gidMin + i ap := &cloud.AccessPoint{ AccessPointId: apId, FileSystemId: fsId, @@ -449,12 +450,12 @@ func TestCreateVolume(t *testing.T) { AccessPointId: apId, FileSystemId: fsId, PosixUser: &cloud.PosixUser{ - Gid: 1001, - Uid: 1001, + Gid: 2000, + Uid: 2000, }, } - expectedGid := 1001 + expectedGid := 2000 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(lastAccessPoint, nil). @@ -472,7 +473,7 @@ func TestCreateVolume(t *testing.T) { // Allocate last available GID _, err = driver.CreateVolume(ctx, req) if err != nil { - t.Fatalf("CreateVolume failed.") + t.Fatalf("CreateVolume failed unexpectedly: %v", err) } accessPoints = append(accessPoints, lastAccessPoint) diff --git a/pkg/driver/gid_allocator.go b/pkg/driver/gid_allocator.go index 98f5622a1..098cc76ce 100644 --- a/pkg/driver/gid_allocator.go +++ b/pkg/driver/gid_allocator.go @@ -94,7 +94,7 @@ func getNextUnusedGid(usedGids []int64, gidMin, gidMax int) (nextGid int, err er var lookup func(usedGids []int64) lookup = func(usedGids []int64) { - for gid := gidMax; gid > gidMin; gid-- { + for gid := gidMin; gid <= gidMax; gid++ { if !slices.Contains(usedGids, int64(gid)) { nextGid = gid return