feat(cinder-csi): add mkfs-options StorageClass parameter

Add support for a new `mkfs-options` StorageClass parameter that allows
cluster admins to pass custom mkfs flags when formatting volumes.
For example, this is useful for Ceph-backed storage where passing `-E
nodiscard` to mkfs.ext4 can dramatically speed up provisioning.

Implementation:
- Extract mkfs-options from StorageClass params in CreateVolume and pass
  via VolumeContext (both new-volume and idempotent early-return paths)
- Parse mkfs-options in NodeStageVolume
- Use FormatAndMountSensitiveWithFormatOptions() from k8s.io/mount-utils
  to pass format options separately from mount options
- No behavior change when mkfs-options is not set

Inspired by the Ceph CSI implementation (ceph/ceph-csi#621).

MountMock.Mounter() now returns a stable, memoized SafeFormatAndMount
with a SetMounter escape hatch, enabling tests to script and capture the
mkfs argv via FakeExec.

Author: rybnico

docs/cinder-csi-plugin/using-cinder-csi-plugin.md

@@ -283,6 +283,7 @@ helm install --namespace kube-system --name cinder-csi ./charts/cinder-csi-plugi
 |-------------------------   |-----------------------|-----------------|-----------------|
 | StorageClass `parameters`  | `availability`          | `nova`          | String. Volume Availability Zone |
 | StorageClass `parameters`  | `type`                  | Empty String    | String. Name/ID of Volume type. Corresponding volume type should exist in cinder     |
+| StorageClass `parameters`  | `mkfs-options`           | Empty String    | String. Whitespace-separated arguments passed to mkfs when formatting the volume. |
 | VolumeSnapshotClass `parameters` | `force-create`    | `false`         | Enable to support creating snapshot for a volume in in-use status |
 | VolumeSnapshotClass `parameters` | `type`            | Empty String    | `snapshot` creates a VolumeSnapshot object linked to a Cinder volume snapshot. `backup` creates a VolumeSnapshot object linked to a cinder volume backup. Defaults to `snapshot` if not defined |
 | VolumeSnapshotClass `parameters` | `backup-max-duration-seconds-per-gb`  | `20`    | Defines the amount of time to wait for a backup to complete in seconds per GB of volume size |

pkg/csi/cinder/controllerserver.go

@@ -119,7 +119,11 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		}
 		klog.V(4).Infof("Volume %s already exists in Availability Zone: %s of size %d GiB", vols[0].ID, vols[0].AvailabilityZone, vols[0].Size)
 		accessibleTopology := getTopology(&vols[0], accessibleTopologyReq, cs.Driver.withTopology, ignoreVolumeAZ)
-		return getCreateVolumeResponse(&vols[0], nil, accessibleTopology), nil
+		existingVolCtx := map[string]string{}
+		if mkfsOptions := volParams["mkfs-options"]; mkfsOptions != "" {
+			existingVolCtx["mkfs-options"] = mkfsOptions
+		}
+		return getCreateVolumeResponse(&vols[0], existingVolCtx, accessibleTopology), nil
 	}
 
 	if len(vols) > 1 {
@@ -233,6 +237,11 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		klog.V(4).Infof("CreateVolume: Resolved scheduler hints: affinity=%s, anti-affinity=%s", affinity, antiAffinity)
 	}
 
