dataimportcron: Pass KubeVirt instance type labels to DataVolume and DataSource (#2534)

* dataimportcron: Pass KubeVirt instance type labels to DataVolume and DataSource

Following on from 4fbcb2d509 a requirement
has arisen to expose the default instance type metadata previously
exposed as annotations also as labels to allow callers such as the UI to
have simple server side filtering of these resources.

The unreleased feature implementation in KubeVirt has now
switched to labels and so CDI should now do the same and pass through
the appropriate labels to the underlying resources.

Signed-off-by: Lee Yarwood <lyarwood@redhat.com>

* instancetype: Pass instance type labels from DataVolume to PVC

Unlike annotations not all labels are copied from a given DataVolume to
a PVC during an import. This change corrects this for instance type
labels ensuring they are passed down to the underlying PVC.

The associated constants are also moved into pkg/controller/common/util
to allow access from the DataImportCron and DataVolume controllers.

Signed-off-by: Lee Yarwood <lyarwood@redhat.com>

Signed-off-by: Lee Yarwood <lyarwood@redhat.com>

pkg/controller/common/util.go

@@ -238,6 +238,15 @@ const (
 	ClaimLost = "ClaimLost"
 	// NotFound reason const
 	NotFound = "NotFound"
+
+	// LabelDefaultInstancetype provides a default VirtualMachine{ClusterInstancetype,Instancetype} that can be used by a VirtualMachine booting from a given PVC
+	LabelDefaultInstancetype = "instancetype.kubevirt.io/default-instancetype"
+	// LabelDefaultInstancetypeKind provides a default kind of either VirtualMachineClusterInstancetype or VirtualMachineInstancetype
+	LabelDefaultInstancetypeKind = "instancetype.kubevirt.io/default-instancetype-kind"
+	// LabelDefaultPreference provides a default VirtualMachine{ClusterPreference,Preference} that can be used by a VirtualMachine booting from a given PVC
+	LabelDefaultPreference = "instancetype.kubevirt.io/default-preference"
+	// LabelDefaultPreferenceKind provides a default kind of either VirtualMachineClusterPreference or VirtualMachinePreference
+	LabelDefaultPreferenceKind = "instancetype.kubevirt.io/default-preference-kind"
 )
 
 // Size-detection pod error codes
@@ -661,6 +670,14 @@ func AddAnnotation(obj metav1.Object, key, value string) {
 	obj.GetAnnotations()[key] = value
 }
 
+// AddLabel adds a label to an object
+func AddLabel(obj metav1.Object, key, value string) {
+	if obj.GetLabels() == nil {
+		obj.SetLabels(make(map[string]string))
+	}
+	obj.GetLabels()[key] = value
+}
+
 // HandleFailedPod handles pod-creation errors and updates the pod's PVC without providing sensitive information
 func HandleFailedPod(err error, podName string, pvc *v1.PersistentVolumeClaim, recorder record.EventRecorder, c client.Client) error {
 	if err == nil {

pkg/controller/dataimportcron-controller.go

@@ -485,10 +485,11 @@ func (r *DataImportCronReconciler) updateDataSource(ctx context.Context, dataImp
 	}
 	dataSourceCopy := dataSource.DeepCopy()
 	r.setDataImportCronResourceLabels(dataImportCron, dataSource)
-	passCronAnnotationToDataSource(dataImportCron, dataSource, AnnDefaultInstancetype)
-	passCronAnnotationToDataSource(dataImportCron, dataSource, AnnDefaultInstancetypeKind)
-	passCronAnnotationToDataSource(dataImportCron, dataSource, AnnDefaultPreference)
-	passCronAnnotationToDataSource(dataImportCron, dataSource, AnnDefaultPreferenceKind)
+
+	passCronLabelToDataSource(dataImportCron, dataSource, cc.LabelDefaultInstancetype)
+	passCronLabelToDataSource(dataImportCron, dataSource, cc.LabelDefaultInstancetypeKind)
+	passCronLabelToDataSource(dataImportCron, dataSource, cc.LabelDefaultPreference)
+	passCronLabelToDataSource(dataImportCron, dataSource, cc.LabelDefaultPreferenceKind)
 
 	sourcePVC := dataImportCron.Status.LastImportedPVC
 	if sourcePVC != nil {
@@ -925,10 +926,12 @@ func (r *DataImportCronReconciler) newSourceDataVolume(cron *cdiv1.DataImportCro
 	r.setDataImportCronResourceLabels(cron, dv)
 	passCronAnnotationToDv(cron, dv, AnnImmediateBinding)
 	passCronAnnotationToDv(cron, dv, cc.AnnPodRetainAfterCompletion)
-	passCronAnnotationToDv(cron, dv, AnnDefaultInstancetype)
-	passCronAnnotationToDv(cron, dv, AnnDefaultInstancetypeKind)
-	passCronAnnotationToDv(cron, dv, AnnDefaultPreference)
-	passCronAnnotationToDv(cron, dv, AnnDefaultPreferenceKind)
+
+	passCronLabelToDv(cron, dv, cc.LabelDefaultInstancetype)
+	passCronLabelToDv(cron, dv, cc.LabelDefaultInstancetypeKind)
+	passCronLabelToDv(cron, dv, cc.LabelDefaultPreference)
+	passCronLabelToDv(cron, dv, cc.LabelDefaultPreferenceKind)
+
 	return dv
 }
 
@@ -956,15 +959,21 @@ func untagDigestedDockerURL(dockerURL string) string {
 	return dockerURL
 }
 
+func passCronLabelToDv(cron *cdiv1.DataImportCron, dv *cdiv1.DataVolume, ann string) {
+	if val := cron.Labels[ann]; val != "" {
+		cc.AddLabel(dv, ann, val)
+	}
+}
+
 func passCronAnnotationToDv(cron *cdiv1.DataImportCron, dv *cdiv1.DataVolume, ann string) {
 	if val := cron.Annotations[ann]; val != "" {
 		cc.AddAnnotation(dv, ann, val)
 	}
 }
 
-func passCronAnnotationToDataSource(cron *cdiv1.DataImportCron, ds *cdiv1.DataSource, ann string) {
-	if val := cron.Annotations[ann]; val != "" {
-		cc.AddAnnotation(ds, ann, val)
+func passCronLabelToDataSource(cron *cdiv1.DataImportCron, ds *cdiv1.DataSource, ann string) {
+	if val := cron.Labels[ann]; val != "" {
+		cc.AddLabel(ds, ann, val)
 	}
 }
 

pkg/controller/dataimportcron-controller_test.go

@@ -617,13 +617,15 @@ var _ = Describe("All DataImportCron Tests", func() {
 			Entry("has no tag", imageStreamName, 1),
 		)
 
-		It("should pass through defaultInstancetype and defaultPreference annotations to DataVolume and DataSource", func() {
+		It("should pass through defaultInstancetype and defaultPreference metadata to DataVolume and DataSource", func() {
 			cron = newDataImportCron(cronName)
 			cron.Annotations[AnnSourceDesiredDigest] = testDigest
-			cron.Annotations[AnnDefaultInstancetype] = AnnDefaultInstancetype
-			cron.Annotations[AnnDefaultInstancetypeKind] = AnnDefaultInstancetypeKind
-			cron.Annotations[AnnDefaultPreference] = AnnDefaultPreference
-			cron.Annotations[AnnDefaultPreferenceKind] = AnnDefaultPreferenceKind
+
+			cron.Labels = map[string]string{}
+			cron.Labels[cc.LabelDefaultInstancetype] = cc.LabelDefaultInstancetype
+			cron.Labels[cc.LabelDefaultInstancetypeKind] = cc.LabelDefaultInstancetypeKind
+			cron.Labels[cc.LabelDefaultPreference] = cc.LabelDefaultPreference
+			cron.Labels[cc.LabelDefaultPreferenceKind] = cc.LabelDefaultPreferenceKind
 
 			reconciler = createDataImportCronReconciler(cron)
 			_, err := reconciler.Reconcile(context.TODO(), cronReq)
@@ -638,25 +640,25 @@ var _ = Describe("All DataImportCron Tests", func() {
 			dvName := imports[0].DataVolumeName
 			Expect(dvName).ToNot(BeEmpty())
 
-			ExpectInstancetypeAnnotations := func(annotations map[string]string) {
-				Expect(annotations).ToNot(BeEmpty())
-				Expect(annotations).Should(ContainElement(AnnDefaultInstancetype))
-				Expect(annotations[AnnDefaultInstancetype]).Should(Equal(AnnDefaultInstancetype))
-				Expect(annotations).Should(ContainElement(AnnDefaultInstancetypeKind))
-				Expect(annotations[AnnDefaultInstancetypeKind]).Should(Equal(AnnDefaultInstancetypeKind))
-				Expect(annotations).Should(ContainElement(AnnDefaultPreference))
-				Expect(annotations[AnnDefaultPreference]).Should(Equal(AnnDefaultPreference))
-				Expect(annotations).Should(ContainElement(AnnDefaultPreferenceKind))
-				Expect(annotations[AnnDefaultPreferenceKind]).Should(Equal(AnnDefaultPreferenceKind))
+			ExpectInstancetypeLabels := func(labels map[string]string) {
+				Expect(labels).ToNot(BeEmpty())
+				Expect(labels).Should(ContainElement(cc.LabelDefaultInstancetype))
+				Expect(labels[cc.LabelDefaultInstancetype]).Should(Equal(cc.LabelDefaultInstancetype))
+				Expect(labels).Should(ContainElement(cc.LabelDefaultInstancetypeKind))
+				Expect(labels[cc.LabelDefaultInstancetypeKind]).Should(Equal(cc.LabelDefaultInstancetypeKind))
+				Expect(labels).Should(ContainElement(cc.LabelDefaultPreference))
+				Expect(labels[cc.LabelDefaultPreference]).Should(Equal(cc.LabelDefaultPreference))
+				Expect(labels).Should(ContainElement(cc.LabelDefaultPreferenceKind))
+				Expect(labels[cc.LabelDefaultPreferenceKind]).Should(Equal(cc.LabelDefaultPreferenceKind))
 			}
 
 			dv := &cdiv1.DataVolume{}
 			Expect(reconciler.client.Get(context.TODO(), dvKey(dvName), dv)).To(Succeed())
-			ExpectInstancetypeAnnotations(dv.Annotations)
+			ExpectInstancetypeLabels(dv.Labels)
 
 			dataSource = &cdiv1.DataSource{}
 			Expect(reconciler.client.Get(context.TODO(), dataSourceKey(cron), dataSource)).To(Succeed())
-			ExpectInstancetypeAnnotations(dataSource.Annotations)
+			ExpectInstancetypeLabels(dataSource.Labels)
 		})
 	})
 })

pkg/controller/datavolume/controller.go

@@ -718,6 +718,21 @@ 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 the DataVolume resource.
 // It also sets the appropriate OwnerReferences on the resource
 // which allows handleObject to discover the DataVolume resource
@@ -729,6 +744,8 @@ func (r *ReconcilerBase) newPersistentVolumeClaim(dataVolume *cdiv1.DataVolume,
 	if util.ResolveVolumeMode(targetPvcSpec.VolumeMode) == corev1.PersistentVolumeFilesystem {
 		labels[common.KubePersistentVolumeFillingUpSuppressLabelKey] = common.KubePersistentVolumeFillingUpSuppressLabelValue
 	}
+	labels = passDataVolumeInstancetypeLabelstoPVC(dataVolume.GetLabels(), labels)
+
 	annotations := make(map[string]string)
 
 	for k, v := range dataVolume.ObjectMeta.Annotations {

pkg/controller/datavolume/import-controller_test.go

@@ -101,6 +101,29 @@ var _ = Describe("All DataVolume Tests", func() {
 			Expect(pvc.Labels[common.KubePersistentVolumeFillingUpSuppressLabelKey]).To(Equal(common.KubePersistentVolumeFillingUpSuppressLabelValue))
 		})
 
+		It("Should pass instancetype labels from DV to PVC", func() {
+			dv := NewImportDataVolume("test-dv")
+			dv.Labels = map[string]string{}
+			dv.Labels[LabelDefaultInstancetype] = LabelDefaultInstancetype
+			dv.Labels[LabelDefaultInstancetypeKind] = LabelDefaultInstancetypeKind
+			dv.Labels[LabelDefaultPreference] = LabelDefaultPreference
+			dv.Labels[LabelDefaultPreferenceKind] = LabelDefaultPreferenceKind
+
+			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.Name).To(Equal("test-dv"))
+			Expect(pvc.Labels[LabelDefaultInstancetype]).To(Equal(LabelDefaultInstancetype))
+			Expect(pvc.Labels[LabelDefaultInstancetypeKind]).To(Equal(LabelDefaultInstancetypeKind))
+			Expect(pvc.Labels[LabelDefaultPreference]).To(Equal(LabelDefaultPreference))
+			Expect(pvc.Labels[LabelDefaultPreferenceKind]).To(Equal(LabelDefaultPreferenceKind))
+		})
+
 		It("Should set params on a PVC from import DV.PVC", func() {
 			importDataVolume := NewImportDataVolume("test-dv")
 			importDataVolume.Spec.PVC.AccessModes = []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}

pkg/controller/util.go

@@ -77,17 +77,6 @@ const (
 	ClusterWideProxyConfigMapKey = "ca-bundle.crt"
 )
 
-const (
-	// AnnDefaultInstancetype provides a default VirtualMachine{ClusterInstancetype,Instancetype} that can be used by a VirtualMachine booting from a given PVC
-	AnnDefaultInstancetype = "instancetype.kubevirt.io/default-instancetype"
-	// AnnDefaultInstancetypeKind provides a default kind of either VirtualMachineClusterInstancetype or VirtualMachineInstancetype
-	AnnDefaultInstancetypeKind = "instancetype.kubevirt.io/default-instancetype-kind"
-	// AnnDefaultPreference provides a default VirtualMachine{ClusterPreference,Preference} that can be used by a VirtualMachine booting from a given PVC
-	AnnDefaultPreference = "instancetype.kubevirt.io/default-preference"
-	// AnnDefaultPreferenceKind provides a default kind of either VirtualMachineClusterPreference or VirtualMachinePreference
-	AnnDefaultPreferenceKind = "instancetype.kubevirt.io/default-preference-kind"
-)
-
 var (
 	vddkInfoMatch = regexp.MustCompile(`((.*; )|^)VDDK: (?P<info>{.*})`)
 )