Repository navigation
KVM: allow cluster-wide storage pools for VM deployment from volumes - #14362
Draft
weizhouapache wants to merge 1 commit into
Draft
weizhouapache wants to merge 1 commit into
weizhouapache wants to merge 1 commit into
Conversation
Allow both ZONE and CLUSTER scoped storage pools for VM deployment from existing volumes and snapshots. Previously only ZONE-scoped pools were accepted, restricting deployment flexibility for cluster-wide storage configurations like NFS and RBD pools. Update validation in UserVmManagerImpl to accept CLUSTER scope alongside ZONE scope, and update error messages accordingly. Update test_vm_lifecycle_with_snapshot_or_volume.py smoke tests to: - Accept both ZONE and CLUSTER scoped storage pools - Add RBD-specific handling for snapshot lifecycle (snapshots cannot outlive source volumes on RBD with snapshot.backup.to.secondary=false) - Skip unsupported test scenarios on RBD backends - Fix VM creation to use service configs instead of hardcoded values - Add reconnect logic to SSH client initialization - Remove unused uuid import
boring-cyborg
Bot
added
component:integration-test
Python
14 tasks
Member
Author
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14362 +/- ##
============================================
- Coverage 19.91% 19.90% -0.01%
+ Complexity 20198 20196 -2
============================================
Files 6373 6373
Lines 577234 577235 +1
Branches 70696 70697 +1
============================================
- Hits 114945 114925 -20
- Misses 449724 449747 +23
+ Partials 12565 12563 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
Author
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
4 open findings
ScopeType.CLUSTERfor… · NewsetUpClassusesgetattr(pool, "type", None)when filtering candidate pools, but this helper… · New("NetworkFilesystem", "RBD"), but the ZONE-scope path takes the… · New{"name": "Test-%s" % uuid.uuid4()}with a shared… · New
What changed in this PR
Expands VM deployment-from-volume support to include CLUSTER-scoped primary storage pools (in addition to ZONE), and aligns smoke tests to work across shared-storage backends like NFS and RBD.
Changes:
- Allow CLUSTER scope in
UserVmManagerImplvolume datastore validation and update related unit-test assertions. - Update VM lifecycle smoke tests to accept ZONE/CLUSTER pools, add RBD-specific skips/behavior, and improve SSH reconnect behavior.
| File | Description |
|---|---|
| test/integration/smoke/test_vm_lifecycle_with_snapshot_or_volume.py | Broadens pool selection to include cluster-scoped NFS/RBD, adds RBD snapshot lifecycle constraints, and adjusts VM creation/SSH behavior in smoke tests. |
| server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java | Updates expected validation error message to reflect acceptance of CLUSTER-scoped pools. |
| server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | Updates volume datastore scope validation to accept CLUSTER in addition to ZONE for deploy-from-volume. |
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Comment on lines
+7014
to
+7016
| || (!ScopeType.ZONE.equals(volume.getDataStore().getScope().getScopeType()) | ||
| && !ScopeType.CLUSTER.equals(volume.getDataStore().getScope().getScopeType())))) { | ||
| throw new InvalidParameterValueException("Deployment of virtual machine is supported only for Zone-wide or Cluster-wide storage pools"); |
| parent volume destroys the snapshot too - it can no longer be | ||
| used to deploy a new volume/VM afterward. | ||
| ''' | ||
| return self.storage_pool.type == 'RBD' |
Comment on lines
+91
to
+106
| # Prefer a zone-wide pool, but also accept a cluster-scope NFS/RBD pool. | ||
| storage_pools_response = list_storage_pools(cls.apiclient, | ||
| zoneid=cls.zone.id, | ||
| scope="ZONE") | ||
|
|
||
| if not storage_pools_response: | ||
| cluster_storage_pools = list_storage_pools(cls.apiclient, | ||
| zoneid=cls.zone.id, | ||
| scope="CLUSTER") or [] | ||
| storage_pools_response = [ | ||
| pool for pool in cluster_storage_pools | ||
| if getattr(pool, "type", None) in ("NetworkFilesystem", "RBD") | ||
| ] | ||
|
|
||
| if storage_pools_response: | ||
| cls.zone_wide_storage = storage_pools_response[0] | ||
| cls.storage_pool = storage_pools_response[0] |
Comment on lines
+198
to
205
| self.services['small'], | ||
| accountid=self.account.name, | ||
| domainid=self.account.domainid, | ||
| zoneid=self.zone.id, | ||
| serviceofferingid=self.service_offering.id, | ||
| templateid=template.id, | ||
| mode="basic", | ||
| mode=self.services['mode'], | ||
| ) |
Copilot stopped reviewing on behalf of
weizhouapache due to an error
October 8, 2026 12:55
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19473 |
This branch has not been deployed
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
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.
Description
Allow both ZONE and CLUSTER scoped storage pools for VM deployment from existing volumes and snapshots. Previously only ZONE-scoped pools were accepted, restricting deployment flexibility for cluster-wide storage configurations like NFS and RBD pools.
Update validation in UserVmManagerImpl to accept CLUSTER scope alongside ZONE scope, and update error messages accordingly.
Update test_vm_lifecycle_with_snapshot_or_volume.py smoke tests to:
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?