fix(kubevirt-instancetypes): restore persistent EFI/TPM state - #3154
Conversation
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 reverts a previous change that disabled persistent EFI and TPM state for Windows virtual machine preferences. The original change was implemented to avoid potential live-migration issues, but it has been determined that KubeVirt v1.4+ correctly handles live-migration for RWO-Filesystem backend storage by creating and copying the necessary state blobs. Restoring this persistence ensures that in-guest security features like Microsoft UEFI CA and Secure Boot remain functional across VM lifecycle events. 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
|
|
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)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoves a Makefile workaround that stripped persistent TPM and EFI fields during preference generation. Six Windows preference resources now explicitly set ChangesWindows preferredTPM persistence enabled
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request removes a temporary workaround in the kubevirt-instancetypes Makefile that previously stripped persistent EFI and TPM state from the KubeVirt preferences. Consequently, the preferences.yaml template has been updated to restore persistent: true for both preferredTPM and preferredEfi across various configurations. I have no feedback to provide as the changes are straightforward and correctly revert the temporary workaround.
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.
|
Andrei Kvapil (@kvaps) rebase please |
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]>
35e0f00 to
59f035f
Compare
myasnikovdaniil
left a comment
There was a problem hiding this comment.
LGTM — correct revert. On KubeVirt v1.8.4 (what we ship) RWO-Filesystem backend storage live-migrates by copy (kubevirt/kubevirt#12629, v1.4+) and VMPersistentState is GA, so persistent EFI/TPM no longer pins the VM — #3005's premise doesn't hold. Worth closing #3005.
|
Successfully created backport PR for |
What this PR does
Reverts #3006, restoring upstream's persistent EFI/TPM firmware state for the
windows.11/windows.2k22/windows.2k25preferences (and their.virtiovariants).#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
replicatedstorage 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
Summary by CodeRabbit
Bug Fixes
Chores