Skip to content

Commit a59b49e

Browse files
piyush5netappSrivastava, Piyush
piyush5netapp
and
Srivastava, Piyush
authored
Feature/cstackex 213: Upstream PR 13578 comments resolution (#78)
### 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>
1 parent a50c534 commit a59b49e

4 files changed

Lines changed: 19 additions & 20 deletions

File tree

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,6 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
344344
logger.error("attachZone : ONTAP primary storage is supported only for KVM hypervisor");
345345
throw new CloudRuntimeException("ONTAP primary storage is supported only for KVM hypervisor");
346346
}
347-
storagePool.setHypervisor(hypervisorType);
348-
storagePoolDao.update(storagePool.getId(),storagePool);
349-
logger.debug("attachZone : Set Hypervisor type for storage pool {} to {}", storagePool.getName(), hypervisorType);
350-
351347
PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore;
352348
List<HostVO> hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), hypervisorType);
353349
logger.debug(String.format("In createPool. Attaching the pool to each of the hosts in %s.", hostsToConnect));
@@ -374,7 +370,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
374370
return false;
375371
}
376372
}
377-
_dataStoreHelper.attachZone(dataStore);
373+
_dataStoreHelper.attachZone(dataStore, hypervisorType);
378374
return true;
379375
}
380376

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ public Volume createStorageVolume(String volumeName, Long size) {
266266
final long availableBytes = aggrResp.getAvailableBlockStorageSpace().longValue();
267267
logger.debug("Aggregate " + aggr.getName() + " available bytes=" + availableBytes + ", requested=" + size);
268268

269-
if (availableBytes <= size) {
269+
if (availableBytes < size) {
270270
logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Required=" +
271271
size + " bytes, available=" + availableBytes + " bytes. Skipping this aggregate.");
272272
continue;

‎plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ public void testAttachZone_positive() throws Exception {
612612
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
613613
.thenReturn(mockHosts);
614614
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
615-
when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
615+
when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore);
616616

617617
try (MockedStatic<OntapStorageUtils> utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
618618
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
@@ -633,7 +633,7 @@ public void testAttachZone_positive() throws Exception {
633633
verify(storagePoolDetailsDao, times(1)).listDetailsKeyPairs(1L);
634634
verify(storageStrategy, times(1)).createAccessGroup(any(AccessGroup.class));
635635
verify(_storageMgr, times(2)).connectHostToSharedPool(any(HostVO.class), eq(1L));
636-
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class));
636+
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM));
637637
}
638638
}
639639

@@ -647,7 +647,7 @@ public void testAttachZone_withSingleHost() throws Exception {
647647
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
648648
.thenReturn(singleHost);
649649
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
650-
when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
650+
when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore);
651651

652652
try (MockedStatic<OntapStorageUtils> utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
653653
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
@@ -662,7 +662,7 @@ public void testAttachZone_withSingleHost() throws Exception {
662662
// Verify
663663
assertTrue(result, "attachZone should return true with single host");
664664
verify(_storageMgr, times(1)).connectHostToSharedPool(any(HostVO.class), eq(1L));
665-
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class));
665+
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM));
666666
}
667667
}
668668

@@ -679,7 +679,7 @@ public void testAttachZone_withMultipleHosts() throws Exception {
679679
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
680680
.thenReturn(mockHosts);
681681
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
682-
when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
682+
when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore);
683683

684684
try (MockedStatic<OntapStorageUtils> utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
685685
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
@@ -694,7 +694,7 @@ public void testAttachZone_withMultipleHosts() throws Exception {
694694
// Verify
695695
assertTrue(result, "attachZone should return true with multiple hosts");
696696
verify(_storageMgr, times(3)).connectHostToSharedPool(any(HostVO.class), eq(1L));
697-
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class));
697+
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM));
698698
}
699699
}
700700

@@ -724,7 +724,7 @@ public void testAttachZone_hostConnectionFailure() throws Exception {
724724
verify(storageStrategy, times(1)).createAccessGroup(any(AccessGroup.class));
725725
verify(_storageMgr, times(1)).connectHostToSharedPool(any(HostVO.class), eq(1L));
726726
// _dataStoreHelper.attachZone should NOT be called due to early return
727-
verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class));
727+
verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM));
728728
}
729729
}
730730

@@ -737,7 +737,7 @@ public void testAttachZone_emptyHostList() throws Exception {
737737
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
738738
.thenReturn(emptyHosts);
739739
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
740-
when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
740+
when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore);
741741

742742
try (MockedStatic<OntapStorageUtils> utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
743743
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
@@ -751,7 +751,7 @@ public void testAttachZone_emptyHostList() throws Exception {
751751
// Verify
752752
assertTrue(result, "attachZone should return true even with no hosts");
753753
verify(_storageMgr, times(0)).connectHostToSharedPool(any(HostVO.class), anyLong());
754-
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class));
754+
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM));
755755
}
756756
}
757757

@@ -780,7 +780,7 @@ public void testAttachZone_secondHostConnectionFails() throws Exception {
780780
// Verify
781781
assertFalse(result, "attachZone should return false when any host connection fails");
782782
verify(_storageMgr, times(2)).connectHostToSharedPool(any(HostVO.class), eq(1L));
783-
verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class));
783+
verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM));
784784
}
785785
}
786786

@@ -791,7 +791,7 @@ public void testAttachZone_createAccessGroupCalled() throws Exception {
791791
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
792792
.thenReturn(mockHosts);
793793
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
794-
when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
794+
when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore);
795795

796796
try (MockedStatic<OntapStorageUtils> utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
797797
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
@@ -838,7 +838,7 @@ public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception {
838838
when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM)))
839839
.thenReturn(mockHosts);
840840
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails);
841-
when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore);
841+
when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore);
842842

843843
try (MockedStatic<OntapStorageUtils> utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) {
844844
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any()))
@@ -850,7 +850,7 @@ public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception {
850850
dataStore, zoneScope, Hypervisor.HypervisorType.KVM);
851851

852852
assertTrue(result, "attachZone should succeed for KVM hypervisor");
853-
verify(storagePoolDao, times(1)).update(eq(1L), any(StoragePoolVO.class));
853+
verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM));
854854
}
855855
}
856856

‎server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
6464
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver;
6565
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
66+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
6667
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
6768
import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
6869
import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
@@ -4851,7 +4852,9 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L
48514852
// Reload volume from DB after grantAccess — managed storage drivers (e.g. ONTAP)
48524853
// may update the volume's path and iScsiName during grantAccess, so the local
48534854
// volumeToAttach object can be stale.
4854-
volumeToAttach = _volsDao.findById(volumeToAttach.getId());
4855+
if(DataStoreProvider.ONTAP_PLUGIN_NAME.equals(volumeToAttachStoragePool.getStorageProviderName())){
4856+
volumeToAttach = _volsDao.findById(volumeToAttach.getId());
4857+
}
48554858
}
48564859

48574860
if (sendCommand) {

0 commit comments

Comments
 (0)