Skip to content

Commit d7fa2d8

Browse files
committed
feat(cinder-csi): add mkfsOptions StorageClass parameter
Add support for a new `mkfsOptions` StorageClass parameter that allows cluster admins to pass custom mkfs flags when formatting volumes. This is particularly useful for Ceph-backed storage where passing `-E nodiscard` to mkfs.ext4 can dramatically speed up provisioning (e.g. from ~7 minutes to seconds for a 100 GB volume). The parameter value is comma-separated, with each token further split on whitespace to produce individual mkfs command-line arguments. Example: `mkfsOptions: "-E nodiscard,-O ^metadata_csum"` Implementation: - Extract mkfsOptions from StorageClass params in CreateVolume and pass via VolumeContext (both new-volume and idempotent early-return paths) - Parse mkfsOptions in NodeStageVolume using parseMkfsOptions() helper - Use FormatAndMountSensitiveWithFormatOptions() from k8s.io/mount-utils to pass format options separately from mount options - No behavior change when mkfsOptions is not set Inspired by the Ceph CSI implementation (ceph/ceph-csi#621).
1 parent 9c80105 commit d7fa2d8

File tree

5 files changed

+218
-5
lines changed

5 files changed

+218
-5
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ helm install --namespace kube-system --name cinder-csi ./charts/cinder-csi-plugi
283283
|------------------------- |-----------------------|-----------------|-----------------|
284284
| StorageClass `parameters` | `availability` | `nova` | String. Volume Availability Zone |
285285
| StorageClass `parameters` | `type` | Empty String | String. Name/ID of Volume type. Corresponding volume type should exist in cinder |
286+
| StorageClass `parameters` | `mkfsOptions` | Empty String | String. Comma-separated mkfs options passed to mkfs when formatting the volume. Useful for passing `-E nodiscard` on Ceph-backed storage to speed up provisioning. |
286287
| VolumeSnapshotClass `parameters` | `force-create` | `false` | Enable to support creating snapshot for a volume in in-use status |
287288
| 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 |
288289
| 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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,11 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
119119
}
120120
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)
121121
accessibleTopology := getTopology(&vols[0], accessibleTopologyReq, cs.Driver.withTopology, ignoreVolumeAZ)
122-
return getCreateVolumeResponse(&vols[0], nil, accessibleTopology), nil
122+
existingVolCtx := map[string]string{}
123+
if mkfsOptions := volParams["mkfsOptions"]; mkfsOptions != "" {
124+
existingVolCtx["mkfsOptions"] = mkfsOptions
125+
}
126+
return getCreateVolumeResponse(&vols[0], existingVolCtx, accessibleTopology), nil
123127
}
124128

125129
if len(vols) > 1 {
@@ -233,6 +237,11 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
233237
klog.V(4).Infof("CreateVolume: Resolved scheduler hints: affinity=%s, anti-affinity=%s", affinity, antiAffinity)
234238
}
235239

