[release-v1.58] CNV-52722: Pass through extra VDDK configuration opti…

…ons to importer pod. (#3610)

* CNV-52722: Pass through extra VDDK configuration options to importer pod. (#3572)

* Add annotation for extra VDDK library arguments.

The VDDK library itself accepts infrequently-used arguments in a
configuration file, and some of these arguments have been tested to show
a significant transfer speedup in some environments. This adds an
annotation that references a ConfigMap holding the contents of this VDDK
configuration file, and mounts it to the importer pod. The first file in
the mounted directory is passed to the VDDK.

Signed-off-by: Matthew Arnold <[email protected]>

* Add functional test for VDDK args annotation.

Signed-off-by: Matthew Arnold <[email protected]>

* Add unit test for extra VDDK arguments annotation.

Signed-off-by: Matthew Arnold <[email protected]>

* Add documentation for extra VDDK arguments.

Signed-off-by: Matthew Arnold <[email protected]>

* Simplify new functional test annotation creation.

Signed-off-by: Matthew Arnold <[email protected]>

* Look for specific file instead of first file.

Instead of listing the mounted VDDK arguments directory and filtering
out hidden files, just hard-code the expected file name and ConfigMap
key.

Signed-off-by: Matthew Arnold <[email protected]>

* Move extra VDDK arguments functional test.

Put this in import_test and assert the values there, instead of in the
VDDK test plugin. The VDDK plugin logs the given values, and then the
test scans the log for what it expects to see.

Signed-off-by: Matthew Arnold <[email protected]>

* Clean up lint error.

Signed-off-by: Matthew Arnold <[email protected]>

* Move VDDK configuration test back, change test ID.

Signed-off-by: Matthew Arnold <[email protected]>

* Avoid using kubectl for scanning nbdkit logs.

Signed-off-by: Matthew Arnold <[email protected]>

* Temporary: show whole nbdkit log after failure.

Signed-off-by: Matthew Arnold <[email protected]>

* Revert "Temporary: show whole nbdkit log after failure."

This reverts commit 488849f.

Signed-off-by: Matthew Arnold <[email protected]>

* Copy extra VDDK args annotation for populators.

Also add a related unit test.

Signed-off-by: Matthew Arnold <[email protected]>

* Correct VDDK args config map mount comment.

Signed-off-by: Matthew Arnold <[email protected]>

---------

Signed-off-by: Matthew Arnold <[email protected]>

* Patch merge conflict in test, "core" vs. "v1".

Signed-off-by: Matthew Arnold <[email protected]>

* Correct merge conflict in extra args volume mount.

Signed-off-by: Matthew Arnold <[email protected]>

---------

Signed-off-by: Matthew Arnold <[email protected]>

doc/datavolumes.md

@@ -286,6 +286,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 with the key `vddk-config-file` 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 mount directory will have a file named `vddk-config-file` with the contents of the file. This means that the ConfigMap must be placed in the same namespace as the DataVolume, and the ConfigMap should only have one file entry, `vddk-config-file`.
+
+[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

@@ -239,6 +239,12 @@ 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"
+	// VddkArgsVolName is the name of the volume referencing the extra VDDK arguments ConfigMap
+	VddkArgsVolName = "vddk-extra-args"
+	// VddkArgsKeyName is the name of the key that must be present in the VDDK arguments ConfigMap
+	VddkArgsKeyName = "vddk-config-file"
 
 	// UploadContentTypeHeader is the header upload clients may use to set the content type explicitly
 	UploadContentTypeHeader = "x-cdi-content-type"

pkg/controller/common/util.go

@@ -146,6 +146,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

@@ -114,6 +114,7 @@ type importerPodArgs struct {
 	imagePullSecrets        []corev1.LocalObjectReference
 	workloadNodePlacement   *sdkapi.NodePlacement
 	vddkImageName           *string
+	vddkExtraArgs           *string
 	priorityClassName       string
 }
 
@@ -475,6 +476,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)
@@ -503,6 +505,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)
@@ -518,6 +525,7 @@ func (r *ImportReconciler) createImporterPod(pvc *corev1.PersistentVolumeClaim)
 		pvc:               pvc,
 		scratchPvcName:    scratchPvcName,
 		vddkImageName:     vddkImageName,
+		vddkExtraArgs:     vddkExtraArgs,
 		priorityClassName: cc.GetPriorityClass(pvc),
 	}
 
@@ -1108,6 +1116,23 @@ func makeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
 		})
 	}
 
