diff --git a/go.mod b/go.mod index d58e3b838e..700fe69ca9 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 00945f8f8d..6fa42da602 100644 --- a/go.sum +++ b/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= diff --git a/pkg/csi/service/common/vsphereutil.go b/pkg/csi/service/common/vsphereutil.go index d0ac30cd6e..0c245f4a60 100644 --- a/pkg/csi/service/common/vsphereutil.go +++ b/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 diff --git a/pkg/csi/service/vanilla/controller.go b/pkg/csi/service/vanilla/controller.go index ef93af6a84..92e03a43ba 100644 --- a/pkg/csi/service/vanilla/controller.go +++ b/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) { diff --git a/pkg/csi/service/vanilla/controller_test.go b/pkg/csi/service/vanilla/controller_test.go index a18d5febcb..8934f1b346 100644 --- a/pkg/csi/service/vanilla/controller_test.go +++ b/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 diff --git a/pkg/csi/service/wcp/controller.go b/pkg/csi/service/wcp/controller.go index dc1276d3df..faa43c6db9 100644 --- a/pkg/csi/service/wcp/controller.go +++ b/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) { diff --git a/pkg/csi/service/wcp/controller_test.go b/pkg/csi/service/wcp/controller_test.go index ce8c5c0a54..4fe423f0d2 100644 --- a/pkg/csi/service/wcp/controller_test.go +++ b/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{