Feature/cstackex 213: Upstream PR 13578 comments resolution#78
Merged
Conversation
piyush5netapp
requested review from
rajiv-jain-netapp,
sandeeplocharla and
suryag1201
as code owners
There was a problem hiding this comment.
Pull request overview
This PR updates ONTAP primary storage attach-zone behavior to use the PrimaryDataStoreHelper.attachZone(DataStore, HypervisorType) overload (rather than updating the pool directly), adjusts ONTAP unit tests accordingly, and changes volume-attach behavior around reloading VolumeVO after grantAccess.
Changes:
- Update ONTAP primary datastore lifecycle to call
_dataStoreHelper.attachZone(dataStore, hypervisorType)and remove directstoragePoolDao.update(...)for hypervisor persistence. - Update ONTAP lifecycle unit tests to stub/verify the new
attachZone(DataStore, HypervisorType)helper call. - Modify
VolumeApiServiceImplto reload the volume from DB aftergrantAccessonly when the storage provider is ONTAP.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java | Alters post-grantAccess volume reload logic during attach. |
| plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | Switches zone attach to use helper overload that sets hypervisor/scope. |
| plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java | Updates mocks/verifications for the helper overload and adjusts assertions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
852
to
+853
| assertTrue(result, "attachZone should succeed for KVM hypervisor"); | ||
| verify(storagePoolDao, times(1)).update(eq(1L), any(StoragePoolVO.class)); | ||
| verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); |
Comment on lines
+4855
to
+4857
| if(DataStoreProvider.ONTAP_PLUGIN_NAME.equals(volumeToAttachStoragePool.getStorageProviderName())){ | ||
| volumeToAttach = _volsDao.findById(volumeToAttach.getId()); | ||
| } |
Comment on lines
850
to
+853
| dataStore, zoneScope, Hypervisor.HypervisorType.KVM); | ||
|
|
||
| assertTrue(result, "attachZone should succeed for KVM hypervisor"); | ||
| verify(storagePoolDao, times(1)).update(eq(1L), any(StoragePoolVO.class)); | ||
| verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); |
rajiv-jain-netapp
approved these changes
Jul 10, 2026
piyush5netapp
added a commit
that referenced
this pull request
Jul 10, 2026
### Description This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): Storage pool <img width="1240" height="439" alt="image" src="https://github.com/user-attachments/assets/cfc7bfdc-9807-470c-b043-ff1905acd0d0" /> VM instance <img width="1230" height="353" alt="image" src="https://github.com/user-attachments/assets/e8bf157d-105a-4c85-8452-9c391c467778" /> ONTAP flexvol <img width="1261" height="241" alt="image" src="https://github.com/user-attachments/assets/f7159199-a26a-4147-a41f-44524fe7ca8e" /> ONTAP Luns <img width="1267" height="207" alt="image" src="https://github.com/user-attachments/assets/a3645d25-d7b3-412c-bed7-2c43d6a0e6b8" /> ONTAP Igroup <img width="1254" height="630" alt="image" src="https://github.com/user-attachments/assets/1af9953a-d761-4b53-884d-e7c9f7ba8b6f" /> ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> --------- Co-authored-by: Srivastava, Piyush <Piyush.Srivastava@netapp.com>
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
This PR...
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
Storage pool
VM instance
ONTAP flexvol
ONTAP Luns
ONTAP Igroup
How Has This Been Tested?
How did you try to break this feature and the system with this change?