Repository navigation
fix(kubevirt-instancetypes): drop persistent EFI/TPM to unblock live-migration - #3006
Conversation
…migration TEMPORARY revert. Restoring persistent EFI/TPM state (4cc60e3) makes windows.11/2k22/2k25 VMs get a backend-storage PVC that is ReadWriteOnce on the default StorageClass, which pins the VM to its node and blocks live-migration and node drains (with evictionStrategy: LiveMigrate this can stall cluster upgrades). See #3005. Strip persistent EFI/TPM from the preferences again, but with `yq del` so an emptied preferredTPM stays `{}` (the v1.8 CRD rejects null) — the bug the original sed strip caused. Secure Boot and the vTPM stay present, just non-persistent. Stopgap until vm-state can request an RWX backend (RWX-Block via the upstream KubeVirt change, or an RWX-Filesystem vmStateStorageClass). Co-Authored-By: Claude <[email protected]> Signed-off-by: Andrei Kvapil <[email protected]>
Andrei Kvapil (kvaps)
added
kind/bug
github-actions
Bot
added
area/uncategorized
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ChangesRemove persistent TPM/EFI from Windows VM preferences
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a temporary fix to unblock live-migration for Windows VMs by disabling persistent EFI and TPM states. Currently, these features trigger the creation of RWO PVCs that pin VMs to specific nodes, hindering cluster maintenance operations like node drains and upgrades. The changes ensure that while Secure Boot and vTPM remain functional, they are no longer persistent, pending a future solution that supports RWX storage for VM state. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request temporarily removes persistent EFI and TPM state from the KubeVirt preferences to prevent the creation of RWO backend-storage PVCs, which can block live-migration and node draining. The Makefile has been updated to strip these fields using yq, and the generated preferences.yaml has been updated accordingly. Feedback is provided to simplify the yq command in the Makefile by passing multiple comma-separated paths to a single del operator instead of chaining them.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # which pins the VM to its node and blocks live-migration / drain. Use `del` (not | ||
| # `sed -d`) so an emptied preferredTPM stays `{}` rather than null, which the | ||
| # KubeVirt v1.8 CRD structural schema rejects. Revert once vm-state can request RWX. | ||
| yq -i 'del(.spec.firmware.preferredEfi.persistent) | del(.spec.devices.preferredTPM.persistent)' templates/preferences.yaml |
There was a problem hiding this comment.
In yq (v4+), you can pass multiple comma-separated paths to the del operator instead of chaining them with |. This is slightly cleaner and more idiomatic.
yq -i 'del(.spec.firmware.preferredEfi.persistent, .spec.devices.preferredTPM.persistent)' templates/preferences.yaml
|
Successfully created backport PR for |
Reverts #3006. Dropping persistent EFI/TPM was motivated by a live-migration concern that rested on an outdated assumption — that an RWO Filesystem backend-storage PVC pins the VM to its node. KubeVirt has migrated RWO-Filesystem backend storage since v1.4 (kubevirt/kubevirt#12629): on migration it creates a fresh target state PVC and copies the small state blob, so persistent-EFI/TPM VMs on the default replicated storage live-migrate fine (verified: the VM reports LiveMigratable=True and the copy-on-target PVC is created on migration). Restore upstream's persistent firmware state for the windows.* preferences. Assisted-By: Claude <[email protected]> Signed-off-by: Andrei Kvapil <[email protected]>
## What this PR does Reverts #3006, restoring upstream's persistent EFI/TPM firmware state for the `windows.11` / `windows.2k22` / `windows.2k25` preferences (and their `.virtio` variants). #3006 dropped persistent EFI/TPM to "unblock live-migration", on the assumption that an RWO Filesystem backend-storage PVC pins the VM to its node. That assumption was outdated: KubeVirt has live-migrated RWO-Filesystem backend storage since v1.4 (kubevirt/kubevirt#12629) — on migration it creates a fresh target state PVC and copies the small state blob. So persistent-EFI/TPM VMs on the default `replicated` storage live-migrate fine; the regression described in #3005 was a misdiagnosis. Verified on a KubeVirt v1.8.2 cluster: a persistent-EFI VM with an RWO Filesystem backend PVC reports `LiveMigratable=True`, and the copy-on-target backend PVC is created when a migration starts. KubeVirt maintainers confirmed the same upstream. This restores persistent firmware state so in-guest Microsoft UEFI CA / Secure Boot enrollment survives reboots and migrations. ### Release note ```release-note fix(kubevirt-instancetypes): restore persistent EFI/TPM state for Windows preferences — RWO-Filesystem backend storage already live-migrates (KubeVirt v1.4+), so persistence no longer needs to be stripped ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated Windows instance type preferences to keep TPM persistence enabled, improving compatibility and reducing storage-related issues. * Ensured firmware and TPM persistence settings are preserved during instance type preference generation for more reliable outcomes. * **Chores** * Streamlined the preference generation step so instance type templates are produced consistently without removing persistence configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
What this PR does
Temporary revert. v1.5.0 restored persistent EFI/TPM state for the
windows.11/windows.2k22/windows.2k25profiles. With persistence on, KubeVirt creates a backend-storage PVC (persistent-state-for-<vm>) which, on the defaultreplicatedStorageClass, is ReadWriteOnce — it pins the VM to its node and blocks live-migration and node drains. WithevictionStrategy: LiveMigratethis can stall cluster upgrades. See #3005.This strips persistent EFI/TPM from the preferences again — but via
yq delinstead of the originalsed -d, so an emptiedpreferredTPMstays{}(the v1.8 CRD rejectsnull, which is the bug the original strip caused). Secure Boot and the vTPM remain present, just non-persistent — Windows VMs still boot; they just don't persist EFI/TPM across reboots.vmStateStorageClass. Once that lands, persistence can be re-enabled with live-migration intact.Screenshots
No UI changes.
Release note
Summary by CodeRabbit