CherryPicked: [v2.10] fix: support vSphere template cloning with configurable resource pool - #587
Open
manage-repositories-app[bot] wants to merge 1 commit into
Open
Conversation
…#583) * fix: support vSphere template cloning with configurable resource pool Add priority-based resource pool selection for VM cloning operations to support cloning from vSphere templates. Resolves DVS port conflicts by enabling template usage as clone sources. Priority order: 1. Configured copyoffload.resource_pool (explicit override) 2. Target ESXi host's pool (automatic compatibility) 3. Source VM's pool (preserve original location) 4. Cluster-wide pool search (fallback for templates) Related: MTV-5787 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: query vSphere directly for template networks when not in inventory Forklift inventory only lists VMs, not templates. When a VM is converted to a template, it no longer appears in the inventory VM list, causing network mapping queries to fail. Add fallback logic to query vSphere API directly for template network information when inventory lookup fails. This allows copy-offload tests to work with template-based clone sources. Changes: - get_vm_or_template_networks: Try inventory first, fall back to vSphere API - _get_networks_from_vsphere: Extract network names from VM/template devices Related: MTV-5787 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: query networks from cloned VMs not original template names The multus_network_name fixture was querying networks using the original template names from class_plan_config instead of the cloned VM names from prepared_plan. This caused Forklift inventory lookups to fail because: 1. Template is cloned to new VM (e.g., auto-zzsx-xcopy-template-test-...) 2. Fixture queried original template name (xcopy-template-test) 3. Template not in Forklift VM inventory 4. Fallback to vSphere API queried wrong object Fix: Use prepared_plan which contains the cloned VM names that exist in Forklift inventory, eliminating the need for template fallback logic. Related: MTV-5787 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: address CodeRabbit review feedback Address all high and medium priority review comments: 1. Exception Handling (HIGH): - Distinguish "VM not found" from "networks not found" errors - Only fall back to vSphere when inventory lookup fails (template case) - Propagate network mapping failures immediately 2. Network Query Coverage (HIGH): - Reuse _get_network_name_from_device() for complete backing type support - Properly handle DVS portgroups via portgroupKey resolution - Remove duplicate network extraction logic 3. Resource Pool Compatibility (HIGH): - Add datastore compatibility check in cluster fallback logic - Verify target datastore is accessible from compute resource - Prevent incompatible pool selection 4. Resource Cleanup (MEDIUM): - Add try/finally for container view cleanup - Follow established pattern from get_obj method 5. Documentation (Required): - Document resource_pool configuration in .providers.json.example - Explain priority order and automatic selection behavior - Clarify when explicit configuration is needed vs automatic Related: MTV-5787 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features:
🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
@tshefi — AI automatically resolved merge conflicts for this cherry-pick. Please review the changes carefully and verify correctness before merging. |
Collaborator
|
/lgtm |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick from
mainbranch, original PR: #583, PR owner: tshefi