Skip to content

vm-adoption-controller: cross-namespace adoption leaks disk copies and leaves the source VM running #3924

Closed

Description

@kvaps

Context

Follow-up from the review of #1982 (VMware VM import via Forklift). The catastrophic data-destruction paths and the wrong release-namespace stamp were fixed in that PR; this issue tracks the two cross-namespace clone-lifecycle findings deliberately deferred from it, because both need a reconcile-loop lifecycle change rather than a spot fix.

Both are on the cross-namespace (virt-v2v) adoption path in packages/system/vm-adoption-controller/images/controller/main.go.

1. Cross-namespace adoption leaks disk copies

The virt-v2v path clones the imported PVC into <vmDiskName>-src (a DataVolume), and the VMDisk then clones that into vm-disk-<name>. Nothing ever deletes the -src intermediate, and the original PVC in the conversion namespace also stays (the source VM is only labeled, never deleted). Net result: three full copies of every migrated disk on replicated storage — one referenced and two permanent garbage with no ownerReference for GC. The controller's ClusterRole also grants no delete on datavolumes/PVCs, so it could not clean them up as it stands.

2. Cross-namespace adoption leaves the source VM running

On the cross-namespace branch labelVMAsAdopted only labels the Forklift VM — it never halts or deletes it. The source VM keeps its inherited runStrategy (default Always) and keeps running off the original disk next to the tenant clone, so both boot with the same guest identity and MAC. The original PVC backing it is never reclaimed either.

Proposed fix

Track the vm-disk-<name> clone to Succeeded, then delete the intermediate -src DataVolume, the original PVC, and the source VM. This requires:

  • Keeping the import in the reconcile loop until the clone completes. Today the cross-namespace source VM is labeled adopted, so it drops out of getForkliftVMs and the controller never revisits the import — there is no place to run the deferred cleanup. The loop needs to revisit an in-flight cross-namespace adoption until its clone is done.
  • Halting the source VM early (safe without completion tracking, and it releases the RWO source PVC so the CDI clone is not blocked by a running guest), then deleting it once the clone is confirmed.
  • A delete verb on cdi.kubevirt.io/datavolumes and core persistentvolumeclaims in the ClusterRole.

Care is needed so the source PVC (the clone source) is not removed before the clone completes; the same-namespace/raw-copy path already avoids the double-copy via an early return and is not affected.

Activity

  1. lexfrei commented on Sep 2, 2026

    @lexfrei
    Contributor

    PR #1982 never merged and packages/system/vm-adoption-controller does not exist on main, so nothing here describes shipped code. The VMware import is being redone in #3931 with a different controller; I carried the two lifecycle findings there as review points. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    triage/needs-triageIndicates an issue needs triage by a maintainer

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions