rbd: introduce a helper function to detect multi writer,block & rwofile

SINGLE_NODE_WRITER capability ambiguity has been fixed in csi spec v1.5
which allows the SP drivers to declare more granular WRITE capability in form
of SINGLE_NODE_SINGLE_WRITER or SINGLE_NODE_MULTI_WRITER.

These are not really new capabilities rather capabilities introduced to
get the desired functionality from CO side based on the capabilities SP
driver support for various CSI operations, this new capabilities also help
to address new access mode RWOP (readwriteoncepod).

This commit adds a helper function which identity the request is of
multiwriter mode and also validates whether it is filesystem mode or
block mode. Based on the inspection it fails to allow multi write
requests for filesystem mode and only allow multi write request against
block mode.

This commit also adds unit tests for isMultiWriterBlock function which
validates various accesstypes and accessmodes.

Signed-off-by: Humble Chirammal <hchiramm@redhat.com>

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
+}

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)
+			}
+		})
+	}
+}

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)
+	// below capability check indicates that we support both {SINGLE_NODE or MULTI_NODE} WRITERs and the `isMultiWriter`
-	isBlock, isMultiNode := csicommon.IsBlockMultiNode(req.VolumeCapabilities)
+	// 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(),
+		req.GetParameters(),
-		(isMultiNode && isBlock), false)
+		req.GetSecrets(),
+		isMultiWriter && isBlock,
+		false)
 	if err != nil {
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}