Append Labels From DV to Importer Pod (#3744)

* append labels from pvc to importer pod

Signed-off-by: dsanatar <dsanatar@redhat.com>

* add unit test to validate AppendLabels func

Signed-off-by: dsanatar <dsanatar@redhat.com>

* change AppendLabel func to be src into dest instead of dest into src

Signed-off-by: dsanatar <dsanatar@redhat.com>

* add new functional test for label propogation

Signed-off-by: dsanatar <dsanatar@redhat.com>

* rename test

Signed-off-by: dsanatar <dsanatar@redhat.com>

* make test cases more ginkgo-like

Signed-off-by: dsanatar <dsanatar@redhat.com>

* replace and remove duplicate function calls

Signed-off-by: dsanatar <dsanatar@redhat.com>

* fix linter errors

Signed-off-by: dsanatar <dsanatar@redhat.com>

* fix ginkgo comparison in unit test

Signed-off-by: dsanatar <dsanatar@redhat.com>

* update unit test to be DescribeTable

Signed-off-by: dsanatar <dsanatar@redhat.com>

* copy labels to prime pvc, update test to account for populators

Signed-off-by: dsanatar <dsanatar@redhat.com>

---------

Signed-off-by: dsanatar <dsanatar@redhat.com>

pkg/controller/import-controller.go

@@ -879,6 +879,9 @@ func createImporterPod(ctx context.Context, log logr.Logger, client client.Clien
 
 	util.SetRecommendedLabels(pod, installerLabels, "cdi-controller")
 
+	// add any labels from pvc to the importer pod
+	util.MergeLabels(args.pvc.Labels, pod.Labels)
+
 	if err = client.Create(context.TODO(), pod); err != nil {
 		return nil, err
 	}

pkg/controller/populators/populator-base.go

@@ -209,6 +209,7 @@ func (r *ReconcilerBase) createPVCPrime(pvc *corev1.PersistentVolumeClaim, sourc
 	}
 	cc.CopyAllowedAnnotations(pvc, pvcPrime)
 	util.SetRecommendedLabels(pvcPrime, r.installerLabels, "cdi-controller")
+	util.MergeLabels(pvc.Labels, pvcPrime.Labels)
 
 	// We use the populator-specific pvcModifierFunc to add required annotations
 	if updatePVCForPopulation != nil {

pkg/util/util.go

@@ -169,19 +169,6 @@ func MergeLabels(src, dest map[string]string) map[string]string {
 	return dest
 }
 
-// AppendLabels append dest labels to source (update if key has existed)
-func AppendLabels(src, dest map[string]string) map[string]string {
-	if src == nil {
-		src = map[string]string{}
-	}
-
-	for k, v := range dest {
-		src[k] = v
-	}
-
-	return src
-}
-
 // GetRecommendedInstallerLabelsFromCr returns the recommended labels to set on CDI resources
 func GetRecommendedInstallerLabelsFromCr(cr *cdiv1.CDI) map[string]string {
 	labels := map[string]string{}

pkg/util/util_test.go

@@ -183,3 +183,45 @@ var _ = Describe("Usable Space calculation", func() {
 		Entry("40Gi virtual size, large overhead to be 40Gi if <= 40Gi and 41Gi if > 40Gi", 40*Gi, largeOverhead),
 	)
 })
+
+var _ = Describe("Merge Labels", func() {
+
+	var (
+		someLabels, emptyLabels, existingLabels, expectedMergedLabels map[string]string
+	)
+
+	BeforeEach(func() {
+		someLabels = map[string]string{
+			"label1": "val1",
+			"label2": "val2",
+			"label3": "val3",
+		}
+		emptyLabels = make(map[string]string)
+		existingLabels = map[string]string{
+			"label4": "val4",
+			"label5": "val5",
+		}
+		expectedMergedLabels = map[string]string{
+			"label1": "val1",
+			"label2": "val2",
+			"label3": "val3",
+			"label4": "val4",
+			"label5": "val5",
+		}
+	})
+
+	DescribeTable("Should properly merge labels", func(original, merged, expected map[string]string) {
+		// copies entries from original to merged
+		MergeLabels(original, merged)
+		Expect(merged).To(HaveLen(len(expected)))
+		for key, val := range merged {
+			Expect(val).To(Equal(expected[key]))
+		}
+	},
+		Entry("original is empty", emptyLabels, someLabels, someLabels),
+		Entry("original has values", someLabels, existingLabels, expectedMergedLabels),
+		Entry("original empty, adding empty", emptyLabels, emptyLabels, emptyLabels),
+		Entry("original has values, adding empty", someLabels, emptyLabels, someLabels),
+	)
+
+})

tests/import_test.go

@@ -2122,6 +2122,55 @@ var _ = Describe("Containerdisk envs to PVC labels", func() {
 	)
 })
 
+var _ = Describe("Propagate DV Labels to Importer Pod", func() {
+	f := framework.NewFramework(namespacePrefix)
+
+	const (
+		testKubevirtKey    = "test.kubevirt.io/test"
+		testKubevirtValue  = "true"
+		testNonKubevirtKey = "testLabel"
+		testNonKubevirtVal = "none"
+	)
+
+	DescribeTable("Import pod should inherit any labels from Data Volume", func(usePopulator string) {
+
+		dataVolume := utils.NewDataVolumeWithHTTPImport("label-test", "100Mi", fmt.Sprintf(utils.TinyCoreIsoURL, f.CdiInstallNs))
+		dataVolume.Annotations[controller.AnnImmediateBinding] = "true"
+		dataVolume.Annotations[controller.AnnPodRetainAfterCompletion] = "true"
+		dataVolume.Annotations[controller.AnnUsePopulator] = usePopulator
+
+		dataVolume.Labels = map[string]string{
+			testKubevirtKey:    testKubevirtValue,
+			testNonKubevirtKey: testNonKubevirtVal,
+		}
+
+		By(fmt.Sprintf("Create new datavolume %s", dataVolume.Name))
+		dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dataVolume)
+		Expect(err).ToNot(HaveOccurred())
+
+		By("Verify pvc was created")
+		_, err = utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name)
+		Expect(err).ToNot(HaveOccurred())
+
+		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 pod")
+		importer, err := utils.FindPodByPrefix(f.K8sClient, dataVolume.Namespace, common.ImporterPodName, common.CDILabelSelector)
+		Expect(err).ToNot(HaveOccurred())
+
+		By("Check labels were appended")
+		importLabels := importer.GetLabels()
+		Expect(importLabels).Should(HaveKeyWithValue(testKubevirtKey, testKubevirtValue))
+		Expect(importLabels).Should(HaveKeyWithValue(testNonKubevirtKey, testNonKubevirtVal))
+
+	},
+		Entry("With Populators", "true"),
+		Entry("Without Populators", "false"),
+	)
+})
+
 var _ = Describe("pull image failure", func() {
 	var (
 		f = framework.NewFramework(namespacePrefix)