Ensure that for RWX volumes for VMFS, clusteredVmdk is enabled (#3551)

Author: skogta

pkg/csi/service/wcp/controller_helper.go

@@ -53,6 +53,7 @@ import (
 var (
 	vmfsNamespace         = "com.vmware.storage.volumeallocation"
 	vmfsNamespaceEztValue = "Fully initialized"
+	vmfsClusteredVmdk     = "Clustered"
 )
 
 // validateCreateBlockReqParam is a helper function used to validate the parameter
@@ -91,32 +92,45 @@ func validateCreateFileReqParam(paramName, value string) bool {
 		paramName == common.AttributeIsLinkedCloneKey
 }
 
-// verifyStoragePolicyForVmfsWithEageredZeroThick goes through each rule in the policy to
-// find out if it is fully intialized for VMFS datastores.
-// This check is required for RWX shared block volumes as for VMFS, the policy must be EZT.
-func verifyStoragePolicyForVmfsWithEageredZeroThick(ctx context.Context,
+// verifyStoragePolicyForVmfsUtil goes through each rule in the policy to
+// find out if it is fully intialized and has clustered VMDK enabled for VMFS datastores.
+// This check is required for RWX shared block volumes as for VMFS, the policy must be EZT
+// and clustered.
+func verifyStoragePolicyForVmfsUtil(ctx context.Context,
 	spbmPolicyContentList []vsphere.SpbmPolicyContent,
 	storagePolicyId string) error {
 	log := logger.GetLogger(ctx)
 
+	isEzt := false
+	isClustered := false
+	isVmfsNamespace := false
+
 	for _, polictContent := range spbmPolicyContentList {
 		for _, profile := range polictContent.Profiles {
 			for _, rule := range profile.Rules {
 				if rule.Ns == vmfsNamespace {
-					if rule.Value != vmfsNamespaceEztValue {
-						msg := fmt.Sprintf("Policy %s is for VMFS datastores. It must be fully initialized for "+
-							"RWX block volumes", storagePolicyId)
-						err := errors.New(msg)
-						log.Errorf(msg)
-						return err
+					isVmfsNamespace = true
+					switch rule.Value {
+					case vmfsNamespaceEztValue:
+						isEzt = true
+					case vmfsClusteredVmdk:
+						isClustered = true
 					}
-					log.Infof("Policy %s is for VMFS and is fully initialized", storagePolicyId)
-					return nil
 				}
 			}
 		}
 	}
 
+	// If any policy is for VMFS and it is not EZT or clustered, then send back error.
+	if isVmfsNamespace && (!isEzt || !isClustered) {
+		msg := fmt.Sprintf("Policy %s is for VMFS datastores. "+
+			"It must be fully initialized and must have clustered VMDK enabled for "+
+			"RWX block volumes", storagePolicyId)
+		err := errors.New(msg)
+		log.Errorf(msg)
+		return err
+	}
+
 	log.Debugf("Policy %s validated correctly", storagePolicyId)
 	return nil
 }
@@ -133,7 +147,7 @@ func validateStoragePolicyForVmfs(ctx context.Context,
 		return err
 	}
 
-	return verifyStoragePolicyForVmfsWithEageredZeroThick(ctx, spbmPolicyContentList, storagePolicyId)
+	return verifyStoragePolicyForVmfsUtil(ctx, spbmPolicyContentList, storagePolicyId)
 }
 
 // validateWCPCreateVolumeRequest is the helper function to validate

pkg/csi/service/wcp/controller_helper_test.go

@@ -133,66 +133,93 @@ func TestGetPodVMUUID(t *testing.T) {
 	})
 }
 
-func TestVerifyStoragePolicyForVmfsWithEageredZeroThick(t *testing.T) {
+func TestVerifyStoragePolicyForVmfsUtil(t *testing.T) {
 	ctx := context.TODO()
-
-	t.Run("Valid VMFS EZT policy", func(t *testing.T) {
-		policyList := []cnsvsphere.SpbmPolicyContent{
-			{
-				Profiles: []cnsvsphere.SpbmPolicySubProfile{
-					{
-						Rules: []cnsvsphere.SpbmPolicyRule{
-							{Ns: vmfsNamespace, Value: vmfsNamespaceEztValue},
+	policyId := "policy-123"
+
+	tests := []struct {
+		name        string
+		input       []cnsvsphere.SpbmPolicyContent
+		expectError bool
+	}{
+		{
+			name: "Valid VMFS policy with EZT and Clustered VMDK",
+			input: []cnsvsphere.SpbmPolicyContent{
+				{
+					Profiles: []cnsvsphere.SpbmPolicySubProfile{
+						{
+							Rules: []cnsvsphere.SpbmPolicyRule{
+								{Ns: vmfsNamespace, Value: vmfsNamespaceEztValue},
+								{Ns: vmfsNamespace, Value: vmfsClusteredVmdk},
+							},
 						},
 					},
 				},
 			},
-		}
-
-		err := verifyStoragePolicyForVmfsWithEageredZeroThick(ctx, policyList, "valid-policy-id")
-		assert.NoError(t, err)
-	})
-
-	t.Run("Invalid VMFS policy - not EZT", func(t *testing.T) {
-		policyList := []cnsvsphere.SpbmPolicyContent{
-			{
-				Profiles: []cnsvsphere.SpbmPolicySubProfile{
-					{
-						Rules: []cnsvsphere.SpbmPolicyRule{
-							{Ns: vmfsNamespace, Value: "Thin"},
+			expectError: false,
+		},
+		{
+			name: "VMFS policy missing EZT",
+			input: []cnsvsphere.SpbmPolicyContent{
+				{
+					Profiles: []cnsvsphere.SpbmPolicySubProfile{
+						{
+							Rules: []cnsvsphere.SpbmPolicyRule{
+								{Ns: vmfsNamespace, Value: vmfsClusteredVmdk},
+							},
 						},
 					},
 				},
 			},
-		}
-
-		err := verifyStoragePolicyForVmfsWithEageredZeroThick(ctx, policyList, "invalid-policy-id")
-		assert.Error(t, err)
-		assert.Contains(t, err.Error(), "must be fully initialized")
-	})
-
-	t.Run("Policy with no VMFS rule", func(t *testing.T) {
-		policyList := []cnsvsphere.SpbmPolicyContent{
-			{
-				Profiles: []cnsvsphere.SpbmPolicySubProfile{
-					{
-						Rules: []cnsvsphere.SpbmPolicyRule{
-							{Ns: "VSAN", Value: "Whatever"},
+			expectError: true,
+		},
+		{
+			name: "VMFS policy missing Clustered VMDK",
+			input: []cnsvsphere.SpbmPolicyContent{
+				{
+					Profiles: []cnsvsphere.SpbmPolicySubProfile{
+						{
+							Rules: []cnsvsphere.SpbmPolicyRule{
+								{Ns: vmfsNamespace, Value: vmfsNamespaceEztValue},
+							},
 						},
 					},
 				},
 			},
-		}
-
-		err := verifyStoragePolicyForVmfsWithEageredZeroThick(ctx, policyList, "no-vmfs-rule-policy")
-		assert.NoError(t, err)
-	})
+			expectError: true,
+		},
+		{
+			name: "Non-VMFS policy should pass",
+			input: []cnsvsphere.SpbmPolicyContent{
+				{
+					Profiles: []cnsvsphere.SpbmPolicySubProfile{
+						{
+							Rules: []cnsvsphere.SpbmPolicyRule{
+								{Ns: "non-vmfs", Value: "some-value"},
+							},
+						},
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name:        "Empty policy list should pass",
+			input:       []cnsvsphere.SpbmPolicyContent{},
+			expectError: false,
+		},
+	}
 
-	t.Run("Empty policy list", func(t *testing.T) {
-		var policyList []cnsvsphere.SpbmPolicyContent
-		err := verifyStoragePolicyForVmfsWithEageredZeroThick(ctx, policyList, "empty-policy")
-		assert.NoError(t, err)
-	})
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := verifyStoragePolicyForVmfsUtil(ctx, tt.input, policyId)
+			if tt.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
 }
 
 func newMockPod(name, namespace, nodeName string, volumes []string,