<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The VM import handoff sets the transferred `PersistentVolume` to `Retain` so the data survives the swap from the Forklift-populated claim to the VMDisk's claim, and then never gives the policy back. Every imported disk therefore ends up with `persistentVolumeReclaimPolicy: Retain`, while its StorageClass says `Delete` and every VMDisk created directly carries `Delete`. When the tenant later deletes an imported VMDisk through the normal path, its DataVolume and claim go away, but the volume stays `Released` with its backing storage: an orphaned, billed volume that no tenant can see or remove. #### How it was measured On a lab cluster with a LINSTOR-backed `replicated` class (`reclaimPolicy: Delete`): a 16 GiB VM imported from vSphere produced a VMDisk whose PV was `Retain`, while every non-imported VMDisk's PV in the cluster was `Delete`. Deleting the VMInstance and then the VMDisk removed the DataVolume and the claim and left the PV `Released`, still `Retain`, with its LINSTOR resource in place. Patching that PV back to `Delete` made the CSI provisioner delete both the PV and the LINSTOR resource, which confirms the reclaim policy is the only thing keeping it. #### The fix `Retain` is now held only for the window it protects. A new step, `settleReclaimPolicy`, restores the policy the volume's StorageClass declares (`Delete` when the class names none or no longer exists) once the handoff is provably over, and until then reports "not yet" and touches nothing: - the replacement claim is `Bound` and the PV is `Bound` with a `claimRef` naming that claim by UID, as the PV controller reports it rather than as the controller wrote it, so no release is in flight; - the transferred claim is gone; - the claim's one and only owner is the VMDisk's DataVolume, as a controller reference whose UID matches the live DataVolume (CDI's adoption), so the only deletion that can reach the volume is the VMDisk's own; - the VMDisk exists and carries this task's output markers. `fulfill` runs this step after adopting every disk and before creating the VMInstance and deleting the scaffolding VM, so a VM is reported `Succeeded` only once its volumes carry their class's policy, and an import that fails after this point still leaves disks that clean up like any other. The step is a compare-and-set, so repeated passes and restarts are no-ops once settled. It also runs on the path that recovers outputs whose status was never written, for tasks caught mid-handoff by an upgrade. The comment that called permanent `Retain` "part of the contract" and the matching paragraph in `packages/system/migration-controller/README.md` are updated; the README also says how to fix volumes left on `Retain` by imports completed before this change, since those tasks are terminal and are not reconciled again. In practice a VM now stays in `Creating` for one more requeue (about 20 s) after the swap, while the PV controller rebinds and CDI adopts the claim. #### Tests - `TestSettleRestoresTheClassReclaimPolicy`: a finished handoff gets the class's policy back: `Delete`, `Retain` when the class asks for it, and `Delete` when the class names none or is gone. - `TestSettleIsIdempotent`: a second pass over a settled volume reports settled and does not rewrite the PV. - `TestSettleKeepsRetainWhileTheHandoffIsOpen`: thirteen cases, each breaking one condition (claim not created by this import, claim not bound, PV still `Released`, `claimRef` still on the old claim, `claimRef` on the new claim with a stale UID, transferred claim still present, claim not yet adopted by CDI, an extra owner, a DataVolume owner that is not the controller, an owner reference to a previous DataVolume, DataVolume missing, VMDisk missing, VMDisk not created by this import). Every one keeps `Retain`. - `TestFulfillRestoresTheReclaimPolicyBeforeFinishing`: behavioural regression over `fulfill`. The first pass swaps the volume, keeps `Retain`, creates no VMInstance and keeps the scaffolding VM. After simulating the PV controller and CDI, the second pass restores `Delete`, creates the VMInstance and removes the scaffolding VM. With the new step turned into a no-op, this test fails. - `TestFulfillSettlesReclaimedOutputs`: the recovery path for outputs whose status was never written. With the handoff still open the volume keeps `Retain`; once it is finished the policy goes back to `Delete`. - The existing `adoptVolume` tests still assert `Retain` during the swap. `go test ./internal/migrationcontroller/...` and `make test-controllers` pass, `go vet` is clean, and `make generate` leaves the tree unchanged. ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `internal/migrationcontroller/`, `packages/system/migration-controller/README.md` and a comment in `packages/system/migration-controller/templates/rbac.yaml`, which match no entry of the trigger map; the website's VMware migration page does not describe the reclaim policy, so it needs no change. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(migration): disks imported from VMware no longer keep a Retain reclaim policy after the import. The volume gets its StorageClass policy back once it is bound to its VMDisk, so deleting an imported VMDisk deletes its volume instead of leaving an orphaned Released PersistentVolume. Volumes imported before this fix keep Retain and have to be patched back by hand. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Imported volumes are protected during claim transfers, then returned to their StorageClass reclaim policy after the replacement claim and volume are safely adopted. If the class or policy is unavailable, the policy defaults to `Delete`. * VM imports wait for volume adoption and reclaim-policy settlement before completing, helping prevent premature volume deletion and incomplete imports. Previously unrecorded handoff outputs are also settled before being returned. * **Documentation** * Clarified reclaim-policy behavior during imports and how to restore cleanup behavior for volumes from older imports. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
migration-controller
Reconciles forklift.cozystack.io: VMImportSource connections and the VMImportTask operations that run over them. Konveyor Forklift is the transfer engine underneath; this controller owns the tenant-facing API, drives Forklift's objects, and turns each finished transfer into a Cozystack VMDisk and VMInstance.
Opt-in. Add all three packages to bundles.enabledPackages:
bundles:
enabledPackages:
- cozystack.forklift-operator
- cozystack.forklift
- cozystack.migration-controller
Before the first import
Four checks, each of which otherwise fails late and obscurely. All were hit on a real cluster.
- The storage class must bind
Immediate. AWaitForFirstConsumerclass deadlocks an import: nothing consumes the claim while it is being populated. The task refuses such a class up front rather than hanging, but the cluster default is frequentlyWaitForFirstConsumer, sospec.storageClassusually has to be named explicitly. Check withkubectl get storageclass. - The ESXi host must be reachable, and must not collide with the Service CIDR. See When the cluster cannot reach the ESXi host. This is the one that costs a whole transfer before it announces itself.
- The vCenter account needs its domain.
mtv-migrationis rejected as an incorrect username;mtv-migration@vsphere.localis not. The engine reports it as a credentials failure either way, which reads like the wrong password. - The engine's own certificates must be current. Forklift rotates its serving certificates and updates the secret, but does not restart its pods, so a long-lived deployment can serve a certificate its own published CA no longer matches. The controller verifies that CA and will refuse the connection.
kubectl -n cozy-forklift rollout restart deploy/forklift-controllerresolves it; the same staleness eventually expires the certificate outright.
Platform configuration
One key, and only for VMware:
vmImport:
vddkImage: registry.example.com/vddk:8.0.3
Cozystack neither ships nor mirrors that image: it is built from VMware's proprietary Virtual Disk Development Kit, which we have no licence to redistribute. An operator who holds one builds it, pushes it where the cluster can pull from, and names it here. Only the reference travels to the controller — never a credential.
Leaving it empty is a normal, supported state. A vSphere VMImportSource then reports Ready=False with reason VDDKNotConfigured at the moment it is created, so a tenant learns the path is unavailable at registration rather than watching a transfer die halfway through. Nothing is materialized for such a source — not even its credentials Secret.
What a tenant creates
A connection, reusable across imports:
apiVersion: forklift.cozystack.io/v1alpha1
kind: VMImportSource
metadata:
name: vcenter-prod
namespace: tenant-foo
spec:
type: vsphere
url: https://vcenter.example.com/sdk
credentials:
username: mtv-migration@vsphere.local
password: "..."
caCert: |
-----BEGIN CERTIFICATE-----
...
-----END CERTIFICATE-----
Then an import, which is one-shot:
apiVersion: forklift.cozystack.io/v1alpha1
kind: VMImportTask
metadata:
name: import-web-tier
namespace: tenant-foo
spec:
sourceRef:
name: vcenter-prod
vms:
- id: vm-1234 # managed-object reference, from the vSphere client URL or `govc ls -i`
name: web-01
storageClass: replicated
Deleting the task removes the migration machinery and leaves web-01 and its disks in place: they are ordinary Cozystack objects from the moment they exist, and carry no owner reference back to the task. Deleting the source deregisters the connection — its Forklift Provider and projected Secret go with it, and nothing already imported is touched.
Created VMs start Halted, not running. A freshly imported guest usually needs its network reconfigured, and booting it the instant the transfer finishes can put a second copy of a still-running production machine on the network.
When the cluster cannot reach the ESXi host
Disk data does not travel through vCenter. The VDDK opens its connection straight to the ESXi host holding the VM, at whatever address vCenter advertises for that host — and that address is often one the cluster cannot use.
Check it before the first import, because the failure comes late. The address vCenter advertises must be routable from the worker nodes and, critically, must not fall inside the cluster's Service CIDR: an address in that range is claimed by service routing, the packets never leave the node, and the transfer dies after validation has already passed. Compare the two:
# what vCenter advertises for the host, as the engine sees it
kubectl -n cozy-forklift exec deploy/forklift-controller -c inventory -- \
curl -sk "https://localhost:8443/providers/vsphere/<provider-uid>/hosts"
# the cluster's Service CIDR, via the address of the kubernetes service
kubectl get svc -n default kubernetes -o jsonpath='{.spec.clusterIP}'
A host that is unreachable, or that collides, gets an entry on the source:
spec:
hosts:
- id: host-10 # the host's managed-object id, as the VM's inventory record names it
address: 10.31.0.29 # an address the cluster can actually reach
credentials:
username: root
password: "..."
insecureSkipVerify: true
The credentials are the ESXi host's own account, not the vCenter one: the host authenticates this connection itself rather than honouring the vCenter session, and the engine connection-tests the pair before any transfer starts. That is also why the field takes a full credential rather than a bare address.
Symptom when the override is missing: the task reaches Transferring, sits there, and then fails with an NBD error — nbd_connect_uri: the server has no export named '' — which names neither the address nor the reason.
TLS: caCert or insecureSkipVerify, not a thumbprint
Exactly one of these must be set, and the API rejects the object at admission otherwise.
caCert is the PEM certificate authority that signed the endpoint's certificate. insecureSkipVerify: true turns verification off — convenient in a lab and a poor idea anywhere else, since the credentials above travel over that same connection.
A SHA-1 thumbprint does not work here, and this is worth stating plainly because the engine's own error is misleading. Forklift's vSphere provider validation requires user, password, and either a cacert key or insecureSkipVerify; given a thumbprint instead it marks the provider SecretNotValid with the message "The secret is not valid", which reads like a credentials problem and is not. A thumbprint is what the engine wants for a direct ESXi host connection, a path this version does not expose.
vCenter privileges
Neither Administrator nor a plain read-only role is right. Read-only cannot do the job, and Administrator hands the cluster a credential whose blast radius is the entire vCenter — and that credential lands in a Secret in the tenant's own namespace.
Clone the read-only role, add the privileges Forklift documents, and assign it to a service account created for the migration campaign and disabled afterwards. Scope it to the datacenter being migrated, propagated to children — not the whole vCenter.
Eleven privileges cover a cold migration, and several of them are writes, which is why read-only is not enough:
Datastore.Browse,Datastore.FileManagementSessions.ValidateSessionVirtualMachine.Interact.PowerOff,.PowerOn,.GuestControlVirtualMachine.Provisioning.DiskRandomAccess,.FileRandomAccess,.DiskRandomRead,.GetVmFiles,.PutVmFiles
Warm migration adds VirtualMachine.State.CreateSnapshot and .RemoveSnapshot; encrypted guests need the Cryptographer.* set. Older vendor documentation asks for all fifteen Provisioning privileges — the remainder only matter for template migration, which this path does not do.
Two things about that role are worth knowing before you build it. vCenter silently ignores an unknown privilege, so a typo produces a role that looks correct and is quietly under-privileged; enumerate the valid identifiers with govc role.ls Admin rather than trusting a document. And the identifiers themselves have drifted between versions — what one document calls Cryptographic.Direct access is Cryptographer.Access on vSphere 7.
Note that the VDDK opens NFC connections to the ESXi hosts on TCP 443 and 902, not only to vCenter. That matters more than it sounds: vCenter frequently advertises a host on an address the cluster cannot reach, and the transfer then fails with VixDiskLib_Open ... The server refused connection while everything about the provider still looks healthy. When that address is unusable, list the host under spec.hosts with an address the cluster can reach and its own ESXi credentials — the host authenticates that connection itself rather than honouring the vCenter session.
A related trap on the cluster side: if the cluster's Service CIDR overlaps the ESXi network, the VDDK breaks before anything else does.
The privilege detail above comes from the operational work on the original VMware import implementation, validated against a real vCenter.
Storage
Every disk of a task lands on spec.storageClass, or the cluster default when it is omitted. Either way the class must bind Immediate.
A WaitForFirstConsumer class does not fail an import, it hangs it: nothing consumes the claim while CDI populates it, so the transfer waits forever. The controller checks the binding mode before creating anything and fails the task with a message naming the class. On a stock Cozystack install the default class is local, which is WaitForFirstConsumer — so a task that names no class fails immediately with that explanation. replicated qualifies.
The transferred volume is handed to its VMDisk without being copied: the PersistentVolume is retained and re-bound into the claim the disk expects. Retain is held for the swap only. Between deleting the transferred claim and re-pointing the volume's claimRef, the volume is Released, and under Delete the provisioner would reclaim it on the spot. Once the new claim and the volume both report Bound to each other, the transferred claim is gone, and CDI has adopted the claim into the VMDisk's DataVolume as its sole owner, the controller sets the volume back to the reclaim policy its StorageClass declares (Delete when the class names none or no longer exists). Only then does it create the VMInstance and mark the VM Succeeded, so a task in Creating a little longer than the transfer itself is this step waiting on the PV controller and CDI. From then on an imported disk behaves like any other: deleting the VMDisk removes its DataVolume, garbage collection takes the claim, and the volume and its backing storage go with it.
Imports finished by a controller older than this behaviour left their volumes on Retain for good, so deleting such a VMDisk leaves a Released volume that no tenant can see or remove. Setting persistentVolumeReclaimPolicy back to the class's policy on those volumes, while their VMDisk still exists, lets them clean up like the rest; on one already Released, the same patch makes the provisioner delete it.
Budget roughly twice each disk's size against the tenant quota while an import runs. CDI allocates a prime volume of the target size and a scratch volume of about 106% of it, so a 230 GiB disk occupies around 474 GiB until the transfer finishes. On a tenant near its quota this shows up as the import stalling on exceeded quota rather than as anything resembling a storage error, and it clears on its own once the previous disk releases its transit volumes.
Also expect a second phase that nothing reports. After the download, CDI runs qemu-img convert from scratch onto the target volume — around half an hour per 230 GiB — while the DataVolume's own progress sits frozen at 99.92%, because that counter only ever measured the download. The real percentage is only visible in the importer pod's log.
Timings, for planning
Measured on a real vSphere source during the original implementation work, on a 16 GiB UEFI Linux guest:
| Path | Duration |
|---|---|
| VDDK raw copy (what this version does) | ≈2 min 55 s |
| virt-v2v conversion plus its cross-namespace clone | ≈5 min 46 s |
The factor of two is the second copy, not the conversion. For scale at the other end: a 160 GiB Windows Server OVF expanding to 295 GiB took about an hour and five minutes end to end, with the disk transfer itself running near 80 MiB/s.
Privilege model
Nothing on the transfer path is privileged. With guest conversion off — the only mode this version offers — no Forklift pod moves the bytes at all: Forklift emits a CDI DataVolume with a VDDK source and CDI's own importer does the work, and that pod satisfies the restricted Pod Security Standard. Verified against Forklift v2.11.5 and CDI v1.64.0, and confirmed on a live cluster by importing into a namespace enforcing restricted with no admission denial.
Guest conversion is a separate matter: it needs a node-level seccomp profile, because libguestfs starts passt, which unconditionally creates its own namespaces. It is deliberately not part of this version.
Tenants get read and write on both kinds through the standard aggregation labels, and no access at all to forklift.konveyor.io — the Forklift objects a task builds are internal machinery.
Failure modes worth recognising
A task stuck in Creating with the transfer finished usually means the engine reported success without producing anything. A Plan that already has a failed attempt in its history can report SUCCEEDED on a retry while creating neither the volume nor the VM; the task surfaces this rather than hanging, and the remedy is a new task (which builds a new Plan) rather than retrying the old one.
A transfer whose progress sits at 0 for many minutes is not reported as an error on any of the migration objects. Look at the target namespace's claim events — the usual cause is the VDDK failing to reach an ESXi host, which surfaces there and nowhere else. When it eventually does fail, the message is Unable to connect to vddk data source: nbd_connect_uri: the server has no export named '', which sounds like a missing disk and is almost always an unreachable host address: see When the cluster cannot reach the ESXi host.
A VM that reports SecretNotValid or an authentication failure on a vCenter source is usually the username, not the password: vCenter wants the SSO domain (user@vsphere.local), and rejects a bare account name with the same message it uses for a wrong password.
server-side dry run does not protect you here. kubectl apply --dry-run=server is not honoured by Cozystack's aggregated API: for apps.cozystack.io resources it really creates the object. That applies to the VMInstance and VMDisk an import produces, so do not use it as a safety net when experimenting.
Instance types are not interchangeable after the fact. A VirtualMachineInstance is immutable, so changing instanceType on an imported VM requires deleting the VMI to have it re-created. Some catalog sizes also request hugepages, which a node without them cannot satisfy — the VM then stays Pending with Insufficient hugepages-2Mi rather than failing outright.
Uninstalling
Remove the ForkliftController operand before the operator, or CRD deletion deadlocks on the operand's finalizer with nothing left running to clear it:
kubectl patch forkliftcontroller -n cozy-forklift forklift-controller \
--type merge -p '{"metadata":{"finalizers":null}}'