CNV-52722: Pass through extra VDDK configuration options to importer pod.

doc/datavolumes.md

@@ -349,6 +349,13 @@ spec:
 [Get VDDK ConfigMap example](../manifests/example/vddk-configmap.yaml)
 [Ways to find thumbprint](https://libguestfs.org/nbdkit-vddk-plugin.1.html#THUMBPRINTS)
 
+#### Extra VDDK Configuration Options
+
+The VDDK library itself looks in a configuration file (such as `/etc/vmware/config`) for extra options to fine tune data transfers. To pass these options through to the VDDK, store the configuration file contents in a ConfigMap and add a `cdi.kubevirt.io/storage.pod.vddk.extraargs` annotation to the DataVolume specification. The ConfigMap will be mounted to the importer pod as a volume, and the first file in the mounted directory will be passed to the VDDK. This means that the ConfigMap must be placed in the same namespace as the DataVolume, and the ConfigMap should only have one file entry.
+
+[Example annotation](../manifests/example/vddk-args-annotation.yaml)
+[Example ConfigMap](../manifests/example/vddk-args-configmap.yaml)
+
 ## Multi-stage Import
  In a multi-stage import, multiple pods are started in succession to copy different parts of the source to an existing base disk image. Currently only the [ImageIO](#multi-stage-imageio-import) and [VDDK](#multi-stage-vddk-import) data sources support multi-stage imports.
 

manifests/example/vddk-args-annotation.yaml

@@ -0,0 +1,22 @@
+apiVersion: cdi.kubevirt.io/v1beta1
+kind: DataVolume
+metadata:
+  name: "vddk-dv"
+  namespace: "cdi"
+  annotations:
+    cdi.kubevirt.io/storage.pod.vddk.extraargs: vddk-arguments
+spec:
+    source:
+        vddk:
+           backingFile: "[iSCSI_Datastore] vm/vm_1.vmdk" # From 'Hard disk'/'Disk File' in vCenter/ESX VM settings
+           url: "https://vcenter.corp.com"
+           uuid: "52260566-b032-36cb-55b1-79bf29e30490"
+           thumbprint: "20:6C:8A:5D:44:40:B3:79:4B:28:EA:76:13:60:90:6E:49:D9:D9:A3" # SSL fingerprint of vCenter/ESX host
+           secretRef: "vddk-credentials"
+           initImageURL: "registry:5000/vddk-init:latest"
+    storage:
+       accessModes:
+         - ReadWriteOnce
+       resources:
+         requests:
+           storage: "32Gi"

manifests/example/vddk-args-configmap.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  namespace: cdi
+  name: vddk-arguments
+data:
+  vddk-config-file: -| 
+      VixDiskLib.nfcAio.Session.BufSizeIn64KB=16
+      VixDiskLib.nfcAio.Session.BufCount=4

pkg/common/common.go

@@ -254,6 +254,8 @@ const (
 	VddkConfigDataKey = "vddk-init-image"
 	// AwaitingVDDK is a Pending condition reason that indicates the PVC is waiting for a VDDK image
 	AwaitingVDDK = "AwaitingVDDK"
+	// VddkArgsDir is the path to the volume mount containing extra VDDK arguments
+	VddkArgsDir = "/vddk-args"
 
 	// UploadContentTypeHeader is the header upload clients may use to set the content type explicitly
 	UploadContentTypeHeader = "x-cdi-content-type"

pkg/controller/common/util.go

@@ -147,6 +147,8 @@ const (
 	AnnVddkHostConnection = AnnAPIGroup + "/storage.pod.vddk.host"
 	// AnnVddkInitImageURL saves a per-DV VDDK image URL on the PVC
 	AnnVddkInitImageURL = AnnAPIGroup + "/storage.pod.vddk.initimageurl"
+	// AnnVddkExtraArgs references a ConfigMap that holds arguments to pass directly to the VDDK library
+	AnnVddkExtraArgs = AnnAPIGroup + "/storage.pod.vddk.extraargs"
 
 	// AnnRequiresScratch provides a const for our PVC requiring scratch annotation
 	AnnRequiresScratch = AnnAPIGroup + "/storage.import.requiresScratch"

pkg/controller/import-controller.go

@@ -116,6 +116,7 @@ type importerPodArgs struct {
 	imagePullSecrets        []corev1.LocalObjectReference
 	workloadNodePlacement   *sdkapi.NodePlacement
 	vddkImageName           *string
+	vddkExtraArgs           *string
 	priorityClassName       string
 }
 
@@ -480,6 +481,7 @@ func (r *ImportReconciler) createImporterPod(pvc *corev1.PersistentVolumeClaim)
 	r.log.V(1).Info("Creating importer POD for PVC", "pvc.Name", pvc.Name)
 	var scratchPvcName *string
 	var vddkImageName *string
+	var vddkExtraArgs *string
 	var err error
 
 	requiresScratch := r.requiresScratchSpace(pvc)
@@ -508,6 +510,11 @@ func (r *ImportReconciler) createImporterPod(pvc *corev1.PersistentVolumeClaim)
 			}
 			return errors.New(message)
 		}
+
+		if extraArgs, ok := anno[cc.AnnVddkExtraArgs]; ok && extraArgs != "" {
+			r.log.V(1).Info("Mounting extra VDDK args ConfigMap to importer pod", "ConfigMap", extraArgs)
+			vddkExtraArgs = &extraArgs
+		}
 	}
 
 	podEnvVar, err := r.createImportEnvVar(pvc)
@@ -523,6 +530,7 @@ func (r *ImportReconciler) createImporterPod(pvc *corev1.PersistentVolumeClaim)
 		pvc:               pvc,
 		scratchPvcName:    scratchPvcName,
 		vddkImageName:     vddkImageName,
+		vddkExtraArgs:     vddkExtraArgs,
 		priorityClassName: cc.GetPriorityClass(pvc),
 	}
 
@@ -999,6 +1007,12 @@ func makeImporterContainerSpec(args *importerPodArgs) []corev1.Container {
 			MountPath: "/opt",
 		})
 	}
+	if args.vddkExtraArgs != nil {
+		containers[0].VolumeMounts = append(containers[0].VolumeMounts, corev1.VolumeMount{
+			Name:      VddkArgsVolName,
+			MountPath: common.VddkArgsDir,
+		})
+	}
 	if args.podEnvVar.certConfigMap != "" {
 		containers[0].VolumeMounts = append(containers[0].VolumeMounts, corev1.VolumeMount{
 			Name:      CertVolName,
@@ -1070,6 +1084,18 @@ func makeImporterVolumeSpec(args *importerPodArgs) []corev1.Volume {
 			},
 		})
 	}
+	if args.vddkExtraArgs != nil {
+		volumes = append(volumes, corev1.Volume{
+			Name: VddkArgsVolName,
+			VolumeSource: corev1.VolumeSource{
+				ConfigMap: &v1.ConfigMapVolumeSource{
+					LocalObjectReference: v1.LocalObjectReference{
+						Name: *args.vddkExtraArgs,
+					},
+				},
+			},
+		})
+	}
 	if args.podEnvVar.certConfigMap != "" {
 		volumes = append(volumes, createConfigMapVolume(CertVolName, args.podEnvVar.certConfigMap))
 	}

pkg/controller/import-controller_test.go

