Update evictionStrategy for imported VMs #956
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds documentation for a known issue with imported virtual machines where the eviction strategy field is not configured during import, which prevents live migration. The documentation provides both individual VM fixes and a bulk update command for affected VMs.
Changes:
- Added a new "Eviction strategy is not set" section to the Known Issues documentation
- Provided kubectl commands to fix individual VMs and bulk update all affected VMs
- Updated documentation for versions 1.6 and 1.7
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| versioned_docs/version-v1.7/advanced/addons/vmimport.md | Added eviction strategy known issue and remediation commands for v1.7 |
| versioned_docs/version-v1.6/advanced/addons/vmimport.md | Added eviction strategy known issue and remediation commands for v1.6 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| To configure all incorrectly configured virtual machines at once, please use the following command: | ||
|
|
||
| ```shell | ||
| for vm in $(kubectl get VirtualMachine -A -o json | jq -r '.items[] | select(.spec.template.spec.evictionStrategy == null) | "\(.metadata.namespace):\(.metadata.name)"'); do \ | ||
| kubectl patch VirtualMachine ${vm#*:} -n ${vm%:*} --type=merge -p '{"spec":{"template":{"spec":{"evictionStrategy":"LiveMigrateIfPossible"}}}}'; \ | ||
| done | ||
| ``` |
There was a problem hiding this comment.
The documentation should clarify the prerequisites for running the bulk update command. The command requires jq to be installed, which may not be immediately obvious to all users. Consider adding a note that jq is required for the second command, or provide an alternative approach for users who don't have jq installed.
| To configure all incorrectly configured virtual machines at once, please use the following command: | ||
|
|
||
| ```shell | ||
| for vm in $(kubectl get VirtualMachine -A -o json | jq -r '.items[] | select(.spec.template.spec.evictionStrategy == null) | "\(.metadata.namespace):\(.metadata.name)"'); do \ | ||
| kubectl patch VirtualMachine ${vm#*:} -n ${vm%:*} --type=merge -p '{"spec":{"template":{"spec":{"evictionStrategy":"LiveMigrateIfPossible"}}}}'; \ | ||
| done | ||
| ``` |
There was a problem hiding this comment.
The documentation should clarify the prerequisites for running the bulk update command. The command requires jq to be installed, which may not be immediately obvious to all users. Consider adding a note that jq is required for the second command, or provide an alternative approach for users who don't have jq installed.
| }' | ||
| ``` | ||
|
|
||
| To configure all incorrectly configured virtual machines at once, please use the following command: |
There was a problem hiding this comment.
The phrasing "To configure all incorrectly configured virtual machines" is redundant. Consider simplifying to "To configure all affected virtual machines at once" or "To update all virtual machines missing the eviction strategy" for better clarity and conciseness.
| To configure all incorrectly configured virtual machines at once, please use the following command: | |
| To update all virtual machines missing the eviction strategy, run the following command: |
| }' | ||
| ``` | ||
|
|
||
| To configure all incorrectly configured virtual machines at once, please use the following command: |
There was a problem hiding this comment.
The phrasing "To configure all incorrectly configured virtual machines" is redundant. Consider simplifying to "To configure all affected virtual machines at once" or "To update all virtual machines missing the eviction strategy" for better clarity and conciseness.
| To configure all incorrectly configured virtual machines at once, please use the following command: | |
| To update all virtual machines missing the eviction strategy at once, use the following command: |
| - In the `VirtualMachineImport` CRD spec, set the `forcePowerOff` field to `true`. | ||
| - Install VMware Tools or [open-vm-tools](https://knowledge.broadcom.com/external/article?legacyId=2073803). | ||
|
|
||
| ##### Eviction strategy is not set |
There was a problem hiding this comment.
The heading capitalization is inconsistent with other headings in the same section. Other headings like "Source Virtual Machine Name Is Not RFC1123-Compliant" and "VMware-Based Virtual Machine Without VMware Tools Is Not Migrated" use title case with major words capitalized. This heading should be "Eviction Strategy Is Not Set" for consistency.
| ##### Eviction strategy is not set | |
| ##### Eviction Strategy Is Not Set |
| - In the `VirtualMachineImport` CRD spec, set the `forcePowerOff` field to `true`. | ||
| - Install VMware Tools or [open-vm-tools](https://knowledge.broadcom.com/external/article?legacyId=2073803). | ||
|
|
||
| ##### Eviction strategy is not set |
There was a problem hiding this comment.
The heading capitalization is inconsistent with other headings in the same section. Other headings like "Source Virtual Machine Name Is Not RFC1123-Compliant" and "VMware-Based Virtual Machine Without VMware Tools Is Not Migrated" use title case with major words capitalized. This heading should be "Eviction Strategy Is Not Set" for consistency.
| ##### Eviction strategy is not set | |
| ##### Eviction Strategy Is Not Set |
| When importing virtual machines, the eviction strategy field is not configured, which subsequently prevents live migration of the virtual machine. | ||
| To fix this, please run the following command: |
There was a problem hiding this comment.
The phrase "To fix this, please run the following command:" could be more consistent with the documentation style used in the previous known issue. Consider using a more formal and consistent phrasing such as "To address the issue, run the following command:" which matches the style of "To address the issue, perform one of the following workarounds:" used in the VMware Tools issue above.
| When importing virtual machines, the eviction strategy field is not configured, which subsequently prevents live migration of the virtual machine. | ||
| To fix this, please run the following command: |
There was a problem hiding this comment.
The phrase "To fix this, please run the following command:" could be more consistent with the documentation style used in the previous known issue. Consider using a more formal and consistent phrasing such as "To address the issue, run the following command:" which matches the style of "To address the issue, perform one of the following workarounds:" used in the VMware Tools issue above.
|
The versions 1.6 and 1.7 have been updated because they are currently considered relevant. All older versions that support the VM Importer plugin are also affected, but are no longer supported anyway. Related to: harvester/harvester#9849 Signed-off-by: Volker Theile <vtheile@suse.com>
votdev
force-pushed
the
issue_9849_eviction_strategy
branch
from
568b15f to
111caee
Compare
1 participant
The versions 1.6 and 1.7 have been updated because they are currently considered relevant. All older versions that support the VM Importer plugin are also affected, but are no longer supported anyway.
Problem:
Solution:
Related Issue(s):
harvester/harvester#9849
Test plan:
Additional documentation or context