-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change storage pool scope from Cluster to Zone and vise versa #8875
Conversation
@blueorangutan package |
@rohityadavcloud 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9138 |
@blueorangutan test |
@andrijapanicsb a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
...src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java
Outdated
Show resolved
Hide resolved
...g/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
Outdated
Show resolved
Hide resolved
...n/java/org/apache/cloudstack/storage/datastore/lifecycle/AdaptiveDataStoreLifeCycleImpl.java
Outdated
Show resolved
Hide resolved
...age/src/main/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java
Show resolved
Hide resolved
...src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a quick review. Left some comments.
...src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java
Outdated
Show resolved
Hide resolved
engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #8875 +/- ##
============================================
+ Coverage 15.01% 15.77% +0.76%
- Complexity 11086 11121 +35
============================================
Files 5393 5033 -360
Lines 471529 442426 -29103
Branches 59743 53754 -5989
============================================
- Hits 70788 69782 -1006
+ Misses 392854 364872 -27982
+ Partials 7887 7772 -115
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@abh1sar lint Github action is failing due to missing eof in some files. |
...g/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
Outdated
Show resolved
Hide resolved
...g/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
Outdated
Show resolved
Hide resolved
...g/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
Outdated
Show resolved
Hide resolved
[SF] Trillian test result (tid-9682)
|
@blueorangutan test |
@kiranchavala a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
[SF] Trillian test result (tid-10643)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Tested it manually
Was able to change the scope of primary storage from clusterwide to zonewide (vice-versa)
Tested with nfs, ceph,powerflex, vmware, KVM environments
The api calls are working fine
change storagepoolscop
list affectedvmsforstoragescopechange
Please find the test case list
Test Case | Result |
---|---|
Change the scope of cluster wide storage to zone wide storage | Pass |
Change the scope of storage pool from zone wide to cluster wide | Pass |
Change the scope of a local storage | Pass |
Events should be logged whenever there is primary storage scope changed | Pass |
Non admin user should not be allowed to change the primary storage scope | Pass |
The database entries in the op_host_capacity and storage_pool tables must be updated | Pass |
Test the storage scope change for a ceph primary storage | Pass |
Test the strorage scope change for a Powerflex storage | Pass |
Test the storage scope change on a vmware env | Pass |
Test the api call List affectedvmsforstoragescopechange | Pass |
Test the list affectedvmsforstoragescopechange api call when the storage pool is in enabled state | Pass |
[SF] Trillian test result (tid-10644)
|
@blueorangutan package |
@abh1sar 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10188 |
@blueorangutan package |
@sureshanaparti 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. |
@blueorangutan package |
@abh1sar 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10197 |
@blueorangutan test |
@abh1sar a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
[SF] Trillian test result (tid-10679)
|
Merging based on reviews, manual & smoke tests. |
…#8875) * New feature: Change storage pool scope * Added checks for Ceph/RBD * Update op_host_capacity table on primary storage scope change * Storage pool scope change integration test * pull 8875 : Addressed review comments * Pull 8875: remove storage checks, AbstractPrimayStorageLifeCycleImpl class * Pull 8875: Fixed integration test failure * Pull 8875: Review comments * Pull 8875: review comments + broke changeStoragePoolScope into smaller functions * Added UT for changeStoragePoolScope * Rename AbstractPrimaryDataStoreLifeCycleImpl to BasePrimaryDataStoreLifeCycleImpl * Pull 8875: Dao review comments * Pull 8875: Rename changeStoragePoolScope.vue to ChangeStoragePoolScope.vue * Pull 8875: Created a new smokes test file + A single warning msg in ui * Pull 8875: Added cleanup in test_primary_storage_scope.py * Pull 8875: Type in en.json * Pull 8875: cleanup array in test_primary_storage_scope.py * Pull:8875 Removing extra whitespace at eof of StorageManagerImplTest * Pull 8875: Added UT for PrimaryDataStoreHelper and BasePrimaryDataStoreLifeCycleImpl * Pull 8875: Added license header * Pull 8875: Fixed sql query for vmstates * Pull 8875: Changed icon plus info on disabled mode in apidoc * Pull 8875: Change scope should not work for local storage * Pull 8875: Change scope completion event * Pull 8875: Added api findAffectedVmsForStorageScopeChange * Pull 8875: Added UT for findAffectedVmsForStorageScopeChange and removed listByPoolIdVMStatesNotInCluster * Pull 8875: Review comments + Vm name in response * Pull 8875: listByVmsNotInClusterUsingPool was returning duplicate VM entries because of multiple volumes in the VM satisfying the criteria * Pull 8875: fixed listAffectedVmsForStorageScopeChange UT * listAffectedVmsForStorageScopeChange should work if the pool is not disabled * Fix listAffectedVmsForStorageScopeChangeTest UT * Pull 8875: add volume.removed not null check in VmsNotInClusterUsingPool query * Pull 8875: minor refactoring in changeStoragePoolScopeToCluster * Update server/src/main/java/com/cloud/storage/StorageManagerImpl.java * fix eof * changeStoragePoolScopeToZone should connect pool to all Up hosts Co-authored-by: Suresh Kumar Anaparti <[email protected]>
Description
This PR adds the functionality to change the scope of the Primary storage from zone to cluster scope and vice versa.
Feature is tested and supported for
Hypervisors : KVM and VMware (Simulator will also work)
Protocol : NFS and Ceph, and
Storage Provider : Default Primary
Documentation PR : apache/cloudstack-documentation#395
The option for scope change is provided as a new action button to root admin once a Primary storage has been disabled.
A new api changeStoragePoolScope handles the execution after doing all relevant checks. It makes DB changes and also connects to the hypervisor hosts to add or remove the storage pool as required. It adds the storage pool to the applicable hosts when the scope is changing from Cluster to Zone. And it removes the storage pool from the hosts when the scope is changing from Zone to Cluster. Management server or the Cloudstack agents don’t need to be restarted for this.
Scope change from Zone to Cluster cannot be done if there are volumes associated with running-VMs belonging to other clusters. Api will throw an error in this case.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
Error while moving the scope from zone to cluster and "list affectedvmsforstoragescopechange" response
How Has This Been Tested?
KVM + NFS
Cluster to Zone
Zone to Cluster
VMware + NFS
Tested basic functionality.
How did you try to break this feature and the system with this change?