From 232907a62e862b6724872e564c70ae8f2d1a685c Mon Sep 17 00:00:00 2001 From: Drew Sirenko Date: Fri, 6 Oct 2023 14:47:11 +0000 Subject: [PATCH] Support clustered allocation when formatting ext4 filesystem --- docs/parameters.md | 1 + pkg/driver/constants.go | 25 +++++++++++++++++-------- pkg/driver/controller.go | 20 ++++++++++++++++---- pkg/driver/controller_test.go | 14 ++++++++++++++ pkg/driver/node.go | 4 ++++ pkg/driver/node_test.go | 14 ++++++++++++++ tests/e2e/format_options.go | 4 ++++ 7 files changed, 70 insertions(+), 12 deletions(-) diff --git a/docs/parameters.md b/docs/parameters.md index 0818b71e2d..6b141ab960 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -20,6 +20,7 @@ The AWS EBS CSI Driver supports [tagging](tagging.md) through `StorageClass.para | "inodeSize" | | | The inode size to use when formatting the underlying filesystem. Only supported on linux nodes and with fstype `ext2`, `ext3`, `ext4`, or `xfs`. | | "bytesPerInode" | | | The `bytes-per-inode` to use when formatting the underlying filesystem. Only supported on linux nodes and with fstype `ext2`, `ext3`, `ext4`. | | "numberOfInodes" | | | The `number-of-inodes` to use when formatting the underlying filesystem. Only supported on linux nodes and with fstype `ext2`, `ext3`, `ext4`. | +| "ext4BigAllocClusterSize" | | | The cluster size to use when formatting an `ext4` filesystem with the `bigalloc` option enabled. Only supported on linux nodes and with fstype `ext4`. | ## Restrictions * `gp3` is currently not supported on outposts. Outpost customers need to use a different type for their volumes. * If the requested IOPS (either directly from `iops` or from `iopsPerGB` multiplied by the volume's capacity) produces a value above the maximum IOPS allowed for the [volume type](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html), the IOPS will be capped at the maximum value allowed. If the value is lower than the minimal supported IOPS value per volume, either an error is returned (the default behavior), or the value is increased to fit into the supported range when `allowautoiopspergbincrease` is `"true"`. diff --git a/pkg/driver/constants.go b/pkg/driver/constants.go index 43b5e346d0..eb8b1241b7 100644 --- a/pkg/driver/constants.go +++ b/pkg/driver/constants.go @@ -94,6 +94,9 @@ const ( // NumberOfInodesKey configures the `number-of-inodes` when formatting a volume NumberOfInodesKey = "numberofinodes" + // Ext4BigAllocClusterSizeKey enables the ext4 bigalloc option configures the cluster size when formatting an ext4 volume + Ext4BigAllocClusterSizeKey = "ext4bigallocclustersize" + // TagKeyPrefix contains the prefix of a volume parameter that designates it as // a tag to be attached to the resource TagKeyPrefix = "tagSpecification" @@ -188,26 +191,32 @@ func (fsConfig fileSystemConfig) isParameterSupported(paramName string) bool { var ( FileSystemConfigs = map[string]fileSystemConfig{ FSTypeExt2: { - NotSupportedParams: map[string]struct{}{}, + NotSupportedParams: map[string]struct{}{ + Ext4BigAllocClusterSizeKey: {}, + }, }, FSTypeExt3: { - NotSupportedParams: map[string]struct{}{}, + NotSupportedParams: map[string]struct{}{ + Ext4BigAllocClusterSizeKey: {}, + }, }, FSTypeExt4: { NotSupportedParams: map[string]struct{}{}, }, FSTypeXfs: { NotSupportedParams: map[string]struct{}{ - BytesPerInodeKey: {}, - NumberOfInodesKey: {}, + BytesPerInodeKey: {}, + NumberOfInodesKey: {}, + Ext4BigAllocClusterSizeKey: {}, }, }, FSTypeNtfs: { NotSupportedParams: map[string]struct{}{ - BlockSizeKey: {}, - InodeSizeKey: {}, - BytesPerInodeKey: {}, - NumberOfInodesKey: {}, + BlockSizeKey: {}, + InodeSizeKey: {}, + BytesPerInodeKey: {}, + NumberOfInodesKey: {}, + Ext4BigAllocClusterSizeKey: {}, }, }, } diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 34b83b06fd..c6eaa9e3b9 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -137,10 +137,11 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol cloud.VolumeNameTagKey: volName, cloud.AwsEbsDriverTagKey: isManagedByDriver, } - blockSize string - inodeSize string - bytesPerInode string - numberOfInodes string + blockSize string + inodeSize string + bytesPerInode string + numberOfInodes string + ext4BigAllocClusterSize string ) tProps := new(template.PVProps) @@ -207,6 +208,11 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, status.Errorf(codes.InvalidArgument, "Could not parse numberOfInodes (%s): %v", value, err) } numberOfInodes = value + case Ext4BigAllocClusterSizeKey: + if isAlphanumeric := util.StringIsAlphanumeric(value); !isAlphanumeric { + return nil, status.Errorf(codes.InvalidArgument, "Could not parse ext4BigAllocClusterSize (%s): %v", value, err) + } + ext4BigAllocClusterSize = value default: if strings.HasPrefix(key, TagKeyPrefix) { scTags = append(scTags, value) @@ -242,6 +248,12 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, err } } + if len(ext4BigAllocClusterSize) > 0 { + responseCtx[Ext4BigAllocClusterSizeKey] = ext4BigAllocClusterSize + if err = validateVolumeCapabilities(req.GetVolumeCapabilities(), Ext4BigAllocClusterSizeKey, FileSystemConfigs); err != nil { + return nil, err + } + } if blockExpress && volumeType != cloud.VolumeTypeIO2 { return nil, status.Errorf(codes.InvalidArgument, "Block Express is only supported on io2 volumes") diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index a39b819c6a..f2ff156d53 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -1688,6 +1688,13 @@ func TestCreateVolumeWithFormattingParameters(t *testing.T) { }, errExpected: false, }, + { + name: "success with ext4 bigalloc cluster size", + formattingOptionParameters: map[string]string{ + Ext4BigAllocClusterSizeKey: "16384", + }, + errExpected: false, + }, { name: "failure with block size", formattingOptionParameters: map[string]string{ @@ -1716,6 +1723,13 @@ func TestCreateVolumeWithFormattingParameters(t *testing.T) { }, errExpected: true, }, + { + name: "failure with ext4 cluster size", + formattingOptionParameters: map[string]string{ + Ext4BigAllocClusterSizeKey: "wrong_value", + }, + errExpected: true, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 32bb7b9d3b..421bb5f84e 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -173,6 +173,7 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol if err != nil { return nil, err } + ext4BigAllocClusterSize, err := recheckFormattingOptionParameter(context, Ext4BigAllocClusterSizeKey, FileSystemConfigs, fsType) if err != nil { return nil, err } @@ -262,6 +263,9 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol if len(numInodes) > 0 { formatOptions = append(formatOptions, "-N", numInodes) } + if len(ext4BigAllocClusterSize) > 0 { + formatOptions = append(formatOptions, "-O", "bigalloc", "-C", ext4BigAllocClusterSize) + } err = d.mounter.FormatAndMountSensitiveWithFormatOptions(source, target, fsType, mountOptions, nil, formatOptions) if err != nil { msg := fmt.Sprintf("could not format %q and mount it at %q: %v", source, target, err) diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index fbf785e29b..e2fd4de71b 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -342,6 +342,20 @@ func TestNodeStageVolume(t *testing.T) { mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any(), gomock.Nil(), gomock.Eq([]string{"-N", "13107200"})) }, }, + { + name: "success with cluster size in ext4", + request: &csi.NodeStageVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: devicePath}, + StagingTargetPath: targetPath, + VolumeCapability: stdVolCap, + VolumeId: volumeID, + VolumeContext: map[string]string{Ext4BigAllocClusterSizeKey: "16384"}, + }, + expectMock: func(mockMounter MockMounter, mockDeviceIdentifier MockDeviceIdentifier) { + successExpectMock(mockMounter, mockDeviceIdentifier) + mockMounter.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq(devicePath), gomock.Eq(targetPath), gomock.Eq(defaultFsType), gomock.Any(), gomock.Nil(), gomock.Eq([]string{"-O", "bigalloc", "-C", "16384"})) + }, + }, { name: "fail no VolumeId", request: &csi.NodeStageVolumeRequest{ diff --git a/tests/e2e/format_options.go b/tests/e2e/format_options.go index 6bb365db7b..66210415d0 100644 --- a/tests/e2e/format_options.go +++ b/tests/e2e/format_options.go @@ -46,6 +46,10 @@ var ( CreateVolumeParameterKey: ebscsidriver.NumberOfInodesKey, CreateVolumeParameterValue: "200192", }, + { + CreateVolumeParameterKey: ebscsidriver.Ext4BigAllocClusterSizeKey, + CreateVolumeParameterValue: "16384", + }, } )