Repository navigation
feat(kubevirt)!: drop HotplugVolumes for declarative volume hotplug - #4453
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (11)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughMigration 59→60 copies eligible hotplug volumes and matching disks from running VMIs into their owning VM templates. The change also updates the KubeVirt feature gates and sets the Kubernetes CSI driver’s VMI hotplug fallback flag to false. ChangesVMI hotplug handling
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Migration59
participant kubectl
participant KubernetesAPI
Migration59->>kubectl: Check KubeVirt CRD and list VMs and VMIs
kubectl->>KubernetesAPI: Read CRD, VM, and VMI resources
KubernetesAPI-->>kubectl: Return resource data
kubectl-->>Migration59: Return resource data
Migration59->>kubectl: Patch eligible VM with resource-version test
kubectl->>KubernetesAPI: Apply JSON patch
KubernetesAPI-->>kubectl: Return patch result
Migration59->>kubectl: Apply version 60 ConfigMap
Merge Risk: 🔵 Low · up to This change moves volume hotplug to the declarative path and backfills hotplugged volumes from running VMIs into their VMs. The migration has not run on a live cluster and no N-1 upgrade has been tested. It can be merged, but operators should know about that gap and the documented edge cases. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The upgrade moves live disk attachments into persistent VM configuration. Failure handling protects against incomplete updates, but ownership identity and concurrent detachment can leave incorrect persistent attachments. A tenant authorization bypass has not been established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (10 skipped: 10 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1204f62 to
94968e3
Compare
45618b0 to
ac6b98b
Compare
IvanHunters
left a comment
There was a problem hiding this comment.
The migration itself checks out. Predicate 59:495 matches KubeVirt v1.9.0 IsDeclarativeHotplugVolume exactly, the copy set is computed from live VM.spec so a re-run or a rebase re-run finds nothing to do, reads go to the API server rather than an informer, and every patch carries a resourceVersion test so a VM that changes between list and patch fails the patch instead of clobbering it. A stopped VM has nothing to copy. The gate flip lines up with the vendored csi driver on both attach and detach: virt-api still accepts the VM-level addvolume/removevolume the driver uses, and after migration 59 no VM-owned VMI has a VMI-only volume left for the two upgrade-order windows to disagree about, so a kubevirt-versus-tenant HR skew produces transient CSI retries, not lost volumes. The red E2E (in-tree) on the head is not this change: grafana-deployment is crashlooping on a 404 from the sandbox plugin mirror, while the KubeVirt, CDI and vm-disk HelmReleases this PR touches all reported UpgradeSucceeded and the install reached 98/98.
What I want before merge is one disclosure. This is a feat!, and there is a behavior change it does not state. A hotplug attached with virtctl addvolume --persist=false on a VM-owned VMI carries the same hotpluggable: true and is indistinguishable in the VMI, so migration 59 writes it into VM.spec as a permanent disk, and the VMI-level ephemeral path stops working once the gate is off. Copying is safer than unplugging it live, so the code is right; the gap is that neither the migration header nor a breaking-change note tells an operator their scratch volume just became persistent and can now only be removed through the VM. A sentence in each closes it.
Two smaller notes, neither blocking. A tenant whose kubernetes HelmRelease is suspended or failed keeps its old csi driver, and a driver predating upstream csi-driver #130 (attach moved to the VM subresource in March 2025) has every attach rejected after the flip, so that is worth a line in the breaking-change note too. And the fail-closed hook is a single list followed by conflict-tested patches, so a busy cluster leans on the Job backoffLimit: 3 to get through. Because this flips a gate on running VMs and the attach/detach data-safety is only verified statically here, I would run an N-1 upgrade through cozystack-pr-test before merging.
| | ($vols | map(.name)) as $have | ||
| | ($disks | map(.name)) as $haveDisks | ||
| | [.spec.volumes[]? | ||
| | select(.dataVolume.hotpluggable == true or .persistentVolumeClaim.hotpluggable == true) |
There was a problem hiding this comment.
[MINOR] feat! does not disclose that ephemeral (--persist=false) VMI hotplugs become persistent and VMI-level ephemeral hotplug stops working
There was a problem hiding this comment.
Added a paragraph to the migration header. It says the copy makes such a volume permanent, and that with the gate off virt-api refuses VMI-level addvolume and removevolume on a VM-owned VMI, so the volume is removed through the VM from then on. The release note says the same now.
74bd4ee to
dc56658
Compare
ac6b98b to
acf41e7
Compare
dc56658 to
1eba94d
Compare
acf41e7 to
142c9a8
Compare
1eba94d to
6ca1ea5
Compare
142c9a8 to
4f55061
Compare
6ca1ea5 to
1448fca
Compare
4f55061 to
12105bd
Compare
|
IvanHunters On the other notes: The suspended or failed tenant case is in the body and the release note now. A driver from before July 2025 attaches through the VMI, so such a tenant can't attach anything until its release reconciles. I left the hook as it is. Each Job retry lists again and skips the VMs already patched, so it fails only if a conflict hits on every attempt. Re-reading just the conflicting VM is listed in the body as a possible improvement. I haven't run the N-1 upgrade. It can't be started from this repo's workflows, and #4463 tracks adding that test. Whether to merge before it exists is up to you. |
1448fca to
bb1b2a6
Compare
12105bd to
9573940
Compare
…otplug Dropping the HotplugVolumes gate lets virt-controller rebuild each running VMI's hotplug volumes from VM.spec, and it removes every hotpluggable VMI volume the VM spec does not list. kubevirt-csi attached tenant volumes at the VMI level until mid-2025 and live migration carries such a VMI across KubeVirt updates, so a tenant node that was never recreated can still hold a mounted disk only its VMI knows about; the first reconcile after the upgrade would unplug it. Migration 59 writes each such volume and its disk into the VM template as they are on the VMI, before the upgrade re-points cozystack-packages and so before the KubeVirt CR and the kubevirt-csi flag change. Hotplug volume changes are a valid live update, so no VM needs a restart. An ephemeral hotplug a user made at the VMI level, such as virtctl addvolume without --persist, has the same shape, because virt-api gives every VMI-level addvolume that shape. It is copied too and becomes permanent: leaving it out would not keep it ephemeral, since the first reconcile would unplug it. Only Running VMIs are considered, matching virt-controller, which reconciles no other; a stopped VM's next start builds its VMI from the VM spec, so its VMI-only volumes are not resurrected. Only the variants with migrations enabled run it. The default variant renders no migration hook, so a cluster there that installed KubeVirt by hand has to check for such volumes itself before upgrading. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
KubeVirt 1.9 deprecates HotplugVolumes in favour of DeclarativeHotplugVolumes, which is Beta and therefore on by default, but virt-controller skips declarative reconciliation for as long as the old gate is on. Keeping it was only needed because kubevirt-csi detached through the VMI-level removevolume, which virt-api accepts on a VM-owned VMI only with the old gate. kubevirt-csi can now skip that call, so its fallback goes off together with the gate. The two live in different HelmReleases and roll out one after the other; in between the driver may issue a VMI-level call virt-api rejects, and the unpublish fails and is retried until both sides agree. The chart now renders --enable-vmi-hotplug-fallback=false, which a kubevirt-csi image built before the flag existed rejects at startup. The driver image is stamped at release preparation, so this needs the rebuilt image that ships in the same release. DeclarativeHotplugVolumes is now listed explicitly. With HotplugVolumes gone it is the only hotplug path, and disabling it through disabledFeatureGates would make virt-api refuse every addvolume, so the chart refuses that render instead. This changes volume hotplug platform-wide. A VM-level addvolume or removevolume now edits VM.spec directly and virt-controller reconciles the VMI hotplug volumes against it, instead of going through VM.status.volumeRequests. On the first reconcile it also removes every hotpluggable VMI volume missing from VM.spec; platform migration 59 copies those into VM.spec beforehand on the variants that run migrations. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
9573940 to
8dbc7bc
Compare
|
IvanHunters I added one more case to the body and the release note. A driver from before July 2025 on a suspended or failing tenant can still attach a volume at the VMI level after migration 59 has run and before the gate flips. The migration never sees that volume, so the first declarative reconcile unplugs it from the running VM. The code is the same, the branch is only rebased onto the updated #4451. |
|
IvanHunters the ephemeral-hotplug disclosure is in the migration 59 header and in the release note at 8dbc7bc, and so is the suspended-tenant driver case. The N-1 upgrade test is tracked in #4463. Could you take another look? |
IvanHunters
left a comment
There was a problem hiding this comment.
Verdict
LGTM with non-blocking notes
The migration I asked for last round is disclosed and the copy path verifies clean; the notes below do not block.
Operational risks
The migration and the gate flip hold up. I re-ran the checks rather than re-reading them: the migration's jq against its own fixtures copies exactly the VMI-only hotplugged volumes into VM.spec and no more, keeps every volume and disk the VM already lists, de-dups a disk the VM already names, and leaves orphan, non-controller-owned, stopped and non-Running VMIs alone. The 7 bats cases pass, and dropping the new Running-phase guard reddens them, so the guard is actually held. helm unittest is green on both charts (kubevirt 34, kubernetes 317), the rendered CR carries DeclarativeHotplugVolumes with HotplugVolumes gone, and the only non-comment change since my last round is that Running guard, which is a safety improvement. CI is green on the head, E2E included.
The disclosure I asked for last round is here: the migration header and the release note both state that an ephemeral VMI-level hotplug (virtctl addvolume --persist=false) is copied into the VM and becomes permanent, and that with the gate off virt-api refuses VMI-level addvolume/removevolume on a VM-owned VMI, so such a volume is managed through the VM from then on. That closes it.
A few non-blocking notes, none of which hold up the merge:
The migration aborts the whole run on the first resourceVersion conflict, leaning on the Job backoffLimit: 3. On a busy fleet a VM that changes between the list and its patch fails the run; the re-run is correct and idempotent, but re-reading just the conflicting VM would avoid the dependence on retries. You flagged this as a possible improvement already.
The rollout skew window is real and disclosed: a tenant whose kubernetes release is suspended or failing keeps an old VMI-level driver, and a volume it attaches between the migration's VMI read and the gate flip is not copied and gets unplugged on the first declarative reconcile. It is in the breaking-change note now, which is the right place for it.
One thing worth a line in the PR body or a backport label: the chart now renders --enable-vmi-hotplug-fallback=false unconditionally, but the pinned kubevirt-csi-driver:v1.6.0 image predates the flag (it is only in #4451), and Go's flag package exits on an unknown flag. The release path rebuilds the image so this is the same documented lag as migrations.image, but a backport of this chart change to a 1.6.x line without #4451's driver commit would crash-loop every tenant's CSI controller.
The one real residual risk is that an N-1 upgrade carrying a live VMI-only volume has never run end to end; #4463 tracks adding that lane. The data-safety path is covered statically and fail-closed, so I am comfortable with LGTM, but I would still run it through cozystack-pr-test before merge if you can. Merge ordering: this is stacked on #4451 and merges after it.
) <!-- 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 This lets the kubevirt-csi controller detach a volume from a VM instance that no VM owns. Right now that call is always denied. Closes #4452. The driver has a fallback for a volume that is still attached to the VMI but is no longer in the VM spec. It calls the VMI-level `removevolume` subresource. The `-kcsi` Role grants only the two VM-level subresources, so the call ends in 403. Nothing else reaches this service account with that grant. Only its own Role and ClusterRole name it as a subject, and the group bindings that cover every service account grant nothing on this subresource. At v1.9.0, `kubevirt.io:default` grants only `kubevirts`, `version` and `guestfs`. `kubevirt.io:edit` and `kubevirt.io:admin` do include the VMI subresource, but they aggregate only into the built-in `edit` and `admin` roles, and no tenant binding gives those to this account. The cozystack tenant roles aggregate on their own labels and don't list it. The fallback is still needed after the flag in #4453 goes to false. The flag only skips VMIs that a VM owns. When a VM is deleted with `propagationPolicy=Orphan`, the KubeVirt VM controller drops its finalizer and leaves the VMI running (`vm.go` at v1.9.0). The tenant node stays up with its volumes attached. When a pod moves off that node, the driver has to detach through the VMI, and on a 403 the hot-plug pod keeps holding the infra volume. Until #4453 lands, the fallback also runs for VM-owned VMIs and gets the same 403. The grant is `update` on `virtualmachineinstances/removevolume` and nothing more. The account can already remove a volume from any VM in the namespace, so it gets no new reach. virt-api also rejects the VMI-level call on a VM-owned VMI once `HotplugVolumes` is off. `virtualmachineinstances/addvolume` stays out because the driver only attaches through the VM. The new helm-unittest pins the Role's `subresources.kubevirt.io` rule to these three subresources with `update`. I haven't reproduced the 403 on a cluster. This shows it on a tenant namespace, before and after: ```bash kubectl auth can-i update virtualmachineinstances.subresources.kubevirt.io --subresource=removevolume \ --namespace <tenant-ns> --as system:serviceaccount:<tenant-ns>:<release>-kcsi ``` Upstream kubevirt/csi-driver has the same gap in `deploy/infra-cluster-service-account.yaml`. ### 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. --> No UI change. ### 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: ### 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(kubernetes): the kubevirt-csi controller can now hot-unplug a volume from a VMI that no VM owns, instead of failing with 403 Forbidden. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Kubernetes CSI volume removal for virtual machine instances, including cases where a volume is not listed in the virtual machine specification. This helps the driver complete volume removal in scenarios where the Kubernetes virtualization API accepts the instance-level request. Existing permissions for adding and removing volumes from virtual machines remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
IvanHunters
left a comment
There was a problem hiding this comment.
Re-reviewed after the rebase onto main. This addresses everything from the last round: the migration header, the PR body, the release-note block and the kubevirt CR comment now all state that an ephemeral virtctl addvolume --persist=false hotplug on a VM-owned VMI becomes permanent after the migration and that VMI-level ephemeral hotplug stops working once the gate is off, and the suspended-tenant / pre-July-2025 driver skew and the disabledFeatureGates render-fail are documented too.
The migration holds up on a fresh look. Its predicate (dataVolume.hotpluggable or persistentVolumeClaim.hotpluggable) is byte-for-byte the removal predicate IsDeclarativeHotplugVolume in KubeVirt v1.9.0's filterHotplugVMIVolumes, the virt-api refusal of VMI-level add/removevolume on a VM-owned VMI is gated on HotplugVolumesEnabled() so it activates exactly when the gate drops, and it runs as a pre-upgrade hook so the copy lands before the flip reaches the cluster. The rebase stamping is clean (merge base at 58, this adds 59 and bumps the target to 60). I ran the migration bats here: 7/7, including the idempotent second run and all four fail-closed abort paths, and the check-runs on the head are green.
Two non-blocking heads-ups. Enabling declarative hotplug means VM.spec.volumes now drives the running VMI, so a vm-instance whose hotplug volume was added out of band (not in .Values.disks) can have it detached live on the next chart reconcile, where before the VMI was left alone; this does not touch CAPK tenant nodes, but it is worth a line next to the ephemeral-hotplug note. And the migration is one list followed by sequential resourceVersion-tested patches, so on a large fleet under status churn it leans on the Job backoffLimit: 3 to get through.
Because the live attach/detach across the flip is only verified statically here, an N-1 upgrade through cozystack-pr-test (the #4463 check) before merge would be the thing that turns this from a careful read into a tested one. Not gating it on that.
What this PR does
This turns off the deprecated KubeVirt
HotplugVolumesgate and moves the platform to declarative volume hotplug. It changes how every volume hotplug on the platform works, so the title carries!. Closes #4032.This PR is stacked on #4451 and has to merge after it. #4451 adds the kubevirt-csi flag this PR flips.
The replacement gate is already on in KubeVirt 1.9, and the old gate is the only thing keeping it off.
DeclarativeHotplugVolumesis Beta at v1.9.0, and Beta gates are on by default. virt-controller skipshandleDeclarativeVolumeHotplugwhileHotplugVolumesis on, so dropping the old gate is what switches the new one on.The kubevirt-csi fallback goes off in the same change. With the old gate off, virt-api rejects the VMI-level
removevolumeon a VM-owned VMI, so the chart now passes--enable-vmi-hotplug-fallback=false. The comments next to the gate and the flag point at each other.This needs the rebuilt kubevirt-csi image. A driver built before #4451 does not know the flag and exits on it at startup. The image is stamped at release preparation and ships in the same release as this change, so only a
make applyfrom main before then would crash-loop the CSI controller.What changes at runtime on an upgrade:
addvolumeandremovevolumecalls now editVM.specdirectly. virt-controller then makes the VMI hotplug volumes match it, instead of going throughVM.status.volumeRequests.VM.spec, so they stay attached.A new platform migration keeps old VMI-only volumes attached. On its first pass virt-controller removes every hotpluggable VMI volume that is missing from
VM.spec. Until July 2025 kubevirt-csi attached volumes at the VMI level (the Role then grantedvirtualmachineinstances/addvolume), and live migration carries such a VMI across KubeVirt updates. Without help, a tenant node not recreated since then would lose a mounted disk on upgrade.Migration 59 copies each such volume and its disk from the VMI into the VM template before anything else changes. Adding a hotplug volume to the VM spec is a valid live update in KubeVirt, so no VM needs a restart. It runs as the platform chart's pre-upgrade hook, before the upgrade re-points
cozystack-packages. The KubeVirt CR and the tenant kubernetes charts both render from that source, so the gate and the flag cannot change before the migration has finished. It only touches VMs that need it, logs each one, and a second run changes nothing.The migration also makes ephemeral hotplugs permanent. A volume a user added at the VMI level, for example with
virtctl addvolumewithout--persistfrom an older virtctl, has the same shape as kubevirt-csi's old attachments, because virt-api builds both the same way. There is nothing reliable to tell them apart by, so both are copied. Leaving the user's volume out would not keep it ephemeral: the first reconcile would unplug it from the running VM. After the upgrade it is part ofVM.specand attaches again on a restart, andvirtctl removevolumedrops it.Ephemeral hotplug stops working on VMs. With the old gate off, virt-api refuses VMI-level
addvolumeandremovevolumeon a VMI that a VM owns (vmiVolumePatchin v1.9.0). A volume on such a VMI is added and removed through the VM from then on. A VMI without a VM is not affected.Only running VMIs are copied. virt-controller reconciles no other VMI, and a stopped VM builds its next VMI from
VM.spec, so copying from a stopped one would bring back volumes the restart was going to drop.Migrations run only on the isp-full, isp-full-generic and isp-hosted variants. KubeVirt comes from the platform only on isp-full and isp-full-generic, so those are covered. On the default variant, where packages are installed by hand, nothing runs the migration.
This optional check lists the VMIs and volumes the migration would copy. It uses the same rule. On the default variant, run it before upgrading and recreate each tenant node it lists:
Empty output means nothing is affected. I checked the query against the migration's test fixtures only. Neither the query nor the migration has run on a live cluster.
#4287 also adds a migration 59. If it merges first, this one moves to 60: the file, the version it stamps,
migrations.targetVersion, the bats file (itsrun_migrationargument and expectedSTAMP) andhack/testdata/migration-59-vmi-hotplug.A VM with a pending hotplug request at upgrade time may also get stuck. virt-controller still processes
VM.status.volumeRequestsbefore the declarative handler, and it applies them through the VMI subresource, which virt-api rejects once the old gate is off. The sync then fails before the VM spec is updated, so nothing clears the request, and every later sync of that VM fails the same way. A retry through the VM subresource edits the spec and clears it, which covers kubevirt-csi's own requests. A one-shot request stays stuck until the VM restarts. I read this from the v1.9.0 code and haven't reproduced it. Waiting until this lists nothing before upgrading narrows the window, but a request made during the rollout can still hit it:The two halves live in different HelmReleases, so there is a short window during the rollout where the driver and the gate disagree. In that window an unpublish fails and is retried until both are updated.
A tenant cluster whose
kubernetesHelmRelease is suspended or failing keeps its old driver for longer. Drivers from before July 2025 attach through the VMI subresource, so on such a tenant every attach is refused until its release reconciles. Such a driver can also attach a volume after the migration has run and before the gate flips. The migration doesn't see that volume, and the first declarative reconcile unplugs it from the running VM.The migration lists the VMs once and patches each one with a
resourceVersiontest, so a VM that changes in between fails the hook Job. The Job hasbackoffLimit: 3, and each retry lists again and skips the VMs already patched. Re-reading only the conflicting VM would avoid the retry; I left that out.The new gate is listed in the CR even though it is on by default. With the old gate gone it is the only hotplug path, and the chart refuses a render that disables it. I found no other user of
HotplugVolumesin the platform or in the downstream repositories.The VMI-level fallback may have been failing already. The kcsi Role has no rule for the VMI-level
removevolume(#4452), so that call likely got 403 with or without the gate. I haven't checked that on a cluster.hack/migration-59-vmi-hotplug-volumes.batsruns the migration against a fake kubectl: a VM that needs the copy, VMs and VMIs that must be left alone (a VMI that is not Running among them), a second run, a cluster without KubeVirt and the fail-closed paths. The helm-unittests for the KubeVirt CR pin the whole gate list, acceptHotplugVolumesindisabledFeatureGatesand refuseDeclarativeHotplugVolumesthere. The CSI test pins the flag at false, and themake updateidempotency test still passes. None of that exercises hotplug itself. The real test is e2e with volume attach and detach on a tenant cluster, including a teardown. No CI lane upgrades from the previous release yet, so an upgrade with a live VMI-only volume is untested; #4463 tracks that test.Screenshots
No UI change.
Downstream repositories
Release note
Summary by CodeRabbit