Skip to content

Commit cbc2205

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 cbc2205

File tree

5 files changed

+154
-6
lines changed

5 files changed

+154
-6
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` | `mkfs-options` | Empty String | String. Comma-separated mkfs options passed to mkfs when formatting the volume. |
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["mkfs-options"]; mkfsOptions != "" {
124+
existingVolCtx["mkfs-options"] = 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["mkfs-options"]; mkfsOptions != "" {
242+
volCtx["mkfs-options"] = 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 mkfs-options 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 mkfs-options
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+
"mkfs-options": "-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 mkfs-options is in VolumeContext
244+
assert.NotNil(actualRes.Volume)
245+
assert.Equal("-E nodiscard", actualRes.Volume.VolumeContext["mkfs-options"])
246+
}
247+
248+
// Test CreateVolume passes mkfs-options 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 mkfs-options
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+
"mkfs-options": "-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 mkfs-options is in VolumeContext even on idempotent path
287+
assert.NotNil(actualRes.Volume)
288+
assert.Equal("-E nodiscard", actualRes.Volume.VolumeContext["mkfs-options"])
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: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"google.golang.org/grpc/status"
3030
"k8s.io/klog/v2"
3131
utilpath "k8s.io/utils/path"
32+
"k8s.io/cloud-provider-openstack/pkg/util"
3233

3334
sharedcsi "k8s.io/cloud-provider-openstack/pkg/csi"
3435
"k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack"
@@ -219,8 +220,16 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
219220
mountFlags := mnt.GetMountFlags()
220221
options = append(options, collectMountOptions(fsType, mountFlags)...)
221222
}
223+
224+
// Parse mkfs options from volume context
225+
var formatOptions []string
226+
if mkfsOpts, ok := volumeContext["mkfs-options"]; ok && mkfsOpts != "" {
227+
formatOptions = util.SplitTrim(mkfsOpts, ',')
228+
klog.V(4).Infof("NodeStageVolume: mkfs-options for volume %s: %v", volumeID, formatOptions)
229+
}
230+
222231
// Mount
223-
err = ns.formatAndMountRetry(devicePath, stagingTarget, fsType, options)
232+
err = ns.formatAndMountRetry(devicePath, stagingTarget, fsType, options, formatOptions)
224233
if err != nil {
225234
return nil, status.Error(codes.Internal, err.Error())
226235
}
@@ -248,9 +257,9 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
248257

249258
// formatAndMountRetry attempts to format and mount a device at the given path.
250259
// 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 {
260+
func (ns *nodeServer) formatAndMountRetry(devicePath, stagingTarget, fsType string, options []string, formatOptions []string) error {
252261
m := ns.Mount
253-
err := m.Mounter().FormatAndMount(devicePath, stagingTarget, fsType, options)
262+
err := m.Mounter().FormatAndMountSensitiveWithFormatOptions(devicePath, stagingTarget, fsType, options, nil, formatOptions)
254263
if err != nil {
255264
klog.Infof("Initial format and mount failed: %v. Attempting rescan.", err)
256265
// Attempting rescan if the initial mount fails
@@ -260,7 +269,7 @@ func (ns *nodeServer) formatAndMountRetry(devicePath, stagingTarget, fsType stri
260269
return err
261270
}
262271
klog.Infof("Rescan succeeded, retrying format and mount")
263-
err = m.Mounter().FormatAndMount(devicePath, stagingTarget, fsType, options)
272+
err = m.Mounter().FormatAndMountSensitiveWithFormatOptions(devicePath, stagingTarget, fsType, options, nil, formatOptions)
264273
}
265274
return err
266275
}

pkg/csi/cinder/nodeserver_test.go

Lines changed: 42 additions & 1 deletion
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+
"mkfs-options": "-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()
@@ -394,5 +436,4 @@ func TestNodeGetVolumeStatsFs(t *testing.T) {
394436

395437
assert.NoError(err)
396438
assert.Equal(expectedFsRes, fsRes)
397-
398439
}

0 commit comments

Comments
 (0)