Merge pull request #1706 from AndrewSirenko/ext4BigAllocFormatOptions

Support clustered allocation when formatting ext4 filesystem
kubernetes-sigs · Oct 12, 2023 · 1b9d078 · 1b9d078
2 parents b7a6060 + 0734fbe
commit 1b9d078
Show file tree

Hide file tree

Showing 9 changed files with 170 additions and 35 deletions.
diff --git a/docs/faq.md b/docs/faq.md
@@ -0,0 +1,9 @@
+# Frequently Asked Questions
+
+## CreateVolume (`StorageClass`) Parameters
+
+### `ext4BigAlloc` and `ext4ClusterSize`
+
+Warnings:
+- Ext4's `bigalloc` is an experimental feature, under active development. Please pay particular attention to your node's kernel version. 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.** 
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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