Skip to content

Commit 8ec1120

Browse files
committed
Block RWX volume creation for VMFS datastores if policy is not EZT
1 parent b7552df commit 8ec1120

File tree

5 files changed

+463
-6
lines changed

5 files changed

+463
-6
lines changed

pkg/csi/service/common/util.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ import (
4545
const (
4646
defaultK8sCloudOperatorServicePort = 10000
4747
MissingSnapshotAggregatedCapacity = "csi.vsphere.missing-snapshot-aggregated-capacity"
48+
vmfsNamespace = "com.vmware.storage.volumeallocation"
49+
vmfsNamespaceEztValue = "Fully initialized"
4850
)
4951

5052
var ErrAvailabilityZoneCRNotRegistered = errors.New("AvailabilityZone custom resource not registered")
@@ -535,3 +537,69 @@ func GetCNSVolumeInfoPatch(ctx context.Context, CapacityInMb int64, volumeId str
535537
}
536538
return patch, nil
537539
}
540+
541+
// ValidateStoragePolicyForRWXVolume returns an error if the storagepolicy is not compatible
542+
// for RWX volumes.
543+
// Currently it returns an error if policy is for VMFS datastores but it is not EagerZeroedThick.
544+
func ValidateStoragePolicyForRWXVolume(ctx context.Context,
545+
vc *cnsvsphere.VirtualCenter, storagePolicyId string) error {
546+
log := logger.GetLogger(ctx)
547+
548+
policies, err := vc.PbmRetrieveContent(ctx, []string{storagePolicyId})
549+
if err != nil {
550+
log.Errorf("failed to retrieve policy %s. Err %s", storagePolicyId, err)
551+
return err
552+
}
553+
554+
if len(policies) != 1 {
555+
msg := fmt.Sprintf("Retrieved %d polciies for policyID %s", len(policies), storagePolicyId)
556+
log.Errorf(msg)
557+
return errors.New(msg)
558+
}
559+
560+
return verifyStoragePolicyForVmfsWithEagerZeroedThick(ctx, policies[0], storagePolicyId)
561+
}
562+
563+
// verifyStoragePolicyForVmfsWithEagerZeroedThick goes through each rule in the policy to
564+
// find out if it is fully intialized for VMFS datastores.
565+
// This check is required for RWX shared block volumes as for VMFS datastores, the policy must be EZT.
566+
func verifyStoragePolicyForVmfsWithEagerZeroedThick(
567+
ctx context.Context,
568+
policy cnsvsphere.SpbmPolicyContent,
569+
storagePolicyID string,
570+
) error {
571+
log := logger.GetLogger(ctx)
572+
573+
log.Infof("Validating policy %s", storagePolicyID)
574+
575+
for _, profile := range policy.Profiles {
576+
isVmfs, isEzt := isVmfsEagerZeroed(profile)
577+
if !isVmfs {
578+
continue
579+
}
580+
if !isEzt {
581+
msg := fmt.Sprintf(
582+
"Policy %s is for VMFS datastores. It must be Thick Provision Eager Zero for RWX block volumes.",
583+
storagePolicyID,
584+
)
585+
log.Errorf(msg)
586+
return errors.New(msg)
587+
}
588+
log.Infof("Policy %s is for VMFS and is fully initialized", storagePolicyID)
589+
return nil
590+
}
591+
592+
return nil
593+
}
594+
595+
// isVmfsEagerZeroed returns two boolean values:
596+
// 1. First indicates if the policy is for a VMFS datastores.
597+
// 2. Second indicates if the policy is eager zeroed thick or fully initialised.
598+
func isVmfsEagerZeroed(profile cnsvsphere.SpbmPolicySubProfile) (bool, bool) {
599+
for _, rule := range profile.Rules {
600+
if rule.Ns == vmfsNamespace {
601+
return true, rule.Value == vmfsNamespaceEztValue
602+
}
603+
}
604+
return false, false
605+
}

pkg/csi/service/common/util_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"github.com/stretchr/testify/assert"
3737

3838
"github.com/container-storage-interface/spec/lib/go/csi"
39+
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/vsphere"
3940
)
4041

