Check VMI status when detaching, not just VM spec - #184

)

## What this PR does

During node reprovisioning in tenant Kubernetes clusters, the VMI (and
its hot-plug pod) can outlive the parent VirtualMachine. The upstream
kubevirt-csi-driver treats a missing VM as "detach succeeded", so the
hot-plug pod stays alive and keeps the infra storage device exclusively
attached to the source host — every subsequent attach of that volume
fails, on DRBD with `failed to set source device readwrite`. This was
hit repeatedly in production tenant clusters (see
kubevirt/csi-driver#182).

This PR:

- updates the pinned `kubevirt.io/csi-driver` module to the latest
upstream commit;
- backports kubevirt/csi-driver#184 into the controller wrapper: after
the upstream unpublish returns success, the wrapper re-checks
`VMI.status.volumeStatus` and, if the hot-plug is still reported there
(e.g. because the parent VM is already gone), detaches it from the VMI
directly via the `removevolume` subresource and waits until it
disappears.

The backport is self-contained in the wrapper and can be dropped once
kubevirt/csi-driver#184 merges upstream and the module pin is bumped
past it.

Credit to @mattia-eleuteri for the root-cause analysis and the upstream
fix.

### Screenshots

Not a UI change.

### Release note

```release-note
fix(csi): tenant CSI driver no longer leaves orphan hot-plug volumes exclusively attached to the source host when the VMI outlives its deleted VirtualMachine during node reprovisioning
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Enhanced hotplug volume unpublishing with automatic verification to
ensure volumes are fully detached before completion, including retry
logic with up to 2-minute timeout.
* **Dependencies**
  * Updated KubeVirt API and CSI driver dependencies to latest versions.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Upstream treats a missing parent VM as detach success, so when the VMI
outlives its VM during node reprovisioning the hot-plug pod keeps the
infra device exclusively attached to the source host and subsequent
attaches fail. Re-check VMI volume status after upstream unpublish and
detach from the VMI directly. Backport of kubevirt/csi-driver#184.

Co-Authored-By: Claude <[email protected]>
Signed-off-by: Andrei Kvapil <[email protected]>

ControllerUnpublishVolume used to return success as soon as the parent
VirtualMachine was missing, on the assumption that a missing VM meant
the hot-plug was already gone. In practice the VMI (and its hot-plug
pod) can outlive the parent VM during a CAPI/CAPK teardown race, so the
infra-side storage device stays exclusively attached to the source host.
The next attempt to attach the same volume to a different VM then fails
with 'Multi-Attach error' from the attach/detach controller or, on
storage backends that enforce exclusive attachment at the device layer
(e.g. DRBD), with 'failed to set source device readwrite'.

The same divergence happens when virt-api updates VM.spec optimistically
on a removevolume call but virt-handler has not finished unplugging the
QEMU device yet — VM.spec no longer references the volume, but the VMI
status still does.

This commit makes ControllerUnpublishVolume consult both surfaces:

  - EnsureVolumeRemovedVMI (new) reports whether the volume is still in
    VMI.Status.VolumeStatus as a HotplugVolume.
  - The new volumeStillAttached helper combines the existing VM-spec
    check with the new VMI-status check and only short-circuits to
    success when both agree the volume is gone.

removeVolumeFromVm is refactored to fall through to RemoveVolumeFromVMI
when the parent VM is gone but the VMI still reports the hot-plug,
closing the orphan path end-to-end.

Refs: kubevirt#83, kubevirt#119, kubevirt#134.

Signed-off-by: Mattia Eleuteri <[email protected]>

kubevirt/csi-driver cb4770e4e475 adds the flag so the driver can run
against KubeVirt without the deprecated HotplugVolumes gate: with it
off, a volume missing from VM.spec is not unplugged through the
VMI-level removevolume on a VM-owned VMI, which virt-api rejects once
that gate is gone. Bump to it and hand the flag to the upstream
controller service. The flag defaults to true, which keeps the detach
path as it was.

The wrapper's own hot-unplug override goes. It was a backport of
kubevirt/csi-driver#184, and the previous pin already contained that
change: upstream detaches from the VMI when the VM is gone and waits
for the VMI status to drop the volume before returning, so the
wrapper's re-check never found it attached. Keeping it would have
meant a second copy of the new ownership rule behind a branch nothing
reaches.

The bump also brings in the upstream node-service changes merged since
the previous pin: NodeUnpublishVolume now uses CleanupMountPoint
instead of Unmount (kubevirt/csi-driver#378), and device lookup runs
udevadm trigger on block devices without a serial
(kubevirt/csi-driver#363). The image carries no udevadm, so that rescan
only logs an error and the lookup behaves as before.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>

kubevirt/csi-driver cb4770e4e475 adds the flag so the driver can run
against KubeVirt without the deprecated HotplugVolumes gate: with it
off, a volume missing from VM.spec is not unplugged through the
VMI-level removevolume on a VM-owned VMI, which virt-api rejects once
that gate is gone. Bump to it and hand the flag to the upstream
controller service. The flag defaults to true, which keeps the detach
path as it was.

The wrapper's own hot-unplug override goes. It was a backport of
kubevirt/csi-driver#184, and the previous pin already contained that
change: upstream detaches from the VMI when the VM is gone and waits
for the VMI status to drop the volume before returning, so the
wrapper's re-check never found it attached. Keeping it would have
meant a second copy of the new ownership rule behind a branch nothing
reaches.

The bump also brings in the upstream node-service changes merged since
the previous pin: NodeUnpublishVolume now uses CleanupMountPoint
instead of Unmount (kubevirt/csi-driver#378), and device lookup runs
udevadm trigger on block devices without a serial
(kubevirt/csi-driver#363). The image carries no udevadm, so that rescan
only logs an error and the lookup behaves as before.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>

kubevirt/csi-driver cb4770e4e475 adds the flag so the driver can run
against KubeVirt without the deprecated HotplugVolumes gate: with it
off, a volume missing from VM.spec is not unplugged through the
VMI-level removevolume on a VM-owned VMI, which virt-api rejects once
that gate is gone. Bump to it and hand the flag to the upstream
controller service. The flag defaults to true, which keeps the detach
path as it was. cb4770e4e475 is the previous pin plus that one change.

The wrapper's own hot-unplug override goes. It was a backport of
kubevirt/csi-driver#184, and the previous pin already contained that
change: upstream detaches from the VMI when the VM is gone and waits
for the VMI status to drop the volume before returning, so the
wrapper's re-check never found it attached. Keeping it would have
meant a second copy of the new ownership rule behind a branch nothing
reaches.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>

kubevirt/csi-driver cb4770e4e475 adds the flag so the driver can run
against KubeVirt without the deprecated HotplugVolumes gate: with it
off, a volume missing from VM.spec is not unplugged through the
VMI-level removevolume on a VM-owned VMI, which virt-api rejects once
that gate is gone. Bump to it and hand the flag to the upstream
controller service. The flag defaults to true, which keeps the detach
path as it was. cb4770e4e475 is the previous pin plus that one change.

The wrapper's own hot-unplug override goes. It was a backport of
kubevirt/csi-driver#184, and the previous pin already contained that
change: upstream detaches from the VMI when the VM is gone and waits
for the VMI status to drop the volume before returning, so the
wrapper's re-check never found it attached. Keeping it would have
meant a second copy of the new ownership rule behind a branch nothing
reaches.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>

…gate (#4451)

## What this PR does

This lets kubevirt-csi detach volumes without the deprecated KubeVirt
`HotplugVolumes` gate. The default stays as it is, so nothing changes at
runtime yet. Part of #4032.

The driver got a switch upstream for exactly this, and I bumped to it.
kubevirt/csi-driver#382 added `--enable-vmi-hotplug-fallback`, merged as
cb4770e4e475. With it set to false, the driver no longer calls the
VMI-level `removevolume` on a VMI owned by a VM. virt-api rejects that
call once `HotplugVolumes` is off.

Our wrapper now passes the flag to upstream and no longer has its own
hot-unplug path. `unpublishHotplugVolume` was a backport of
kubevirt/csi-driver#184, and the previous pin already contained that
change. Upstream detaches from the VMI when the VM is gone, then waits
for the VMI status to drop the volume before it returns. So the
wrapper's re-check never found the volume attached. Adding the ownership
check there would have been dead code.

Closes #4450

The chart leaves the flag at its default of true and does not render it.
The driver image in the chart is stamped only at release preparation, so
between releases main runs the previous release's binary, which does not
know the flag. Go's `flag` package exits on an unknown flag. Rendering
`=true` would crash-loop the CSI controller for no effect. The right
value depends on the KubeVirt CR, not on the tenant, so the comment next
to `HotplugVolumes` in `kubevirt-cr.yaml` points at the controller
Deployment. The follow-up PR drops the gate and renders the flag as
false.

Nothing changes on the node side. Main already pins f5de10d39556, and
cb4770e4e475 is that commit plus kubevirt/csi-driver#382.

A Go unit test runs an unpublish through the wrapper during a teardown,
with the flag on, off with a VM-owned VMI, and off with an orphaned VMI.
It checks that the flag reaches upstream and that only upstream makes
the VMI-level call. Upstream's own tests cover the ownership rule. A
second test holds that an NFS volume only releases the node from its
CiliumNetworkPolicy and never reaches the upstream hot-unplug. A
helm-unittest pins that the chart renders no such argument. I haven't
built the image or run e2e. Volume attach and detach on a tenant cluster
in e2e is the real check.

### Screenshots

No UI change.

### Downstream repositories

- [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:

### Release note

```release-note
feat(kubernetes): kubevirt-csi supports --enable-vmi-hotplug-fallback, so it can detach volumes without the KubeVirt HotplugVolumes feature gate. The flag stays true for now and behaviour is unchanged.
```

)

removeVolumeFromVm treats NotFound from GetWorkloadManagingVirtualMachine
as "no VM", but then checks vm != nil. A typed client-go Get returns an
empty, non-nil object together with NotFound, so the check passes and
vm.Spec.Template, which is nil, is dereferenced. ControllerUnpublishVolume
panics on every retry for a VMI that outlives its VM, and the volume stays
on the VMI: the case the VMI fallback path was added for (#182, #184).

Reset vm to nil on NotFound. The test mock now returns the empty object a
real client returns, which makes the existing VM-gone tests panic without
the fix.

Signed-off-by: Moep90 <[email protected]>