@@ -935,6 +935,36 @@ var _ = Describe("Create Importer Pod", func() {
 		Entry("with long PVC name", strings.Repeat("test-pvc-", 20), "snap1"),
 		Entry("with long PVC and checkpoint names", strings.Repeat("test-pvc-", 20), strings.Repeat("repeating-checkpoint-id-", 10)),
 	)
+
+	It("should mount extra VDDK arguments ConfigMap when annotation is set", func() {
+		pvcName := "testPvc1"
+		podName := "testpod"
+		extraArgs := "testing-123"
+		annotations := map[string]string{
+			cc.AnnEndpoint:         testEndPoint,
+			cc.AnnImportPod:        podName,
+			cc.AnnSource:           cc.SourceVDDK,
+			cc.AnnVddkInitImageURL: "testing-vddk",
+			cc.AnnVddkExtraArgs:    extraArgs,
+		}
+		pvc := cc.CreatePvcInStorageClass(pvcName, "default", &testStorageClass, annotations, nil, corev1.ClaimBound)
+		reconciler := createImportReconciler(pvc)
+
+		_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: pvcName, Namespace: "default"}})
+		Expect(err).ToNot(HaveOccurred())
+
+		pod := &corev1.Pod{}
+		err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: podName, Namespace: "default"}, pod)
+		Expect(err).ToNot(HaveOccurred())
+
+		found := false // Look for vddk-args mount
+		for _, volume := range pod.Spec.Volumes {
+			if volume.ConfigMap != nil && volume.ConfigMap.Name == extraArgs {
+				found = true
+			}
+		}
+		Expect(found).To(BeTrue())
+	})
 })
 
 var _ = Describe("Import test env", func() {

pkg/controller/util.go

@@ -52,6 +52,9 @@ const (
 	//nolint:gosec // This is not a real secret
 	SecretVolName = "cdi-secret-vol"
 
+	// VddkArgsVolName is the name of the volume referencing the extra VDDK arguments ConfigMap
+	VddkArgsVolName = "vddk-extra-args"
+
 	// AnnOwnerRef is used when owner is in a different namespace
 	AnnOwnerRef = cc.AnnAPIGroup + "/storage.ownerRef"
 

pkg/image/nbdkit.go

@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -168,6 +169,13 @@ func NewNbdkitVddk(nbdkitPidFile, socket string, args NbdKitVddkPluginArgs) (Nbd
 	pluginArgs = append(pluginArgs, "-D", "nbdkit.backend.datapath=0")
 	pluginArgs = append(pluginArgs, "-D", "vddk.datapath=0")
 	pluginArgs = append(pluginArgs, "-D", "vddk.stats=1")
+	config, err := getVddkConfig()
+	if err != nil {
+		return nil, err
+	}
+	if config != "" {
+		pluginArgs = append(pluginArgs, "config="+config)
+	}
 	p := getVddkPluginPath()
 	n := &Nbdkit{
 		NbdPidFile: nbdkitPidFile,
@@ -228,6 +236,30 @@ func getVddkPluginPath() NbdkitPlugin {
 	return NbdkitVddkPlugin
 }
 
+// Extra VDDK configuration options are stored in a ConfigMap mounted to the
+// importer pod. Just look for the first file in the mounted directory, and
+// pass that through nbdkit via the "config=" option.
+func getVddkConfig() (string, error) {
+	withHidden, err := os.ReadDir(common.VddkArgsDir)
+	if err != nil {
+		if os.IsNotExist(err) { // No extra arguments ConfigMap specified, so mount directory does not exist
+			return "", nil
+		}
+		return "", err
+	}
+	files := []fs.DirEntry{}
+	for _, file := range withHidden { // Ignore hidden files
+		if !strings.HasPrefix(file.Name(), ".") {
+			files = append(files, file)
+		}
+	}
+	if len(files) < 1 {
+		return "", fmt.Errorf("no VDDK configuration files found under %s", common.VddkArgsDir)
+	}
+	path := filepath.Join(common.VddkArgsDir, files[0].Name())
+	return path, nil
+}
+
 func (n *Nbdkit) getSourceArg(s string) string {
 	var source string
 	switch n.plugin {

tests/datavolume_test.go

@@ -1184,6 +1184,25 @@ var _ = Describe("[vendor:[email protected]][level:component]DataVolume tests",
 			return dv
 		}
 
+		createVddkDataVolumeWithExtraArgs := func(dataVolumeName, size, url string) *cdiv1.DataVolume {
+			dv := createVddkDataVolumeWithInitImageURL(dataVolumeName, size, url)
+			extraArguments := v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: f.Namespace.Name,
+					Name:      "vddk-extra-args-map",
+				},
+				Data: map[string]string{ // Must match vddk-test-plugin
+					"vddk-config-file": "VixDiskLib.nfcAio.Session.BufSizeIn64KB=16",
+				},
+			}
+			_, err := f.K8sClient.CoreV1().ConfigMaps(f.Namespace.Name).Create(context.TODO(), &extraArguments, metav1.CreateOptions{})
+			if !k8serrors.IsAlreadyExists(err) {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			controller.AddAnnotation(dv, controller.AnnVddkExtraArgs, extraArguments.Name)
+			return dv
+		}
+
 		// Similar to previous table, but with additional cleanup steps to save and restore VDDK image config map
 		DescribeTable("should", Serial, func(args dataVolumeTestArguments) {
 			_, err := utils.CopyConfigMap(f.K8sClient, f.CdiInstallNs, common.VddkConfigMap, f.CdiInstallNs, savedVddkConfigMap, "")
@@ -1254,6 +1273,30 @@ var _ = Describe("[vendor:[email protected]][level:component]DataVolume tests",
 					Message: "Import Complete",
 					Reason:  "Completed",
 				}}),
+			Entry("[test_id:5083]succeed importing VDDK data volume with extra arguments ConfigMap set", dataVolumeTestArguments{
+				name:             "dv-import-vddk",
+				size:             "1Gi",
+				url:              vcenterURL,
+				dvFunc:           createVddkDataVolumeWithExtraArgs,
+				eventReason:      dvc.ImportSucceeded,
+				phase:            cdiv1.Succeeded,
+				checkPermissions: false,
+				readyCondition: &cdiv1.DataVolumeCondition{
+					Type:   cdiv1.DataVolumeReady,
+					Status: v1.ConditionTrue,
+				},
+				boundCondition: &cdiv1.DataVolumeCondition{
+					Type:    cdiv1.DataVolumeBound,
+					Status:  v1.ConditionTrue,
+					Message: "PVC dv-import-vddk Bound",
+					Reason:  "Bound",
+				},
+				runningCondition: &cdiv1.DataVolumeCondition{
+					Type:    cdiv1.DataVolumeRunning,
+					Status:  v1.ConditionFalse,
+					Message: "Import Complete",
+					Reason:  "Completed",
+				}}),
 		)
 
 		// similar to other tables but with check of quota

tools/vddk-test/vddk-test-plugin.c

@@ -13,6 +13,7 @@
 #define NBDKIT_API_VERSION 2
 #include <nbdkit-plugin.h>
 #include <fcntl.h>
+#include <stdio.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -32,6 +33,26 @@ int fakevddk_config(const char *key, const char *value) {
     if (strcmp(key, "snapshot") == 0) {
         expected_arg_count = 9; // Expect one for 'snapshot' and one for 'transports'
     }
+    if (strcmp(key, "config") == 0) {
+        expected_arg_count = 8;
+        nbdkit_debug("Extra config option set to: %s\n", value);
+
+        FILE *f  = fopen(value, "r");
+        if (f == NULL) {
+            nbdkit_error("Failed to open VDDK extra configuration file %s!\n", value);
+            return -1;
+        }
+        char extras[50];
+        if (fgets(extras, 50, f) == NULL) { // Expect only one line of test data
+            nbdkit_error("Failed to read VDDK extra configuration file %s! Error was: %s", value, strerror(errno));
+            return -1;
+        }
+        if (strcmp(extras, "VixDiskLib.nfcAio.Session.BufSizeIn64KB=16") != 0) { // Must match datavolume_test
+            nbdkit_error("Unexpected content in VDDK extra configuration file %s: %s\n", value, extras);
+            return -1;
+        }
+        fclose(f);
+    }
     return 0;
 }