VDDK: Add CRD field for extra configuration arguments (#3622)

* Add ExtraArgs field to VDDK CRD.

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Add tests for VDDK ExtraArgs field.

Add one unit test and rework existing functional test into a table for
both annotation and field entries.

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Update ExtraArgs DataVolume documentation.

Signed-off-by: Matthew Arnold <marnold@redhat.com>

---------

Signed-off-by: Matthew Arnold <marnold@redhat.com>

api/openapi-spec/swagger.json

@@ -4934,6 +4934,10 @@
       "description": "BackingFile is the path to the virtual hard disk to migrate from vCenter/ESXi",
       "type": "string"
      },
+     "extraArgs": {
+      "description": "ExtraArgs is a reference to a ConfigMap containing extra arguments to pass directly to the VDDK library",
+      "type": "string"
+     },
      "initImageURL": {
       "description": "InitImageURL is an optional URL to an image containing an extracted VDDK library, overrides v2v-vmware config map",
       "type": "string"

doc/datavolumes.md

@@ -351,8 +351,9 @@ spec:
 
 #### 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`.
+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 put the name of this ConfigMap in either the `spec.source.vddk.extraArgs` field or a `cdi.kubevirt.io/storage.pod.vddk.extraargs` annotation in 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 field](../manifests/example/vddk-args-field.yaml)
 [Example annotation](../manifests/example/vddk-args-annotation.yaml)
 [Example ConfigMap](../manifests/example/vddk-args-configmap.yaml)
 

manifests/example/vddk-args-field.yaml

@@ -0,0 +1,20 @@
+apiVersion: cdi.kubevirt.io/v1beta1
+kind: DataVolume
+metadata:
+  name: "vddk-dv"
+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"
+           extraArgs: "vddk-arguments"
+    storage:
+       accessModes:
+         - ReadWriteOnce
+       resources:
+         requests:
+           storage: "32Gi"

pkg/apis/core/v1beta1/openapi_generated.go

@@ -18405,6 +18405,13 @@ func schema_pkg_apis_core_v1beta1_DataVolumeSourceVDDK(ref common.ReferenceCallb
 							Format:      "",
 						},
 					},
+					"extraArgs": {
+						SchemaProps: spec.SchemaProps{
+							Description: "ExtraArgs is a reference to a ConfigMap containing extra arguments to pass directly to the VDDK library",
+							Type:        []string{"string"},
+							Format:      "",
+						},
+					},
 				},
 			},
 		},

pkg/controller/common/util.go

@@ -1703,6 +1703,9 @@ func UpdateVDDKAnnotations(annotations map[string]string, vddk *cdiv1.DataVolume
 	if vddk.InitImageURL != "" {
 		annotations[AnnVddkInitImageURL] = vddk.InitImageURL
 	}
+	if vddk.ExtraArgs != "" {
+		annotations[AnnVddkExtraArgs] = vddk.ExtraArgs
+	}
 }
 
 // UpdateImageIOAnnotations updates the passed annotations for proper imageIO import

pkg/controller/datavolume/import-controller_test.go

@@ -1088,6 +1088,19 @@ var _ = Describe("All DataVolume Tests", func() {
 			Expect(pvc).ToNot(BeNil())
 			Expect(pvc.GetAnnotations()[AnnVddkInitImageURL]).To(Equal("test://image"))
 		})
+
+		It("Should copy extra VDDK args to PVC", func() {
+			dv := newVDDKDataVolume("test-dv")
+			dv.Spec.Source.VDDK.ExtraArgs = "vddk-extra-args"
+			reconciler = createImportReconciler(dv)
+			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
+			Expect(err).ToNot(HaveOccurred())
+			pvc := &corev1.PersistentVolumeClaim{}
+			err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(pvc).ToNot(BeNil())
+			Expect(pvc.GetAnnotations()[AnnVddkExtraArgs]).To(Equal("vddk-extra-args"))
+		})
 	})
 
 	var _ = Describe("Reconcile Datavolume status", func() {

pkg/operator/resources/crds_generated.go

@@ -6129,6 +6129,11 @@ spec:
                                 description: BackingFile is the path to the virtual
                                   hard disk to migrate from vCenter/ESXi
                                 type: string
+                              extraArgs:
+                                description: ExtraArgs is a reference to a ConfigMap
+                                  containing extra arguments to pass directly to the
+                                  VDDK library
+                                type: string
                               initImageURL:
                                 description: InitImageURL is an optional URL to an
                                   image containing an extracted VDDK library, overrides
@@ -7084,6 +7089,10 @@ spec:
                         description: BackingFile is the path to the virtual hard disk
                           to migrate from vCenter/ESXi
                         type: string
+                      extraArgs:
+                        description: ExtraArgs is a reference to a ConfigMap containing
+                          extra arguments to pass directly to the VDDK library
+                        type: string
                       initImageURL:
                         description: InitImageURL is an optional URL to an image containing
                           an extracted VDDK library, overrides v2v-vmware config map
@@ -8090,6 +8099,10 @@ spec:
                         description: BackingFile is the path to the virtual hard disk
                           to migrate from vCenter/ESXi
                         type: string
+                      extraArgs:
+                        description: ExtraArgs is a reference to a ConfigMap containing
+                          extra arguments to pass directly to the VDDK library
+                        type: string
                       initImageURL:
                         description: InitImageURL is an optional URL to an image containing
                           an extracted VDDK library, overrides v2v-vmware config map

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go

@@ -262,6 +262,8 @@ type DataVolumeSourceVDDK struct {
 	SecretRef string `json:"secretRef,omitempty"`
 	// InitImageURL is an optional URL to an image containing an extracted VDDK library, overrides v2v-vmware config map
 	InitImageURL string `json:"initImageURL,omitempty"`
+	// ExtraArgs is a reference to a ConfigMap containing extra arguments to pass directly to the VDDK library
+	ExtraArgs string `json:"extraArgs,omitempty"`
 }
 
 // DataVolumeSourceRef defines an indirect reference to the source of data for the DataVolume

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types_swagger_generated.go

@@ -138,6 +138,7 @@ func (DataVolumeSourceVDDK) SwaggerDoc() map[string]string {
 		"thumbprint":   "Thumbprint is the certificate thumbprint of the vCenter or ESXi host",
 		"secretRef":    "SecretRef provides a reference to a secret containing the username and password needed to access the vCenter or ESXi host",
 		"initImageURL": "InitImageURL is an optional URL to an image containing an extracted VDDK library, overrides v2v-vmware config map",
+		"extraArgs":    "ExtraArgs is a reference to a ConfigMap containing extra arguments to pass directly to the VDDK library",
 	}
 }
 

tests/datavolume_test.go

@@ -3466,68 +3466,73 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
 		)
 	})
 
