From 2f8f6f203229728a84b14f8424c30aa276510c78 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/faq.md | 9 ++++ docs/parameters.md | 3 ++ pkg/driver/constants.go | 32 +++++++++---- pkg/driver/controller.go | 35 ++++++++++++-- pkg/driver/controller_test.go | 31 ++++++++++++ pkg/driver/node.go | 11 +++++ pkg/driver/node_test.go | 28 +++++++++++ tests/e2e/format_options.go | 47 ++++++++++++------- tests/e2e/testsuites/format_options_tester.go | 9 +--- 9 files changed, 170 insertions(+), 35 deletions(-) create mode 100644 docs/faq.md diff --git a/docs/faq.md b/docs/faq.md new file mode 100644 index 0000000000..5e940984d4 --- /dev/null +++ b/docs/faq.md @@ -0,0 +1,9 @@ +# Frequently Asked Questions + +## CreateVolume (`StorageClass`) Parameters + +### `ext4BigAlloc` and `ext4ClusterSize` + +Warnings: +- The `bigalloc` feature may not be fully supported with your node's Linux kernel or may have various bugs. Please see the [ext4(5) man-page](https://man7.org/linux/man-pages/man5/ext4.5.html) for details. +- Linux kernel release 4.15 added support for resizing ext4 filesystems using clustered allocation. **Resizing volumes mounted to nodes running a Linux kernel version prior to 4.15 will fail and require manual intervention and downtime of the volume.** diff --git a/docs/parameters.md b/docs/parameters.md index 0818b71e2d..a992ebfddb 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -20,6 +20,9 @@ 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`. | +| "ext4BigAlloc" | true, false | false | Changes the `ext4` filesystem to use clustered block allocation by enabling the `bigalloc` formatting option. Warning: `bigalloc` may not be fully supported with your node's Linux kernel. Please see our [FAQ](/docs/faq.md). | +| "ext4ClusterSize" | | | The cluster size to use when formatting an `ext4` filesystem when the `bigalloc` feature is enabled. Note: The `ext4BigAlloc` parameter must be set to true. See our [FAQ](/docs/faq.md). | + ## 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..3854bf8ea9 100644 --- a/pkg/driver/constants.go +++ b/pkg/driver/constants.go @@ -94,6 +94,12 @@ const ( // NumberOfInodesKey configures the `number-of-inodes` when formatting a volume NumberOfInodesKey = "numberofinodes" + // Ext4ClusterSizeKey enables the bigalloc option when formatting an ext4 volume + Ext4BigAllocKey = "ext4bigalloc" + + // Ext4ClusterSizeKey configures the cluster size when formatting an ext4 volume with the bigalloc option enabled + Ext4ClusterSizeKey = "ext4clustersize" + // TagKeyPrefix contains the prefix of a volume parameter that designates it as // a tag to be attached to the resource TagKeyPrefix = "tagSpecification" @@ -188,26 +194,36 @@ func (fsConfig fileSystemConfig) isParameterSupported(paramName string) bool { var ( FileSystemConfigs = map[string]fileSystemConfig{ FSTypeExt2: { - NotSupportedParams: map[string]struct{}{}, + NotSupportedParams: map[string]struct{}{ + Ext4BigAllocKey: {}, + Ext4ClusterSizeKey: {}, + }, }, FSTypeExt3: { - NotSupportedParams: map[string]struct{}{}, + NotSupportedParams: map[string]struct{}{ + Ext4BigAllocKey: {}, + Ext4ClusterSizeKey: {}, + }, }, FSTypeExt4: { NotSupportedParams: map[string]struct{}{}, }, FSTypeXfs: { NotSupportedParams: map[string]struct{}{ - BytesPerInodeKey: {}, - NumberOfInodesKey: {}, + BytesPerInodeKey: {}, + NumberOfInodesKey: {}, + Ext4BigAllocKey: {}, + Ext4ClusterSizeKey: {}, }, }, FSTypeNtfs: { NotSupportedParams: map[string]struct{}{ - BlockSizeKey: {}, - InodeSizeKey: {}, - BytesPerInodeKey: {}, - NumberOfInodesKey: {}, + BlockSizeKey: {}, + InodeSizeKey: {}, + BytesPerInodeKey: {}, + NumberOfInodesKey: {}, + Ext4BigAllocKey: {}, + Ext4ClusterSizeKey: {}, }, }, } diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 34b83b06fd..cff2eb35a2 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -137,10 +137,12 @@ 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 + ext4BigAlloc bool + ext4ClusterSize string ) tProps := new(template.PVProps) @@ -207,6 +209,15 @@ 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 Ext4BigAllocKey: + if value == "true" { + ext4BigAlloc = true + } + case Ext4ClusterSizeKey: + if isAlphanumeric := util.StringIsAlphanumeric(value); !isAlphanumeric { + return nil, status.Errorf(codes.InvalidArgument, "Could not parse ext4ClusterSize (%s): %v", value, err) + } + ext4ClusterSize = value default: if strings.HasPrefix(key, TagKeyPrefix) { scTags = append(scTags, value) @@ -242,6 +253,22 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, err } } + if ext4BigAlloc { + responseCtx[Ext4BigAllocKey] = "true" + if err = validateVolumeCapabilities(req.GetVolumeCapabilities(), Ext4BigAllocKey, FileSystemConfigs); err != nil { + return nil, err + } + } + if len(ext4ClusterSize) > 0 { + responseCtx[Ext4ClusterSizeKey] = ext4ClusterSize + if err = validateVolumeCapabilities(req.GetVolumeCapabilities(), Ext4ClusterSizeKey, FileSystemConfigs); err != nil { + return nil, err + } + } + + if !ext4BigAlloc && len(ext4ClusterSize) > 0 { + return nil, status.Errorf(codes.InvalidArgument, "Cannot set ext4BigAllocClusterSize when ext4BigAlloc is false") + } 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..dbf113e4ad 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -1688,6 +1688,21 @@ func TestCreateVolumeWithFormattingParameters(t *testing.T) { }, errExpected: false, }, + { + name: "success with ext4 big alloc option", + formattingOptionParameters: map[string]string{ + Ext4BigAllocKey: "true", + }, + errExpected: false, + }, + { + name: "success with ext4 bigalloc option and custom cluster size", + formattingOptionParameters: map[string]string{ + Ext4BigAllocKey: "true", + Ext4ClusterSizeKey: "16384", + }, + errExpected: false, + }, { name: "failure with block size", formattingOptionParameters: map[string]string{ @@ -1716,6 +1731,22 @@ func TestCreateVolumeWithFormattingParameters(t *testing.T) { }, errExpected: true, }, + { + name: "failure with ext4 custom cluster size", + formattingOptionParameters: map[string]string{ + Ext4BigAllocKey: "true", + Ext4ClusterSizeKey: "wrong_value", + }, + errExpected: true, + }, + { + name: "failure with ext4 bigalloc option and cluster size mismatch", + formattingOptionParameters: map[string]string{ + Ext4BigAllocKey: "false", + Ext4ClusterSizeKey: "16384", + }, + 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..ba5c326a80 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -173,6 +173,11 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol if err != nil { return nil, err } + ext4BigAlloc, err := recheckFormattingOptionParameter(context, Ext4BigAllocKey, FileSystemConfigs, fsType) + if err != nil { + return nil, err + } + ext4ClusterSize, err := recheckFormattingOptionParameter(context, Ext4ClusterSizeKey, FileSystemConfigs, fsType) if err != nil { return nil, err } @@ -262,6 +267,12 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol if len(numInodes) > 0 { formatOptions = append(formatOptions, "-N", numInodes) } + if ext4BigAlloc == "true" { + formatOptions = append(formatOptions, "-O", "bigalloc") + } + if len(ext4ClusterSize) > 0 { + formatOptions = append(formatOptions, "-C", ext4ClusterSize) + } 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..a8be6083c2 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -342,6 +342,34 @@ 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 bigalloc feature flag enabled in ext4", + request: &csi.NodeStageVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: devicePath}, + StagingTargetPath: targetPath, + VolumeCapability: stdVolCap, + VolumeId: volumeID, + VolumeContext: map[string]string{Ext4BigAllocKey: "true"}, + }, + 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"})) + }, + }, + { + name: "success with custom cluster size and bigalloc feature flag enabled in ext4", + request: &csi.NodeStageVolumeRequest{ + PublishContext: map[string]string{DevicePathKey: devicePath}, + StagingTargetPath: targetPath, + VolumeCapability: stdVolCap, + VolumeId: volumeID, + VolumeContext: map[string]string{Ext4BigAllocKey: "true", Ext4ClusterSizeKey: "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..f42f088c24 100644 --- a/tests/e2e/format_options.go +++ b/tests/e2e/format_options.go @@ -29,22 +29,37 @@ import ( var ( testedFsTypes = []string{ebscsidriver.FSTypeExt4} - formatOptionTests = []testsuites.FormatOptionTest{ - { - CreateVolumeParameterKey: ebscsidriver.BlockSizeKey, - CreateVolumeParameterValue: "1024", + formatOptionTests = map[string]testsuites.FormatOptionTest{ + ebscsidriver.BlockSizeKey: { + CreateVolumeParameters: map[string]string{ + ebscsidriver.BlockSizeKey: "1024", + }, }, - { - CreateVolumeParameterKey: ebscsidriver.InodeSizeKey, - CreateVolumeParameterValue: "512", + ebscsidriver.InodeSizeKey: { + CreateVolumeParameters: map[string]string{ + ebscsidriver.InodeSizeKey: "512", + }, }, - { - CreateVolumeParameterKey: ebscsidriver.BytesPerInodeKey, - CreateVolumeParameterValue: "8192", + ebscsidriver.BytesPerInodeKey: { + CreateVolumeParameters: map[string]string{ + ebscsidriver.BytesPerInodeKey: "8192", + }, }, - { - CreateVolumeParameterKey: ebscsidriver.NumberOfInodesKey, - CreateVolumeParameterValue: "200192", + ebscsidriver.NumberOfInodesKey: { + CreateVolumeParameters: map[string]string{ + ebscsidriver.NumberOfInodesKey: "200192", + }, + }, + ebscsidriver.Ext4BigAllocKey: { + CreateVolumeParameters: map[string]string{ + ebscsidriver.Ext4BigAllocKey: "true", + }, + }, + ebscsidriver.Ext4ClusterSizeKey: { + CreateVolumeParameters: map[string]string{ + ebscsidriver.Ext4BigAllocKey: "true", + ebscsidriver.Ext4ClusterSizeKey: "16384", + }, }, } ) @@ -67,13 +82,13 @@ var _ = Describe("[ebs-csi-e2e] [single-az] [format-options] Formatting a volume for _, fsType := range testedFsTypes { Context(fmt.Sprintf("using an %s filesystem", fsType), func() { - for _, formatOptionTestCase := range formatOptionTests { + for testedParameter, formatOptionTestCase := range formatOptionTests { formatOptionTestCase := formatOptionTestCase - if fsTypeDoesNotSupportFormatOptionParameter(fsType, formatOptionTestCase.CreateVolumeParameterKey) { + if fsTypeDoesNotSupportFormatOptionParameter(fsType, testedParameter) { continue } - Context(fmt.Sprintf("with a custom %s parameter", formatOptionTestCase.CreateVolumeParameterKey), func() { + Context(fmt.Sprintf("with a custom %s parameter", testedParameter), func() { It("successfully mounts and is resizable", func() { formatOptionTestCase.Run(cs, ns, ebsDriver, fsType) }) diff --git a/tests/e2e/testsuites/format_options_tester.go b/tests/e2e/testsuites/format_options_tester.go index a031fac0dc..7d11538a85 100644 --- a/tests/e2e/testsuites/format_options_tester.go +++ b/tests/e2e/testsuites/format_options_tester.go @@ -25,8 +25,7 @@ import ( // FormatOptionTest will provision required StorageClass(es), PVC(s) and Pod(s) in order to test that volumes with // a specified custom format options will mount and are able to be later resized. type FormatOptionTest struct { - CreateVolumeParameterKey string - CreateVolumeParameterValue string + CreateVolumeParameters map[string]string } const ( @@ -56,10 +55,6 @@ func (t *FormatOptionTest) Run(client clientset.Interface, namespace *v1.Namespa } func createFormatOptionVolumeDetails(fsType string, t *FormatOptionTest) *VolumeDetails { - additionalParameters := map[string]string{ - t.CreateVolumeParameterKey: t.CreateVolumeParameterValue, - } - allowVolumeExpansion := true volume := VolumeDetails{ @@ -72,7 +67,7 @@ func createFormatOptionVolumeDetails(fsType string, t *FormatOptionTest) *Volume MountPathGenerate: DefaultMountPath, }, AllowVolumeExpansion: &allowVolumeExpansion, - AdditionalParameters: additionalParameters, + AdditionalParameters: t.CreateVolumeParameters, } return &volume