+	if args.vddkExtraArgs != nil {
+		pod.Spec.Volumes = append(pod.Spec.Volumes, corev1.Volume{
+			Name: common.VddkArgsVolName,
+			VolumeSource: corev1.VolumeSource{
+				ConfigMap: &v1.ConfigMapVolumeSource{
+					LocalObjectReference: v1.LocalObjectReference{
+						Name: *args.vddkExtraArgs,
+					},
+				},
+			},
+		})
+		pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, v1.VolumeMount{
+			Name:      common.VddkArgsVolName,
+			MountPath: common.VddkArgsDir,
+		})
+	}
+
 	if args.podEnvVar.certConfigMap != "" {
 		vm := corev1.VolumeMount{
 			Name:      CertVolName,

pkg/controller/import-controller_test.go

@@ -860,6 +860,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/populators/import-populator_test.go

@@ -442,6 +442,62 @@ var _ = Describe("Import populator tests", func() {
 			Expect(pvcPrime.GetAnnotations()[AnnCurrentCheckpoint]).To(Equal("current"))
 			Expect(pvcPrime.GetAnnotations()[AnnFinalCheckpoint]).To(Equal("true"))
 		})
+
+		It("Should create PVC prime with proper VDDK import annotations", func() {
+			targetPvc := CreatePvcInStorageClass(targetPvcName, metav1.NamespaceDefault, &sc.Name, map[string]string{}, nil, corev1.ClaimPending)
+			targetPvc.Spec.DataSourceRef = dataSourceRef
+			targetPvc.Annotations[AnnVddkExtraArgs] = "vddk-extras"
+
+			volumeImportSource := getVolumeImportSource(true, metav1.NamespaceDefault)
+			volumeImportSource.Spec.Source = &cdiv1.ImportSourceType{
+				VDDK: &cdiv1.DataVolumeSourceVDDK{
+					BackingFile: "testBackingFile",
+					SecretRef:   "testSecret",
+					Thumbprint:  "testThumbprint",
+					URL:         "testUrl",
+					UUID:        "testUUID",
+				},
+			}
+
+			By("Reconcile")
+			reconciler = createImportPopulatorReconciler(targetPvc, volumeImportSource, sc)
+			result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: targetPvcName, Namespace: metav1.NamespaceDefault}})
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(result).To(Not(BeNil()))
+
+			By("Checking events recorded")
+			close(reconciler.recorder.(*record.FakeRecorder).Events)
+			found := false
+			for event := range reconciler.recorder.(*record.FakeRecorder).Events {
+				if strings.Contains(event, createdPVCPrimeSuccessfully) {
+					found = true
+				}
+			}
+			reconciler.recorder = nil
+			Expect(found).To(BeTrue())
+
+			By("Checking PVC' annotations")
+			pvcPrime, err := reconciler.getPVCPrime(targetPvc)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pvcPrime).ToNot(BeNil())
+			// make sure we didnt inflate size
+			Expect(pvcPrime.Spec.Resources.Requests[corev1.ResourceStorage]).To(Equal(resource.MustParse("1G")))
+			Expect(pvcPrime.GetAnnotations()).ToNot(BeNil())
+			Expect(pvcPrime.GetAnnotations()[AnnImmediateBinding]).To(Equal(""))
+			Expect(pvcPrime.GetAnnotations()[AnnUploadRequest]).To(Equal(""))
+			Expect(pvcPrime.GetAnnotations()[AnnPopulatorKind]).To(Equal(cdiv1.VolumeImportSourceRef))
+			Expect(pvcPrime.GetAnnotations()[AnnPreallocationRequested]).To(Equal("true"))
+			Expect(pvcPrime.GetAnnotations()[AnnBackingFile]).To(Equal("testBackingFile"))
+			Expect(pvcPrime.GetAnnotations()[AnnSecret]).To(Equal("testSecret"))
+			Expect(pvcPrime.GetAnnotations()[AnnThumbprint]).To(Equal("testThumbprint"))
+			Expect(pvcPrime.GetAnnotations()[AnnEndpoint]).To(Equal("testUrl"))
+			Expect(pvcPrime.GetAnnotations()[AnnUUID]).To(Equal("testUUID"))
+			Expect(pvcPrime.GetAnnotations()[AnnSource]).To(Equal(SourceVDDK))
+			Expect(pvcPrime.GetLabels()[LabelExcludeFromVeleroBackup]).To(Equal("true"))
+
+			Expect(pvcPrime.GetAnnotations()[AnnVddkExtraArgs]).To(Equal("vddk-extras"))
+		})
+
 	})
 
 	var _ = Describe("Import populator progress report", func() {

pkg/controller/populators/populator-base.go

@@ -180,6 +180,9 @@ func (r *ReconcilerBase) createPVCPrime(pvc *corev1.PersistentVolumeClaim, sourc
 	if _, ok := pvc.Annotations[cc.AnnPodRetainAfterCompletion]; ok {
 		annotations[cc.AnnPodRetainAfterCompletion] = pvc.Annotations[cc.AnnPodRetainAfterCompletion]
 	}
+	if vddkExtraArgs, ok := pvc.Annotations[cc.AnnVddkExtraArgs]; ok && vddkExtraArgs != "" {
+		annotations[cc.AnnVddkExtraArgs] = vddkExtraArgs
+	}
 
 	// Assemble PVC' spec
 	pvcPrime := &corev1.PersistentVolumeClaim{

pkg/image/nbdkit.go

@@ -169,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,
@@ -229,6 +236,22 @@ func getVddkPluginPath() NbdkitPlugin {
 	return NbdkitVddkPlugin
 }
 
+// Extra VDDK configuration options are stored in a ConfigMap mounted to the
+// importer pod. Make sure a "vddk-config-file" exists in that directory and
+// pass that through nbdkit via the "config=" option.
+func getVddkConfig() (string, error) {
+	path := filepath.Join(common.VddkArgsDir, common.VddkArgsKeyName)
+	_, err := os.Stat(path)
+	if err != nil {
+		if os.IsNotExist(err) { // No configuration file found, so no extra arguments to give to VDDK
+			return "", nil
+		}
+		return "", err
+	}
+
+	return path, nil
+}
+
 func (n *Nbdkit) getSourceArg(s string) string {
 	var source string
 	switch n.plugin {

tests/datavolume_test.go

@@ -3367,6 +3367,69 @@ var _ = Describe("[vendor:[email protected]][level:component]DataVolume tests",
 			}),
 		)
 	})
+
+	Describe("extra configuration options for VDDK imports", func() {
+		It("[test_id:XXXX]succeed importing VDDK data volume with extra arguments ConfigMap set", Label("VDDK"), func() {
+			vddkConfigOptions := []string{
+				"VixDiskLib.nfcAio.Session.BufSizeIn64KB=16",
+				"vixDiskLib.nfcAio.Session.BufCount=4",
+			}
+
+			vddkConfigMap := &v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "vddk-extras",
+				},
+				Data: map[string]string{
+					common.VddkArgsKeyName: strings.Join(vddkConfigOptions, "\n"),
+				},
+			}
+
+			_, err := f.K8sClient.CoreV1().ConfigMaps(f.Namespace.Name).Create(context.TODO(), vddkConfigMap, metav1.CreateOptions{})
+			if !k8serrors.IsAlreadyExists(err) {
+				Expect(err).ToNot(HaveOccurred())
+			}
+
+			vcenterURL := fmt.Sprintf(utils.VcenterURL, f.CdiInstallNs)
+			dataVolume := createVddkDataVolume("import-pod-vddk-config-test", "100Mi", vcenterURL)
+
+			By(fmt.Sprintf("Create new DataVolume %s", dataVolume.Name))
+			controller.AddAnnotation(dataVolume, controller.AnnPodRetainAfterCompletion, "true")
+			controller.AddAnnotation(dataVolume, controller.AnnVddkExtraArgs, "vddk-extras")
+			dataVolume, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dataVolume)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Verify PVC was created")
+			pvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name)
+			Expect(err).ToNot(HaveOccurred())
+			f.ForceBindIfWaitForFirstConsumer(pvc)
+
+			By("Wait for import to be completed")
+			err = utils.WaitForDataVolumePhase(f, dataVolume.Namespace, cdiv1.Succeeded, dataVolume.Name)
+			Expect(err).ToNot(HaveOccurred(), "DataVolume not in phase succeeded in time")
+
+			By("Find importer pods after completion")
+			pvcName := dataVolume.Name
+			// When using populators, the PVC Prime name is used to build the importer pod
+			if usePopulator, _ := dvc.CheckPVCUsingPopulators(pvc); usePopulator {
+				pvcName = populators.PVCPrimeName(pvc)
+			}
+			By("Find importer pod " + pvcName)
+			importer, err := utils.FindPodByPrefixOnce(f.K8sClient, dataVolume.Namespace, common.ImporterPodName, common.CDILabelSelector)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(importer.DeletionTimestamp).To(BeNil())
+
+			Eventually(func() (string, error) {
+				out, err := f.K8sClient.CoreV1().
+					Pods(importer.Namespace).
+					GetLogs(importer.Name, &v1.PodLogOptions{SinceTime: &metav1.Time{Time: CurrentSpecReport().StartTime}}).
+					DoRaw(context.Background())
+				return string(out), err
+			}, time.Minute, pollingInterval).Should(And(
+				ContainSubstring(vddkConfigOptions[0]),
+				ContainSubstring(vddkConfigOptions[1]),
+			))
+		})
+	})
 })
 
 func SetFilesystemOverhead(f *framework.Framework, globalOverhead, scOverhead string) {

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,21 @@ 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[512]; // Importer test will scan debug log for given values, just pass them back
+        while (fgets(extras, 512, f) != NULL) {
+            nbdkit_debug("Extra configuration data: %s\n", extras);
+        }
+        fclose(f);
+    }
     return 0;
 }