diff --git a/internal/csi-common/utils.go b/internal/csi-common/utils.go index f8096406d..f464f1b1c 100644 --- a/internal/csi-common/utils.go +++ b/internal/csi-common/utils.go @@ -320,3 +320,72 @@ func IsBlockMultiNode(caps []*csi.VolumeCapability) (bool, bool) { return isBlock, isMultiNode } + +// IsFileRWO checks if it is of type RWO and file mode, if it is return value +// will be set to true. +func IsFileRWO(caps []*csi.VolumeCapability) bool { + // the return value has been set to true, if the volume is of file mode and if the capabilities are of RWO + // kind, ie SINGLE NODE but flexible to have one or more writers. This is also used as a validation in caller + // to preserve the backward compatibility we had with file mode RWO volumes. + + // to preserve backward compatibility we allow RWO filemode, ideally SINGLE_NODE_WRITER check is good enough, + // however more granular level check could help us in future, so keeping it here as an additional measure. + for _, cap := range caps { + if cap.AccessMode != nil { + if cap.GetMount() != nil { + switch cap.AccessMode.Mode { //nolint:exhaustive // only check what we want + case csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + csi.VolumeCapability_AccessMode_SINGLE_NODE_MULTI_WRITER, + csi.VolumeCapability_AccessMode_SINGLE_NODE_SINGLE_WRITER: + return true + } + } + } + } + + return false +} + +// IsReaderOnly check and set return value true only when the access mode is `READER ONLY` regardless of file +// or block mode. +func IsReaderOnly(caps []*csi.VolumeCapability) bool { + for _, cap := range caps { + if cap.AccessMode != nil { + switch cap.AccessMode.Mode { //nolint:exhaustive // only check what we want + case csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY, + csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY: + return true + } + } + } + + return false +} + +// IsBlockMultiWriter validates the volume capability slice against the access modes and access type. +// if the capability is of multi write the first return value will be set to true and if the request +// is of type block, the second return value will be set to true. +func IsBlockMultiWriter(caps []*csi.VolumeCapability) (bool, bool) { + // multiWriter has been set and returned after validating multi writer caps regardless of + // single or multi node access mode. The caps check is agnostic to whether it is a filesystem or block + // mode volume. + var multiWriter bool + + // block has been set and returned if the passed in capability is of block volume mode. + var block bool + + for _, cap := range caps { + if cap.AccessMode != nil { + switch cap.AccessMode.Mode { //nolint:exhaustive // only check what we want + case csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, + csi.VolumeCapability_AccessMode_SINGLE_NODE_MULTI_WRITER: + multiWriter = true + } + } + if cap.GetBlock() != nil { + block = true + } + } + + return multiWriter, block +} diff --git a/internal/csi-common/utils_test.go b/internal/csi-common/utils_test.go index 2cb409c0f..3b339dd00 100644 --- a/internal/csi-common/utils_test.go +++ b/internal/csi-common/utils_test.go @@ -177,3 +177,421 @@ func TestIsBlockMultiNode(t *testing.T) { assert.Equal(t, isMultiNode, test.isMultiNode, test.name) } } + +func TestIsFileRWO(t *testing.T) { + t.Parallel() + tests := []struct { + name string + caps []*csi.VolumeCapability + rwoFile bool + }{ + { + name: "non valid", + caps: []*csi.VolumeCapability{ + { + AccessMode: nil, + AccessType: nil, + }, + }, + rwoFile: false, + }, + + { + name: "single writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_SINGLE_WRITER, + }, + }, + }, + rwoFile: true, + }, + { + name: "single node multi writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_MULTI_WRITER, + }, + }, + }, + rwoFile: true, + }, + { + name: "multi node multi writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, + }, + }, + }, + rwoFile: false, + }, + { + name: "multi node multi reader FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY, + }, + }, + }, + rwoFile: false, + }, + { + name: "single node reader FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY, + }, + }, + }, + rwoFile: false, + }, + } + + for _, tt := range tests { + newtt := tt + t.Run(newtt.name, func(t *testing.T) { + t.Parallel() + rwoFile := IsFileRWO(newtt.caps) + if rwoFile != newtt.rwoFile { + t.Errorf("IsFileRWO() rwofile = %v, want %v", rwoFile, newtt.rwoFile) + } + }) + } +} + +func TestIsBlockMultiWriter(t *testing.T) { + t.Parallel() + tests := []struct { + name string + caps []*csi.VolumeCapability + multiWriter bool + block bool + }{ + { + name: "non valid", + caps: []*csi.VolumeCapability{ + { + AccessMode: nil, + AccessType: nil, + }, + }, + multiWriter: false, + block: false, + }, + { + name: "multi node multi writer block mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, + }, + }, + }, + multiWriter: true, + block: true, + }, + { + name: "single node multi writer block mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_MULTI_WRITER, + }, + }, + }, + multiWriter: true, + block: true, + }, + { + name: "single writer block mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_SINGLE_WRITER, + }, + }, + }, + multiWriter: false, + block: true, + }, + { + name: "single writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_SINGLE_WRITER, + }, + }, + }, + multiWriter: false, + block: false, + }, + { + name: "single node multi writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_MULTI_WRITER, + }, + }, + }, + multiWriter: true, + block: false, + }, + { + name: "multi node multi writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, + }, + }, + }, + multiWriter: true, + block: false, + }, + { + name: "multi node multi reader FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY, + }, + }, + }, + multiWriter: false, + block: false, + }, + { + name: "single node reader FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY, + }, + }, + }, + multiWriter: false, + block: false, + }, + { + name: "multi node reader block mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY, + }, + }, + }, + multiWriter: false, + block: true, + }, + { + name: "single node reader block mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY, + }, + }, + }, + multiWriter: false, + block: true, + }, + } + + for _, tt := range tests { + newtt := tt + t.Run(newtt.name, func(t *testing.T) { + t.Parallel() + multiWriter, block := IsBlockMultiWriter(newtt.caps) + if multiWriter != newtt.multiWriter { + t.Errorf("IsBlockMultiWriter() multiWriter = %v, want %v", multiWriter, newtt.multiWriter) + } + if block != newtt.block { + t.Errorf("IsBlockMultiWriter block = %v, want %v", block, newtt.block) + } + }) + } +} + +func TestIsReaderOnly(t *testing.T) { + t.Parallel() + tests := []struct { + name string + caps []*csi.VolumeCapability + roOnly bool + }{ + { + name: "non valid", + caps: []*csi.VolumeCapability{ + { + AccessMode: nil, + AccessType: nil, + }, + }, + roOnly: false, + }, + + { + name: "single writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_SINGLE_WRITER, + }, + }, + }, + roOnly: false, + }, + { + name: "single node multi writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_MULTI_WRITER, + }, + }, + }, + roOnly: false, + }, + { + name: "multi node multi writer FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, + }, + }, + }, + roOnly: false, + }, + { + name: "multi node multi reader FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY, + }, + }, + }, + roOnly: true, + }, + { + name: "single node reader FS mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY, + }, + }, + }, + roOnly: true, + }, + { + name: "multi node reader block mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY, + }, + }, + }, + roOnly: true, + }, + { + name: "single node reader block mode", + caps: []*csi.VolumeCapability{ + { + AccessType: &csi.VolumeCapability_Block{ + Block: &csi.VolumeCapability_BlockVolume{}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY, + }, + }, + }, + roOnly: true, + }, + } + + for _, tt := range tests { + newtt := tt + t.Run(newtt.name, func(t *testing.T) { + t.Parallel() + roOnly := IsReaderOnly(newtt.caps) + if roOnly != newtt.roOnly { + t.Errorf("isReadOnly() roOnly = %v, want %v", roOnly, newtt.roOnly) + } + }) + } +} diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 29c40186f..bef83e771 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -101,11 +101,17 @@ func (cs *ControllerServer) parseVolCreateRequest( req *csi.CreateVolumeRequest) (*rbdVolume, error) { // TODO (sbezverk) Last check for not exceeding total storage capacity - // RO modes need to be handled independently (ie right now even if access mode is RO, they'll be RW upon attach) - isBlock, isMultiNode := csicommon.IsBlockMultiNode(req.VolumeCapabilities) + // below capability check indicates that we support both {SINGLE_NODE or MULTI_NODE} WRITERs and the `isMultiWriter` + // flag has been set accordingly. + isMultiWriter, isBlock := csicommon.IsBlockMultiWriter(req.VolumeCapabilities) + // below return value has set, if it is RWO mode File PVC. + isRWOFile := csicommon.IsFileRWO(req.VolumeCapabilities) + + // below return value has set, if it is ReadOnly capability. + isROOnly := csicommon.IsReaderOnly(req.VolumeCapabilities) // We want to fail early if the user is trying to create a RWX on a non-block type device - if isMultiNode && !isBlock { + if !isRWOFile && !isBlock && !isROOnly { return nil, status.Error( codes.InvalidArgument, "multi node access modes are only supported on rbd `block` type volumes") @@ -115,11 +121,13 @@ func (cs *ControllerServer) parseVolCreateRequest( return nil, status.Error(codes.InvalidArgument, "missing required parameter imageFeatures") } - // if it's NOT SINGLE_NODE_WRITER and it's BLOCK we'll set the parameter to ignore the in-use checks + // if it's NOT SINGLE_NODE_WRITER, and it's BLOCK we'll set the parameter to ignore the in-use checks rbdVol, err := genVolFromVolumeOptions( ctx, - req.GetParameters(), req.GetSecrets(), - (isMultiNode && isBlock), false) + req.GetParameters(), + req.GetSecrets(), + isMultiWriter && isBlock, + false) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) }