-	Describe("extra configuration options for VDDK imports", func() {
+	DescribeTable("extra configuration options for VDDK imports", Label("VDDK"), func(tweakDataVolume func(*cdiv1.DataVolume)) {
-		It("[test_id:XXXX]succeed importing VDDK data volume with extra arguments ConfigMap set", Label("VDDK"), func() {
+		vddkConfigOptions := []string{
-			vddkConfigOptions := []string{
+			"VixDiskLib.nfcAio.Session.BufSizeIn64KB=16",
-				"VixDiskLib.nfcAio.Session.BufSizeIn64KB=16",
+			"vixDiskLib.nfcAio.Session.BufCount=4",
-				"vixDiskLib.nfcAio.Session.BufCount=4",
+		}
-			}
 
-			vddkConfigMap := &v1.ConfigMap{
+		vddkConfigMap := &v1.ConfigMap{
-				ObjectMeta: metav1.ObjectMeta{
+			ObjectMeta: metav1.ObjectMeta{
-					Name: "vddk-extras",
+				Name: "vddk-extras",
-				},
+			},
-				Data: map[string]string{
+			Data: map[string]string{
-					common.VddkArgsKeyName: strings.Join(vddkConfigOptions, "\n"),
+				common.VddkArgsKeyName: strings.Join(vddkConfigOptions, "\n"),
-				},
+			},
-			}
+		}
 
-			_, err := f.K8sClient.CoreV1().ConfigMaps(f.Namespace.Name).Create(context.TODO(), vddkConfigMap, metav1.CreateOptions{})
+		_, err := f.K8sClient.CoreV1().ConfigMaps(f.Namespace.Name).Create(context.TODO(), vddkConfigMap, metav1.CreateOptions{})
-			if !k8serrors.IsAlreadyExists(err) {
+		if !k8serrors.IsAlreadyExists(err) {
-				Expect(err).ToNot(HaveOccurred())
+			Expect(err).ToNot(HaveOccurred())
-			}
+		}
 
-			vcenterURL := fmt.Sprintf(utils.VcenterURL, f.CdiInstallNs)
+		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))
+		dataVolume := createVddkDataVolume("import-pod-vddk-config-test", "100Mi", vcenterURL)
-			controller.AddAnnotation(dataVolume, controller.AnnPodRetainAfterCompletion, "true")
+		By(fmt.Sprintf("Create new DataVolume %s", dataVolume.Name))
+		tweakDataVolume(dataVolume)
+		controller.AddAnnotation(dataVolume, controller.AnnPodRetainAfterCompletion, "true")
+		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, &core.PodLogOptions{SinceTime: &meta.Time{Time: CurrentSpecReport().StartTime}}).
+				DoRaw(context.Background())
+			return string(out), err
+		}, time.Minute, pollingInterval).Should(And(
+			ContainSubstring(vddkConfigOptions[0]),
+			ContainSubstring(vddkConfigOptions[1]),
+		))
+	},
+		Entry("[test_id:XXXX]succeed importing VDDK data volume with extra arguments ConfigMap annotation set", func(dataVolume *cdiv1.DataVolume) {
 			controller.AddAnnotation(dataVolume, controller.AnnVddkExtraArgs, "vddk-extras")
-			dataVolume, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dataVolume)
+		}),
-			Expect(err).ToNot(HaveOccurred())
+		Entry("[test_id:XXXX]succeed importing VDDK data volume with extra arguments ConfigMap field set", func(dataVolume *cdiv1.DataVolume) {
-
+			dataVolume.Spec.Source.VDDK.ExtraArgs = "vddk-extras"
-			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, &core.PodLogOptions{SinceTime: &meta.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) {