rbd: use helper functions from csi-common for VolumeCapability checking

The internal/csi-common package offers helper functions like
`IsReaderOnly()` and `IsBlockMultiNode()`. These should be used instead
of checking the VolumeCapability that is passed in a request in
different places.

This also suggested that adding the "ro" mount option in
`NodeServer.mountVolumeToStagePath()` is not appropriate, as the
csi-common helper `ConstructMountOptions()` can take care of that
already too.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
This commit is contained in:
Niels de Vos 2025-04-24 17:08:38 +02:00 committed by mergify[bot]
parent 34be059dd5
commit ea7be34396
2 changed files with 14 additions and 22 deletions

View File

@ -93,6 +93,14 @@ func ConstructMountOptions(mountOptions []string, volCap *csi.VolumeCapability)
} }
} }
// add "ro" in case the capabilities indicate READER_ONLY
rOnly := "ro"
if IsReaderOnly([]*csi.VolumeCapability{volCap}) {
if !MountOptionContains(mountOptions, rOnly) {
mountOptions = append(mountOptions, rOnly)
}
}
return mountOptions return mountOptions
} }

View File

@ -165,10 +165,12 @@ func (ns *NodeServer) populateRbdVol(
) (*rbdVolume, error) { ) (*rbdVolume, error) {
var err error var err error
volID := req.GetVolumeId() volID := req.GetVolumeId()
isBlock := req.GetVolumeCapability().GetBlock() != nil
isBlock, isMultiNode := csicommon.IsBlockMultiNode([]*csi.VolumeCapability{req.GetVolumeCapability()})
disableInUseChecks := false disableInUseChecks := false
// MULTI_NODE_MULTI_WRITER is supported by default for Block access type volumes // MULTI_NODE_MULTI_WRITER is supported by default for Block access type volumes
if req.GetVolumeCapability().GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER { if isMultiNode {
if !isBlock { if !isBlock {
log.WarningLog( log.WarningLog(
ctx, ctx,
@ -216,6 +218,7 @@ func (ns *NodeServer) populateRbdVol(
} }
rv.DisableInUseChecks = disableInUseChecks rv.DisableInUseChecks = disableInUseChecks
rv.readOnly = csicommon.IsReaderOnly([]*csi.VolumeCapability{req.GetVolumeCapability()})
err = rv.Connect(cr) err = rv.Connect(cr)
if err != nil { if err != nil {
@ -404,13 +407,6 @@ func (ns *NodeServer) stageTransaction(
var err error var err error
// Allow image to be mounted on multiple nodes if it is ROX
if req.GetVolumeCapability().GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY {
log.ExtendedLog(ctx, "setting disableInUseChecks on rbd volume to: %v", req.GetVolumeId)
volOptions.DisableInUseChecks = true
volOptions.readOnly = true
}
err = flattenImageBeforeMapping(ctx, volOptions) err = flattenImageBeforeMapping(ctx, volOptions)
if err != nil { if err != nil {
return transaction, err return transaction, err
@ -760,7 +756,6 @@ func (ns *NodeServer) mountVolumeToStagePath(
stagingPath, devicePath string, stagingPath, devicePath string,
fileEncryption bool, fileEncryption bool,
) error { ) error {
readOnly := false
fsType := req.GetVolumeCapability().GetMount().GetFsType() fsType := req.GetVolumeCapability().GetMount().GetFsType()
diskMounter := &mount.SafeFormatAndMount{Interface: ns.Mounter, Exec: utilexec.New()} diskMounter := &mount.SafeFormatAndMount{Interface: ns.Mounter, Exec: utilexec.New()}
// rbd images are thin-provisioned and return zeros for unwritten areas. A freshly created // rbd images are thin-provisioned and return zeros for unwritten areas. A freshly created
@ -784,18 +779,7 @@ func (ns *NodeServer) mountVolumeToStagePath(
opt = append(opt, "_netdev") opt = append(opt, "_netdev")
opt = csicommon.ConstructMountOptions(opt, req.GetVolumeCapability()) opt = csicommon.ConstructMountOptions(opt, req.GetVolumeCapability())
isBlock := req.GetVolumeCapability().GetBlock() != nil isBlock := req.GetVolumeCapability().GetBlock() != nil
rOnly := "ro" readOnly := csicommon.IsReaderOnly([]*csi.VolumeCapability{req.GetVolumeCapability()})
mode := req.GetVolumeCapability().GetAccessMode().GetMode()
if mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY ||
mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY {
if !csicommon.MountOptionContains(opt, rOnly) {
opt = append(opt, rOnly)
}
}
if csicommon.MountOptionContains(opt, rOnly) {
readOnly = true
}
if existingFormat == "" && !staticVol && !readOnly && !isBlock { if existingFormat == "" && !staticVol && !readOnly && !isBlock {
args := ns.getMkfsArgs(fsType) args := ns.getMkfsArgs(fsType)