From 58c8272befbef724d548580049a1386b33ca1d17 Mon Sep 17 00:00:00 2001 From: Oscar Torreno Date: Thu, 4 Jan 2024 00:35:20 +0100 Subject: [PATCH] Refactor re-use Access Point When reuse access point flag is set to true, there is no point in doing a lot of work that is aimed at preparing the options for the createAccessPoint call (e.g. allocating a GID). We can save quite some time and quite some API calls if the AP with the same clientToken is found. The check for the AP existence has been moved from the CreateAccessPoint function one level up. The controller in this case checks if there is an access point with such client token, if so, it proceeds to return a CreateVolumeResponse. If it is not found, then it follows the usual flow. --- pkg/cloud/cloud.go | 35 +--- pkg/cloud/cloud_test.go | 181 +++++++++++++------- pkg/cloud/fakes.go | 10 +- pkg/driver/controller.go | 290 ++++++++++++++++++--------------- pkg/driver/controller_test.go | 82 +++++----- pkg/driver/mocks/mock_cloud.go | 31 +++- 6 files changed, 355 insertions(+), 274 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 5220ef51d..240b80202 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -97,9 +97,10 @@ type Efs interface { type Cloud interface { GetMetadata() MetadataService - CreateAccessPoint(ctx context.Context, clientToken string, accessPointOpts *AccessPointOptions, reuseAccessPoint bool) (accessPoint *AccessPoint, err error) + CreateAccessPoint(ctx context.Context, clientToken string, accessPointOpts *AccessPointOptions) (accessPoint *AccessPoint, err error) DeleteAccessPoint(ctx context.Context, accessPointId string) (err error) DescribeAccessPoint(ctx context.Context, accessPointId string) (accessPoint *AccessPoint, err error) + FindAccessPointByClientToken(ctx context.Context, clientToken, fileSystemId string) (accessPoint *AccessPoint, err error) ListAccessPoints(ctx context.Context, fileSystemId string) (accessPoints []*AccessPoint, err error) DescribeFileSystem(ctx context.Context, fileSystemId string) (fs *FileSystem, err error) DescribeMountTargets(ctx context.Context, fileSystemId, az string) (fs *MountTarget, err error) @@ -164,26 +165,8 @@ func (c *cloud) GetMetadata() MetadataService { return c.metadata } -func (c *cloud) CreateAccessPoint(ctx context.Context, clientToken string, accessPointOpts *AccessPointOptions, reuseAccessPoint bool) (accessPoint *AccessPoint, err error) { +func (c *cloud) CreateAccessPoint(ctx context.Context, clientToken string, accessPointOpts *AccessPointOptions) (accessPoint *AccessPoint, err error) { efsTags := parseEfsTags(accessPointOpts.Tags) - - //if reuseAccessPoint is true, check for AP with same Root Directory exists in efs - // if found reuse that AP - if reuseAccessPoint { - existingAP, err := c.findAccessPointByClientToken(ctx, clientToken, accessPointOpts) - if err != nil { - return nil, fmt.Errorf("failed to find access point: %v", err) - } - if existingAP != nil { - //AP path already exists - klog.V(2).Infof("Existing AccessPoint found : %+v", existingAP) - return &AccessPoint{ - AccessPointId: existingAP.AccessPointId, - FileSystemId: existingAP.FileSystemId, - CapacityGiB: accessPointOpts.CapacityGiB, - }, nil - } - } createAPInput := &efs.CreateAccessPointInput{ ClientToken: &clientToken, FileSystemId: &accessPointOpts.FileSystemId, @@ -262,22 +245,22 @@ func (c *cloud) DescribeAccessPoint(ctx context.Context, accessPointId string) ( }, nil } -func (c *cloud) findAccessPointByClientToken(ctx context.Context, clientToken string, accessPointOpts *AccessPointOptions) (accessPoint *AccessPoint, err error) { - klog.V(5).Infof("AccessPointOptions to find AP : %+v", accessPointOpts) +func (c *cloud) FindAccessPointByClientToken(ctx context.Context, clientToken, fileSystemId string) (accessPoint *AccessPoint, err error) { + klog.V(5).Infof("Filesystem ID to find AP : %+v", fileSystemId) klog.V(2).Infof("ClientToken to find AP : %s", clientToken) describeAPInput := &efs.DescribeAccessPointsInput{ - FileSystemId: &accessPointOpts.FileSystemId, + FileSystemId: &fileSystemId, MaxResults: aws.Int64(AccessPointPerFsLimit), } res, err := c.efs.DescribeAccessPointsWithContext(ctx, describeAPInput) if err != nil { if isAccessDenied(err) { - return + return nil, ErrAccessDenied } if isFileSystemNotFound(err) { - return + return nil, ErrNotFound } - err = fmt.Errorf("failed to list Access Points of efs = %s : %v", accessPointOpts.FileSystemId, err) + err = fmt.Errorf("failed to list Access Points of efs = %s : %v", fileSystemId, err) return } for _, ap := range res.AccessPoints { diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go index e16cc5c63..48651edbf 100644 --- a/pkg/cloud/cloud_test.go +++ b/pkg/cloud/cloud_test.go @@ -35,7 +35,7 @@ func TestCreateAccessPoint(t *testing.T) { testFunc func(t *testing.T) }{ { - name: "Success - AP does not exist", + name: "Success", testFunc: func(t *testing.T) { mockCtl := gomock.NewController(t) mockEfs := mocks.NewMockEfs(mockCtl) @@ -74,63 +74,9 @@ func TestCreateAccessPoint(t *testing.T) { }, } - describeAPOutput := &efs.DescribeAccessPointsOutput{ - AccessPoints: nil, - } - ctx := context.Background() - mockEfs.EXPECT().DescribeAccessPointsWithContext(gomock.Eq(ctx), gomock.Any()).Return(describeAPOutput, nil) mockEfs.EXPECT().CreateAccessPointWithContext(gomock.Eq(ctx), gomock.Any()).Return(output, nil) - res, err := c.CreateAccessPoint(ctx, clientToken, req, true) - - if err != nil { - t.Fatalf("CreateAccessPointFailed is failed: %v", err) - } - - if res == nil { - t.Fatal("Result is nil") - } - - if accessPointId != res.AccessPointId { - t.Fatalf("AccessPointId mismatched. Expected: %v, Actual: %v", accessPointId, res.AccessPointId) - } - - if fsId != res.FileSystemId { - t.Fatalf("FileSystemId mismatched. Expected: %v, Actual: %v", fsId, res.FileSystemId) - } - mockCtl.Finish() - }, - }, - { - name: "Success - AP already exists", - testFunc: func(t *testing.T) { - mockCtl := gomock.NewController(t) - mockEfs := mocks.NewMockEfs(mockCtl) - c := &cloud{ - efs: mockEfs, - } - - tags := make(map[string]string) - tags["cluster"] = "efs" - - req := &AccessPointOptions{ - FileSystemId: fsId, - Uid: uid, - Gid: gid, - DirectoryPerms: directoryPerms, - DirectoryPath: directoryPath, - Tags: tags, - } - - describeAPOutput := &efs.DescribeAccessPointsOutput{ - AccessPoints: []*efs.AccessPointDescription{ - {AccessPointId: aws.String(accessPointId), FileSystemId: aws.String(fsId), ClientToken: aws.String(clientToken), RootDirectory: &efs.RootDirectory{Path: aws.String(directoryPath)}, Tags: []*efs.Tag{{Key: aws.String(PvcNameTagKey), Value: aws.String(volName)}}}, - }, - } - - ctx := context.Background() - mockEfs.EXPECT().DescribeAccessPointsWithContext(gomock.Eq(ctx), gomock.Any()).Return(describeAPOutput, nil) - res, err := c.CreateAccessPoint(ctx, clientToken, req, true) + res, err := c.CreateAccessPoint(ctx, clientToken, req) if err != nil { t.Fatalf("CreateAccessPointFailed is failed: %v", err) @@ -164,14 +110,10 @@ func TestCreateAccessPoint(t *testing.T) { DirectoryPerms: directoryPerms, DirectoryPath: directoryPath, } - describeAPOutput := &efs.DescribeAccessPointsOutput{ - AccessPoints: nil, - } ctx := context.Background() - mockEfs.EXPECT().DescribeAccessPointsWithContext(gomock.Eq(ctx), gomock.Any()).Return(describeAPOutput, nil) mockEfs.EXPECT().CreateAccessPointWithContext(gomock.Eq(ctx), gomock.Any()).Return(nil, errors.New("CreateAccessPointWithContext failed")) - _, err := c.CreateAccessPoint(ctx, clientToken, req, true) + _, err := c.CreateAccessPoint(ctx, clientToken, req) if err == nil { t.Fatalf("CreateAccessPoint did not fail") } @@ -195,7 +137,7 @@ func TestCreateAccessPoint(t *testing.T) { ctx := context.Background() mockEfs.EXPECT().CreateAccessPointWithContext(gomock.Eq(ctx), gomock.Any()).Return(nil, awserr.New(AccessDeniedException, "Access Denied", errors.New("Access Denied"))) - _, err := c.CreateAccessPoint(ctx, clientToken, req, false) + _, err := c.CreateAccessPoint(ctx, clientToken, req) if err == nil { t.Fatalf("CreateAccessPoint did not fail") } @@ -551,6 +493,119 @@ func TestDescribeAccessPoint(t *testing.T) { } } +func TestFindAccessPointByClientToken(t *testing.T) { + var ( + fsId = "fs-abcd1234" + accessPointId = "ap-abc123" + clientToken = "token" + path = "/myDir" + Gid int64 = 1000 + Uid int64 = 1000 + ) + testCases := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Success - clientToken found", + testFunc: func(t *testing.T) { + mockctl := gomock.NewController(t) + mockEfs := mocks.NewMockEfs(mockctl) + c := &cloud{efs: mockEfs} + + output := &efs.DescribeAccessPointsOutput{ + AccessPoints: []*efs.AccessPointDescription{ + { + AccessPointId: aws.String(accessPointId), + FileSystemId: aws.String(fsId), + ClientToken: aws.String(clientToken), + RootDirectory: &efs.RootDirectory{ + Path: aws.String(path), + }, + PosixUser: &efs.PosixUser{ + Gid: aws.Int64(Gid), + Uid: aws.Int64(Uid), + }, + }, + }, + NextToken: nil, + } + + ctx := context.Background() + mockEfs.EXPECT().DescribeAccessPointsWithContext(gomock.Eq(ctx), gomock.Any()).Return(output, nil) + res, err := c.FindAccessPointByClientToken(ctx, clientToken, fsId) + if err != nil { + t.Fatalf("Find Access Point by Client Token failed: %v", err) + } + + if res == nil { + t.Fatal("Result is nil") + } + + mockctl.Finish() + }, + }, + { + name: "Success - nil result if clientToken is not found", + testFunc: func(t *testing.T) { + mockctl := gomock.NewController(t) + mockEfs := mocks.NewMockEfs(mockctl) + c := &cloud{efs: mockEfs} + + output := &efs.DescribeAccessPointsOutput{ + AccessPoints: []*efs.AccessPointDescription{ + { + AccessPointId: aws.String(accessPointId), + FileSystemId: aws.String(fsId), + ClientToken: aws.String("differentToken"), + RootDirectory: &efs.RootDirectory{ + Path: aws.String(path), + }, + PosixUser: &efs.PosixUser{ + Gid: aws.Int64(Gid), + Uid: aws.Int64(Uid), + }, + }, + }, + NextToken: nil, + } + + ctx := context.Background() + mockEfs.EXPECT().DescribeAccessPointsWithContext(gomock.Eq(ctx), gomock.Any()).Return(output, nil) + res, err := c.FindAccessPointByClientToken(ctx, clientToken, fsId) + if err != nil { + t.Fatalf("Find Access Point by Client Token failed: %v", err) + } + + if res != nil { + t.Fatal("Result should be nil. No access point with the specified token") + } + + mockctl.Finish() + }, + }, + { + name: "Fail - Access Denied", + testFunc: func(t *testing.T) { + mockctl := gomock.NewController(t) + mockEfs := mocks.NewMockEfs(mockctl) + c := &cloud{efs: mockEfs} + ctx := context.Background() + mockEfs.EXPECT().DescribeAccessPointsWithContext(gomock.Eq(ctx), gomock.Any()).Return(nil, awserr.New(AccessDeniedException, "Access Denied", errors.New("Access Denied"))) + _, err := c.FindAccessPointByClientToken(ctx, clientToken, fsId) + if err == nil { + t.Fatalf("Find Access Point by Client Token should have failed: %v", err) + } + + mockctl.Finish() + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, tc.testFunc) + } +} + func TestListAccessPoints(t *testing.T) { var ( fsId = "fs-abcd1234" @@ -1024,7 +1079,7 @@ func Test_findAccessPointByPath(t *testing.T) { tt.prepare(mockEfs) } - gotAccessPoint, err := c.findAccessPointByClientToken(ctx, tt.args.clientToken, tt.args.accessPointOpts) + gotAccessPoint, err := c.FindAccessPointByClientToken(ctx, tt.args.clientToken, tt.args.accessPointOpts.FileSystemId) if (err != nil) != tt.wantErr { t.Errorf("findAccessPointByClientToken() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/cloud/fakes.go b/pkg/cloud/fakes.go index 49953665b..8f910ca88 100644 --- a/pkg/cloud/fakes.go +++ b/pkg/cloud/fakes.go @@ -27,7 +27,7 @@ func (c *FakeCloudProvider) GetMetadata() MetadataService { return c.m } -func (c *FakeCloudProvider) CreateAccessPoint(ctx context.Context, clientToken string, accessPointOpts *AccessPointOptions, usePvcName bool) (accessPoint *AccessPoint, err error) { +func (c *FakeCloudProvider) CreateAccessPoint(ctx context.Context, clientToken string, accessPointOpts *AccessPointOptions) (accessPoint *AccessPoint, err error) { ap, exists := c.accessPoints[clientToken] if exists { if accessPointOpts.CapacityGiB == ap.CapacityGiB { @@ -98,6 +98,14 @@ func (c *FakeCloudProvider) DescribeMountTargets(ctx context.Context, fileSystem return nil, ErrNotFound } +func (c *FakeCloudProvider) FindAccessPointByClientToken(ctx context.Context, clientToken, fileSystemId string) (accessPoint *AccessPoint, err error) { + if ap, exists := c.accessPoints[clientToken]; exists { + return ap, nil + } else { + return nil, nil + } +} + func (c *FakeCloudProvider) ListAccessPoints(ctx context.Context, fileSystemId string) ([]*AccessPoint, error) { accessPoints := []*AccessPoint{ c.accessPoints[fileSystemId], diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index be4630e63..86c6baaf1 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -144,21 +144,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", ProvisioningMode) } - // Create tags - tags := map[string]string{ - DefaultTagKey: DefaultTagValue, - } - - // Append input tags to default tag - if len(d.tags) != 0 { - for k, v := range d.tags { - tags[k] = v - } - } - accessPointsOptions := &cloud.AccessPointOptions{ CapacityGiB: volSize, - Tags: tags, } if value, ok := volumeParams[FsId]; ok { @@ -170,162 +157,197 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", FsId) } - uid = -1 - if value, ok := volumeParams[Uid]; ok { - uid, err = strconv.ParseInt(value, 10, 64) - if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", Uid, err) - } - if uid < 0 { - return nil, status.Errorf(codes.InvalidArgument, "%v must be greater or equal than 0", Uid) - } + localCloud, roleArn, crossAccountDNSEnabled, err = getCloud(req.GetSecrets(), d) + if err != nil { + return nil, err } - gid = -1 - if value, ok := volumeParams[Gid]; ok { - gid, err = strconv.ParseInt(value, 10, 64) + var accessPoint *cloud.AccessPoint + //if reuseAccessPoint is true, check for AP with same Root Directory exists in efs + // if found reuse that AP + if reuseAccessPoint { + existingAP, err := localCloud.FindAccessPointByClientToken(ctx, clientToken, accessPointsOptions.FileSystemId) if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", Gid, err) + return nil, fmt.Errorf("failed to find access point: %v", err) } - if gid < 0 { - return nil, status.Errorf(codes.InvalidArgument, "%v must be greater or equal than 0", Gid) + if existingAP != nil { + //AP path already exists + klog.V(2).Infof("Existing AccessPoint found : %+v", existingAP) + accessPoint = &cloud.AccessPoint{ + AccessPointId: existingAP.AccessPointId, + FileSystemId: existingAP.FileSystemId, + CapacityGiB: accessPointsOptions.CapacityGiB, + } } } - if value, ok := volumeParams[GidMin]; ok { - gidMin, err = strconv.ParseInt(value, 10, 64) - if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", GidMin, err) - } - if gidMin <= 0 { - return nil, status.Errorf(codes.InvalidArgument, "%v must be greater than 0", GidMin) + if accessPoint == nil { + // Create tags + tags := map[string]string{ + DefaultTagKey: DefaultTagValue, } - } - if value, ok := volumeParams[GidMax]; ok { - // Ensure GID min is provided with GID max - if gidMin == 0 { - return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", GidMin) + // Append input tags to default tag + if len(d.tags) != 0 { + for k, v := range d.tags { + tags[k] = v + } } - gidMax, err = strconv.ParseInt(value, 10, 64) - if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", GidMax, err) + + accessPointsOptions.Tags = tags + + uid = -1 + if value, ok := volumeParams[Uid]; ok { + uid, err = strconv.ParseInt(value, 10, 64) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", Uid, err) + } + if uid < 0 { + return nil, status.Errorf(codes.InvalidArgument, "%v must be greater or equal than 0", Uid) + } } - if gidMax <= gidMin { - return nil, status.Errorf(codes.InvalidArgument, "%v must be greater than %v", GidMax, GidMin) + + gid = -1 + if value, ok := volumeParams[Gid]; ok { + gid, err = strconv.ParseInt(value, 10, 64) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", Gid, err) + } + if uid < 0 { + return nil, status.Errorf(codes.InvalidArgument, "%v must be greater or equal than 0", Gid) + } } - } else { - // Ensure GID max is provided with GID min - if gidMin != 0 { - return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", GidMax) + + if value, ok := volumeParams[GidMin]; ok { + gidMin, err = strconv.ParseInt(value, 10, 64) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", GidMin, err) + } + if gidMin <= 0 { + return nil, status.Errorf(codes.InvalidArgument, "%v must be greater than 0", GidMin) + } } - } - // Assign default GID ranges if not provided - if gidMin == 0 && gidMax == 0 { - gidMin = DefaultGidMin - gidMax = DefaultGidMax - } + if value, ok := volumeParams[GidMax]; ok { + // Ensure GID min is provided with GID max + if gidMin == 0 { + return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", GidMin) + } + gidMax, err = strconv.ParseInt(value, 10, 64) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "Failed to parse invalid %v: %v", GidMax, err) + } + if gidMax <= gidMin { + return nil, status.Errorf(codes.InvalidArgument, "%v must be greater than %v", GidMax, GidMin) + } + } else { + // Ensure GID max is provided with GID min + if gidMin != 0 { + return nil, status.Errorf(codes.InvalidArgument, "Missing %v parameter", GidMax) + } + } - if value, ok := volumeParams[DirectoryPerms]; ok { - accessPointsOptions.DirectoryPerms = value - } + // Assign default GID ranges if not provided + if gidMin == 0 && gidMax == 0 { + gidMin = DefaultGidMin + gidMax = DefaultGidMax + } - // Storage class parameter `az` will be used to fetch preferred mount target for cross account mount. - // If the `az` storage class parameter is not provided, a random mount target will be picked for mounting. - // This storage class parameter different from `az` mount option provided by efs-utils https://github.com/aws/efs-utils/blob/v1.31.1/src/mount_efs/__init__.py#L195 - // The `az` mount option provided by efs-utils is used for cross az mount or to provide az of efs one zone file system mount within the same aws-account. - // To make use of the `az` mount option, add it under storage class's `mountOptions` section. https://kubernetes.io/docs/concepts/storage/storage-classes/#mount-options - if value, ok := volumeParams[AzName]; ok { - azName = value - } + if value, ok := volumeParams[DirectoryPerms]; ok { + accessPointsOptions.DirectoryPerms = value + } - localCloud, roleArn, crossAccountDNSEnabled, err = getCloud(req.GetSecrets(), d) - if err != nil { - return nil, err - } + // Storage class parameter `az` will be used to fetch preferred mount target for cross account mount. + // If the `az` storage class parameter is not provided, a random mount target will be picked for mounting. + // This storage class parameter different from `az` mount option provided by efs-utils https://github.com/aws/efs-utils/blob/v1.31.1/src/mount_efs/__init__.py#L195 + // The `az` mount option provided by efs-utils is used for cross az mount or to provide az of efs one zone file system mount within the same aws-account. + // To make use of the `az` mount option, add it under storage class's `mountOptions` section. https://kubernetes.io/docs/concepts/storage/storage-classes/#mount-options + if value, ok := volumeParams[AzName]; ok { + azName = value + } - // Check if file system exists. Describe FS or List APs handle appropriate error codes - // With dynamic uid/gid provisioning we can save a call to describe FS, as list APs fails if FS ID does not exist - var accessPoints []*cloud.AccessPoint - if uid == -1 || gid == -1 { - accessPoints, err = localCloud.ListAccessPoints(ctx, accessPointsOptions.FileSystemId) - } else { - _, err = localCloud.DescribeFileSystem(ctx, accessPointsOptions.FileSystemId) - } - if err != nil { - if err == cloud.ErrAccessDenied { - return nil, status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) + // Check if file system exists. Describe FS or List APs handle appropriate error codes + // With dynamic uid/gid provisioning we can save a call to describe FS, as list APs fails if FS ID does not exist + var accessPoints []*cloud.AccessPoint + if uid == -1 || gid == -1 { + accessPoints, err = localCloud.ListAccessPoints(ctx, accessPointsOptions.FileSystemId) + } else { + _, err = localCloud.DescribeFileSystem(ctx, accessPointsOptions.FileSystemId) } - if err == cloud.ErrNotFound { - return nil, status.Errorf(codes.InvalidArgument, "File System does not exist: %v", err) + if err != nil { + if err == cloud.ErrAccessDenied { + return nil, status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) + } + if err == cloud.ErrNotFound { + return nil, status.Errorf(codes.InvalidArgument, "File System does not exist: %v", err) + } + return nil, status.Errorf(codes.Internal, "Failed to fetch Access Points or Describe File System: %v", err) } - return nil, status.Errorf(codes.Internal, "Failed to fetch Access Points or Describe File System: %v", err) - } - var allocatedGid int64 - if uid == -1 || gid == -1 { - allocatedGid, err = d.gidAllocator.getNextGid(accessPointsOptions.FileSystemId, accessPoints, gidMin, gidMax) - if err != nil { - return nil, err + var allocatedGid int64 + if uid == -1 || gid == -1 { + allocatedGid, err = d.gidAllocator.getNextGid(accessPointsOptions.FileSystemId, accessPoints, gidMin, gidMax) + if err != nil { + return nil, err + } + } + if uid == -1 { + uid = allocatedGid + } + if gid == -1 { + gid = allocatedGid } - } - if uid == -1 { - uid = allocatedGid - } - if gid == -1 { - gid = allocatedGid - } - if value, ok := volumeParams[BasePath]; ok { - basePath = value - } + if value, ok := volumeParams[BasePath]; ok { + basePath = value + } - rootDirName := volName - // Check if a custom structure should be imposed on the access point directory - if value, ok := volumeParams[SubPathPattern]; ok { - // Try and construct the root directory and check it only contains supported components - val, err := interpolateRootDirectoryName(value, volumeParams) - if err == nil { - klog.Infof("Using user-specified structure for access point directory.") - rootDirName = val - if value, ok := volumeParams[EnsureUniqueDirectory]; ok { - if ensureUniqueDirectory, err := strconv.ParseBool(value); !ensureUniqueDirectory && err == nil { - klog.Infof("Not appending PVC UID to path.") + rootDirName := volName + // Check if a custom structure should be imposed on the access point directory + if value, ok := volumeParams[SubPathPattern]; ok { + // Try and construct the root directory and check it only contains supported components + val, err := interpolateRootDirectoryName(value, volumeParams) + if err == nil { + klog.Infof("Using user-specified structure for access point directory.") + rootDirName = val + if value, ok := volumeParams[EnsureUniqueDirectory]; ok { + if ensureUniqueDirectory, err := strconv.ParseBool(value); !ensureUniqueDirectory && err == nil { + klog.Infof("Not appending PVC UID to path.") + } else { + klog.Infof("Appending PVC UID to path.") + rootDirName = fmt.Sprintf("%s-%s", val, uuid.New().String()) + } } else { klog.Infof("Appending PVC UID to path.") rootDirName = fmt.Sprintf("%s-%s", val, uuid.New().String()) } } else { - klog.Infof("Appending PVC UID to path.") - rootDirName = fmt.Sprintf("%s-%s", val, uuid.New().String()) + return nil, err } } else { - return nil, err + klog.Infof("Using PV name for access point directory.") } - } else { - klog.Infof("Using PV name for access point directory.") - } - rootDir := path.Join("/", basePath, rootDirName) - if ok, err := validateEfsPathRequirements(rootDir); !ok { - return nil, err - } - klog.Infof("Using %v as the access point directory.", rootDir) + rootDir := path.Join("/", basePath, rootDirName) + if ok, err := validateEfsPathRequirements(rootDir); !ok { + return nil, err + } + klog.Infof("Using %v as the access point directory.", rootDir) - accessPointsOptions.Uid = uid - accessPointsOptions.Gid = gid - accessPointsOptions.DirectoryPath = rootDir + accessPointsOptions.Uid = uid + accessPointsOptions.Gid = gid + accessPointsOptions.DirectoryPath = rootDir - accessPointId, err := localCloud.CreateAccessPoint(ctx, clientToken, accessPointsOptions, reuseAccessPoint) - if err != nil { - if err == cloud.ErrAccessDenied { - return nil, status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) - } - if err == cloud.ErrAlreadyExists { - return nil, status.Errorf(codes.AlreadyExists, "Access Point already exists") + accessPoint, err = localCloud.CreateAccessPoint(ctx, clientToken, accessPointsOptions) + if err != nil { + if err == cloud.ErrAccessDenied { + return nil, status.Errorf(codes.Unauthenticated, "Access Denied. Please ensure you have the right AWS permissions: %v", err) + } + if err == cloud.ErrAlreadyExists { + return nil, status.Errorf(codes.AlreadyExists, "Access Point already exists") + } + return nil, status.Errorf(codes.Internal, "Failed to create Access point in File System %v : %v", accessPointsOptions.FileSystemId, err) } - return nil, status.Errorf(codes.Internal, "Failed to create Access point in File System %v : %v", accessPointsOptions.FileSystemId, err) } volContext := map[string]string{} @@ -352,7 +374,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) return &csi.CreateVolumeResponse{ Volume: &csi.Volume{ CapacityBytes: volSize, - VolumeId: accessPointsOptions.FileSystemId + "::" + accessPointId.AccessPointId, + VolumeId: accessPointsOptions.FileSystemId + "::" + accessPoint.AccessPointId, VolumeContext: volContext, }, }, nil diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 3725ce026..b625f508b 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -79,8 +79,8 @@ func TestCreateVolume(t *testing.T) { FileSystemId: fsId, } 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, accessPointsOptions *cloud.AccessPointOptions, reuseAccessPointName bool) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Eq(volumeName), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointsOptions *cloud.AccessPointOptions) { if accessPointsOptions.Uid != 1000 { t.Fatalf("Uid mismatched. Expected: %v, actual: %v", 1000, accessPointsOptions.Uid) } @@ -146,8 +146,8 @@ func TestCreateVolume(t *testing.T) { FileSystemId: fsId, } mockCloud.EXPECT().DescribeFileSystem(gomock.Eq(ctx), gomock.Any()).Return(fileSystem, nil) - 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.Uid != 1000 { t.Fatalf("Uid mismatched. Expected: %v, actual: %v", 1000, accessPointOpts.Uid) } @@ -228,8 +228,8 @@ func TestCreateVolume(t *testing.T) { var expectedGid int64 = 1003 //1001 and 1002 are taken, next available is 1003 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). - Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.Uid != expectedGid { t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) } @@ -323,8 +323,8 @@ func TestCreateVolume(t *testing.T) { var expectedGid int64 = 1004 // 1001-1003 is taken. 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). - Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(ap2, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.Uid != expectedGid { t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) } @@ -340,8 +340,8 @@ func TestCreateVolume(t *testing.T) { expectedGid = 1001 // 1001 is now free and lowest possible, if no GID return would happen allocator would pick 1005. mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), false).Return(ap3, nil). - Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(ap3, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.Uid != expectedGid { t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) } @@ -357,8 +357,8 @@ func TestCreateVolume(t *testing.T) { expectedGid = 1002 // 1001 and 1004 are now taken, lowest available is 1002 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). - Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(ap2, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.Uid != expectedGid { t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) } @@ -444,8 +444,8 @@ func TestCreateVolume(t *testing.T) { expectedGid := 2000 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). - Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, reuseAccessPointName bool) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(lastAccessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.Uid != int64(expectedGid) { t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) } @@ -512,8 +512,8 @@ func TestCreateVolume(t *testing.T) { expectedGid := 1000 // Allocator should pick lowest available GID 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.Uid != int64(expectedGid) { t.Fatalf("Uid mismatched. Expected: %v, actual: %v", expectedGid, accessPointOpts.Uid) } @@ -574,7 +574,7 @@ func TestCreateVolume(t *testing.T) { } accessPoints := []*cloud.AccessPoint{accessPoint} mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Eq(volumeName), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Eq(volumeName), gomock.Any()).Return(accessPoint, nil) res, err := driver.CreateVolume(ctx, req) @@ -630,7 +630,7 @@ func TestCreateVolume(t *testing.T) { } accessPoints := []*cloud.AccessPoint{accessPoint} mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) res, err := driver.CreateVolume(ctx, req) @@ -689,7 +689,7 @@ func TestCreateVolume(t *testing.T) { } accessPoints := []*cloud.AccessPoint{accessPoint} mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) res, err := driver.CreateVolume(ctx, req) @@ -748,7 +748,7 @@ func TestCreateVolume(t *testing.T) { } accessPoints := []*cloud.AccessPoint{accessPoint} mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil) res, err := driver.CreateVolume(ctx, req) @@ -810,9 +810,7 @@ func TestCreateVolume(t *testing.T) { Uid: 1000, }, } - accessPoints := []*cloud.AccessPoint{accessPoint} - mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(accessPoints, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Eq(get64LenHash(pvcNameVal)), gomock.Any(), gomock.Any()).Return(accessPoint, nil) + mockCloud.EXPECT().FindAccessPointByClientToken(gomock.Eq(ctx), gomock.Any(), gomock.Eq(fsId)).Return(accessPoint, nil) res, err := driver.CreateVolume(ctx, req) @@ -875,8 +873,8 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil) - 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, reuseAccessPoint bool) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", directoryCreated, @@ -943,8 +941,8 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil) - 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", directoryCreated, @@ -1014,8 +1012,8 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil) - 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", directoryCreated, @@ -1085,8 +1083,8 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil) - 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.DirectoryPath != directoryCreated { t.Fatalf("Root directory mismatch. Expected: %v, actual: %v", directoryCreated, @@ -1155,8 +1153,8 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil) - 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", directoryCreated, @@ -1220,8 +1218,8 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil) - 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.DirectoryPath != "/" { t.Fatalf("Root directory mismatch. Expected: %v, actual: %v", "/", @@ -1286,8 +1284,8 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil) - 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if accessPointOpts.DirectoryPath != "/" { t.Fatalf("Root directory mismatch. Expected: %v, actual: %v", "/", @@ -1354,8 +1352,8 @@ func TestCreateVolume(t *testing.T) { } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return(nil, nil) - 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) { + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(accessPoint, nil). + Do(func(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) { if !verifyPathWhenUUIDIncluded(accessPointOpts.DirectoryPath, directoryCreated) { t.Fatalf("Root directory mismatch. Expected: %v (with UID appended), actual: %v", directoryCreated, @@ -2366,7 +2364,7 @@ func TestCreateVolume(t *testing.T) { ctx := context.Background() mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return([]*cloud.AccessPoint{}, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("CreateAccessPoint call failed")) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(nil, errors.New("CreateAccessPoint call failed")) _, err := driver.CreateVolume(ctx, req) if err == nil { t.Fatal("CreateVolume did not fail") @@ -2405,7 +2403,7 @@ func TestCreateVolume(t *testing.T) { ctx := context.Background() mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return([]*cloud.AccessPoint{}, nil) - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, cloud.ErrAccessDenied) + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(nil, cloud.ErrAccessDenied) _, err := driver.CreateVolume(ctx, req) if err == nil { t.Fatal("CreateVolume did not fail") @@ -2460,7 +2458,7 @@ func TestCreateVolume(t *testing.T) { }, } mockCloud.EXPECT().ListAccessPoints(gomock.Eq(ctx), gomock.Any()).Return([]*cloud.AccessPoint{ap1, ap2}, nil).AnyTimes() - mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any(), gomock.Any()).Return(ap2, nil).AnyTimes() + mockCloud.EXPECT().CreateAccessPoint(gomock.Eq(ctx), gomock.Any(), gomock.Any()).Return(ap2, nil).AnyTimes() var err error // All GIDs from available range are taken, CreateVolume should fail. diff --git a/pkg/driver/mocks/mock_cloud.go b/pkg/driver/mocks/mock_cloud.go index eacf69fae..96cef7e69 100644 --- a/pkg/driver/mocks/mock_cloud.go +++ b/pkg/driver/mocks/mock_cloud.go @@ -162,18 +162,18 @@ func (m *MockCloud) EXPECT() *MockCloudMockRecorder { } // CreateAccessPoint mocks base method. -func (m *MockCloud) CreateAccessPoint(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions, usePvcName bool) (*cloud.AccessPoint, error) { +func (m *MockCloud) CreateAccessPoint(ctx context.Context, clientToken string, accessPointOpts *cloud.AccessPointOptions) (*cloud.AccessPoint, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateAccessPoint", ctx, clientToken, accessPointOpts, usePvcName) + ret := m.ctrl.Call(m, "CreateAccessPoint", ctx, clientToken, accessPointOpts) ret0, _ := ret[0].(*cloud.AccessPoint) ret1, _ := ret[1].(error) return ret0, ret1 } // CreateAccessPoint indicates an expected call of CreateAccessPoint. -func (mr *MockCloudMockRecorder) CreateAccessPoint(ctx, clientToken, accessPointOpts, usePvcName interface{}) *gomock.Call { +func (mr *MockCloudMockRecorder) CreateAccessPoint(ctx, clientToken, accessPointOpts interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateAccessPoint", reflect.TypeOf((*MockCloud)(nil).CreateAccessPoint), ctx, clientToken, accessPointOpts, usePvcName) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateAccessPoint", reflect.TypeOf((*MockCloud)(nil).CreateAccessPoint), ctx, clientToken, accessPointOpts) } // DeleteAccessPoint mocks base method. @@ -235,6 +235,21 @@ func (mr *MockCloudMockRecorder) DescribeMountTargets(ctx, fileSystemId, az inte return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeMountTargets", reflect.TypeOf((*MockCloud)(nil).DescribeMountTargets), ctx, fileSystemId, az) } +// FindAccessPointByClientToken mocks base method. +func (m *MockCloud) FindAccessPointByClientToken(ctx context.Context, clientToken, fileSystemId string) (*cloud.AccessPoint, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "FindAccessPointByClientToken", ctx, clientToken, fileSystemId) + ret0, _ := ret[0].(*cloud.AccessPoint) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// FindAccessPointByClientToken indicates an expected call of FindAccessPointByClientToken. +func (mr *MockCloudMockRecorder) FindAccessPointByClientToken(ctx, clientToken, fileSystemId interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FindAccessPointByClientToken", reflect.TypeOf((*MockCloud)(nil).FindAccessPointByClientToken), ctx, clientToken, fileSystemId) +} + // GetMetadata mocks base method. func (m *MockCloud) GetMetadata() cloud.MetadataService { m.ctrl.T.Helper() @@ -250,16 +265,16 @@ func (mr *MockCloudMockRecorder) GetMetadata() *gomock.Call { } // ListAccessPoints mocks base method. -func (m *MockCloud) ListAccessPoints(arg0 context.Context, arg1 string) ([]*cloud.AccessPoint, error) { +func (m *MockCloud) ListAccessPoints(ctx context.Context, fileSystemId string) ([]*cloud.AccessPoint, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListAccessPoints", arg0, arg1) + ret := m.ctrl.Call(m, "ListAccessPoints", ctx, fileSystemId) ret0, _ := ret[0].([]*cloud.AccessPoint) ret1, _ := ret[1].(error) return ret0, ret1 } // ListAccessPoints indicates an expected call of ListAccessPoints. -func (mr *MockCloudMockRecorder) ListAccessPoints(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockCloudMockRecorder) ListAccessPoints(ctx, fileSystemId interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAccessPoints", reflect.TypeOf((*MockCloud)(nil).ListAccessPoints), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListAccessPoints", reflect.TypeOf((*MockCloud)(nil).ListAccessPoints), ctx, fileSystemId) }