diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 1b42b9931..e407169bb 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "regexp" + "strconv" "testing" "github.com/google/uuid" @@ -171,6 +172,322 @@ func TestCreateVolume(t *testing.T) { mockCtl.Finish() }, }, + { + name: "Success: avoiding GID collision", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + driver := &Driver{ + endpoint: endpoint, + cloud: mockCloud, + gidAllocator: NewGidAllocator(mockCloud), + } + + 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: "1003", + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + accessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + } + accessPoints := []*cloud.AccessPoint{ + { + AccessPointId: apId, + FileSystemId: fsId, + PosixUser: &cloud.PosixUser{ + Gid: 1003, + Uid: 1003, + }, + }, + { + AccessPointId: apId, + FileSystemId: fsId, + PosixUser: &cloud.PosixUser{ + Gid: 1002, + Uid: 1002, + }, + }, + } + + 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()).Return(accessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.Uid != expectedGid { + t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) + } + if accessPointOpts.Gid != expectedGid { + t.Fatalf("Gid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Gid) + } + }) + + res, err := driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: reuse released GID", + testFunc: func(t *testing.T) { + mockCtl := gomock.NewController(t) + mockCloud := mocks.NewMockCloud(mockCtl) + + driver := &Driver{ + endpoint: endpoint, + cloud: mockCloud, + gidAllocator: NewGidAllocator(mockCloud), + } + + 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: "1004", + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + ap1 := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + PosixUser: &cloud.PosixUser{ + Gid: 1001, + Uid: 1001, + }, + } + ap2 := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + PosixUser: &cloud.PosixUser{ + Gid: 1002, + Uid: 1002, + }, + } + ap3 := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + PosixUser: &cloud.PosixUser{ + Gid: 1003, + Uid: 1003, + }, + } + ap4 := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + PosixUser: &cloud.PosixUser{ + Gid: 1004, + Uid: 1004, + }, + } + + // Let allocator jump over some GIDS. + accessPoints := []*cloud.AccessPoint{ap3, ap4} + var expectedGid int64 = 1002 // 1003 and 1004 is taken. + + 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()).Return(ap2, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.Uid != expectedGid { + t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) + } + if accessPointOpts.Gid != expectedGid { + t.Fatalf("Gid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Gid) + } + }) + + res, err := driver.CreateVolume(ctx, req) + + // 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. + + 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()).Return(ap3, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.Uid != expectedGid { + t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) + } + if accessPointOpts.Gid != expectedGid { + t.Fatalf("Gid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Gid) + } + }) + + res, err = driver.CreateVolume(ctx, req) + //// + accessPoints = []*cloud.AccessPoint{ap1, ap4} + + 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()).Return(ap2, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + if accessPointOpts.Uid != expectedGid { + t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) + } + if accessPointOpts.Gid != expectedGid { + t.Fatalf("Gid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Gid) + } + }) + + res, err = driver.CreateVolume(ctx, req) + + if err != nil { + t.Fatalf("CreateVolume failed: %v", err) + } + + if res.Volume == nil { + t.Fatal("Volume is nil") + } + + if res.Volume.VolumeId != volumeId { + t.Fatalf("Volume Id mismatched. Expected: %v, Actual: %v", volumeId, res.Volume.VolumeId) + } + mockCtl.Finish() + }, + }, + { + name: "Success: 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(mockCloud), + } + + 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: "1200", + }, + } + + ctx := context.Background() + fileSystem := &cloud.FileSystem{ + FileSystemId: fsId, + } + + accessPoints := []*cloud.AccessPoint{} + for i := 0; i < 119; i++ { + gidMax, err := strconv.Atoi(req.Parameters[GidMax]) + if err != nil { + t.Fatalf("Failed to convert GidMax Parameter to int.") + } + userGid := gidMax - i + ap := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + PosixUser: &cloud.PosixUser{ + Gid: int64(userGid), + Uid: int64(userGid), + }, + } + accessPoints = append(accessPoints, ap) + } + + lastAccessPoint := &cloud.AccessPoint{ + AccessPointId: apId, + FileSystemId: fsId, + PosixUser: &cloud.PosixUser{ + Gid: 1081, + Uid: 1081, + }, + } + + expectedGid := 1081 + 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()).Return(lastAccessPoint, nil). + Do(func(ctx context.Context, volumeName string, accessPointOpts *cloud.AccessPointOptions) { + 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 + + // Allocate last available GID + _, err = driver.CreateVolume(ctx, req) + if err != nil { + t.Fatalf("CreateVolume failed.") + } + + accessPoints = append(accessPoints, lastAccessPoint) + 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()).Return(lastAccessPoint, nil).AnyTimes() + + // All 120 GIDs are taken now, internal limit should take effect causing CreateVolume to fail. + _, err = driver.CreateVolume(ctx, req) + if err == nil { + t.Fatalf("CreateVolume should have failed.") + } + mockCtl.Finish() + }, + }, { name: "Success: Normal flow", testFunc: func(t *testing.T) {