+	// Pass mkfsOptions to node via volume context
+	if mkfsOptions := volParams["mkfs-options"]; mkfsOptions != "" {
+		volCtx["mkfs-options"] = mkfsOptions
+	}
+
 	vol, err := cloud.CreateVolume(ctx, opts, schedulerHints)
 	if err != nil {
 		klog.Errorf("Failed to CreateVolume: %v", err)

pkg/csi/cinder/controllerserver_test.go

@@ -200,6 +200,94 @@ func TestCreateVolumeWithParam(t *testing.T) {
 	assert.Equal(FakeAvailability, actualRes.Volume.AccessibleTopology[0].GetSegments()[topologyKey])
 }
 
+// Test CreateVolume passes mkfs-options to VolumeContext
+func TestCreateVolumeWithMkfsOptions(t *testing.T) {
+	fakeCs, osmock := fakeControllerServer()
+
+	// mock OpenStack
+	properties := map[string]string{cinderCSIClusterIDKey: FakeCluster}
+	osmock.On("CreateVolume", FakeVolName, mock.AnythingOfType("int"), FakeVolType, FakeAvailability, "", "", "", properties).Return(&FakeVol, nil)
+	osmock.On("GetVolumesByName", FakeVolName).Return(FakeVolListEmpty, nil)
+	osmock.On("GetBlockStorageOpts").Return(openstack.BlockStorageOpts{})
+
+	assert := assert.New(t)
+
+	// Fake request with mkfs-options
+	fakeReq := &csi.CreateVolumeRequest{
+		Name: FakeVolName,
+		VolumeCapabilities: []*csi.VolumeCapability{
+			{
+				AccessMode: &csi.VolumeCapability_AccessMode{
+					Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
+				},
+			},
+		},
+		Parameters: map[string]string{
+			"mkfs-options": "-E nodiscard",
+		},
+		AccessibilityRequirements: &csi.TopologyRequirement{
+			Requisite: []*csi.Topology{
+				{
+					Segments: map[string]string{topologyKey: FakeAvailability},
+				},
+			},
+		},
+	}
+
+	// Invoke CreateVolume
+	actualRes, err := fakeCs.CreateVolume(FakeCtx, fakeReq)
+	if err != nil {
+		t.Errorf("failed to CreateVolume: %v", err)
+	}
+
+	// Assert mkfs-options is in VolumeContext
+	assert.NotNil(actualRes.Volume)
+	assert.Equal("-E nodiscard", actualRes.Volume.VolumeContext["mkfs-options"])
+}
+
+// Test CreateVolume passes mkfs-options on idempotent path (volume already exists)
+func TestCreateVolumeExistingWithMkfsOptions(t *testing.T) {
+	fakeCs, osmock := fakeControllerServer()
+
+	// mock OpenStack - volume already exists
+	osmock.On("GetVolumesByName", FakeVolName).Return(FakeVolList, nil)
+	osmock.On("GetBlockStorageOpts").Return(openstack.BlockStorageOpts{})
+
+	assert := assert.New(t)
+
+	// Fake request with mkfs-options
+	fakeReq := &csi.CreateVolumeRequest{
+		Name: FakeVolName,
+		VolumeCapabilities: []*csi.VolumeCapability{
+			{
+				AccessMode: &csi.VolumeCapability_AccessMode{
+					Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
+				},
+			},
+		},
+		Parameters: map[string]string{
+			"mkfs-options": "-E nodiscard",
+		},
+		AccessibilityRequirements: &csi.TopologyRequirement{
+			Requisite: []*csi.Topology{
+				{
+					Segments: map[string]string{topologyKey: FakeAvailability},
+				},
+			},
+		},
+	}
+
+	// Invoke CreateVolume
+	actualRes, err := fakeCs.CreateVolume(FakeCtx, fakeReq)
+	if err != nil {
+		t.Errorf("failed to CreateVolume: %v", err)
+	}
+
+	// Assert mkfs-options is in VolumeContext even on idempotent path
+	assert.NotNil(actualRes.Volume)
+	assert.Equal("-E nodiscard", actualRes.Volume.VolumeContext["mkfs-options"])
+}
+
 // Test CreateVolume with ignore-volume-az option
 func TestCreateVolumeWithIgnoreVolumeAZ(t *testing.T) {
 	fakeCs, osmock := fakeControllerServer()

pkg/csi/cinder/nodeserver.go

@@ -219,8 +219,16 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 			mountFlags := mnt.GetMountFlags()
 			options = append(options, collectMountOptions(fsType, mountFlags)...)
 		}
+
+		// Parse mkfs options from volume context
+		var formatOptions []string
+		if mkfsOpts, ok := volumeContext["mkfs-options"]; ok && mkfsOpts != "" {
+			formatOptions = strings.Fields(mkfsOpts)
+			klog.V(4).Infof("NodeStageVolume: mkfs-options for volume %s: %v", volumeID, formatOptions)
+		}
+
 		// Mount
-		err = ns.formatAndMountRetry(devicePath, stagingTarget, fsType, options)
+		err = ns.formatAndMountRetry(devicePath, stagingTarget, fsType, options, formatOptions)
 		if err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
@@ -248,9 +256,9 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 
 // formatAndMountRetry attempts to format and mount a device at the given path.
 // If the initial mount fails, it rescans the device and retries the mount operation.
-func (ns *nodeServer) formatAndMountRetry(devicePath, stagingTarget, fsType string, options []string) error {
+func (ns *nodeServer) formatAndMountRetry(devicePath, stagingTarget, fsType string, options []string, formatOptions []string) error {
 	m := ns.Mount
-	err := m.Mounter().FormatAndMount(devicePath, stagingTarget, fsType, options)
+	err := m.Mounter().FormatAndMountSensitiveWithFormatOptions(devicePath, stagingTarget, fsType, options, nil, formatOptions)
 	if err != nil {
 		klog.Infof("Initial format and mount failed: %v. Attempting rescan.", err)
 		// Attempting rescan if the initial mount fails
@@ -260,7 +268,7 @@ func (ns *nodeServer) formatAndMountRetry(devicePath, stagingTarget, fsType stri
 			return err
 		}
 		klog.Infof("Rescan succeeded, retrying format and mount")
-		err = m.Mounter().FormatAndMount(devicePath, stagingTarget, fsType, options)
+		err = m.Mounter().FormatAndMountSensitiveWithFormatOptions(devicePath, stagingTarget, fsType, options, nil, formatOptions)
 	}
 	return err
 }

pkg/csi/cinder/nodeserver_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
@@ -30,6 +31,9 @@ import (
 	"k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack"
 	"k8s.io/cloud-provider-openstack/pkg/util/metadata"
 	"k8s.io/cloud-provider-openstack/pkg/util/mount"
+	mountutil "k8s.io/mount-utils"
+	utilsexec "k8s.io/utils/exec"
+	testingexec "k8s.io/utils/exec/testing"
 )
 
 func fakeNodeServer() (*nodeServer, *openstack.OpenStackMock, *mount.MountMock, *metadata.MetadataMock) {
@@ -236,6 +240,134 @@ func TestNodeStageVolumeBlock(t *testing.T) {
 	assert.Equal(expectedRes, actualRes)
 }
 
+// TestNodeStageVolume_MkfsOptions_ArgvCapture drives NodeStageVolume with a
+// range of mkfs-options values and asserts that the argv handed to mkfs
+// starts with the user-supplied arguments.
+func TestNodeStageVolume_MkfsOptions_ArgvCapture(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skipf("mkfs argv capture only runs on linux (GOOS=%s)", runtime.GOOS)
+	}
+
+	stdMountVolCap := &csi.VolumeCapability{
+		AccessType: &csi.VolumeCapability_Mount{
+			Mount: &csi.VolumeCapability_MountVolume{},
+		},
+		AccessMode: &csi.VolumeCapability_AccessMode{
+			Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
+		},
+	}
+
+	cases := []struct {
+		name              string
+		mkfsOptions       string
+		expectedArguments []string
+	}{
+		{
+			name:              "no options",
+			mkfsOptions:       "",
+			expectedArguments: []string{},
+		},
+		{
+			name:              "single flag",
+			mkfsOptions:       "-j",
+			expectedArguments: []string{"-j"},
+		},
+		{
+			name:              "flag and value",
+			mkfsOptions:       "-E nodiscard",
+			expectedArguments: []string{"-E", "nodiscard"},
+		},
+		{
+			name:              "multiple options",
+			mkfsOptions:       "-E nodiscard -j",
+			expectedArguments: []string{"-E", "nodiscard", "-j"},
+		},
+		{
+			// Demonstrates the bug in util.SplitTrim: it also splits on
+			// whitespace, so the comma-containing value was shredded into
+			// two separate mkfs args and treated by mkfs as a block count.
+			name:              "commas inside option value",
+			mkfsOptions:       "-E lazy_itable_init=0,lazy_journal_init=0",
+			expectedArguments: []string{"-E", "lazy_itable_init=0,lazy_journal_init=0"},
+		},
+		{
+			name:              "extra whitespace",
+			mkfsOptions:       "  -j   -F  ",
+			expectedArguments: []string{"-j", "-F"},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			fakeNs, omock, mmock, _ := fakeNodeServer()
+
+			mmock.On("GetDevicePath", FakeVolID).Return(FakeDevicePath, nil)
+			mmock.On("IsLikelyNotMountPointAttach", FakeStagingTargetPath).Return(true, nil)
+			omock.On("GetVolume", FakeVolID).Return(FakeVol, nil)
+
+			// Script exec:
+			//   1. blkid → exit 2 (device is unformatted) to trigger the mkfs path.
+			//   2. mkfs.ext4 → succeed, and capture the argv it was called with.
+			var capturedMkfsArgs []string
+
+			fakeExec := &testingexec.FakeExec{}
+
+			blkidCmd := &testingexec.FakeCmd{
+				CombinedOutputScript: []testingexec.FakeAction{
+					func() ([]byte, []byte, error) {
+						return nil, nil, &testingexec.FakeExitError{Status: 2}
+					},
+				},
+			}
+			fakeExec.CommandScript = append(fakeExec.CommandScript,
+				func(cmd string, args ...string) utilsexec.Cmd {
+					return testingexec.InitFakeCmd(blkidCmd, cmd, args...)
+				},
+			)
+
+			mkfsCmd := &testingexec.FakeCmd{
+				CombinedOutputScript: []testingexec.FakeAction{
+					func() ([]byte, []byte, error) { return nil, nil, nil },
+				},
+			}
+			fakeExec.CommandScript = append(fakeExec.CommandScript,
+				func(cmd string, args ...string) utilsexec.Cmd {
+					capturedMkfsArgs = append([]string(nil), args...)
+					return testingexec.InitFakeCmd(mkfsCmd, cmd, args...)
+				},
+			)
+
+			mmock.SetMounter(&mountutil.SafeFormatAndMount{
+				Interface: mount.NewFakeMounter(),
+				Exec:      fakeExec,
+			})
+
+			volCtx := map[string]string{}
+			if tc.mkfsOptions != "" {
+				volCtx["mkfs-options"] = tc.mkfsOptions
+			}
+
+			fakeReq := &csi.NodeStageVolumeRequest{
+				VolumeId:          FakeVolID,
+				PublishContext:    map[string]string{"DevicePath": FakeDevicePath},
+				StagingTargetPath: FakeStagingTargetPath,
+				VolumeCapability:  stdMountVolCap,
+				VolumeContext:     volCtx,
+			}
+
+			_, err := fakeNs.NodeStageVolume(FakeCtx, fakeReq)
+			assert.NoError(t, err)
+
+			// mount-utils constructs mkfs argv as: formatOptions ++ ["-F", "-m0", devicePath].
+			// Our user-supplied options therefore occupy the argv prefix.
+			assert.GreaterOrEqual(t, len(capturedMkfsArgs), len(tc.expectedArguments),
+				"captured argv = %v", capturedMkfsArgs)
+			assert.Equal(t, tc.expectedArguments, capturedMkfsArgs[:len(tc.expectedArguments)],
+				"mkfs argv prefix mismatch; full argv = %v", capturedMkfsArgs)
+		})
+	}
+}
+
 // Test NodeUnpublishVolume
 func TestNodeUnpublishVolume(t *testing.T) {
 	fakeNs, omock, mmock, _ := fakeNodeServer()
@@ -394,5 +526,4 @@ func TestNodeGetVolumeStatsFs(t *testing.T) {
 
 	assert.NoError(err)
 	assert.Equal(expectedFsRes, fsRes)
-
 }

pkg/util/mount/mount_mock.go

@@ -28,10 +28,22 @@ import (
 // ORIGINALLY GENERATED BY mockery with hand edits
 type MountMock struct {
 	mock.Mock
+
+	// safeMounter is lazily constructed on first Mounter() call and returned
+	// unchanged on subsequent calls, so tests can inspect or pre-configure
+	// the associated FakeExec. Tests that need a custom exec script should
+	// call SetMounter before NodeStageVolume runs.
+	safeMounter *mount.SafeFormatAndMount
 }
 
 // revive:enable:exported
 
+// SetMounter injects a caller-built SafeFormatAndMount. Useful in tests that
+// want to script or capture exec invocations via a custom FakeExec.
+func (_m *MountMock) SetMounter(sfm *mount.SafeFormatAndMount) {
+	_m.safeMounter = sfm
+}
+
 func (_m *MountMock) GetDeviceStats(path string) (*DeviceStats, error) {
 	ret := _m.Called(path)
 
@@ -146,6 +158,10 @@ func (_m *MountMock) UnmountPath(mountPath string) error {
 
 // GetBaseMounter provides a mock function
 func (_m *MountMock) Mounter() *mount.SafeFormatAndMount {
+	if _m.safeMounter != nil {
+		return _m.safeMounter
+	}
+
 	scripts := []struct {
 		cmd    string
 		output []byte
@@ -177,10 +193,11 @@ func (_m *MountMock) Mounter() *mount.SafeFormatAndMount {
 		fakeexec.CommandScript = append(fakeexec.CommandScript, cmdAction)
 	}
 
-	return &mount.SafeFormatAndMount{
+	_m.safeMounter = &mount.SafeFormatAndMount{
 		Interface: NewFakeMounter(),
 		Exec:      fakeexec,
 	}
+	return _m.safeMounter
 }
 
 func (_m *MountMock) MakeDir(pathname string) error {