Support of snapshot copy to primary storage in different zones.#9478
Support of snapshot copy to primary storage in different zones.#9478sureshanaparti merged 15 commits intoapache:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9478 +/- ##
============================================
- Coverage 17.35% 17.33% -0.02%
- Complexity 15189 15198 +9
============================================
Files 5883 5883
Lines 524514 525259 +745
Branches 64007 64131 +124
============================================
+ Hits 91013 91042 +29
- Misses 423216 423924 +708
- Partials 10285 10293 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
slavkap
force-pushed
the
support-snapshot-copy-on-primary
branch
from
8cc7b49 to
0df6764
Compare
slavkap
changed the title
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
slavkap
force-pushed
the
support-snapshot-copy-on-primary
branch
from
0df6764 to
e0f4283
Compare
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10756 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-11155)
|
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
slavkap
force-pushed
the
support-snapshot-copy-on-primary
branch
from
e0f4283 to
6f209d8
Compare
api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java
Show resolved
Hide resolved
.../src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java
Outdated
Show resolved
Hide resolved
api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CopySnapshotCmd.java
Outdated
Show resolved
Hide resolved
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
.../src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@slavkap - great job! I've identified some minor issues that need to be addressed:
- Resource Cleanup: Failed snapshots left in unusable "Allocated" state - need proper rollback/cleanup
- UI Label: Shows label.useStorageReplication instead of "Use storage replication"
Below is a list of the test env I've used and everything that was covered:
Test Environment
3-zone CloudStack environment
NFS primary storage (NetworkFilesystem, DefaultPrimary provider)
6 test VMs deployed across all zones
Admin and regular user accounts configured
CloudMonkey CLI profiles for both user types
Test Areas Covered:
RBAC (Role-Based Access Control) Testing
- Admin user snapshot creation with usestoragereplication=true
- Admin user snapshot creation with storageids parameter
- Regular user snapshot creation with usestoragereplication=true
- Regular user snapshot creation with storageids parameter ( ISSUE: Api call fails but snapshot created / allocated)
- Resource isolation testing - regular user accessing admin volumes (properly blocked)
- Cross-user access validation - admin accessing regular user resources (working correctly)
- Permission boundary testing for all new parameters
Snapshot Creation Testing
- Traditional snapshot creation (admin user) - BackedUp state
- Traditional snapshot creation (regular user) - BackedUp state
- Snapshot creation with usestoragereplication=true (both users) - Error 530, Allocated state (this is due to the usage of NFS storage)
- Snapshot creation with usestoragereplication=false (both users) - BackedUp state
- Snapshot creation with usestoragereplication=invalid_value (both users) - BackedUp state (parameter ignored)
- Snapshot creation with storageids parameter (both users) - Error 530, Allocated state in UI
- VM state validation - snapshots require stopped VMs
Cross-Zone Copy Testing
- Traditional cross-zone copy (admin user) - SUCCESS via secondary storage
- Traditional cross-zone copy (regular user) - SUCCESS via secondary storage
- Cross-zone copy with usestoragereplication=true (admin) - Error 431 (expected NFS limitation)
- Cross-zone copy with usestoragereplication=true (regular user) - Error 431 (expected NFS limitation)
- Cross-zone copy with storageids parameter (admin) - Error 530
- Cross-zone copy with storageids parameter (regular user) - Error 431 (ISSUE: Should be blocked)
- Cross-zone copy of "Allocated" state snapshots - Properly blocked
- Cross-zone copy parameter validation (invalid zones, invalid storage IDs)
Snapshot Policy Testing
- Traditional hourly policies (both users) - SUCCESS
- Traditional daily policies (both users) - SUCCESS
- Traditional weekly policies (both users) - SUCCESS
- Policies with usestoragereplication=true parameter (both users) - Accepted
- Policies with storageids parameter (both users) - Accepted
- Policy schedule format validation - Proper rejection of invalid formats
- Policy storage pool assignment - Automatic assignment working
Parameter Validation Testing
- Boolean parameter testing: true, false, invalid_value
- Storage ID validation: valid IDs, invalid IDs, non-existent IDs
- Zone ID validation: valid zones, invalid zones
- Parameter combination testing: multiple parameters together
- Empty parameter testing
- Malformed parameter testing
Backward Compatibility Testing
- Traditional createSnapshot (no new parameters) - 100% working
- Traditional copySnapshot via secondary storage - 100% working
- Traditional createSnapshotPolicy - 100% working
- Existing API endpoints unchanged - 100% compatible
- No performance regression in traditional operations
- Zero breaking changes in existing workflows
Error Handling Testing
- Error consistency across user types - Same error codes for same operations
- Graceful failure testing - No system crashes or exceptions
- Invalid resource access - Proper error messages
- Storage capability mismatch errors - Clear messaging
- Permission denied scenarios - Appropriate responses
- Malformed request handling - Robust validation
UI Integration Testing
- New UI toggle functionality validation
- Storage pool dropdown behavior testing
- Form validation and submission testing
- User type UI difference validation
- Error message display in UI
- UI label issue identified: label.useStorageReplication should be "Use storage replication"
Security Testing
- Resource isolation between user types - Working correctly
- Admin privilege validation - Working correctly
- Regular user restriction testing - Mostly working (storageids issue)
- Cross-account operation attempts - Properly blocked
- Permission boundary enforcement - Partial (storageids bypass)
- Privilege escalation prevention - Working (except storageids)
Integration Testing
- End-to-end snapshot lifecycle - Complete workflow functional
- Multi-zone operation workflows - Cross-zone logic working
- Template creation from snapshots - SUCCESS (regression test)
- Volume reversion from snapshots - SUCCESS (regression test)
- Policy execution simulation - Scheduling working
- System integration validation - All components connected properly
Regression Testing
- Pre-existing snapshot functionality - 100% preserved
- Traditional cross-zone copy methods - 100% working
- Snapshot policy execution - 100% functional
- API backward compatibility - 100% maintained
- System stability - No regressions introduced
|
[SF] Trillian test result (tid-13973)
|
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
1 similar comment
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@slavkap can you check these issues and resolve the conflicts |
…n zones Added support to copy a snapshot to another StorPool primary storage in different zones.
Added drop down to choose the primary storage pools to copy a snapshot Small fixes
hide the primary storage from the users in the UI refactor smoke test
fix copy when reccuring snapshot fix UI after rebasing
Pool type to volumes that are created from snapshots Added StorPool tags to snapshots that are copied from remote
|
@blueorangutan package |
|
@slavkap a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Thank you, @rosi-shapeblue, for the detailed information! The UI issue with the label has been fixed in the latest commit. Additionally, snapshot allocation will no longer occur if the storage doesn't support copying to another zone. I’ve tested snapshot copying between zones using StorPool, and confirmed that this feature doesn't break existing functionality for copying snapshots between zones on secondary storage. Testing was also done with NFS as the primary storage. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14503 |
|
@rosi-shapeblue can you check if your concerns are addressed? |
RosiKyu
left a comment
RosiKyu
left a comment
There was a problem hiding this comment.
LGTM. The issues that were discussed have been addressed.
|
@blueorangutan test |
|
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14022)
|
10 participants
Description
This PR allows copying a snapshot from a primary storage pool in one zone to a primary storage pool in different zones without involving secondary storage.
This feature is currently implemented only in the StorPool storage plugin. Other storage plugins can add this feature if the storage pools support direct copies of snapshots from one pool to another.
Added additional API param
usestoragereplicationand one more for Admin usersstorageidsin CloudStack API calls:The option
snapshot.backup.to.secondarydoes not apply to the copy functionality. The snapshots will be copied only to the required primary storage in a different zone.The user can create volumes/templates from the copied snapshots. The user can make copies only to a primary or to a secondary storage - at the moment, there is no option to do the copy on both.
The destination zone is a mandatory parameter if the users want to copy a snapshot, and the
usestoragereplicationif the copy has to be on the primary storageFor Admin users:
Those users can define the primary destination storage for the copy or to enable the setting
use.storage.replicationin the Primary storage settings.For regular Users:
The Admin should enable the configuration setting
use.storage.replicationin the Primary storage settings.For other storage plugins that want to adopt this functionality:
CAN_COPY_SNAPSHOT_BETWEEN_ZONEScopySnapshotmethod in their SnapshotStrategy and that the driver can handle the COPY operationTypes of changes
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
Manual and smoke tests with StorPool primary storage on multiple zones