240+
// Pass mkfsOptions to node via volume context
241+
if mkfsOptions := volParams["mkfsOptions"]; mkfsOptions != "" {
242+
volCtx["mkfsOptions"] = mkfsOptions
243+
}
244+
236245
vol, err := cloud.CreateVolume(ctx, opts, schedulerHints)
237246
if err != nil {
238247
klog.Errorf("Failed to CreateVolume: %v", err)

pkg/csi/cinder/controllerserver_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,94 @@ func TestCreateVolumeWithParam(t *testing.T) {
200200
assert.Equal(FakeAvailability, actualRes.Volume.AccessibleTopology[0].GetSegments()[topologyKey])
201201
}
202202

203+
// Test CreateVolume passes mkfsOptions to VolumeContext
204+
func TestCreateVolumeWithMkfsOptions(t *testing.T) {
205+
fakeCs, osmock := fakeControllerServer()
206+
207+
// mock OpenStack
208+
properties := map[string]string{cinderCSIClusterIDKey: FakeCluster}
209+
osmock.On("CreateVolume", FakeVolName, mock.AnythingOfType("int"), FakeVolType, FakeAvailability, "", "", "", properties).Return(&FakeVol, nil)
210+
osmock.On("GetVolumesByName", FakeVolName).Return(FakeVolListEmpty, nil)
211+
osmock.On("GetBlockStorageOpts").Return(openstack.BlockStorageOpts{})
212+
213+
assert := assert.New(t)
214+
215+
// Fake request with mkfsOptions
216+
fakeReq := &csi.CreateVolumeRequest{
217+
Name: FakeVolName,
218+
VolumeCapabilities: []*csi.VolumeCapability{
219+
{
220+
AccessMode: &csi.VolumeCapability_AccessMode{
221+
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
222+
},
223+
},
224+
},
225+
Parameters: map[string]string{
226+
"mkfsOptions": "-E nodiscard",
227+
},
228+
AccessibilityRequirements: &csi.TopologyRequirement{
229+
Requisite: []*csi.Topology{
230+
{
231+
Segments: map[string]string{topologyKey: FakeAvailability},
232+
},
233+
},
234+
},
235+
}
236+
237+
// Invoke CreateVolume
238+
actualRes, err := fakeCs.CreateVolume(FakeCtx, fakeReq)
239+
if err != nil {
240+
t.Errorf("failed to CreateVolume: %v", err)
241+
}
242+
243+
// Assert mkfsOptions is in VolumeContext
244+
assert.NotNil(actualRes.Volume)
245+
assert.Equal("-E nodiscard", actualRes.Volume.VolumeContext["mkfsOptions"])
246+
}
247+
248+
// Test CreateVolume passes mkfsOptions on idempotent path (volume already exists)
249+
func TestCreateVolumeExistingWithMkfsOptions(t *testing.T) {
250+
fakeCs, osmock := fakeControllerServer()
251+
252+
// mock OpenStack - volume already exists
253+
osmock.On("GetVolumesByName", FakeVolName).Return(FakeVolList, nil)
254+
osmock.On("GetBlockStorageOpts").Return(openstack.BlockStorageOpts{})
255+
256+
assert := assert.New(t)
257+
258+
// Fake request with mkfsOptions
259+
fakeReq := &csi.CreateVolumeRequest{
260+
Name: FakeVolName,
261+
VolumeCapabilities: []*csi.VolumeCapability{
262+
{
263+
AccessMode: &csi.VolumeCapability_AccessMode{
264+
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
265+
},
266+
},
267+
},
268+
Parameters: map[string]string{
269+
"mkfsOptions": "-E nodiscard",
270+
},
271+
AccessibilityRequirements: &csi.TopologyRequirement{
272+
Requisite: []*csi.Topology{
273+
{
274+
Segments: map[string]string{topologyKey: FakeAvailability},
275+
},
276+
},
277+
},
278+
}
279+
280+
// Invoke CreateVolume
281+
actualRes, err := fakeCs.CreateVolume(FakeCtx, fakeReq)
282+
if err != nil {
283+
t.Errorf("failed to CreateVolume: %v", err)
284+
}
285+
286+
// Assert mkfsOptions is in VolumeContext even on idempotent path
287+
assert.NotNil(actualRes.Volume)
288+
assert.Equal("-E nodiscard", actualRes.Volume.VolumeContext["mkfsOptions"])
289+
}
290+
203291
// Test CreateVolume with ignore-volume-az option
204292
func TestCreateVolumeWithIgnoreVolumeAZ(t *testing.T) {
205293
fakeCs, osmock := fakeControllerServer()

pkg/csi/cinder/nodeserver.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,16 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
219219
mountFlags := mnt.GetMountFlags()
220220
options = append(options, collectMountOptions(fsType, mountFlags)...)
221221
}
222+
223+
// Parse mkfs options from volume context
224+
var formatOptions []string
225+
if mkfsOpts, ok := volumeContext["mkfsOptions"]; ok && mkfsOpts != "" {
226+
formatOptions = parseMkfsOptions(mkfsOpts)
227+
klog.V(4).Infof("NodeStageVolume: mkfsOptions for volume %s: %v", volumeID, formatOptions)
228+
}
229+
222230
// Mount
223-
err = ns.formatAndMountRetry(devicePath, stagingTarget, fsType, options)
231+
err = ns.formatAndMountRetry(devicePath, stagingTarget, fsType, options, formatOptions)
224232
if err != nil {
225233
return nil, status.Error(codes.Internal, err.Error())
226234
}
@@ -248,9 +256,9 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
248256

249257
// formatAndMountRetry attempts to format and mount a device at the given path.
250258
// If the initial mount fails, it rescans the device and retries the mount operation.
251-
func (ns *nodeServer) formatAndMountRetry(devicePath, stagingTarget, fsType string, options []string) error {
259+
func (ns *nodeServer) formatAndMountRetry(devicePath, stagingTarget, fsType string, options []string, formatOptions []string) error {
252260
m := ns.Mount
253-
err := m.Mounter().FormatAndMount(devicePath, stagingTarget, fsType, options)
261+
err := m.Mounter().FormatAndMountSensitiveWithFormatOptions(devicePath, stagingTarget, fsType, options, nil, formatOptions)
254262
if err != nil {
255263
klog.Infof("Initial format and mount failed: %v. Attempting rescan.", err)
256264
// Attempting rescan if the initial mount fails
@@ -260,7 +268,7 @@ func (ns *nodeServer) formatAndMountRetry(devicePath, stagingTarget, fsType stri
260268
return err
261269
}
262270
klog.Infof("Rescan succeeded, retrying format and mount")
263-
err = m.Mounter().FormatAndMount(devicePath, stagingTarget, fsType, options)
271+
err = m.Mounter().FormatAndMountSensitiveWithFormatOptions(devicePath, stagingTarget, fsType, options, nil, formatOptions)
264272
}
265273
return err
266274
}
@@ -429,6 +437,20 @@ func getDevicePath(volumeID string, m mount.IMount) (string, error) {
429437
return devicePath, nil
430438
}
431439

440+
// parseMkfsOptions splits a comma-separated mkfsOptions string into individual
441+
// command-line arguments. Each comma-separated token is further split on
442+
// whitespace. Empty tokens are filtered out.
443+
func parseMkfsOptions(mkfsOpts string) []string {
444+
var formatOptions []string
445+
for _, opt := range strings.Split(mkfsOpts, ",") {
446+
opt = strings.TrimSpace(opt)
447+
if opt != "" {
448+
formatOptions = append(formatOptions, strings.Fields(opt)...)
449+
}
450+
}
451+
return formatOptions
452+
}
453+
432454
func collectMountOptions(fsType string, mntFlags []string) []string {
433455
var options []string
434456
options = append(options, mntFlags...)

pkg/csi/cinder/nodeserver_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,48 @@ func TestNodeStageVolumeBlock(t *testing.T) {
236236
assert.Equal(expectedRes, actualRes)
237237
}
238238

239+
// Test NodeStageVolume with mkfsOptions in VolumeContext
240+
func TestNodeStageVolumeWithMkfsOptions(t *testing.T) {
241+
fakeNs, omock, mmock, _ := fakeNodeServer()
242+
243+
mmock.On("GetDevicePath", FakeVolID).Return(FakeDevicePath, nil)
244+
mmock.On("IsLikelyNotMountPointAttach", FakeStagingTargetPath).Return(true, nil)
245+
omock.On("GetVolume", FakeVolID).Return(FakeVol, nil)
246+
247+
assert := assert.New(t)
248+
249+
// Expected Result
250+
expectedRes := &csi.NodeStageVolumeResponse{}
251+
stdVolCap := &csi.VolumeCapability{
252+
AccessType: &csi.VolumeCapability_Mount{
253+
Mount: &csi.VolumeCapability_MountVolume{},
254+
},
255+
AccessMode: &csi.VolumeCapability_AccessMode{
256+
Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
257+
},
258+
}
259+
260+
// Fake request with mkfsOptions in VolumeContext
261+
fakeReq := &csi.NodeStageVolumeRequest{
262+
VolumeId: FakeVolID,
263+
PublishContext: map[string]string{"DevicePath": FakeDevicePath},
264+
StagingTargetPath: FakeStagingTargetPath,
265+
VolumeCapability: stdVolCap,
266+
VolumeContext: map[string]string{
267+
"mkfsOptions": "-E nodiscard",
268+
},
269+
}
270+
271+
// Invoke NodeStageVolume
272+
actualRes, err := fakeNs.NodeStageVolume(FakeCtx, fakeReq)
273+
if err != nil {
274+
t.Errorf("failed to NodeStageVolume: %v", err)
275+
}
276+
277+
// Assert
278+
assert.Equal(expectedRes, actualRes)
279+
}
280+
239281
// Test NodeUnpublishVolume
240282
func TestNodeUnpublishVolume(t *testing.T) {
241283
fakeNs, omock, mmock, _ := fakeNodeServer()
@@ -396,3 +438,54 @@ func TestNodeGetVolumeStatsFs(t *testing.T) {
396438
assert.Equal(expectedFsRes, fsRes)
397439

398440
}
441+
442+
func TestParseMkfsOptions(t *testing.T) {
443+
tests := []struct {
444+
name string
445+
input string
446+
expected []string
447+
}{
448+
{
449+
name: "single option with flag",
450+
input: "-E nodiscard",
451+
expected: []string{"-E", "nodiscard"},
452+
},
453+
{
454+
name: "multiple comma-separated options",
455+
input: "-E nodiscard,-O ^metadata_csum",
456+
expected: []string{"-E", "nodiscard", "-O", "^metadata_csum"},
457+
},
458+
{
459+
name: "trailing comma",
460+
input: "-E nodiscard,",
461+
expected: []string{"-E", "nodiscard"},
462+
},
463+
{
464+
name: "double comma",
465+
input: "-E nodiscard,,-b 4096",
466+
expected: []string{"-E", "nodiscard", "-b", "4096"},
467+
},
468+
{
469+
name: "whitespace around commas",
470+
input: " -E nodiscard , -b 4096 ",
471+
expected: []string{"-E", "nodiscard", "-b", "4096"},
472+
},
473+
{
474+
name: "single flag no value",
475+
input: "-F",
476+
expected: []string{"-F"},
477+
},
478+
{
479+
name: "empty string",
480+
input: "",
481+
expected: nil,
482+
},
483+
}
484+
485+
for _, tt := range tests {
486+
t.Run(tt.name, func(t *testing.T) {
487+
result := parseMkfsOptions(tt.input)
488+
assert.Equal(t, tt.expected, result)
489+
})
490+
}
491+
}

0 commit comments

Comments
 (0)