Repository navigation
Conversation
The CPU model decides which node selectors KubeVirt puts on the migration target pod: the model label for a named model, the host model and the source node's required features for host-model, and only the vendor for host-passthrough. None of this is part of the LiveMigratable condition, so a VMI can report LiveMigratable True and still fail to migrate because the target pod stays unschedulable. Add a section with the selectors per model, the host-model and host-passthrough pitfalls, and using a named model to migrate between nodes with different CPUs. Signed-off-by: YoungjuKim <[email protected]> Assisted-by: Claude <[email protected]>
kubevirt-prow
Bot
added
the
dco-signoff: yes
kubevirt-prow
Bot
added
the
needs-ok-to-test
|
Hi @fjvbn2003. Thanks for your PR. I'm waiting for a kubevirt member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
dhiller
left a comment
dhiller
left a comment
There was a problem hiding this comment.
@fjvbn2003 sound like a reasonable addition to the user guide, however the details are above me, so I'd ask @jean-edouard or @iholder101 to either take a closer look or forward this further.\
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhiller The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubevirt-prow
Bot
added
the
approved
dhiller
left a comment
dhiller
left a comment
There was a problem hiding this comment.
Caution: AI generated review
Reviewed the added documentation for English wording and technical correctness against KubeVirt v1.9.0. The inline comments cover three factual corrections: the scope of the LiveMigratable CPU check, the destination labels used to determine host-model compatibility, and the timeout's default value and starting point. There is also one non-blocking English wording suggestion. Each comment includes proposed replacement text.
🤖 Assisted by Kilo using model openai/gpt-6.1-sol
| - The `LiveMigratable` condition does not take the CPU into account. A VMI | ||
| can report `LiveMigratable: True` while no other node can run its CPU. |
There was a problem hiding this comment.
[P2] Correctness: narrow this to destination compatibility. The condition does take the CPU into account: calculateLiveMigrationCondition calls isHostModelMigratable, which rejects a host-model VMI when the host CPU model is unavailable, using reason CPUModeNotMigratable (v1.9.0 source, check implementation). What it does not establish is whether another node satisfies the CPU requirements.
| - The `LiveMigratable` condition does not take the CPU into account. A VMI | |
| can report `LiveMigratable: True` while no other node can run its CPU. | |
| - The `LiveMigratable` condition does not verify that another node satisfies | |
| the VMI's CPU model and feature requirements. A VMI can report | |
| `LiveMigratable: True` even when no other node meets those requirements. |
There was a problem hiding this comment.
You're right, thanks. isHostModelMigratable does check the source side: a host-model VMI gets CPUModeNotMigratable when the source node has no host CPU model (vm.go L1255-L1257, L2441-L2451; same on main). It does not check the destination. I applied your wording in 4468901.
I also added one sentence: when no other node has the required labels for a host-model VMI, virt-controller records a NoSuitableNodesForHostModelMigration warning event on the VMI (migration.go L2482-L2516). In my test cluster that event appeared on the VMI in the same second the target pod was created. I can drop the sentence if you'd rather keep this item short.
(Written with help from Claude (AI).)
| VMI started on the source node cannot be migrated there. Compare the | ||
| `host-model-cpu.node.kubevirt.io/*` and | ||
| `host-model-required-features.node.kubevirt.io/*` labels of your nodes to | ||
| see where a VMI can go. |
There was a problem hiding this comment.
[P2] Correctness: compare the migration selectors with the destination's compatibility labels, not the host-model identity and required-feature labels across nodes. The destination can have a different host model and still support the source model. KubeVirt translates the source's host-model and required-feature labels into cpu-model-migration.* and cpu-feature.* selectors (v1.9.0 source). The current instruction can incorrectly rule out suitable destinations. The replacement also makes the referent of "there" explicit.
| VMI started on the source node cannot be migrated there. Compare the | |
| `host-model-cpu.node.kubevirt.io/*` and | |
| `host-model-required-features.node.kubevirt.io/*` labels of your nodes to | |
| see where a VMI can go. | |
| VMI started on the source node cannot be migrated to that other node. | |
| To identify compatible destinations, check that each candidate node has | |
| labels matching the migration target pod's | |
| `cpu-model-migration.node.kubevirt.io/<model>`, | |
| `cpu-feature.node.kubevirt.io/<feature>`, and | |
| `cpu-vendor.node.kubevirt.io/<vendor>` selectors. For a VMI that has | |
| already migrated, use its current pod's CPU selectors rather than | |
| deriving new requirements from its current node's host-model labels. |
There was a problem hiding this comment.
Agreed, the old instruction was wrong. The node-labeller sets cpu-model-migration.node.kubevirt.io/<model> for every model the node knows, not just its own host model (node_labeller.go L258-L260). So a node with a different host model can still be a valid destination. I checked this against the labels I saved from my nodes: every node with a different host model still had the cpu-model-migration label for the other nodes' host model. Applied your wording in 4468901. It also matches the check in alertIfHostModelIsUnschedulable.
(Written with help from Claude (AI).)
| `didn't match Pod's node affinity/selector` scheduling message, and the | ||
| migration fails once the target pod has been unschedulable for 5 minutes. |
There was a problem hiding this comment.
[P3] Correctness: five minutes is the default timeout, and its clock starts at target-pod creation, not when the pod first becomes unschedulable. The controller uses timeSinceCreationSeconds and supports an annotation override (timeout accounting, override handling). Qualifying the default avoids promising a fixed five-minute unschedulable interval.
| `didn't match Pod's node affinity/selector` scheduling message, and the | |
| migration fails once the target pod has been unschedulable for 5 minutes. | |
| `didn't match Pod's node affinity/selector` scheduling message. By | |
| default, the migration fails if the target pod is still unschedulable | |
| five minutes after the pod was created. |
There was a problem hiding this comment.
Agreed. The timeout is the default defaultUnschedulablePendingTimeoutSeconds, and handlePendingPodTimeout measures it from the target pod's CreationTimestamp (L1717-L1758). In my run the pod was created and reported unschedulable in the same second, so the measurement could not tell the two apart. Applied your wording in 4468901.
(Written with help from Claude (AI).)
| The CPU model decides which nodes a VMI can be live migrated to. KubeVirt | ||
| adds node selectors to the migration target pod based on the model: |
There was a problem hiding this comment.
nit: "Determines" is more natural than "decides" for a configuration value. Rephrasing also avoids the awkward "can be live migrated to" construction and clarifies that this paragraph concerns CPU compatibility, not every prerequisite for migration.
| The CPU model decides which nodes a VMI can be live migrated to. KubeVirt | |
| adds node selectors to the migration target pod based on the model: | |
| The CPU model determines CPU compatibility with potential live migration | |
| destinations. KubeVirt adds node selectors to the migration target pod | |
| based on the model: |
There was a problem hiding this comment.
Applied as suggested in 4468901.
(Written with help from Claude (AI).)
Address review feedback on the CPU model and live migration section. LiveMigratable does look at the CPU for host-model: virt-handler reports CPUModeNotMigratable when the source node has no host CPU model. What it does not check is whether another node can take the VMI, so say that. A destination is suitable when its labels match the target pod's cpu-model-migration, cpu-feature and cpu-vendor selectors. The node labeller sets cpu-model-migration for every model the node knows, so a node with a different host model can still be a valid destination. Comparing host-model-cpu labels between nodes is therefore wrong. The five minute limit is the default, and it counts from the creation of the target pod. Also mention the NoSuitableNodesForHostModelMigration warning event that virt-controller records on the VMI in this case. Signed-off-by: YoungjuKim <[email protected]> Assisted-by: Claude <[email protected]>
|
Thanks for the review, @dhiller. I addressed all four comments in 4468901 as a separate commit so the changes are easy to review. I'm happy to squash it before merge. @iholder101 @jean-edouard, could one of you take a look at the CPU model details when you have time? (Written with help from Claude (AI).) |
What this PR does / why we need it:
The user guide introduces
host-modelandhost-passthroughindocs/compute/virtual_hardware.md, but it does not say how the CPU model affects live migration.docs/compute/live_migration.mdhas no CPU item under Limitations either. As a result, ahost-modelVMI (the default when no model is set) can reportLiveMigratable: Trueand then fail to migrate after 5 minutes, and nothing in the docs explains why.This PR adds a "CPU model and live migration" subsection under "CPU model special cases":
host-modelandhost-passthrough,LiveMigratablecondition does not consider the CPU, and what the failure looks like,host-model: same host model name, different required features, and which labels to compare,host-passthrough: only the vendor is checked, so use it only with identical CPUs,Sources (kubevirt v1.9.0,
pkg/virt-controller/watch/migration/migration.go):host-model,prepareNodeSelectorForHostCpuModeladds the selectors.host-modelVMI that has already migrated reuses thecpu-feature/cpu-model-migrationselectors of its current pod.getHostCpuModelFromMapturnshost-model-cpu.node.kubevirt.io/<model>intocpu-model-migration.node.kubevirt.io/<model>and eachhost-model-required-features.node.kubevirt.io/<f>intocpu-feature.node.kubevirt.io/<f>.cpu-vendorselector is added for every model ("Ensure migration happens only between nodes with the same CPU vendor").defaultUnschedulablePendingTimeoutSeconds = 60 * 5.What I measured on a test cluster (KubeVirt v1.9.0, Kubernetes 1.36, two workers on different Intel CPU generations; both nodes report host model
SierraForest):LiveMigratablehost-modelcpu-model-migration.node.kubevirt.io/SierraForest, 27cpu-feature.node.kubevirt.io/*,cpu-vendor.node.kubevirt.io/IntelUnschedulable(didn't match Pod's node affinity/selector). Deleted after 5 min, migration failed. The other node lackedcpu-featurelabels forddpd-uandrfds-no, which were in the source node's required features.host-passthroughcpu-vendor.node.kubevirt.io/IntelSkylake-Client-v4cpu-model.node.kubevirt.io/Skylake-Client-v4,cpu-vendor.node.kubevirt.io/IntelCascadelake-Server-v5(no node has it)cpu-model.node.kubevirt.io/Cascadelake-Server-v5PendingThe nodes in this test are nested VMs. That changes which features the nodes report, but not how KubeVirt builds the selectors from the labels.
I did not build the site locally. The table uses the same Markdown table syntax as other pages (for example
user_workloads/vm_templates.md).Special notes for your reviewer:
I put this in
virtual_hardware.mdand not inlive_migration.mdbecause three open PRs (#1020, #1024, #1026) are editinglive_migration.md. If you would like a short pointer from the Limitations list inlive_migration.md, I can add it after those are merged.Prepared with help from Claude (AI); the measurements come from my own nested test cluster.