Pass DV labels to PVC (#2547)

* Pass DV labels to PVC Signed-off-by: Ido Aharon <iaharon@redhat.com> * Rebase and add utests Signed-off-by: Ido Aharon <iaharon@redhat.com> * Transferring annotations in smart clone and passing dv instead of client Signed-off-by: Ido Aharon <iaharon@redhat.com> * CSI clone and Cloning from snapshot source functional tests Signed-off-by: Ido Aharon <iaharon@redhat.com> * CSI clone and Cloning from snapshot source functional tests Signed-off-by: Ido Aharon <iaharon@redhat.com> * uninitialized map case fixed Signed-off-by: Ido Aharon <iaharon@redhat.com> --------- Signed-off-by: Ido Aharon <iaharon@redhat.com>
kubevirt · Mar 17, 2023 · 184ee03 · 184ee03
1 parent 1e96ca3
commit 184ee03
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 33 deletions.
diff --git a/pkg/controller/datavolume/controller-base.go b/pkg/controller/datavolume/controller-base.go
@@ -982,21 +982,6 @@ func buildHTTPClient() *http.Client {
 	return httpClient
 }
 
-func passDataVolumeInstancetypeLabelstoPVC(dataVolumeLabels, pvcLabels map[string]string) map[string]string {
-	instancetypeLabels := []string{
-		cc.LabelDefaultInstancetype,
-		cc.LabelDefaultInstancetypeKind,
-		cc.LabelDefaultPreference,
-		cc.LabelDefaultPreferenceKind,
-	}
-	for _, label := range instancetypeLabels {
-		if dvLabel, hasLabel := dataVolumeLabels[label]; hasLabel {
-			pvcLabels[label] = dvLabel
-		}
-	}
-	return pvcLabels
-}
-
 // newPersistentVolumeClaim creates a new PVC for the DataVolume resource.
 // It also sets the appropriate OwnerReferences on the resource
 // which allows handleObject to discover the DataVolume resource
@@ -1008,7 +993,9 @@ func (r *ReconcilerBase) newPersistentVolumeClaim(dataVolume *cdiv1.DataVolume,
 	if util.ResolveVolumeMode(targetPvcSpec.VolumeMode) == corev1.PersistentVolumeFilesystem {
 		labels[common.KubePersistentVolumeFillingUpSuppressLabelKey] = common.KubePersistentVolumeFillingUpSuppressLabelValue
 	}
-	labels = passDataVolumeInstancetypeLabelstoPVC(dataVolume.GetLabels(), labels)
+	for k, v := range dataVolume.Labels {
+		labels[k] = v
+	}
 
 	annotations := make(map[string]string)
 	for k, v := range dataVolume.ObjectMeta.Annotations {

diff --git a/pkg/controller/datavolume/import-controller_test.go b/pkg/controller/datavolume/import-controller_test.go
@@ -464,14 +464,16 @@ var _ = Describe("All DataVolume Tests", func() {
 			Expect(pvc.Spec.Resources.Requests.Storage().Value()).To(Equal(expectedSize.Value()))
 		})
 
-		It("Should pass annotation from DV to created a PVC on a DV", func() {
+		It("Should pass annotations and labels from DV to created PVC", func() {
 			dv := NewImportDataVolume("test-dv")
 			dv.SetAnnotations(make(map[string]string))
 			dv.GetAnnotations()["test-ann-1"] = "test-value-1"
 			dv.GetAnnotations()["test-ann-2"] = "test-value-2"
 			dv.GetAnnotations()[AnnSource] = "invalid phase should not copy"
 			dv.GetAnnotations()[AnnPodNetwork] = "data-network"
 			dv.GetAnnotations()[AnnPodSidecarInjection] = "false"
+			dv.SetLabels(make(map[string]string))
+			dv.GetLabels()["test"] = "test-label"
 			reconciler = createImportReconciler(dv)
 			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
 			Expect(err).ToNot(HaveOccurred())
@@ -486,6 +488,7 @@ var _ = Describe("All DataVolume Tests", func() {
 			Expect(pvc.GetAnnotations()[AnnPodNetwork]).To(Equal("data-network"))
 			Expect(pvc.GetAnnotations()[AnnPodSidecarInjection]).To(Equal("false"))
 			Expect(pvc.GetAnnotations()[AnnPriorityClassName]).To(Equal("p0"))
+			Expect(pvc.Labels["test"]).To(Equal("test-label"))
 		})
 
 		It("Should pass annotation from DV with S3 source to created a PVC on a DV", func() {

diff --git a/pkg/controller/datavolume/smart-clone-controller.go b/pkg/controller/datavolume/smart-clone-controller.go
@@ -238,7 +238,8 @@ func (r *SmartCloneReconciler) reconcileSnapshot(log logr.Logger, snapshot *snap
 	if err != nil {
 		return reconcile.Result{}, err
 	}
-	newPvc, err := newPvcFromSnapshot(snapshot.Name, snapshot, targetPvcSpec)
+
+	newPvc, err := newPvcFromSnapshot(dataVolume, snapshot.Name, snapshot, targetPvcSpec)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
@@ -352,7 +353,7 @@ func (r *SmartCloneReconciler) getTargetPVC(dataVolume *cdiv1.DataVolume) (*core
 	return pvc, nil
 }
 
-func newPvcFromSnapshot(name string, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec) (*corev1.PersistentVolumeClaim, error) {
+func newPvcFromSnapshot(dv *cdiv1.DataVolume, name string, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec) (*corev1.PersistentVolumeClaim, error) {
 	targetPvcSpecCopy := targetPvcSpec.DeepCopy()
 	restoreSize := snapshot.Status.RestoreSize
 	if restoreSize == nil {
@@ -369,22 +370,31 @@ func newPvcFromSnapshot(name string, snapshot *snapshotv1.VolumeSnapshot, target
 		common.CDILabelKey:       common.CDILabelValue,
 		common.CDIComponentLabel: common.SmartClonerCDILabel,
 	}
+	for k, v := range dv.Labels {
+		labels[k] = v
+	}
+
+	annotations := map[string]string{
+		AnnSmartCloneRequest:          "true",
+		cc.AnnRunningCondition:        string(corev1.ConditionFalse),
+		cc.AnnRunningConditionMessage: cc.CloneComplete,
+		cc.AnnRunningConditionReason:  "Completed",
+		annSmartCloneSnapshot:         key,
+	}
+	for k, v := range dv.ObjectMeta.Annotations {
+		annotations[k] = v
+	}
+
 	if util.ResolveVolumeMode(targetPvcSpecCopy.VolumeMode) == corev1.PersistentVolumeFilesystem {
 		labels[common.KubePersistentVolumeFillingUpSuppressLabelKey] = common.KubePersistentVolumeFillingUpSuppressLabelValue
 	}
 
 	target := &corev1.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: snapshot.Namespace,
-			Labels:    labels,
-			Annotations: map[string]string{
-				AnnSmartCloneRequest:          "true",
-				cc.AnnRunningCondition:        string(corev1.ConditionFalse),
-				cc.AnnRunningConditionMessage: cc.CloneComplete,
-				cc.AnnRunningConditionReason:  "Completed",
-				annSmartCloneSnapshot:         key,
-			},
+			Name:        name,
+			Namespace:   snapshot.Namespace,
+			Labels:      labels,
+			Annotations: annotations,
 		},
 		Spec: corev1.PersistentVolumeClaimSpec{
 			DataSource: &corev1.TypedLocalObjectReference{

diff --git a/pkg/controller/datavolume/smart-clone-controller_test.go b/pkg/controller/datavolume/smart-clone-controller_test.go
@@ -212,8 +212,10 @@ var _ = Describe("All smart clone tests", func() {
 		It("Should create PVC if snapshot ready", func() {
 			dv := newCloneDataVolume("test-dv")
 			q, _ := resource.ParseQuantity("500Mi")
-			// Set annotation on DV which we can verify on PVC later
-			dv.GetAnnotations()["test"] = "test-value"
+			// Set annotation and label on DV which we can verify on PVC later
+			dv.Annotations["test"] = "test-value"
+			dv.Labels = map[string]string{"test": "test-label"}
+
 			snapshot := createSnapshotVolume(dv.Name, dv.Namespace, nil)
 			snapshot.Spec.Source = snapshotv1.VolumeSnapshotSource{
 				PersistentVolumeClaimName: &[]string{"source"}[0],
@@ -234,8 +236,9 @@ var _ = Describe("All smart clone tests", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(pvc.Labels[common.AppKubernetesVersionLabel]).To(Equal("v0.0.0-tests"))
 			Expect(pvc.Labels[common.KubePersistentVolumeFillingUpSuppressLabelKey]).To(Equal(common.KubePersistentVolumeFillingUpSuppressLabelValue))
+			Expect(pvc.Labels["test"]).To(Equal("test-label"))
 			// Verify PVC's annotation
-			Expect(pvc.GetAnnotations()["test"]).To(Equal("test-value"))
+			Expect(pvc.Annotations["test"]).To(Equal("test-value"))
 			event := <-reconciler.recorder.(*record.FakeRecorder).Events
 			Expect(event).To(ContainSubstring("Creating PVC for smart-clone is in progress"))
 		})

diff --git a/pkg/controller/datavolume/snapshot-clone-controller.go b/pkg/controller/datavolume/snapshot-clone-controller.go
@@ -488,7 +488,7 @@ func (r *SnapshotCloneReconciler) getStorageClassCorrespondingToSnapClass(driver
 }
 
 func (r *SnapshotCloneReconciler) makePvcFromSnapshot(pvcName string, dv *cdiv1.DataVolume, snapshot *snapshotv1.VolumeSnapshot, targetPvcSpec *corev1.PersistentVolumeClaimSpec) (*corev1.PersistentVolumeClaim, error) {
-	newPvc, err := newPvcFromSnapshot(pvcName, snapshot, targetPvcSpec)
+	newPvc, err := newPvcFromSnapshot(dv, pvcName, snapshot, targetPvcSpec)
 	if err != nil {
 		return nil, err
 	}

diff --git a/tests/cloner_test.go b/tests/cloner_test.go
@@ -281,11 +281,13 @@ var _ = Describe("all clone tests", func() {
 			DescribeTable("[test_id:1355]Should clone data across different namespaces", func(targetSize string) {
 				pvcDef := utils.NewPVCDefinition(sourcePVCName, "1Gi", nil, nil)
 				pvcDef.Namespace = f.Namespace.Name
+
 				sourcePvc = f.CreateAndPopulateSourcePVC(pvcDef, sourcePodFillerName, fillCommand+testFile+"; chmod 660 "+testBaseDir+testFile)
 				targetNs, err := f.CreateNamespace(f.NsPrefix, map[string]string{
 					framework.NsPrefixLabel: f.NsPrefix,
 				})
 				Expect(err).NotTo(HaveOccurred())
+
 				f.AddNamespaceToDelete(targetNs)
 				doFileBasedCloneTest(f, pvcDef, targetNs, "target-dv", targetSize)
 			},
@@ -2653,6 +2655,8 @@ var _ = Describe("all clone tests", func() {
 
 			for i = 0; i < repeat; i++ {
 				dataVolume := utils.NewDataVolumeForSnapshotCloningAndStorageSpec(fmt.Sprintf("clone-from-snap-%d", i), size, snapshot.Namespace, snapshot.Name, nil, &volumeMode)
+				dataVolume.Labels = map[string]string{"test-label-1": "test-label-value-1"}
+				dataVolume.Annotations = map[string]string{"test-annotation-1": "test-annotation-value-1"}
 				By(fmt.Sprintf("Create new datavolume %s which will clone from volumesnapshot", dataVolume.Name))
 				dataVolume, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, targetNs.Name, dataVolume)
 				Expect(err).ToNot(HaveOccurred())
@@ -2670,6 +2674,9 @@ var _ = Describe("all clone tests", func() {
 				Expect(ok).To(BeFalse())
 				Expect(pvc.Spec.DataSource.Kind).To(Equal("VolumeSnapshot"))
 				Expect(pvc.Spec.DataSourceRef.Kind).To(Equal("VolumeSnapshot"))
+				// All labels and annotations passed
+				Expect(pvc.Labels["test-label-1"]).To(Equal("test-label-value-1"))
+				Expect(pvc.Annotations["test-annotation-1"]).To(Equal("test-annotation-value-1"))
 			}
 
 			By("Verify MD5 on one of the DVs")
@@ -2895,6 +2902,15 @@ func doFileBasedCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolume
 	}
 	// Create targetPvc in new NS.
 	targetDV := utils.NewCloningDataVolume(targetDv, targetSize[0], srcPVCDef)
+	if targetDV.GetLabels() == nil {
+		targetDV.SetLabels(make(map[string]string))
+	}
+	if targetDV.GetAnnotations() == nil {
+		targetDV.SetAnnotations(make(map[string]string))
+	}
+	targetDV.Labels["test-label-1"] = "test-label-key-1"
+	targetDV.Annotations["test-annotation-1"] = "test-annotation-key-1"
+
 	dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, targetNs.Name, targetDV)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -2915,6 +2931,10 @@ func doFileBasedCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolume
 
 	es := resource.MustParse(targetSize[0])
 	Expect(es.Cmp(*targetPvc.Status.Capacity.Storage()) <= 0).To(BeTrue())
+
+	// All labels and annotations passed
+	Expect(targetPvc.Labels["test-label-1"]).To(Equal("test-label-key-1"))
+	Expect(targetPvc.Annotations["test-annotation-1"]).To(Equal("test-annotation-key-1"))
 }
 
 func doInUseCloneTest(f *framework.Framework, srcPVCDef *v1.PersistentVolumeClaim, targetNs *v1.Namespace, targetDv string) {

diff --git a/tests/datavolume_test.go b/tests/datavolume_test.go
@@ -1161,6 +1161,31 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
 			table.Entry("[test_id:8045]for clone DataVolume", createCloneDataVolume, fillCommand),
 		)
 
+		table.DescribeTable("Should pass all DV labels and annotations to the PVC", func(dvFunc func(string, string, string) *cdiv1.DataVolume, url string) {
+			dataVolume := dvFunc("pass-all-labels-dv", "1Gi", url)
+			dataVolume.Labels = map[string]string{"test-label-1": "test-label-1", "test-label-2": "test-label-2"}
+			dataVolume.Annotations = map[string]string{"test-annotation-1": "test-annotation-1", "test-annotation-2": "test-annotation-2"}
+
+			By(fmt.Sprintf("creating new datavolume %s", dataVolume.Name))
+			dataVolume, err := utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dataVolume)
+			Expect(err).ToNot(HaveOccurred())
+
+			// verify PVC was created
+			By("verifying pvc was created and is Bound")
+			pvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name)
+			Expect(err).ToNot(HaveOccurred())
+
+			// All labels and annotations passed
+			Expect(pvc.Labels["test-label-1"]).To(Equal("test-label-1"))
+			Expect(pvc.Labels["test-label-2"]).To(Equal("test-label-2"))
+			Expect(pvc.Annotations["test-annotation-1"]).To(Equal("test-annotation-1"))
+			Expect(pvc.Annotations["test-annotation-2"]).To(Equal("test-annotation-2"))
+		},
+			table.Entry("for import DataVolume", utils.NewDataVolumeWithHTTPImport, tinyCoreIsoURL()),
+			table.Entry("for upload DataVolume", createUploadDataVolume, tinyCoreIsoURL()),
+			table.Entry("for clone DataVolume", createCloneDataVolume, fillCommand),
+		)
+
 		It("Should handle a pre populated DV", func() {
 			By(fmt.Sprintf("initializing dataVolume marked as prePopulated %s", dataVolumeName))
 			dataVolume := utils.NewDataVolumeWithHTTPImport(dataVolumeName, "1Gi", cirrosURL())