For shared block volume, return true in IsFileVolume method (#3451)

Author: skogta

pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go

@@ -974,15 +974,15 @@ func pvcAdded(obj interface{}) {}
 // This ensures that all existing PVs in the cluster are added to the map, even
 // across container restarts.
 func pvAdded(obj interface{}) {
-	_, log := logger.GetNewContextWithLogger()
+	ctx, log := logger.GetNewContextWithLogger()
 	pv, ok := obj.(*v1.PersistentVolume)
 	if pv == nil || !ok {
 		log.Warnf("pvAdded: unrecognized object %+v", obj)
 		return
 	}
 
 	if pv.Spec.CSI != nil && pv.Spec.CSI.Driver == csitypes.Name {
-		if !isFileVolume(pv) && pv.Spec.ClaimRef != nil && pv.Status.Phase == v1.VolumeBound {
+		if !isFileVolume(ctx, pv) && pv.Spec.ClaimRef != nil && pv.Status.Phase == v1.VolumeBound {
 			// We should not be caching file volumes to the map.
 			// Add volume handle to PVC mapping.
 			objKey := pv.Spec.CSI.VolumeHandle
@@ -1010,7 +1010,7 @@ func pvAdded(obj interface{}) {
 
 // pvUpdated updates the volumeIDToPvcMap and pvcToVolumeIDMap when a PV goes to Bound phase.
 func pvUpdated(oldObj, newObj interface{}) {
-	_, log := logger.GetNewContextWithLogger()
+	ctx, log := logger.GetNewContextWithLogger()
 	// Get old and new PV objects.
 	oldPv, ok := oldObj.(*v1.PersistentVolume)
 	if oldPv == nil || !ok {
@@ -1028,7 +1028,7 @@ func pvUpdated(oldObj, newObj interface{}) {
 	if oldPv.Status.Phase != v1.VolumeBound && newPv.Status.Phase == v1.VolumeBound {
 		if newPv.Spec.CSI != nil && newPv.Spec.CSI.Driver == csitypes.Name &&
 			newPv.Spec.ClaimRef != nil {
-			if !isFileVolume(newPv) {
+			if !isFileVolume(ctx, newPv) {
 
 				log.Debugf("pvUpdated: PV %s went to Bound phase", newPv.Name)
 				// Add volume handle to PVC mapping.

pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator_helper.go

@@ -106,10 +106,17 @@ func (c *K8sOrchestrator) updatePVCAnnotations(ctx context.Context,
 
 // isFileVolume checks if the Persistent Volume has ReadWriteMany or
 // ReadOnlyMany support.
-func isFileVolume(pv *v1.PersistentVolume) bool {
+func isFileVolume(ctx context.Context, pv *v1.PersistentVolume) bool {
 	if len(pv.Spec.AccessModes) == 0 {
 		return false
 	}
+
+	if k8sOrchestratorInstance.IsFSSEnabled(ctx, common.SharedDiskFss) &&
+		*pv.Spec.VolumeMode == v1.PersistentVolumeBlock {
+		// If volumeMode is block, then volume is Block volume.
+		return false
+	}
+
 	for _, accessMode := range pv.Spec.AccessModes {
 		if accessMode == v1.ReadWriteMany || accessMode == v1.ReadOnlyMany {
 			return true

pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 
 	cnstypes "github.com/vmware/govmomi/cns/types"
+	v1 "k8s.io/api/core/v1"
 
 	cnsconfig "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
 )
@@ -260,3 +261,61 @@ func TestGetNodesForVolumes(t *testing.T) {
 		t.Errorf("Expected node names %v but got %v", expectedNodeNames, nodeNames)
 	}
 }
+
+func TestIsFileVolume(t *testing.T) {
+	ctx := context.Background()
+
+	tests := []struct {
+		name string
+		pv   *v1.PersistentVolume
+		want bool
+	}{
+		{
+			name: "No AccessModes",
+			pv: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					AccessModes: []v1.PersistentVolumeAccessMode{},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "AccessMode ReadWriteMany",
+			pv: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteMany},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "AccessMode ReadOnlyMany",
+			pv: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadOnlyMany},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "RWO Block volume mode with FSS disabled",
+			pv: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
+					VolumeMode:  func() *v1.PersistentVolumeMode { m := v1.PersistentVolumeBlock; return &m }(),
+				},
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			k8sOrchestratorInstance = &K8sOrchestrator{}
+			got := isFileVolume(ctx, tt.pv)
+			if got != tt.want {
+				t.Errorf("isFileVolume() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/csi/service/common/constants.go

@@ -448,7 +448,7 @@ const (
 	// FileVolumesWithVmService is an FSS to support file volumes with VM service VMs.
 	FileVolumesWithVmService = "file-volume-with-vm-service"
 	// SharedDiskFss is an FSS that tells whether shared disks are supported or not
-	SharedDiskFss = "supports_shared_disks"
+	SharedDiskFss = "supports_shared_disks_with_VM_service_VMs"
 	// CSITranSactionSupport is an FSS for transaction support
 	CSITranSactionSupport = "csi-transaction-support"
 	// VolFromSnapshotOnTargetDs is a FSS that tells whether creation of volumes from
@@ -466,6 +466,7 @@ var WCPFeatureStates = map[string]struct{}{
 	WorkloadDomainIsolation:    {},
 	VPCCapabilitySupervisor:    {},
 	VolFromSnapshotOnTargetDs:  {},
+	SharedDiskFss:              {},
 }
 
 // WCPFeatureStatesSupportsLateEnablement contains capabilities that can be enabled later