Support specifying block size for filesystem format

Signed-off-by: Connor Catlett <[email protected]>

docs/parameters.md

@@ -14,6 +14,7 @@ The AWS EBS CSI Driver supports [tagging](tagging.md) through `StorageClass.para
 | "encrypted"                  | true, false                                        | false   | Whether the volume should be encrypted or not. Valid values are "true" or "false".                                                                                                                                                                                                                                                                                                             |
 | "blockExpress"               | true, false                                        | false   | Enables the creation of [io2 Block Express volumes](https://aws.amazon.com/ebs/provisioned-iops/#Introducing_io2_Block_Express) by increasing the IOPS limit for io2 volumes to 256000. Volumes created with more than 64000 IOPS will fail to mount on instances that do not support io2 Block Express.                                                                                       |
 | "kmsKeyId"                   |                                                    |         | The full ARN of the key to use when encrypting the volume. If not specified, AWS will use the default KMS key for the region the volume is in. This will be an auto-generated key called `/aws/ebs` if not changed.                                                                                                                                                                            |
+| "blockSize"                  |                                                    |         | The block size to use when formatting the underlying filesystem. Only supported on linux nodes and with fstype `ext2`, `ext3`, `ext4`, or `xfs`.                                    |
 
 **Appendix**
 * `gp3` is currently not supported on outposts. Outpost customers need to use a different type for their volumes.

go.mod

@@ -20,9 +20,9 @@ require (
 	k8s.io/component-base v0.25.3
 	k8s.io/klog/v2 v2.80.1
 	k8s.io/kubernetes v1.25.3
-	k8s.io/mount-utils v0.25.3
+	k8s.io/mount-utils v0.26.0-rc.0
 	k8s.io/pod-security-admission v0.25.3
-	k8s.io/utils v0.0.0-20221012122500-cfd413dd9e85
+	k8s.io/utils v0.0.0-20221107191617-1a15be271d1d
 )
 
 require (
@@ -132,7 +132,7 @@ replace (
 	k8s.io/kubelet => k8s.io/kubelet v0.25.3
 	k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.25.3
 	k8s.io/metrics => k8s.io/metrics v0.25.3
-	k8s.io/mount-utils => k8s.io/mount-utils v0.25.3
+	k8s.io/mount-utils => k8s.io/mount-utils v0.26.0-rc.0
 	k8s.io/node-api => k8s.io/node-api v0.25.3
 	k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.25.3
 	k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.25.3

go.sum

@@ -948,14 +948,14 @@ k8s.io/kubelet v0.25.3 h1:PjT3Xo0VL1BpRilBpZrRN8pSy6w5pGQ0YDQQeQWSHvQ=
 k8s.io/kubelet v0.25.3/go.mod h1:YopVc6vLhveZb22I7AzcoWPap+t3/KJKqRZDa2MZmyE=
 k8s.io/kubernetes v1.25.3 h1:Ljx/Ew9+dt7rN9ob3V+N/aoDy7nDSbmr35IbYGRTyqE=
 k8s.io/kubernetes v1.25.3/go.mod h1:lvEY+3iJhh+sGIK1LorGkI56rW0eLGsfalnp68wQwYU=
-k8s.io/mount-utils v0.25.3 h1:Eb4MDClmozX3Vrz4ZtoG0bQ/pGhT5gyo28p3f+0r9EE=
-k8s.io/mount-utils v0.25.3/go.mod h1:odpFnGwJfFjN3SRnjfGS0902ubcj/W6hDOrNDmSSINo=
+k8s.io/mount-utils v0.26.0-rc.0 h1:+eYfAi3jbs0to42UCMhad7I3W/as5YMxaF0ih3PlnNA=
+k8s.io/mount-utils v0.26.0-rc.0/go.mod h1:mFGnSO6Hc1qzZsFZ73pe724HgXtjXZBWfmkJwEXtQjU=
 k8s.io/pod-security-admission v0.25.3 h1:2HnXWKUIDSez2sWtvxeGgGVUFvYnJJHutL4AI1MIuwk=
 k8s.io/pod-security-admission v0.25.3/go.mod h1:xSaLkcMPD6cGKrZ//ZUrCNs0BewZzQdOEcC9LuXBGR4=
 k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
 k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
-k8s.io/utils v0.0.0-20221012122500-cfd413dd9e85 h1:cTdVh7LYu82xeClmfzGtgyspNh6UxpwLWGi8R4sspNo=
-k8s.io/utils v0.0.0-20221012122500-cfd413dd9e85/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
+k8s.io/utils v0.0.0-20221107191617-1a15be271d1d h1:0Smp/HP1OH4Rvhe+4B8nWGERtlqAGSftbSbbmm45oFs=
+k8s.io/utils v0.0.0-20221107191617-1a15be271d1d/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
 rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
 rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=

pkg/driver/constants.go

@@ -72,6 +72,9 @@ const (
 	// BlockExpressKey increases the iops limit for io2 volumes to the block express limit
 	BlockExpressKey = "blockexpress"
 
+	// BlockSizeKey configures the block size when formatting a volume
+	BlockSizeKey = "blocksize"
+
 	// TagKeyPrefix contains the prefix of a volume parameter that designates it as
 	// a tag to be attached to the resource
 	TagKeyPrefix = "tagSpecification"

pkg/driver/controller.go

@@ -132,6 +132,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 			cloud.VolumeNameTagKey:   volName,
 			cloud.AwsEbsDriverTagKey: isManagedByDriver,
 		}
+		blockSize string
 	)
 
 	tProps := new(template.Props)
@@ -178,6 +179,12 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 			if value == "true" {
 				blockExpress = true
 			}
+		case BlockSizeKey:
+			_, err = strconv.Atoi(value)
+			if err != nil {
+				return nil, status.Errorf(codes.InvalidArgument, "Could not parse blockSize: %v", err)
+			}
+			blockSize = value
 		default:
 			if strings.HasPrefix(key, TagKeyPrefix) {
 				scTags = append(scTags, value)
@@ -187,6 +194,26 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 		}
 	}
 
+	if len(blockSize) > 0 {
+		for _, volCap := range req.GetVolumeCapabilities() {
+			switch volCap.GetAccessType().(type) {
+			case *csi.VolumeCapability_Block:
+				return nil, status.Error(codes.InvalidArgument, "Cannot use block size with block volume")
+			}
+
+			mountVolume := volCap.GetMount()
+			if mountVolume == nil {
+				return nil, status.Error(codes.InvalidArgument, "CreateVolume: mount is nil within volume capability")
+			}
+
+			fsType := mountVolume.GetFsType()
+
+			if fsType != "ext2" && fsType != "ext3" && fsType != "ext4" && fsType != "xfs" {
+				return nil, status.Errorf(codes.InvalidArgument, "Cannot use block size with fstype %s", fsType)
+			}
+		}
+	}
+
 	if volumeType == cloud.VolumeTypeIO1 {
 		if iopsPerGB == 0 {
 			return nil, status.Errorf(codes.InvalidArgument, "The parameter IOPSPerGB must be specified for io1 volumes")
@@ -265,7 +292,7 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 		}
 		return nil, status.Errorf(errCode, "Could not create volume %q: %v", volName, err)
 	}
-	return newCreateVolumeResponse(disk), nil
+	return newCreateVolumeResponse(disk, blockSize), nil
 }
 
 func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error {
@@ -754,7 +781,7 @@ func getOutpostArn(requirement *csi.TopologyRequirement) string {
 	return ""
 }
 
-func newCreateVolumeResponse(disk *cloud.Disk) *csi.CreateVolumeResponse {
+func newCreateVolumeResponse(disk *cloud.Disk, blockSize string) *csi.CreateVolumeResponse {
 	var src *csi.VolumeContentSource
 	if disk.SnapshotID != "" {
 		src = &csi.VolumeContentSource{
@@ -777,11 +804,16 @@ func newCreateVolumeResponse(disk *cloud.Disk) *csi.CreateVolumeResponse {
 		segments[AwsOutpostIDKey] = strings.ReplaceAll(arn.Resource, "outpost/", "")
 	}
 
+	context := map[string]string{}
+	if len(blockSize) > 0 {
+		context[BlockSizeKey] = blockSize
+	}
+
 	return &csi.CreateVolumeResponse{
 		Volume: &csi.Volume{
 			VolumeId:      disk.VolumeID,
 			CapacityBytes: util.GiBToBytes(disk.CapacityGiB),
-			VolumeContext: map[string]string{},
+			VolumeContext: context,
 			AccessibleTopology: []*csi.Topology{
 				{
 					Segments: segments,

pkg/driver/controller_test.go

@@ -147,7 +147,9 @@ func TestCreateVolume(t *testing.T) {
 	stdVolCap := []*csi.VolumeCapability{
 		{
 			AccessType: &csi.VolumeCapability_Mount{
-				Mount: &csi.VolumeCapability_MountVolume{},
+				Mount: &csi.VolumeCapability_MountVolume{
+					FsType: "ext4",
+				},
 			},
 			AccessMode: &csi.VolumeCapability_AccessMode{
 				Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
@@ -1585,6 +1587,57 @@ func TestCreateVolume(t *testing.T) {
 				checkExpectedErrorCode(t, err, codes.AlreadyExists)
 			},
 		},
+		{
+			name: "success with block size",
+			testFunc: func(t *testing.T) {
+				req := &csi.CreateVolumeRequest{
+					Name:               "random-vol-name",
+					CapacityRange:      stdCapRange,
+					VolumeCapabilities: stdVolCap,
+					Parameters: map[string]string{
+						BlockSizeKey: "4096",
+					},
+				}
+
+				ctx := context.Background()
+
+				mockDisk := &cloud.Disk{
+					VolumeID:         req.Name,
+					AvailabilityZone: expZone,
+					CapacityGiB:      util.BytesToGiB(stdVolSize),
+				}
+
+				mockCtl := gomock.NewController(t)
+				defer mockCtl.Finish()
+
+				mockCloud := cloud.NewMockCloud(mockCtl)
+				mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.Name), gomock.Any()).Return(mockDisk, nil)
+
+				awsDriver := controllerService{
+					cloud:         mockCloud,
+					inFlight:      internal.NewInFlight(),
+					driverOptions: &DriverOptions{},
+				}
+
+				response, err := awsDriver.CreateVolume(ctx, req)
+				if err != nil {
+					srvErr, ok := status.FromError(err)
+					if !ok {
+						t.Fatalf("Could not get error status code from error: %v", srvErr)
+					}
+					t.Fatalf("Unexpected error: %v", srvErr.Code())
+				}
+
+				context := response.Volume.VolumeContext
+				if blockSize, ok := context[BlockSizeKey]; ok {
+					if blockSize != "4096" {
+						t.Fatalf("Invalid %s in VolumeContext (got %s expected 4096)", BlockSizeKey, blockSize)
+					}
+				} else {
+					t.Fatalf("Missing key %s in VolumeContext", BlockSizeKey)
+				}
+			},
+		},
 	}
 
 	for _, tc := range testCases {

pkg/driver/mount.go

@@ -32,7 +32,7 @@ import (
 type Mounter interface {
 	mountutils.Interface
 
-	FormatAndMount(source string, target string, fstype string, options []string) error
+	FormatAndMountSensitiveWithFormatOptions(source string, target string, fstype string, options []string, sensitiveOptions []string, formatOptions []string) error
 	IsCorruptedMnt(err error) bool
 	GetDeviceNameFromMount(mountPath string) (string, int, error)
 	MakeFile(path string) error
@@ -55,7 +55,7 @@ type NodeMounter struct {
 }
 
 func newNodeMounter() (Mounter, error) {
-	// mounter.NewSafeMounter returns a SafeFormatAndMount
+	// mounter.NewSafeMounter returns a SafeormatAndMount
 	safeMounter, err := mounter.NewSafeMounter()
 	if err != nil {
 		return nil, err

pkg/driver/mount_windows.go

@@ -27,12 +27,12 @@ import (
 	"regexp"
 )
 
-func (m NodeMounter) FormatAndMount(source string, target string, fstype string, options []string) error {
+func (m NodeMounter) FormatAndMountSensitiveWithFormatOptions(source string, target string, fstype string, options []string, sensitiveOptions []string, formatOptions []string) error {
 	proxyMounter, ok := m.SafeFormatAndMount.Interface.(*mounter.CSIProxyMounter)
 	if !ok {
 		return fmt.Errorf("failed to cast mounter to csi proxy mounter")
 	}
-	return proxyMounter.FormatAndMount(source, target, fstype, options)
+	return proxyMounter.FormatAndMountSensitiveWithFormatOptions(source, target, fstype, options, sensitiveOptions, formatOptions)
 }
 
 // GetDeviceNameFromMount returns the volume ID for a mount path.

pkg/driver/node.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	csi "github.com/container-storage-interface/spec/lib/go/csi"
@@ -111,6 +112,17 @@ func newNodeService(driverOptions *DriverOptions) nodeService {
 func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRequest) (*csi.NodeStageVolumeResponse, error) {
 	klog.V(4).Infof("NodeStageVolume: called with args %+v", *req)
 
+	context := req.GetVolumeContext()
+	blockSize, ok := context[BlockSizeKey]
+	if ok {
+		// This check is already performed on the controller side
+		// However, because it is potentially security-sensitive, we redo it here to be safe
+		_, err := strconv.Atoi(blockSize)
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "Invalid blockSize (aborting!): %v", err)
+		}
+	}
+
 	volumeID := req.GetVolumeId()
 	if len(volumeID) == 0 {
 		return nil, status.Error(codes.InvalidArgument, "Volume ID not provided")
@@ -150,7 +162,7 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 		fsType = defaultFsType
 	}
 
-	_, ok := ValidFSTypes[strings.ToLower(fsType)]
+	_, ok = ValidFSTypes[strings.ToLower(fsType)]
 	if !ok {
 		return nil, status.Errorf(codes.InvalidArgument, "NodeStageVolume: invalid fstype %s", fsType)
 	}
@@ -219,7 +231,11 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 
 	// FormatAndMount will format only if needed
 	klog.V(4).Infof("NodeStageVolume: formatting %s and mounting at %s with fstype %s", source, target, fsType)
-	err = d.mounter.FormatAndMount(source, target, fsType, mountOptions)
+	formatOptions := []string{}
+	if len(blockSize) > 0 {
+		formatOptions = append(formatOptions, "-b", blockSize)
+	}
+	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)
 		return nil, status.Error(codes.Internal, msg)