From f229aeb5ff69b3d89b537b39f547eb6c2bc3d5de Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Tue, 10 Jan 2023 17:54:53 +0000 Subject: [PATCH] dataimportcron: Pass KubeVirt instance type labels to DataVolume and DataSource (#2534) * dataimportcron: Pass KubeVirt instance type labels to DataVolume and DataSource Following on from 4fbcb2d509645752850baa33da05ad67a7897262 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 * 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 Signed-off-by: Lee Yarwood --- pkg/controller/common/util.go | 17 +++++++++ pkg/controller/dataimportcron-controller.go | 31 ++++++++++------ .../dataimportcron-controller_test.go | 36 ++++++++++--------- pkg/controller/datavolume/controller.go | 17 +++++++++ .../datavolume/import-controller_test.go | 23 ++++++++++++ pkg/controller/util.go | 11 ------ 6 files changed, 96 insertions(+), 39 deletions(-) diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index 420415ba1..b5078745d 100644 --- a/pkg/controller/common/util.go +++ b/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 { diff --git a/pkg/controller/dataimportcron-controller.go b/pkg/controller/dataimportcron-controller.go index 95e362548..7f2be5ac9 100644 --- a/pkg/controller/dataimportcron-controller.go +++ b/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) } } diff --git a/pkg/controller/dataimportcron-controller_test.go b/pkg/controller/dataimportcron-controller_test.go index 790c70b71..9580437c4 100644 --- a/pkg/controller/dataimportcron-controller_test.go +++ b/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) }) }) }) diff --git a/pkg/controller/datavolume/controller.go b/pkg/controller/datavolume/controller.go index d101aca17..b6076abb2 100644 --- a/pkg/controller/datavolume/controller.go +++ b/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 { diff --git a/pkg/controller/datavolume/import-controller_test.go b/pkg/controller/datavolume/import-controller_test.go index 2816ab07a..1d9afe3ae 100644 --- a/pkg/controller/datavolume/import-controller_test.go +++ b/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} diff --git a/pkg/controller/util.go b/pkg/controller/util.go index a506d0d78..3c3200581 100644 --- a/pkg/controller/util.go +++ b/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{.*})`) )