From c77f4e489f353980386068dbde700eb8d78dad0a Mon Sep 17 00:00:00 2001 From: Mikey032 <26899585+Mikey032@users.noreply.github.com> Date: Mon, 12 May 2025 22:08:42 +0200 Subject: [PATCH 1/2] support for the az multi parameter --- Dockerfile | 2 +- .../kubernetes/cross_account_mount/README.md | 2 + pkg/cloud/cloud.go | 37 ++++++++---- pkg/cloud/cloud_test.go | 19 ++++++ pkg/cloud/ec2_metadata.go | 16 ++++- pkg/cloud/ec2_metadata_test.go | 59 +++++++++++++++---- pkg/cloud/fakes.go | 7 +-- pkg/cloud/metadata.go | 14 ++++- pkg/driver/controller.go | 11 +++- pkg/driver/driver.go | 2 + pkg/driver/mocks/mock_cloud.go | 4 +- pkg/driver/node.go | 29 ++++++++- 12 files changed, 162 insertions(+), 40 deletions(-) diff --git a/Dockerfile b/Dockerfile index bc6959dae..6d2dd3160 100644 --- a/Dockerfile +++ b/Dockerfile @@ -63,7 +63,6 @@ COPY --from=rpm-provider /tmp/rpms/* /tmp/download/ # cd, ls, cat, vim, tcpdump, are for debugging RUN clean_install amazon-efs-utils true && \ clean_install crypto-policies true && \ - clean_install "openssl-3.0.8 openssl-libs-3.0.8" true && \ install_binary \ /usr/bin/cat \ /usr/bin/cd \ @@ -76,6 +75,7 @@ RUN clean_install amazon-efs-utils true && \ /usr/bin/mount \ /usr/bin/umount \ /sbin/mount.nfs4 \ + /usr/bin/openssl \ /usr/bin/sed \ /usr/bin/stat \ /usr/bin/stunnel \ diff --git a/examples/kubernetes/cross_account_mount/README.md b/examples/kubernetes/cross_account_mount/README.md index ae90a4ed7..37353434e 100644 --- a/examples/kubernetes/cross_account_mount/README.md +++ b/examples/kubernetes/cross_account_mount/README.md @@ -25,6 +25,8 @@ parameters: csi.storage.k8s.io/provisioner-secret-namespace: kube-system ``` +**Note**: driver version > 2.1.0 supports the `az: multi` option, which fetches EFS mount targets in all availability zones and consumes one for a pod in the same zone. + ### Prerequisite setup Lets say you have an EKS cluster in aws account `A` & you wish to mount your file system in another aws account `B` using aws-efs-csi-driver, you'll need to perform the following steps before you proceed with cross account mount between accounts `A` & `B` 1. Perform [vpc-peering](https://docs.aws.amazon.com/vpc/latest/peering/working-with-vpc-peering.html) between EKS cluster `vpc` in aws account `A` and EFS `vpc` in another aws account `B`. diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index f0bd50add..98e037701 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -20,12 +20,13 @@ import ( "context" "errors" "fmt" - - "github.com/aws/smithy-go" "math/rand" - "os" "time" + "os" + + "github.com/aws/smithy-go" + "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/credentials/stscreds" @@ -106,7 +107,7 @@ type Cloud interface { 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) + DescribeMountTargets(ctx context.Context, fileSystemId, az string) (fs []*MountTarget, err error) } type cloud struct { @@ -368,7 +369,7 @@ func (c *cloud) DescribeFileSystem(ctx context.Context, fileSystemId string) (fs }, nil } -func (c *cloud) DescribeMountTargets(ctx context.Context, fileSystemId, azName string) (fs *MountTarget, err error) { +func (c *cloud) DescribeMountTargets(ctx context.Context, fileSystemId, azName string) (fs []*MountTarget, err error) { describeMtInput := &efs.DescribeMountTargetsInput{FileSystemId: &fileSystemId} klog.V(5).Infof("Calling DescribeMountTargets with input: %+v", *describeMtInput) res, err := c.efs.DescribeMountTargets(ctx, describeMtInput, func(o *efs.Options) { @@ -395,6 +396,20 @@ func (c *cloud) DescribeMountTargets(ctx context.Context, fileSystemId, azName s return nil, fmt.Errorf("No mount target for file system %v is in available state. Please retry in 5 minutes.", fileSystemId) } + var returneMountTargets []*MountTarget + if azName == "multi" { + // Return all available mount targets + for _, mt := range availableMountTargets { + returneMountTargets = append(returneMountTargets, &MountTarget{ + AZName: *mt.AvailabilityZoneName, + AZId: *mt.AvailabilityZoneId, + MountTargetId: *mt.MountTargetId, + IPAddress: *mt.IpAddress, + }) + } + return returneMountTargets, nil + } + var mountTarget *types.MountTargetDescription if azName != "" { mountTarget = getMountTargetForAz(availableMountTargets, azName) @@ -408,11 +423,13 @@ func (c *cloud) DescribeMountTargets(ctx context.Context, fileSystemId, azName s mountTarget = &availableMountTargets[rand.Intn(len(availableMountTargets))] } - return &MountTarget{ - AZName: *mountTarget.AvailabilityZoneName, - AZId: *mountTarget.AvailabilityZoneId, - MountTargetId: *mountTarget.MountTargetId, - IPAddress: *mountTarget.IpAddress, + return []*MountTarget{ + { + AZName: *mountTarget.AvailabilityZoneName, + AZId: *mountTarget.AvailabilityZoneId, + MountTargetId: *mountTarget.MountTargetId, + IPAddress: *mountTarget.IpAddress, + }, }, nil } diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go index bc833fd37..5b343ddfe 100644 --- a/pkg/cloud/cloud_test.go +++ b/pkg/cloud/cloud_test.go @@ -1005,6 +1005,25 @@ func TestDescribeMountTargets(t *testing.T) { }, expectError: errtyp{}, }, + { + name: "Success: Mount target with multi az. Get all mount targets.", + mockOutput: &efs.DescribeMountTargetsOutput{ + MountTargets: []types.MountTargetDescription{ + { + AvailabilityZoneId: aws.String("az-id"), + AvailabilityZoneName: aws.String("multi"), + FileSystemId: aws.String(fsId), + IpAddress: aws.String("127.0.0.1"), + LifeCycleState: types.LifeCycleStateAvailable, + MountTargetId: aws.String(mtId), + NetworkInterfaceId: aws.String("eni-abcd1234"), + OwnerId: aws.String("1234567890"), + SubnetId: aws.String("subnet-abcd1234"), + }, + }, + }, + expectError: errtyp{}, + }, { name: "Fail: File system does not have any mount targets", mockOutput: &efs.DescribeMountTargetsOutput{ diff --git a/pkg/cloud/ec2_metadata.go b/pkg/cloud/ec2_metadata.go index a134fc107..7322837a9 100644 --- a/pkg/cloud/ec2_metadata.go +++ b/pkg/cloud/ec2_metadata.go @@ -3,6 +3,7 @@ package cloud import ( "context" "fmt" + "io" "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" ) @@ -34,9 +35,18 @@ func (e ec2MetadataProvider) getMetadata() (MetadataService, error) { return nil, fmt.Errorf("could not get valid EC2 availavility zone") } + availabilityZoneIdResponse, err := e.ec2MetadataService.GetMetadata(context.TODO(), &imds.GetMetadataInput{ + Path: "placement/availability-zone-id", + }) + if err != nil { + return nil, fmt.Errorf("could not get placement availability-zone-id from the EC2 instance identity metadata") + } + availabilityZoneID, _ := io.ReadAll(availabilityZoneIdResponse.Content) + return &metadata{ - instanceID: doc.InstanceID, - region: doc.Region, - availabilityZone: doc.AvailabilityZone, + instanceID: doc.InstanceID, + region: doc.Region, + availabilityZone: doc.AvailabilityZone, + availabilityZoneID: string(availabilityZoneID), }, nil } diff --git a/pkg/cloud/ec2_metadata_test.go b/pkg/cloud/ec2_metadata_test.go index 1968c230d..c307407f5 100644 --- a/pkg/cloud/ec2_metadata_test.go +++ b/pkg/cloud/ec2_metadata_test.go @@ -3,6 +3,8 @@ package cloud import ( "context" "fmt" + "io" + "strings" "testing" "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" @@ -12,18 +14,20 @@ import ( ) var ( - stdInstanceID = "instance-1" - stdRegionName = "instance-1" - stdAvailabilityZone = "az-1" + stdInstanceID = "instance-1" + stdRegionName = "instance-1" + stdAvailabilityZone = "az-1" + stdAvailabilityZoneID = "use1-az1" ) func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) { testCases := []struct { - name string - isAvailable bool - isPartial bool - identityDocument imds.InstanceIdentityDocument - err error + name string + isAvailable bool + isPartial bool + identityDocument imds.InstanceIdentityDocument + availabilityZoneID string + err error }{ { name: "success: normal", @@ -33,7 +37,8 @@ func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) { Region: stdRegionName, AvailabilityZone: stdAvailabilityZone, }, - err: nil, + availabilityZoneID: stdAvailabilityZoneID, + err: nil, }, { name: "fail: GetInstanceIdentityDocument returned error", @@ -43,7 +48,8 @@ func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) { Region: stdRegionName, AvailabilityZone: stdAvailabilityZone, }, - err: fmt.Errorf(""), + availabilityZoneID: stdAvailabilityZoneID, + err: fmt.Errorf(""), }, { name: "fail: GetInstanceIdentityDocument returned empty instance", @@ -54,7 +60,8 @@ func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) { Region: stdRegionName, AvailabilityZone: stdAvailabilityZone, }, - err: nil, + availabilityZoneID: stdAvailabilityZoneID, + err: nil, }, { name: "fail: GetInstanceIdentityDocument returned empty region", @@ -65,7 +72,8 @@ func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) { Region: "", AvailabilityZone: stdAvailabilityZone, }, - err: nil, + availabilityZoneID: stdAvailabilityZoneID, + err: nil, }, { name: "fail: GetInstanceIdentityDocument returned empty az", @@ -76,7 +84,19 @@ func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) { Region: stdRegionName, AvailabilityZone: "", }, - err: nil, + availabilityZoneID: stdAvailabilityZoneID, + err: nil, + }, + { + name: "fail: GetInstanceIdentityDocument returned empty az id", + isAvailable: true, + identityDocument: imds.InstanceIdentityDocument{ + InstanceID: stdInstanceID, + Region: stdRegionName, + AvailabilityZone: stdAvailabilityZone, + }, + availabilityZoneID: "", + err: nil, }, } @@ -87,6 +107,15 @@ func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) { if tc.isAvailable { mockEC2Metadata.EXPECT().GetInstanceIdentityDocument(context.TODO(), &imds.GetInstanceIdentityDocumentInput{}).Return(&imds.GetInstanceIdentityDocumentOutput{InstanceIdentityDocument: tc.identityDocument}, tc.err) + + if tc.err == nil && + tc.identityDocument.InstanceID != "" && + tc.identityDocument.Region != "" && + tc.identityDocument.AvailabilityZone != "" { + mockEC2Metadata.EXPECT().GetMetadata(context.TODO(), &imds.GetMetadataInput{ + Path: "placement/availability-zone-id", + }).Return(&imds.GetMetadataOutput{Content: io.NopCloser(strings.NewReader(tc.availabilityZoneID))}, nil) + } } ec2Mp := ec2MetadataProvider{ec2MetadataService: mockEC2Metadata} @@ -108,6 +137,10 @@ func TestRetrieveMetadataFromEC2MetadataService(t *testing.T) { if m.GetAvailabilityZone() != tc.identityDocument.AvailabilityZone { t.Fatalf("GetAvailabilityZone() failed: expected %v, got %v", tc.identityDocument.AvailabilityZone, m.GetAvailabilityZone()) } + + if m.GetAvailabilityZoneID() != tc.availabilityZoneID { + t.Fatalf("GetAvailabilityZoneID() failed: expected %v, got %v", tc.availabilityZoneID, m.GetAvailabilityZoneID()) + } } else { if err == nil { t.Fatal("getEC2Metadata() failed: expected error when GetInstanceIdentityDocument returns partial data, got nothing") diff --git a/pkg/cloud/fakes.go b/pkg/cloud/fakes.go index 8f910ca88..0e00e219e 100644 --- a/pkg/cloud/fakes.go +++ b/pkg/cloud/fakes.go @@ -16,7 +16,7 @@ type FakeCloudProvider struct { func NewFakeCloudProvider() *FakeCloudProvider { return &FakeCloudProvider{ - m: &metadata{"instanceID", "region", "az"}, + m: &metadata{"instanceID", "region", "az", "azId"}, fileSystems: make(map[string]*FileSystem), accessPoints: make(map[string]*AccessPoint), mountTargets: make(map[string]*MountTarget), @@ -90,11 +90,10 @@ func (c *FakeCloudProvider) DescribeFileSystem(ctx context.Context, fileSystemId return fs, nil } -func (c *FakeCloudProvider) DescribeMountTargets(ctx context.Context, fileSystemId, az string) (mountTarget *MountTarget, err error) { +func (c *FakeCloudProvider) DescribeMountTargets(ctx context.Context, fileSystemId, az string) (mountTarget []*MountTarget, err error) { if mt, ok := c.mountTargets[fileSystemId]; ok { - return mt, nil + return []*MountTarget{mt}, nil } - return nil, ErrNotFound } diff --git a/pkg/cloud/metadata.go b/pkg/cloud/metadata.go index e285af451..5d3d6109c 100644 --- a/pkg/cloud/metadata.go +++ b/pkg/cloud/metadata.go @@ -19,6 +19,7 @@ package cloud import ( "context" "fmt" + "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" "k8s.io/client-go/kubernetes" @@ -34,12 +35,14 @@ type MetadataService interface { GetInstanceID() string GetRegion() string GetAvailabilityZone() string + GetAvailabilityZoneID() string } type metadata struct { - instanceID string - region string - availabilityZone string + instanceID string + region string + availabilityZone string + availabilityZoneID string } var _ MetadataService = &metadata{} @@ -61,6 +64,11 @@ func (m *metadata) GetAvailabilityZone() string { return m.availabilityZone } +// GetAvailabilityZoneID returns the Availability Zone Id which the instance is in. +func (m *metadata) GetAvailabilityZoneID() string { + return m.availabilityZoneID +} + // GetNewMetadataProvider returns a MetadataProvider on which can be invoked getMetadata() to extract the metadata. func GetNewMetadataProvider(svc EC2Metadata, clientset kubernetes.Interface) (MetadataProvider, error) { // check if it is running in ECS otherwise default fall back to ec2 diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 07666416c..7ee767eca 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -392,8 +392,14 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) mountTarget, err := localCloud.DescribeMountTargets(ctx, accessPointsOptions.FileSystemId, azName) if err != nil { klog.Warningf("Failed to describe mount targets for file system %v. Skip using `mounttargetip` mount option: %v", accessPointsOptions.FileSystemId, err) + } else if azName == "multi" { + var mountTargets []string + for _, mt := range mountTarget { + mountTargets = append(mountTargets, fmt.Sprintf("%s:%s", mt.AZId, mt.IPAddress)) + } + volContext[MountTargetIp] = strings.Join(mountTargets, ",") } else { - volContext[MountTargetIp] = mountTarget.IPAddress + volContext[MountTargetIp] = mountTarget[0].IPAddress } } @@ -499,7 +505,8 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) } else { mountTarget, err := localCloud.DescribeMountTargets(ctx, fileSystemId, "") if err == nil { - mountOptions = append(mountOptions, MountTargetIp+"="+mountTarget.IPAddress) + // TODO optimize multi zone + mountOptions = append(mountOptions, MountTargetIp+"="+mountTarget[0].IPAddress) } else { klog.Warningf("Failed to describe mount targets for file system %v. Skip using `mounttargetip` mount option: %v", fileSystemId, err) } diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index f8d78dac6..b1fb11c13 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -40,6 +40,7 @@ const ( type Driver struct { endpoint string nodeID string + availabilityZoneID string srv *grpc.Server mounter Mounter efsWatchdog Watchdog @@ -67,6 +68,7 @@ func NewDriver(endpoint, efsUtilsCfgPath, efsUtilsStaticFilesPath, tags string, return &Driver{ endpoint: endpoint, nodeID: cloud.GetMetadata().GetInstanceID(), + availabilityZoneID: cloud.GetMetadata().GetAvailabilityZoneID(), mounter: newNodeMounter(), efsWatchdog: watchdog, cloud: cloud, diff --git a/pkg/driver/mocks/mock_cloud.go b/pkg/driver/mocks/mock_cloud.go index 6385943fa..3a7024fb0 100644 --- a/pkg/driver/mocks/mock_cloud.go +++ b/pkg/driver/mocks/mock_cloud.go @@ -219,10 +219,10 @@ func (mr *MockCloudMockRecorder) DescribeFileSystem(ctx, fileSystemId interface{ } // DescribeMountTargets mocks base method. -func (m *MockCloud) DescribeMountTargets(ctx context.Context, fileSystemId, az string) (*cloud.MountTarget, error) { +func (m *MockCloud) DescribeMountTargets(ctx context.Context, fileSystemId, az string) ([]*cloud.MountTarget, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DescribeMountTargets", ctx, fileSystemId, az) - ret0, _ := ret[0].(*cloud.MountTarget) + ret0, _ := ret[0].([]*cloud.MountTarget) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/pkg/driver/node.go b/pkg/driver/node.go index b730bff0d..9164a51a9 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -100,8 +100,33 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Volume context property %q must be a boolean value: %v", k, err)) } case MountTargetIp: - ipAddr := volContext[MountTargetIp] - mountOptions = append(mountOptions, MountTargetIp+"="+ipAddr) + mountTargetIp := volContext[MountTargetIp] + var ipAddrToMount string + + // check for multiZone volume or not: + if strings.Contains(mountTargetIp, ",") { + mountTargetPairs := strings.Split(mountTargetIp, ",") + azToIp := make(map[string]string) + + for _, pair := range mountTargetPairs { + ipPair := strings.Split(pair, ":") + if len(ipPair) == 2 { + azid := ipPair[0] + ipAddr := ipPair[1] + azToIp[azid] = ipAddr + } + } + + var ok bool + ipAddrToMount, ok = azToIp[d.availabilityZoneID] + if !ok { + return nil, status.Errorf(codes.InvalidArgument, "No mount target IP found for availability zone %s", d.availabilityZoneID) + } + mountOptions = append(mountOptions, MountTargetIp+"="+ipAddrToMount) + } else { + ipAddrToMount = mountTargetIp + } + mountOptions = append(mountOptions, MountTargetIp+"="+ipAddrToMount) case CrossAccount: var err error crossAccountDNSEnabled, err = strconv.ParseBool(v) From 4d604c4744ae62be0c3e07758b601ff7eef158b5 Mon Sep 17 00:00:00 2001 From: Mikey032 <26899585+Mikey032@users.noreply.github.com> Date: Fri, 16 May 2025 13:50:20 +0200 Subject: [PATCH 2/2] small optimization --- pkg/cloud/cloud.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 98e037701..23ec7f01d 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -396,18 +396,18 @@ func (c *cloud) DescribeMountTargets(ctx context.Context, fileSystemId, azName s return nil, fmt.Errorf("No mount target for file system %v is in available state. Please retry in 5 minutes.", fileSystemId) } - var returneMountTargets []*MountTarget if azName == "multi" { // Return all available mount targets + var multiAzMountTargets []*MountTarget for _, mt := range availableMountTargets { - returneMountTargets = append(returneMountTargets, &MountTarget{ + multiAzMountTargets = append(multiAzMountTargets, &MountTarget{ AZName: *mt.AvailabilityZoneName, AZId: *mt.AvailabilityZoneId, MountTargetId: *mt.MountTargetId, IPAddress: *mt.IpAddress, }) } - return returneMountTargets, nil + return multiAzMountTargets, nil } var mountTarget *types.MountTargetDescription