Skip to content

Commit c4239d6

Browse files
authored
Respect pod retain annotation in cross ns host assisted clone (#4050)
May be missing something but an admin might have the relevant RBAC and want to look at the retained pod (for themselves or tenants behalf) Relevant discussion on original PR: https://github.com/kubevirt/containerized-data-importer/pull/2873/changes#r1313295871 Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
1 parent 93e8027 commit c4239d6

2 files changed

Lines changed: 16 additions & 12 deletions

File tree

pkg/controller/populators/clone-populator.go

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -599,20 +599,9 @@ func shouldSkipCleanup(pvc *corev1.PersistentVolumeClaim) bool {
599599
// - AnnPodRetainAfterCompletion annotation is set to true. This means that the user explicitly wants
600600
// to keep the pods.
601601
// - Clone is host-assisted, which is the only clone type with transfer pods worth debugging.
602-
// - Clone occurs in a single namespace, so we avoid retaining pods in a namespace we don't have right to access.
603602
if pvc.Annotations[cc.AnnCloneType] == string(cdiv1.CloneStrategyHostAssisted) &&
604-
pvc.Annotations[cc.AnnPodRetainAfterCompletion] == "true" && !isCrossNamespaceClone(pvc) {
603+
pvc.Annotations[cc.AnnPodRetainAfterCompletion] == "true" {
605604
return true
606605
}
607606
return false
608607
}
609-
610-
func isCrossNamespaceClone(pvc *corev1.PersistentVolumeClaim) bool {
611-
dataSourceNamespace := pvc.Namespace
612-
if ns, ok := pvc.Annotations[AnnDataSourceNamespace]; ok {
613-
dataSourceNamespace = ns
614-
} else if pvc.Spec.DataSourceRef.Namespace != nil {
615-
dataSourceNamespace = *pvc.Spec.DataSourceRef.Namespace
616-
}
617-
return dataSourceNamespace != pvc.Namespace
618-
}

pkg/controller/populators/clone-populator_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,21 @@ var _ = Describe("Clone populator tests", func() {
393393
Expect(pvc.Finalizers).ToNot(ContainElement(cloneFinalizer))
394394
Expect(fp.cleanupCalled).To(BeTrue())
395395
})
396+
397+
It("should skip cleanup for cross namespace host-assisted clone with pod retain annotation", func() {
398+
target := succeededTarget()
399+
target.Annotations[cc.AnnCloneType] = string(cdiv1.CloneStrategyHostAssisted)
400+
target.Annotations[cc.AnnPodRetainAfterCompletion] = "true"
401+
target.Annotations[AnnDataSourceNamespace] = "other-ns"
402+
reconciler := createClonePopulatorReconciler(target)
403+
fp := &fakePlanner{}
404+
reconciler.planner = fp
405+
result, err := reconciler.Reconcile(context.Background(), nn)
406+
isDefaultResult(result, err)
407+
pvc := getTarget(reconciler.client)
408+
Expect(pvc.Finalizers).ToNot(ContainElement(cloneFinalizer))
409+
Expect(fp.cleanupCalled).To(BeFalse())
410+
})
396411
})
397412

398413
// fakePlanner implements Plan interface

0 commit comments

Comments
 (0)