4142
var (
@@ -622,3 +623,106 @@ func TestGetClusterComputeResourceMoIds_SingleClusterPerAZ(t *testing.T) {
622623
gomega.Expect(multiple).To(gomega.BeFalse())
623624
gomega.Expect(moIDs).To(gomega.ContainElements("domain-c1", "domain-c2"))
624625
}
626+
627+
func TestIsVmfsEagerZeroed(t *testing.T) {
628+
tests := []struct {
629+
name string
630+
profile vsphere.SpbmPolicySubProfile
631+
wantIsVmfs bool
632+
wantIsEzt bool
633+
}{
634+
{
635+
name: "VMFS eager zeroed",
636+
profile: makeProfile(makeRule(vmfsNamespace, vmfsNamespaceEztValue)),
637+
wantIsVmfs: true,
638+
wantIsEzt: true,
639+
},
640+
{
641+
name: "VMFS not eager zeroed",
642+
profile: makeProfile(makeRule(vmfsNamespace, "THIN")),
643+
wantIsVmfs: true,
644+
wantIsEzt: false,
645+
},
646+
{
647+
name: "Non-VMFS",
648+
profile: makeProfile(makeRule("NFS", "ANY")),
649+
wantIsVmfs: false,
650+
wantIsEzt: false,
651+
},
652+
{
653+
name: "Empty rules",
654+
profile: makeProfile(),
655+
wantIsVmfs: false,
656+
wantIsEzt: false,
657+
},
658+
}
659+
660+
for _, tt := range tests {
661+
t.Run(tt.name, func(t *testing.T) {
662+
gotVmfs, gotEzt := isVmfsEagerZeroed(tt.profile)
663+
assert.Equal(t, tt.wantIsVmfs, gotVmfs)
664+
assert.Equal(t, tt.wantIsEzt, gotEzt)
665+
})
666+
}
667+
}
668+
669+
func TestVerifyStoragePolicyForVmfsWithEagerZeroedThick(t *testing.T) {
670+
ctx := context.Background()
671+
672+
tests := []struct {
673+
name string
674+
policy vsphere.SpbmPolicyContent
675+
expectError bool
676+
}{
677+
{
678+
name: "VMFS eager zeroed",
679+
policy: makePolicy(
680+
makeProfile(makeRule(vmfsNamespace, vmfsNamespaceEztValue)),
681+
),
682+
expectError: false,
683+
},
684+
{
685+
name: "VMFS but not eager zeroed",
686+
policy: makePolicy(
687+
makeProfile(makeRule(vmfsNamespace, "THIN")),
688+
),
689+
expectError: true,
690+
},
691+
{
692+
name: "Non-VMFS policy",
693+
policy: makePolicy(
694+
makeProfile(makeRule("NFS", "ANY")),
695+
),
696+
expectError: false,
697+
},
698+
{
699+
name: "Empty profiles",
700+
policy: makePolicy(),
701+
expectError: false,
702+
},
703+
}
704+
705+
for _, tt := range tests {
706+
t.Run(tt.name, func(t *testing.T) {
707+
err := verifyStoragePolicyForVmfsWithEagerZeroedThick(ctx, tt.policy, "test-policy")
708+
if tt.expectError {
709+
assert.Error(t, err)
710+
assert.Contains(t, err.Error(), "It must be Thick Provision Eager Zero for RWX block volumes.")
711+
} else {
712+
assert.NoError(t, err)
713+
}
714+
})
715+
}
716+
}
717+
718+
func makeRule(ns, value string) vsphere.SpbmPolicyRule {
719+
return vsphere.SpbmPolicyRule{Ns: ns, Value: value}
720+
}
721+
722+
func makeProfile(rules ...vsphere.SpbmPolicyRule) vsphere.SpbmPolicySubProfile {
723+
return vsphere.SpbmPolicySubProfile{Rules: rules}
724+
}
725+
726+
func makePolicy(profiles ...vsphere.SpbmPolicySubProfile) vsphere.SpbmPolicyContent {
727+
return vsphere.SpbmPolicyContent{Profiles: profiles}
728+
}

pkg/csi/service/wcp/controller.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,16 @@ func (c *controller) createBlockVolume(ctx context.Context, req *csi.CreateVolum
522522
"failed to get vCenter from Manager. Error: %v", err)
523523
}
524524

525+
if commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx,
526+
common.SharedDiskFss) && isSharedRawBlockRequest(ctx, req.VolumeCapabilities) {
527+
log.Infof("Volume request is for shared RWX volume. Validatig if policy is compatible for VMFS datastores.")
528+
err := common.ValidateStoragePolicyForRWXVolume(ctx, vc, storagePolicyID)
529+
if err != nil {
530+
log.Errorf("failed validation for policy %s", storagePolicyID)
531+
return nil, csifault.CSIInternalFault, err
532+
}
533+
}
534+
525535
// Fetch the accessibility requirements from the request.
526536
topologyRequirement = req.GetAccessibilityRequirements()
527537
filterSuspendedDatastores := commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.CnsMgrSuspendCreateVolume)

0 commit comments

Comments
 (0)