From dcf7cfba8a0616cd4931881768057a84fdea960b Mon Sep 17 00:00:00 2001 From: Sibasish Behera Date: Wed, 8 Jan 2025 12:41:41 +0530 Subject: [PATCH] fix: Use storage class from annotation for host-assisted PVC operation modify the `getStorageClassCorrespondingToSnapClass` function to use the storage class from the annotation for host-assisted PVC operations. Fixes: https://github.com/kubevirt/containerized-data-importer/issues/3530 Signed-off-by: Sibasish Behera --- pkg/controller/common/util.go | 3 ++ .../datavolume/snapshot-clone-controller.go | 9 +++- .../snapshot-clone-controller_test.go | 41 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index 8b383a4a34..306ad489ad 100644 --- a/pkg/controller/common/util.go +++ b/pkg/controller/common/util.go @@ -225,6 +225,9 @@ const ( // AnnSourceVolumeMode is the volume mode of the source PVC specified as an annotation on snapshots AnnSourceVolumeMode = AnnAPIGroup + "/storage.import.sourceVolumeMode" + // AnnSnapshotRestoreStorageClass indicates the storage class to use for restoring snapshots. + // This annotation should be added to DataVolumes for the temporary host-assisted source PVC. + AnnSnapshotRestoreStorageClass = AnnAPIGroup + "/snapshot.restore.storageClass" // AnnOpenShiftImageLookup is the annotation for OpenShift image stream lookup AnnOpenShiftImageLookup = "alpha.image.policy.openshift.io/resolve-names" diff --git a/pkg/controller/datavolume/snapshot-clone-controller.go b/pkg/controller/datavolume/snapshot-clone-controller.go index 089ab117a5..6bd2898b72 100644 --- a/pkg/controller/datavolume/snapshot-clone-controller.go +++ b/pkg/controller/datavolume/snapshot-clone-controller.go @@ -418,7 +418,7 @@ func (r *SnapshotCloneReconciler) createTempHostAssistedSourcePvc(dv *cdiv1.Data if err := r.client.Get(context.TODO(), types.NamespacedName{Name: *snapshot.Spec.VolumeSnapshotClassName}, vsc); err != nil { return err } - sc, err := r.getStorageClassCorrespondingToSnapClass(vsc.Driver) + sc, err := r.getStorageClassCorrespondingToSnapClass(dv, vsc.Driver) if err != nil { return err } @@ -450,7 +450,12 @@ func (r *SnapshotCloneReconciler) createTempHostAssistedSourcePvc(dv *cdiv1.Data return nil } -func (r *SnapshotCloneReconciler) getStorageClassCorrespondingToSnapClass(driver string) (string, error) { +func (r *SnapshotCloneReconciler) getStorageClassCorrespondingToSnapClass(dv *cdiv1.DataVolume, driver string) (string, error) { + if sc, found := dv.Annotations[cc.AnnSnapshotRestoreStorageClass]; found { + r.log.V(3).Info("Using storage class from annotation for host-assisted source PVC operation", "storageClass.Name", sc) + return sc, nil + } + matches := []storagev1.StorageClass{} storageClasses := &storagev1.StorageClassList{} diff --git a/pkg/controller/datavolume/snapshot-clone-controller_test.go b/pkg/controller/datavolume/snapshot-clone-controller_test.go index c828ba5487..6ae6aebfbf 100644 --- a/pkg/controller/datavolume/snapshot-clone-controller_test.go +++ b/pkg/controller/datavolume/snapshot-clone-controller_test.go @@ -168,6 +168,47 @@ var _ = Describe("All DataVolume Tests", func() { Expect(err).ToNot(HaveOccurred()) }) + It("Should use the storage class used in annotation AnnSnapshotRestoreStorageClass if present", func() { + dv := newCloneFromSnapshotDataVolume("test-dv") + scName := "testsc" + expectedSnapshotClass := "snap-class" + sc := CreateStorageClassWithProvisioner(scName, map[string]string{ + AnnDefaultStorageClass: "true", + }, map[string]string{}, "csi-plugin") + targetScName := "targetsc" + scSameProvisioner := sc.DeepCopy() + scSameProvisioner.Name = "same-provisioner-as-source-sc" + scUsedInAnnotation := sc.DeepCopy() + scUsedInAnnotation.Name = "sc-used-in-annotation" + dv.Annotations[AnnSnapshotRestoreStorageClass] = scUsedInAnnotation.Name + + tsc := CreateStorageClassWithProvisioner(targetScName, map[string]string{}, map[string]string{}, "another-csi-plugin") + sp := createStorageProfile(scName, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) + sp2 := createStorageProfile(targetScName, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) + sp3 := createStorageProfile(scSameProvisioner.Name, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) + sp4 := createStorageProfile(scUsedInAnnotation.Name, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, BlockMode) + dv.Spec.PVC.StorageClassName = &targetScName + snapshot := createSnapshotInVolumeSnapshotClass("test-snap", metav1.NamespaceDefault, &expectedSnapshotClass, nil, nil, true) + snapClass := createSnapshotClass(expectedSnapshotClass, nil, "csi-plugin") + reconciler = createSnapshotCloneReconciler(sc, scSameProvisioner, tsc, sp, sp2, sp3, sp4, dv, snapshot, snapClass, createDefaultVolumeSnapshotContent(), createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd()) + _, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}}) + Expect(err).ToNot(HaveOccurred()) + By("Verifying that temp host assisted source PVC is being created") + pvc := &corev1.PersistentVolumeClaim{} + tempPvcName := getTempHostAssistedSourcePvcName(dv) + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Namespace: snapshot.Namespace, Name: tempPvcName}, pvc) + Expect(err).ToNot(HaveOccurred()) + Expect(pvc.Labels[common.CDIComponentLabel]).To(Equal("cdi-clone-from-snapshot-source-host-assisted-fallback-pvc")) + Expect(pvc.Labels[common.AppKubernetesPartOfLabel]).To(Equal("testing")) + Expect(*pvc.Spec.StorageClassName).To(Equal(scUsedInAnnotation.Name)) + By("Verifying that target host assisted PVC is being created") + err = reconciler.client.Get(context.TODO(), types.NamespacedName{Namespace: dv.Namespace, Name: dv.Name}, pvc) + Expect(err).ToNot(HaveOccurred()) + Expect(pvc.Labels[common.AppKubernetesPartOfLabel]).To(Equal("testing")) + Expect(pvc.Annotations[AnnCloneRequest]).To(Equal(fmt.Sprintf("%s/%s", snapshot.Namespace, tempPvcName))) + Expect(*pvc.Spec.StorageClassName).To(Equal(targetScName)) + }) + It("Should clean up host assisted source temp PVC when done", func() { dv := newCloneFromSnapshotDataVolume("test-dv") dv.Status.Phase = cdiv1.Succeeded