Unit test changes related to StoragePolicy in vSphere CSI plugin

go.mod

@@ -21,7 +21,7 @@ require (
 	github.com/prometheus/client_golang v1.18.0
 	github.com/stretchr/testify v1.9.0
 	github.com/vmware-tanzu/vm-operator/api v1.8.2
-	github.com/vmware/govmomi v0.36.1
+	github.com/vmware/govmomi v0.37.2
 	go.uber.org/zap v1.26.0
 	golang.org/x/crypto v0.18.0
 	golang.org/x/sync v0.5.0

go.sum

@@ -1298,8 +1298,8 @@ github.com/vishvananda/netns v0.0.2 h1:Cn05BRLm+iRP/DZxyVSsfVyrzgjDbwHwkVt38qvXn
 github.com/vishvananda/netns v0.0.2/go.mod h1:yitZXdAVI+yPFSb4QUe+VW3vOVl4PZPNcBgbPxAtJxw=
 github.com/vmware-tanzu/vm-operator/api v1.8.2 h1:7cZHVusqAmAMFWvsiU7X5xontxdjasknI/sVfe0p0Z4=
 github.com/vmware-tanzu/vm-operator/api v1.8.2/go.mod h1:vauVboD3sQxP+pb28TnI9wfrj+0nH2zSEc9Q7AzWJ54=
-github.com/vmware/govmomi v0.36.1 h1:+E/nlfteQ8JvC0xhuKAfpnMsuIeGeGj7rJwqENUcWm8=
-github.com/vmware/govmomi v0.36.1/go.mod h1:mtGWtM+YhTADHlCgJBiskSRPOZRsN9MSjPzaZLte/oQ=
+github.com/vmware/govmomi v0.37.2 h1:5ANLoaTxWv600ZnoosJ2zXbM3A+EaxqGheEZbRN8YVE=
+github.com/vmware/govmomi v0.37.2/go.mod h1:mtGWtM+YhTADHlCgJBiskSRPOZRsN9MSjPzaZLte/oQ=
 github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=
 github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ=
 github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=

pkg/csi/service/common/vsphereutil.go

@@ -56,8 +56,8 @@ type VanillaCreateBlockVolParamsForMultiVC struct {
 
 // CreateBlockVolumeUtil is the helper function to create CNS block volume.
 func CreateBlockVolumeUtil(ctx context.Context, clusterFlavor cnstypes.CnsClusterFlavor, manager *Manager,
-	spec *CreateVolumeSpec, sharedDatastores []*vsphere.DatastoreInfo, filterSuspendedDatastores bool, useSupervisorId,
-	checkCompatibleDataStores bool, extraParams interface{}) (*cnsvolume.CnsVolumeInfo, string, error) {
+	spec *CreateVolumeSpec, sharedDatastores []*vsphere.DatastoreInfo, filterSuspendedDatastores,
+	useSupervisorId bool, extraParams interface{}) (*cnsvolume.CnsVolumeInfo, string, error) {
 	log := logger.GetLogger(ctx)
 	vc, err := GetVCenter(ctx, manager)
 	if err != nil {
@@ -198,11 +198,9 @@ func CreateBlockVolumeUtil(ctx context.Context, clusterFlavor cnstypes.CnsCluste
 		}
 	}
 
-	if checkCompatibleDataStores {
-		fault, err := isDataStoreCompatible(ctx, vc, spec, datastores, datastoreObj)
-		if err != nil {
-			return nil, fault, err
-		}
+	fault, err := isDataStoreCompatible(ctx, vc, spec, datastores, datastoreObj)
+	if err != nil {
+		return nil, fault, err
 	}
 
 	var containerClusterArray []cnstypes.CnsContainerCluster

pkg/csi/service/vanilla/controller.go

@@ -95,9 +95,8 @@ var (
 	isTopologyAwareFileVolumeEnabled bool
 
 	// variables for list volumes
-	volIDsInK8s               = make([]string, 0)
-	CNSVolumesforListVolume   = make([]cnstypes.CnsVolume, 0)
-	checkCompatibleDataStores = true
+	volIDsInK8s             = make([]string, 0)
+	CNSVolumesforListVolume = make([]cnstypes.CnsVolume, 0)
 
 	// errAllDSFilteredOut is an error thrown by auth service when it
 	// filters out all the potential shared datastores in a volume provisioning call.
@@ -782,8 +781,7 @@ func (c *controller) createBlockVolume(ctx context.Context, req *csi.CreateVolum
 		}
 
 		volumeInfo, faultType, err = common.CreateBlockVolumeUtil(ctx, cnstypes.CnsClusterFlavorVanilla,
-			c.manager, &createVolumeSpec, sharedDatastores, filterSuspendedDatastores, false,
-			checkCompatibleDataStores, nil)
+			c.manager, &createVolumeSpec, sharedDatastores, filterSuspendedDatastores, false, nil)
 		if err != nil {
 			return nil, faultType, logger.LogNewErrorCodef(log, codes.Internal,
 				"failed to create volume. Error: %+v", err)
@@ -1950,10 +1948,7 @@ func (c *controller) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequ
 	start := time.Now()
 	ctx = logger.NewContextWithLogger(ctx)
 	log := logger.GetLogger(ctx)
-	if chkDataStoreCompatibility := req.Parameters["checkCompatibleDatastores"]; chkDataStoreCompatibility == "false" {
-		checkCompatibleDataStores = false
-		delete(req.Parameters, "checkCompatibleDatastores")
-	}
+
 	volumeType := prometheus.PrometheusUnknownVolumeType
 	createVolumeInternal := func() (
 		*csi.CreateVolumeResponse, string, error) {

pkg/csi/service/vanilla/controller_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/vmware/govmomi/cns"
 	cnstypes "github.com/vmware/govmomi/cns/types"
 	"github.com/vmware/govmomi/pbm"
+	"github.com/vmware/govmomi/pbm/types"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
@@ -290,7 +291,7 @@ func TestCreateVolumeWithStoragePolicy(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
+
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -374,6 +375,147 @@ func TestCreateVolumeWithStoragePolicy(t *testing.T) {
 	}
 }
 
+// Create new Storage Policy and pass this storage policy's name through parameters to CreateVolume
+// function call. Verify that volume creation succeeds and it uses newly created storage policy.
+func TestCreateVolumeWithNewlyCreatedStoragePolicy(t *testing.T) {
+	// Create context.
+	ct := getControllerTest(t)
+
+	params := make(map[string]string)
+
+	pc, err := pbm.NewClient(ctx, ct.vcenter.Client.Client)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create new Storage Policy
+	storagePolicyName := "Kubernetes-VSAN-TestPolicy"
+	pbmCreateSpecForVSAN := pbm.CapabilityProfileCreateSpec{
+		Name:        storagePolicyName,
+		Description: "VSAN Test policy create",
+		Category:    string(types.PbmProfileCategoryEnumREQUIREMENT),
+		CapabilityList: []pbm.Capability{
+			{
+				ID:        "hostFailuresToTolerate",
+				Namespace: "VSAN",
+				PropertyList: []pbm.Property{
+					{
+						ID:       "hostFailuresToTolerate",
+						Value:    "2",
+						DataType: "int",
+					},
+				},
+			},
+			{
+				ID:        "stripeWidth",
+				Namespace: "VSAN",
+				PropertyList: []pbm.Property{
+					{
+						ID:       "stripeWidth",
+						Value:    "1",
+						DataType: "int",
+					},
+				},
+			},
+		},
+	}
+
+	// Create PBM capability spec for the above defined user spec
+	createSpecVSAN, err := pbm.CreateCapabilityProfileSpec(pbmCreateSpecForVSAN)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create SPBM VSAN profile
+	vsanProfileID, err := pc.CreateProfile(ctx, *createSpecVSAN)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Logf("VSAN Storage Policy %q successfully created", vsanProfileID.UniqueId)
+
+	defer func() {
+		_, err = pc.DeleteProfile(ctx, []types.PbmProfileId{*vsanProfileID})
+		if err != nil {
+			t.Fatal(err)
+		}
+		t.Logf("VSAN Storage Policy %+v successfully deleted", vsanProfileID.UniqueId)
+	}()
+
+	// Verify if profile created exists by issuing a PbmRetrieveContent request
+	_, err = ct.vcenter.PbmRetrieveContent(ctx, []string{vsanProfileID.UniqueId})
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Logf("Storage Policy %q exists on vCenter", vsanProfileID.UniqueId)
+
+	params[common.AttributeStoragePolicyName] = storagePolicyName
+	capabilities := []*csi.VolumeCapability{
+		{
+			AccessMode: &csi.VolumeCapability_AccessMode{
+				Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER,
+			},
+		},
+	}
+
+	reqCreate := &csi.CreateVolumeRequest{
+		Name: testVolumeName + "-" + uuid.New().String(),
+		CapacityRange: &csi.CapacityRange{
+			RequiredBytes: 1 * common.GbInBytes,
+		},
+		Parameters:         params,
+		VolumeCapabilities: capabilities,
+	}
+
+	// Create volume.
+	// As part of create volume PbmCheckCompatibility and GetStoragePolicyIDByName
+	// functions will get tested.
+	respCreate, err := ct.controller.CreateVolume(ctx, reqCreate)
+	if err != nil {
+		t.Fatal(err)
+	}
+	volID := respCreate.Volume.VolumeId
+
+	queryFilter := cnstypes.CnsQueryFilter{
+		VolumeIds: []cnstypes.CnsVolumeId{
+			{
+				Id: volID,
+			},
+		},
+	}
+	queryResult, err := ct.vcenter.CnsClient.QueryVolume(ctx, queryFilter)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(queryResult.Volumes) != 1 && queryResult.Volumes[0].VolumeId.Id != volID {
+		t.Fatalf("failed to find the newly created volume with ID: %s", volID)
+	}
+
+	// Make sure that volume is using the newly created storage profile
+	if queryResult.Volumes[0].StoragePolicyId != vsanProfileID.UniqueId {
+		t.Fatalf("failed to match volume policy ID: %s", vsanProfileID.UniqueId)
+	}
+
+	// Delete volume
+	reqDelete := &csi.DeleteVolumeRequest{
+		VolumeId: volID,
+	}
+	_, err = ct.controller.DeleteVolume(ctx, reqDelete)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Verify the volume has been deleted.
+	queryResult, err = ct.vcenter.CnsClient.QueryVolume(ctx, queryFilter)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(queryResult.Volumes) != 0 {
+		t.Fatalf("Volume should not exist after deletion with ID: %s", volID)
+	}
+}
+
 // When the testbed has multiple shared datastores, but VC user which is used
 // to deploy CSI does not have Datastore.FileManagement privilege on all shared
 // datastores, the create volume should succeed. This test is to simulate CSI

pkg/csi/service/wcp/controller.go

@@ -75,7 +75,6 @@ var (
 		csi.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT,
 		csi.ControllerServiceCapability_RPC_LIST_SNAPSHOTS,
 	}
-	checkCompatibleDataStores = true
 	// volumeInfoService holds the pointer to VolumeInfo service instance
 	// This will hold mapping for VolumeID to Storage policy info for PodVMOnStretchedSupervisor deployments
 	volumeInfoService cnsvolumeinfo.VolumeInfoService
@@ -627,7 +626,7 @@ func (c *controller) createBlockVolume(ctx context.Context, req *csi.CreateVolum
 	if isPodVMOnStretchSupervisorFSSEnabled {
 		volumeInfo, faultType, err = common.CreateBlockVolumeUtil(ctx, cnstypes.CnsClusterFlavorWorkload,
 			c.manager, &createVolumeSpec, candidateDatastores, filterSuspendedDatastores,
-			isTKGSHAEnabled, checkCompatibleDataStores,
+			isTKGSHAEnabled,
 			&cnsvolume.CreateVolumeExtraParams{
 				VolSizeBytes:                         volSizeBytes,
 				StorageClassName:                     req.Parameters[common.AttributeStorageClassName],
@@ -637,7 +636,7 @@ func (c *controller) createBlockVolume(ctx context.Context, req *csi.CreateVolum
 	} else {
 		volumeInfo, faultType, err = common.CreateBlockVolumeUtil(ctx, cnstypes.CnsClusterFlavorWorkload,
 			c.manager, &createVolumeSpec, candidateDatastores, filterSuspendedDatastores,
-			isTKGSHAEnabled, checkCompatibleDataStores, nil)
+			isTKGSHAEnabled, nil)
 	}
 	if err != nil {
 		return nil, faultType, logger.LogNewErrorCodef(log, codes.Internal,
@@ -900,10 +899,7 @@ func (c *controller) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequ
 	start := time.Now()
 	ctx = logger.NewContextWithLogger(ctx)
 	log := logger.GetLogger(ctx)
-	if chkDataStoreCompatibility := req.Parameters["checkCompatibleDatastores"]; chkDataStoreCompatibility == "false" {
-		checkCompatibleDataStores = false
-		delete(req.Parameters, "checkCompatibleDatastores")
-	}
+
 	volumeType := prometheus.PrometheusUnknownVolumeType
 	createVolumeInternal := func() (
 		*csi.CreateVolumeResponse, string, error) {

pkg/csi/service/wcp/controller_test.go

@@ -173,7 +173,6 @@ func TestWCPCreateVolumeWithStoragePolicy(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -285,7 +284,6 @@ func TestWCPCreateVolumeWithZonalLabelPresentButNoStorageTopoType(t *testing.T)
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -381,7 +379,6 @@ func TestWCPCreateDeleteSnapshot(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -497,7 +494,6 @@ func TestListSnapshots(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -617,7 +613,6 @@ func TestListSnapshotsOnSpecificVolume(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -738,7 +733,6 @@ func TestListSnapshotsWithToken(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -869,7 +863,6 @@ func TestListSnapshotsOnSpecificVolumeAndSnapshot(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -988,7 +981,6 @@ func TestCreateVolumeFromSnapshot(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -1194,7 +1186,6 @@ func TestWCPDeleteVolumeWithSnapshots(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{
@@ -1300,7 +1291,6 @@ func TestWCPExpandVolumeWithSnapshots(t *testing.T) {
 			},
 		},
 	}
-	params["checkCompatibleDatastores"] = "false"
 	reqCreate := &csi.CreateVolumeRequest{
 		Name: testVolumeName + "-" + uuid.New().String(),
 		CapacityRange: &csi.CapacityRange{