Allow the usage of AnnPodRetainAfterCompletion with populators

This annotation causes CDI transfer pods (importer, uploader, cloner) to be retained after a successful or failed completion.

This makes debugging and testing easier, as users can get the pod state and logs after completion.

Signed-off-by: Alvaro Romero <alromero@redhat.com>

pkg/controller/datavolume/controller-base.go

@@ -1163,12 +1163,6 @@ func (r *ReconcilerBase) shouldUseCDIPopulator(syncState *dvSyncState) (bool, er
 		return boolUsePopulator, nil
 	}
 	log := r.log.WithValues("DataVolume", dv.Name, "Namespace", dv.Namespace)
-	// currently populators don't support retain pod annotation so don't use populators in that case
-	if retain := dv.Annotations[cc.AnnPodRetainAfterCompletion]; retain == "true" {
-		log.Info("Not using CDI populators, currently we don't support populators with retainAfterCompletion annotation")
-		return false, nil
-	}
-
 	usePopulator, err := storageClassCSIDriverExists(r.client, r.log, syncState.pvcSpec.StorageClassName)
 	if err != nil {
 		return false, err

pkg/controller/datavolume/import-controller_test.go

@@ -1556,17 +1556,20 @@ var _ = Describe("All DataVolume Tests", func() {
 			dv := createDataVolumeWithStorageAPI("test-dv", metav1.NamespaceDefault, httpSource, storageSpec)
 			AddAnnotation(dv, annotation, value)
 
-			reconciler = createImportReconciler()
+			reconciler = createImportReconciler(sc, csiDriver)
 			syncState := dvSyncState{
 				dvMutated: dv,
+				pvcSpec: &corev1.PersistentVolumeClaimSpec{
+					StorageClassName: &scName,
+				},
 			}
 			usePopulator, err := reconciler.shouldUseCDIPopulator(&syncState)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(usePopulator).To(Equal(expected))
 		},
 			Entry("AnnUsePopulator=true return true", AnnUsePopulator, "true", true),
 			Entry("AnnUsePopulator=false return false", AnnUsePopulator, "false", false),
-			Entry("AnnPodRetainAfterCompletion return false", AnnPodRetainAfterCompletion, "true", false),
+			Entry("AnnPodRetainAfterCompletion return true", AnnPodRetainAfterCompletion, "true", true),
 		)
 
 		It("Should return true if storage class has wffc bindingMode and honorWaitForFirstConsumer feature gate is disabled", func() {

pkg/controller/populators/populator-base.go

@@ -176,6 +176,9 @@ func (r *ReconcilerBase) createPVCPrime(pvc *corev1.PersistentVolumeClaim, sourc
 	if waitForFirstConsumer {
 		annotations[cc.AnnSelectedNode] = pvc.Annotations[cc.AnnSelectedNode]
 	}
+	if _, ok := pvc.Annotations[cc.AnnPodRetainAfterCompletion]; ok {
+		annotations[cc.AnnPodRetainAfterCompletion] = pvc.Annotations[cc.AnnPodRetainAfterCompletion]
+	}
 
 	// Assemble PVC' spec
 	pvcPrime := &corev1.PersistentVolumeClaim{

tests/import_proxy_test.go

@@ -22,6 +22,7 @@ import (
 	"kubevirt.io/containerized-data-importer/pkg/common"
 	cont "kubevirt.io/containerized-data-importer/pkg/controller"
 	controller "kubevirt.io/containerized-data-importer/pkg/controller/common"
+	"kubevirt.io/containerized-data-importer/pkg/controller/populators"
 	"kubevirt.io/containerized-data-importer/tests/framework"
 	"kubevirt.io/containerized-data-importer/tests/utils"
 
@@ -129,22 +130,30 @@ var _ = Describe("Import Proxy tests", func() {
 		}, 30*time.Second, time.Second).Should(BeTrue())
 	})
 
-	verifyImportProxyConfigMap := func(pvcName string) {
+	getPVCNameForConfigMap := func(pvc *corev1.PersistentVolumeClaim) string {
+		if pvc.Spec.DataSourceRef != nil {
+			return populators.PVCPrimeName(pvc)
+		}
+		return pvc.Name
+	}
+
+	verifyImportProxyConfigMap := func(pvc *corev1.PersistentVolumeClaim) {
 		By("Verify import proxy ConfigMap copied to the import namespace")
-		trustedCAProxy := cont.GetImportProxyConfigMapName(pvcName)
+		trustedCAProxy := cont.GetImportProxyConfigMapName(getPVCNameForConfigMap(pvc))
 		Eventually(func() error {
 			_, err := f.K8sClient.CoreV1().ConfigMaps(f.Namespace.Name).Get(context.TODO(), trustedCAProxy, metav1.GetOptions{})
 			return err
 		}, time.Second*60, time.Second).Should(BeNil())
 	}
 
-	verifyImportProxyConfigMapIsDeletedOnPodDeletion := func(pvcName string) {
+	verifyImportProxyConfigMapIsDeletedOnPodDeletion := func(pvc *corev1.PersistentVolumeClaim) {
 		By("Verify import proxy ConfigMap is deleted from import namespace on importer pod deletion")
+		pvcName := pvc.Name
 		pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Get(context.TODO(), pvcName, metav1.GetOptions{})
 		Expect(err).ToNot(HaveOccurred())
 		err = utils.DeletePodByName(f.K8sClient, pvc.Annotations[controller.AnnImportPod], f.Namespace.Name, nil)
 		Expect(err).ToNot(HaveOccurred())
-		trustedCAProxy := cont.GetImportProxyConfigMapName(pvcName)
+		trustedCAProxy := cont.GetImportProxyConfigMapName(getPVCNameForConfigMap(pvc))
 		Eventually(func() bool {
 			_, err := f.K8sClient.CoreV1().ConfigMaps(f.Namespace.Name).Get(context.TODO(), trustedCAProxy, metav1.GetOptions{})
 			return k8serrors.IsNotFound(err)
@@ -178,15 +187,15 @@ var _ = Describe("Import Proxy tests", func() {
 			pvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dvName)
 			Expect(err).ToNot(HaveOccurred())
 			f.ForceBindIfWaitForFirstConsumer(pvc)
-			verifyImportProxyConfigMap(dvName)
+			verifyImportProxyConfigMap(pvc)
 			By(fmt.Sprintf("Waiting for datavolume to match phase %s", string(cdiv1.Succeeded)))
 			err = utils.WaitForDataVolumePhase(f, f.Namespace.Name, cdiv1.Succeeded, dv.Name)
 			Expect(err).ToNot(HaveOccurred())
 
 			By("Checking the importer pod information in the proxy log to verify if the requests were proxied")
 			verifyImporterPodInfoInProxyLogs(f, imgURL, args.userAgent, now, args.expected)
 
-			verifyImportProxyConfigMapIsDeletedOnPodDeletion(dvName)
+			verifyImportProxyConfigMapIsDeletedOnPodDeletion(pvc)
 		},
 			Entry("succeed creating import dv with a proxied server (http)", importProxyTestArguments{
 				name:          "dv-import-http-proxy",
@@ -333,15 +342,15 @@ var _ = Describe("Import Proxy tests", func() {
 			pvc, err := utils.WaitForPVC(f.K8sClient, dv.Namespace, dv.Name)
 			Expect(err).ToNot(HaveOccurred())
 			f.ForceBindIfWaitForFirstConsumer(pvc)
-			verifyImportProxyConfigMap(dvName)
+			verifyImportProxyConfigMap(pvc)
 			By(fmt.Sprintf("Waiting for datavolume to match phase %s", string(cdiv1.Succeeded)))
 			err = utils.WaitForDataVolumePhase(f, f.Namespace.Name, cdiv1.Succeeded, dv.Name)
 			Expect(err).ToNot(HaveOccurred())
 
 			By("Checking the importer pod information in the proxy log to verify if the requests were proxied")
 			verifyImporterPodInfoInProxyLogs(f, *dv.Spec.Source.Registry.URL, registryUserAgent, now, BeTrue)
 
-			verifyImportProxyConfigMapIsDeletedOnPodDeletion(dvName)
+			verifyImportProxyConfigMapIsDeletedOnPodDeletion(pvc)
 		},
 			Entry("with http proxy, no auth", false, false),
 			Entry("with http proxy, auth", false, true),
@@ -421,7 +430,9 @@ var _ = Describe("Import Proxy tests", func() {
 				return dvName
 			}, timeout, pollingInterval).ShouldNot(BeEmpty())
 
-			verifyImportProxyConfigMap(dvName)
+			pvc, err := utils.WaitForPVC(f.K8sClient, ns, dvName)
+			Expect(err).ToNot(HaveOccurred())
+			verifyImportProxyConfigMap(pvc)
 
 			By("Wait for DataImportCron UpToDate")
 			Eventually(func() bool {
@@ -436,7 +447,7 @@ var _ = Describe("Import Proxy tests", func() {
 			err = utils.WaitForDataVolumePhase(f, ns, cdiv1.Succeeded, dvName)
 			Expect(err).ToNot(HaveOccurred())
 
-			verifyImportProxyConfigMapIsDeletedOnPodDeletion(dvName)
+			verifyImportProxyConfigMapIsDeletedOnPodDeletion(pvc)
 		},
 			Entry("with http proxy, no auth", false, false),
 			Entry("with http proxy, auth", false, true),