Improve RWX volumes with VM service VMs performace during validation in webhook.

Author: skogta

pkg/syncer/admissionhandler/cnscsi_admissionhandler.go

@@ -37,7 +37,8 @@ const (
 	MutationWebhookPath              = "/mutate"
 	DefaultWebhookPort               = 9883
 	DefaultWebhookMetricsBindAddress = "0"
-	devopsUserLabelKey               = "cns.vmware.com/user-created"
+	vmNameLabelKey                   = "cns.vmware.com/vm-name"
+	pvcNameLabelKey                  = "cns.vmware.com/pvc-name"
 )
 
 var (
@@ -268,11 +269,13 @@ func (h *CSISupervisorMutationWebhook) mutateNewCnsFileAccessConfig(ctx context.
 	if newCnsFileAccessConfig.Labels == nil {
 		newCnsFileAccessConfig.Labels = make(map[string]string)
 	}
-	if _, ok := newCnsFileAccessConfig.Labels[devopsUserLabelKey]; ok {
-		log.Debugf("Devops label already present on instance %s", newCnsFileAccessConfig.Name)
-		return admission.Allowed("")
-	}
-	newCnsFileAccessConfig.Labels[devopsUserLabelKey] = "true"
+
+	// Add VM name and PVC name label.
+	// If someone created this CR with these labels already present, CSI will overrite on them
+	// with the correct values.
+	newCnsFileAccessConfig.Labels[vmNameLabelKey] = newCnsFileAccessConfig.Spec.VMName
+	newCnsFileAccessConfig.Labels[pvcNameLabelKey] = newCnsFileAccessConfig.Spec.PvcName
+
 	newRawCnsFileAccessConfig, err := json.Marshal(newCnsFileAccessConfig)
 	if err != nil {
 		return admission.Errored(http.StatusInternalServerError, err)

pkg/syncer/admissionhandler/validate_cnsfileaccessconfig.go

@@ -7,7 +7,9 @@ import (
 	"regexp"
 
 	admissionv1 "k8s.io/api/admission/v1"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	cnsoperatorv1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator"
 
@@ -42,6 +44,25 @@ func validateCreateCnsFileAccessConfig(ctx context.Context, clientConfig *rest.C
 		}
 	}
 
+	// This validation is not required for PVCSI service account.
+	isPvCSIServiceAccount, err := validatePvCSIServiceAccount(req.UserInfo.Username)
+	if err != nil {
+		// return AdmissionResponse result
+		return &admissionv1.AdmissionResponse{
+			Allowed: false,
+			Result: &metav1.Status{
+				Message: fmt.Sprintf("failed to validate user information: %v", err),
+			},
+		}
+	}
+
+	// If user is PVCSI service account, allow this request.
+	if isPvCSIServiceAccount {
+		return &admissionv1.AdmissionResponse{
+			Allowed: true,
+		}
+	}
+
 	vm := cnsFileAccessConfig.Spec.VMName
 	pvc := cnsFileAccessConfig.Spec.PvcName
 	namespace := cnsFileAccessConfig.Namespace
@@ -87,22 +108,31 @@ func cnsFileAccessConfigAlreadyExists(ctx context.Context, clientConfig *rest.Co
 		return "", err
 	}
 
+	// List only that CnsFileAccessConfig CRs which has the same VM name and PVC name labels.
+	labelSelector := labels.SelectorFromSet(labels.Set{vmNameLabelKey: vm, pvcNameLabelKey: pvc})
 	// Get the list of all CnsFileAccessConfig CRs in the given namespace.
 	cnsFileAccessConfigList := &cnsfileaccessconfigv1alpha1.CnsFileAccessConfigList{}
-	err = cnsOperatorClient.List(ctx, cnsFileAccessConfigList, &client.ListOptions{Namespace: namespace})
+	err = cnsOperatorClient.List(ctx, cnsFileAccessConfigList, &client.ListOptions{
+		Namespace:     namespace,
+		LabelSelector: labelSelector,
+	})
 	if err != nil {
 		log.Errorf("failed to list CnsFileAccessConfigList CRs from %s namesapace. Error: %+v",
 			namespace, err)
 		return "", err
 	}
 
-	for _, cnsFileAccessConfig := range cnsFileAccessConfigList.Items {
-		if cnsFileAccessConfig.Spec.VMName == vm {
-			if cnsFileAccessConfig.Spec.PvcName == pvc {
-				return cnsFileAccessConfig.Name, nil
-			}
-		}
+	if len(cnsFileAccessConfigList.Items) == 1 {
+		// There should be only 1 CFC CR with the same VM and PVC
+		return cnsFileAccessConfigList.Items[0].Name, nil
+	}
+
+	if len(cnsFileAccessConfigList.Items) > 1 {
+		// We should never reach here but it's good to have this check.
+		return "", fmt.Errorf("invalid case, %d CnsFileAccessConfig instances detected "+
+			"with the VM %s and PVC %s", len(cnsFileAccessConfigList.Items), vm, pvc)
 	}
+
 	return "", nil
 }
 
@@ -125,10 +155,10 @@ func validateDeleteCnsFileAccessConfig(ctx context.Context, clientConfig *rest.C
 			},
 		}
 	}
-	// If CR has devops user label, allow this request as
+	// If CR has VM name label, allow this request as
 	// it means that it is created by devops user or K8s admin
 	// and not by VKS (CSI service account).
-	if _, ok := cnsFileAccessConfig.Labels[devopsUserLabelKey]; ok {
+	if _, ok := cnsFileAccessConfig.Labels[vmNameLabelKey]; ok {
 		log.Infof("CnsFileAccessConfig %s has devops user label. Allow this reqeust.",
 			cnsFileAccessConfig.Name)
 		return &admissionv1.AdmissionResponse{