From 991fb3d44d4b54477e652f0a6457471b9ac45ce3 Mon Sep 17 00:00:00 2001 From: piyush5netapp <91685498+piyush5netapp@users.noreply.github.com> Date: Mon, 11 May 2026 12:03:48 +0530 Subject: [PATCH 1/7] bugfix/CSTACKEX-138: Fix volume attach in iSCSI for ONTAP plugin (#54) --- .../main/java/com/cloud/storage/VolumeApiServiceImpl.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 39686b4d3843..4555b4485970 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -5092,6 +5092,11 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L throw new CloudRuntimeException(e.getMessage()); } + + // Reload volume from DB after grantAccess — managed storage drivers (e.g. ONTAP) + // may update the volume's path and iScsiName during grantAccess, so the local + // volumeToAttach object can be stale. + volumeToAttach = _volsDao.findById(volumeToAttach.getId()); } if (sendCommand) { From 2346fae590d040ff6d705b698369a38e3a21d916 Mon Sep 17 00:00:00 2001 From: piyush5netapp <91685498+piyush5netapp@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:07:38 +0530 Subject: [PATCH 2/7] Bugfix/cstackex 188 (#68): Zone scoped Instance creation fix ### Description This PR... ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] 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): image image ### How Has This Been Tested? #### How did you try to break this feature and the system with this change? --------- Co-authored-by: Srivastava, Piyush --- .../OntapPrimaryDatastoreLifecycle.java | 9 ++- .../OntapPrimaryDatastoreLifecycleTest.java | 62 +++++++++++++++++-- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java index 9854f02e6960..a098c68c3d61 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java @@ -337,9 +337,16 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper logger.error("attachZone : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("Storage Pool not found for id: " + dataStore.getId()); } + if (!Hypervisor.HypervisorType.KVM.equals(hypervisorType)){ + logger.error("attachZone : ONTAP primary storage is supported only for KVM hypervisor"); + throw new CloudRuntimeException("ONTAP primary storage is supported only for KVM hypervisor"); + } + storagePool.setHypervisor(hypervisorType); + storagePoolDao.update(storagePool.getId(),storagePool); + logger.debug("attachZone : Set Hypervisor type for storage pool {} to {}", storagePool.getName(), hypervisorType); PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore; - List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), Hypervisor.HypervisorType.KVM); + List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), hypervisorType); logger.debug(String.format("In createPool. Attaching the pool to each of the hosts in %s.", hostsToConnect)); Map details = storagePoolDetailsDao.listDetailsKeyPairs(primaryStore.getId()); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java index 604ab400474c..3b576df97e37 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java @@ -344,17 +344,24 @@ public void testInitialize_clusterNotKVM() { clusterVO.setHypervisorType("XenServer"); when(_clusterDao.findById(2L)).thenReturn(clusterVO); + HashMap detailsMap = new HashMap<>(); + detailsMap.put(OntapStorageConstants.USERNAME, "testUser"); + detailsMap.put(OntapStorageConstants.PASSWORD, "testPassword"); + detailsMap.put(OntapStorageConstants.STORAGE_IP, "10.10.10.10"); + detailsMap.put(OntapStorageConstants.SVM_NAME, "vs0"); + detailsMap.put(OntapStorageConstants.PROTOCOL, "NFS3"); + Map dsInfos = new HashMap<>(); - dsInfos.put("zoneId",1L); - dsInfos.put("podId",1L); + dsInfos.put("zoneId", 1L); + dsInfos.put("podId", 1L); dsInfos.put("clusterId", 2L); dsInfos.put("name", "testStoragePool"); dsInfos.put("providerName", "testProvider"); - dsInfos.put("capacityBytes",200000L); - dsInfos.put("managed",true); + dsInfos.put("capacityBytes", 200000L); + dsInfos.put("managed", true); dsInfos.put("tags", "testTag"); dsInfos.put("isTagARule", false); - dsInfos.put("details", new HashMap()); + dsInfos.put("details", detailsMap); Exception ex = assertThrows(CloudRuntimeException.class, () -> { try (MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) { @@ -802,4 +809,49 @@ public void testAttachZone_createAccessGroupCalled() throws Exception { } } + @Test + public void testAttachZone_nonKvmHypervisorThrowsException() { + // Non-KVM hypervisor type must be rejected before any further processing + Exception ex = assertThrows(CloudRuntimeException.class, () -> + ontapPrimaryDatastoreLifecycle.attachZone( + dataStore, zoneScope, Hypervisor.HypervisorType.VMware)); + + assertTrue(ex.getMessage().contains("ONTAP primary storage is supported only for KVM hypervisor")); + // update should never be reached + verify(storagePoolDao, times(0)).update(anyLong(), any(StoragePoolVO.class)); + } + + @Test + public void testAttachZone_nullHypervisorThrowsException() { + // null hypervisorType is not a valid case — only KVM is accepted + Exception ex = assertThrows(CloudRuntimeException.class, () -> + ontapPrimaryDatastoreLifecycle.attachZone(dataStore, zoneScope, null)); + + assertTrue(ex.getMessage().contains("ONTAP primary storage is supported only for KVM hypervisor")); + verify(storagePoolDao, times(0)).update(anyLong(), any(StoragePoolVO.class)); + } + + @Test + public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception { + // KVM hypervisorType should be set on the pool and persisted via storagePoolDao.update + when(zoneScope.getScopeId()).thenReturn(1L); + when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) + .thenReturn(mockHosts); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + + try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) + .thenReturn(storageStrategy); + when(storageStrategy.createAccessGroup(any(AccessGroup.class))).thenReturn(null); + when(_storageMgr.connectHostToSharedPool(any(HostVO.class), anyLong())).thenReturn(true); + + boolean result = ontapPrimaryDatastoreLifecycle.attachZone( + dataStore, zoneScope, Hypervisor.HypervisorType.KVM); + + assertTrue(result, "attachZone should succeed for KVM hypervisor"); + verify(storagePoolDao, times(1)).update(eq(1L), any(StoragePoolVO.class)); + } + } + } From e377eff94a212a725fa812c125a359ef2279d567 Mon Sep 17 00:00:00 2001 From: Surya Gupta Date: Mon, 22 Jun 2026 22:25:19 +0530 Subject: [PATCH 3/7] BugFixes For CSTACKEX-176, CSTACKEX-172, CSTACKEX-199 and CSTACKEX-181 (#65) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Description This PR... CSTACKEX-176: Storage Pool Backend by ONTAP FlexVolume is not Thin Provisioned CSTACKEX-172: VM create failure due to aggrs not available and UI says "Unable to orchestrate the start of VM instance" CSTACKEX-199: Error message when cluster password is wrong while creating primary storage pool is not user intuitive CSTACKEX-181: Multiple host with same hostname will not follow per host igroup design ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] 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): Screenshot 2026-06-18 at 2 33 52 PM image ### How Has This Been Tested? 1- Create a storage pool with invalid ontap cred - throw the error 2- Create a storage pool with valid ontap cred - success 3- Verify Ontap Volume - thin provisioned 4- VM instance VM1 creation - suceess and igroup created with cs_svmName_hostUUID 5- VM instance VM2 creation - reused the same igroup 6- VM instance VM1 and VM2 delete - igroup also got deleted #### How did you try to break this feature and the system with this change? --------- Co-authored-by: Gupta, Surya --- .../driver/OntapPrimaryDatastoreDriver.java | 4 +- .../storage/feign/model/Volume.java | 75 +++++++++++++++++ .../storage/service/StorageStrategy.java | 37 ++++++--- .../storage/service/UnifiedSANStrategy.java | 4 +- .../storage/utils/OntapStorageConstants.java | 1 + .../storage/utils/OntapStorageUtils.java | 13 ++- .../OntapPrimaryDatastoreDriverTest.java | 3 + .../storage/service/StorageStrategyTest.java | 77 +++++++++++++++++ .../service/UnifiedSANStrategyTest.java | 16 ++-- .../storage/utils/OntapStorageUtilsTest.java | 82 +++++++++++++++++++ 10 files changed, 284 insertions(+), 28 deletions(-) create mode 100644 plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 04996b74d2a5..ece29f7cd0ac 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java @@ -413,7 +413,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map details, String svmName, StoragePoolVO storagePool) { String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME).getValue(); UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); - String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getName()); + String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid()); // Validate if Igroup exist ONTAP for this host as we may be using delete_on_unmap= true and igroup may be deleted by ONTAP automatically Map getAccessGroupMap = Map.of( @@ -506,7 +506,7 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, String svmName = details.get(OntapStorageConstants.SVM_NAME); if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { - String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getName()); + String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid()); // Retrieve LUN name from volume details; if missing, volume may not have been fully created VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Volume.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Volume.java index e7c538d7cd04..736d70de2178 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Volume.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Volume.java @@ -19,9 +19,11 @@ package org.apache.cloudstack.storage.feign.model; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonValue; import java.util.List; import java.util.Objects; @@ -50,6 +52,9 @@ public class Volume { @JsonProperty("space") private VolumeSpace space; + @JsonProperty("guarantee") + private Guarantee guarantee; + @JsonProperty("anti_ransomware") private AntiRansomware antiRansomware; @@ -112,6 +117,14 @@ public void setSpace(VolumeSpace space) { this.space = space; } + public Guarantee getGuarantee() { + return guarantee; + } + + public void setGuarantee(Guarantee guarantee) { + this.guarantee = guarantee; + } + public AntiRansomware getAntiRansomware() { return antiRansomware; } @@ -139,4 +152,66 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hashCode(uuid); } + + public static class Guarantee { + + /** + * ONTAP FlexVolume space guarantee (provisioning) type. + *
    + *
  • {@link #NONE} - thin provisioning (space is not reserved up front)
    • + *
  • {@link #VOLUME} - thick provisioning (full volume size is reserved on the aggregate)
    • + *
+ */ + public enum TypeEnum { + NONE("none"), + + VOLUME("volume"); + + private String value; + + TypeEnum(String value) { + this.value = value; + } + + @JsonValue + public String getValue() { + return value; + } + + @Override + public String toString() { + return String.valueOf(value); + } + + @JsonCreator + public static TypeEnum fromValue(String text) { + if (text == null) return null; + for (TypeEnum b : TypeEnum.values()) { + if (text.equalsIgnoreCase(b.value)) { + return b; + } + } + return null; + } + } + + @JsonProperty("type") + private TypeEnum type; + + public Guarantee() { + } + + public Guarantee(TypeEnum type) { + this.type = type; + } + + public TypeEnum getType() { + return type; + } + + public void setType(TypeEnum type) { + this.type = type; + } + } + } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index bd808a26d6f8..f4ab806d6885 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -49,6 +49,7 @@ import org.apache.logging.log4j.Logger; import java.util.HashMap; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Objects; @@ -135,31 +136,42 @@ public boolean connect() { logger.error("No aggregates are assigned to SVM " + svmName); throw new CloudRuntimeException("No aggregates are assigned to SVM " + svmName); } + // Collect all online aggregates assigned to the SVM. Capacity-based selection is + // intentionally deferred to createStorageVolume(name, size), which validates the + // available space against the actual requested volume size. + List eligibleAggregates = new ArrayList<>(); for (Aggregate aggr : aggrs) { logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid()); Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid()); if (aggrResp == null) { logger.warn("Aggregate details response is null for aggregate " + aggr.getName() + ". Skipping."); - break; + continue; } if (!Objects.equals(aggrResp.getState(), Aggregate.StateEnum.ONLINE)) { logger.warn("Aggregate " + aggr.getName() + " is not in online state. Skipping this aggregate."); continue; - } else if (aggrResp.getSpace() == null || aggrResp.getAvailableBlockStorageSpace() == null || - aggrResp.getAvailableBlockStorageSpace() <= storage.getSize().doubleValue()) { - logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Skipping this aggregate."); - continue; } - logger.info("Selected aggregate: " + aggr.getName() + " for volume operations."); - this.aggregates = List.of(aggr); - break; + logger.debug("Aggregate " + aggr.getName() + " is online and eligible for volume operations."); + eligibleAggregates.add(aggr); } - if (this.aggregates == null || this.aggregates.isEmpty()) { - logger.error("No suitable aggregates found on SVM " + svmName + " for volume creation."); - throw new CloudRuntimeException("No suitable aggregates found on SVM " + svmName + " for volume creation."); + if (eligibleAggregates.isEmpty()) { + logger.error("No suitable aggregates found on SVM " + svmName + " for volume operations."); + throw new CloudRuntimeException("No suitable aggregates found on SVM " + svmName + " for volume operations."); } + this.aggregates = eligibleAggregates; + logger.info("Found " + eligibleAggregates.size() + " online aggregate(s) on SVM " + svmName + " for volume operations."); logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided"); + } catch (FeignException.Unauthorized e) { + logger.error("Authentication failed while connecting to ONTAP cluster at " + storage.getStorageIP() + + ". Please verify the username and password.", e); + throw new CloudRuntimeException("Authentication failed: Invalid credentials for ONTAP cluster at " + + storage.getStorageIP() + ". Please verify the username and password."); + } catch (FeignException.Forbidden e) { + logger.error("Authorization failed while connecting to ONTAP cluster at " + storage.getStorageIP() + + ". The user does not have sufficient privileges.", e); + throw new CloudRuntimeException("Authorization failed: User does not have sufficient privileges on ONTAP cluster at " + + storage.getStorageIP() + ". Please verify user permissions."); } catch (Exception e) { logger.error("Failed to connect to ONTAP cluster: " + e.getMessage(), e); throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e); @@ -211,7 +223,7 @@ public Volume createStorageVolume(String volumeName, Long size) { if (aggrResp == null) { logger.warn("Aggregate details response is null for aggregate " + aggr.getName() + ". Skipping."); - break; + continue; } if (!Objects.equals(aggrResp.getState(), Aggregate.StateEnum.ONLINE)) { @@ -251,6 +263,7 @@ public Volume createStorageVolume(String volumeName, Long size) { volumeRequest.setAggregates(List.of(aggr)); volumeRequest.setSize(size); volumeRequest.setNas(nas); + volumeRequest.setGuarantee(new Volume.Guarantee(Volume.Guarantee.TypeEnum.NONE)); try { JobResponse jobResponse = volumeFeignClient.createVolumeWithJob(authHeader, volumeRequest); if (jobResponse == null || jobResponse.getJob() == null) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 5f1ac265fc50..2b0e65f9f7ea 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -207,7 +207,7 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { igroupRequest.setOsType(Igroup.OsTypeEnum.Linux); for (HostVO host : accessGroup.getHostsToConnect()) { - igroupName = OntapStorageUtils.getIgroupName(svmName, host.getName()); + igroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid()); igroupRequest.setName(igroupName); List initiators = new ArrayList<>(); @@ -271,7 +271,7 @@ public void deleteAccessGroup(AccessGroup accessGroup) { //Get iGroup name per host if(!CollectionUtils.isEmpty(accessGroup.getHostsToConnect())) { for (HostVO host : accessGroup.getHostsToConnect()) { - String igroupName = OntapStorageUtils.getIgroupName(svmName, host.getName()); + String igroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid()); logger.info("deleteAccessGroup: iGroup name '{}'", igroupName); // Get the iGroup to retrieve its UUID diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 4ad0412d2593..e5224237e526 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -96,6 +96,7 @@ public class OntapStorageConstants { public static final String IGROUP_DOT_UUID = "igroup.uuid"; public static final String UNDERSCORE = "_"; public static final String CS = "cs"; + public static final int IGROUP_NAME_MAX_LENGTH = 96; public static final String SRC_CS_VOLUME_ID = "src_cs_volume_id"; public static final String BASE_ONTAP_FV_ID = "base_ontap_fv_id"; public static final String ONTAP_SNAP_ID = "ontap_snap_id"; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java index 596372edcf16..66fd41d5123d 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java @@ -139,10 +139,15 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map OntapStorageConstants.IGROUP_NAME_MAX_LENGTH) { + igroupName = igroupName.substring(0, OntapStorageConstants.IGROUP_NAME_MAX_LENGTH); + } + return igroupName; } public static String generateExportPolicyName(String svmName, String volumeName){ diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index b535217fd235..3c139e23cb88 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -348,6 +348,7 @@ void testGrantAccess_ClusterScope_Success() { when(volumeVO.getId()).thenReturn(100L); when(host.getName()).thenReturn("host1"); + when(host.getUuid()).thenReturn("host-uuid-1"); VolumeDetailVO lunNameDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1", false); when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_NAME)).thenReturn(lunNameDetail); @@ -384,6 +385,7 @@ void testGrantAccess_IgroupNotFound_CreatesNewIgroup() { // Setup - use HostVO mock since production code casts Host to HostVO HostVO hostVO = mock(HostVO.class); when(hostVO.getName()).thenReturn("host1"); + when(hostVO.getUuid()).thenReturn("host-uuid-1"); when(dataStore.getId()).thenReturn(1L); when(volumeInfo.getType()).thenReturn(VOLUME); @@ -477,6 +479,7 @@ void testRevokeAccess_ISCSIVolume_Success() { when(host.getStorageUrl()).thenReturn("iqn.1993-08.org.debian:01:host1"); when(host.getName()).thenReturn("host1"); + when(host.getUuid()).thenReturn("host-uuid-1"); VolumeDetailVO lunNameDetail = new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/lun1", false); when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_NAME)).thenReturn(lunNameDetail); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java index 86ef1d7c79b6..df9afe2542f9 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java @@ -230,6 +230,69 @@ public void testConnect_positive() { verify(svmFeignClient, times(1)).getSvmResponse(anyMap(), anyString()); } + @Test + public void testConnect_succeedsWhenAggregateSpaceBelowPoolCapacity() { + // Regression: connect() must validate connectivity/SVM/aggregate-state ONLY. + // Capacity is validated per-volume in createStorageVolume(name, size). Previously + // connect() compared aggregate free space against the whole storage pool size + // (storage.getSize()), which incorrectly failed data-path operations (volume/LUN + // create, grant/revoke access, delete) once the pool FlexVolume already existed. + Svm svm = new Svm(); + svm.setName("svm1"); + svm.setState(OntapStorageConstants.RUNNING); + svm.setNfsEnabled(true); + + Aggregate aggregate = new Aggregate(); + aggregate.setName("aggr1"); + aggregate.setUuid("aggr-uuid-1"); + svm.setAggregates(List.of(aggregate)); + + OntapResponse svmResponse = new OntapResponse<>(); + svmResponse.setRecords(List.of(svm)); + + when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse); + + // Aggregate is ONLINE but has far less free space than the configured pool size (5GB). + Aggregate aggregateDetail = mock(Aggregate.class); + when(aggregateDetail.getName()).thenReturn("aggr1"); + when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1"); + when(aggregateDetail.getState()).thenReturn(Aggregate.StateEnum.ONLINE); + when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"))).thenReturn(aggregateDetail); + + // Execute & Verify - connect() should succeed regardless of available space. + boolean result = storageStrategy.connect(); + assertTrue(result, "connect() should succeed for an online aggregate even when its free space is below the pool capacity"); + } + + @Test + public void testConnect_noOnlineAggregates() { + // Setup - aggregate assigned to the SVM exists but is not ONLINE + Svm svm = new Svm(); + svm.setName("svm1"); + svm.setState(OntapStorageConstants.RUNNING); + svm.setNfsEnabled(true); + + Aggregate aggregate = new Aggregate(); + aggregate.setName("aggr1"); + aggregate.setUuid("aggr-uuid-1"); + svm.setAggregates(List.of(aggregate)); + + OntapResponse svmResponse = new OntapResponse<>(); + svmResponse.setRecords(List.of(svm)); + + when(svmFeignClient.getSvmResponse(anyMap(), anyString())).thenReturn(svmResponse); + + Aggregate aggregateDetail = mock(Aggregate.class); + when(aggregateDetail.getName()).thenReturn("aggr1"); + when(aggregateDetail.getUuid()).thenReturn("aggr-uuid-1"); + when(aggregateDetail.getState()).thenReturn(null); // not online + when(aggregateFeignClient.getAggregateByUUID(anyString(), eq("aggr-uuid-1"))).thenReturn(aggregateDetail); + + // Execute & Verify + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect()); + assertTrue(ex.getMessage().contains("No suitable aggregates found")); + } + @Test public void testConnect_svmNotFound() { // Setup @@ -342,6 +405,20 @@ public void testConnect_nullSvmResponse() { assertTrue(ex.getMessage().contains("No SVM found")); } + @Test + public void testConnect_invalidCredentials() { + // Setup - ONTAP rejects the supplied username/password with HTTP 401 Unauthorized. + when(svmFeignClient.getSvmResponse(anyMap(), anyString())) + .thenThrow(mock(FeignException.Unauthorized.class)); + + // Execute & Verify - connect() must surface a clear "invalid credentials" error. + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, () -> storageStrategy.connect()); + assertTrue(ex.getMessage().contains("Authentication failed: Invalid credentials"), + "Expected an authentication failure message but got: " + ex.getMessage()); + assertTrue(ex.getMessage().contains("Please verify the username and password"), + "Expected the message to prompt verifying username/password but got: " + ex.getMessage()); + } + // ========== createStorageVolume() Tests ========== @Test diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index 1c0c84ef91dd..ec9023a6c760 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -304,7 +304,7 @@ void testCreateAccessGroup_Success() { List hosts = new ArrayList<>(); HostVO host1 = mock(HostVO.class); - when(host1.getName()).thenReturn("host1"); + when(host1.getUuid()).thenReturn("host1"); when(host1.getStorageUrl()).thenReturn("iqn.1993-08.org.debian:01:host1"); hosts.add(host1); accessGroup.setHostsToConnect(hosts); @@ -357,7 +357,7 @@ void testCreateAccessGroup_AlreadyExists_ReturnsSuccessfully() { List hosts = new ArrayList<>(); HostVO host1 = mock(HostVO.class); - when(host1.getName()).thenReturn("host1"); + when(host1.getUuid()).thenReturn("host1"); when(host1.getStorageUrl()).thenReturn("iqn.1993-08.org.debian:01:host1"); hosts.add(host1); accessGroup.setHostsToConnect(hosts); @@ -396,7 +396,7 @@ void testDeleteAccessGroup_Success() { List hosts = new ArrayList<>(); HostVO host1 = mock(HostVO.class); - lenient().when(host1.getName()).thenReturn("host1"); + lenient().when(host1.getUuid()).thenReturn("host1"); hosts.add(host1); accessGroup.setHostsToConnect(hosts); @@ -444,7 +444,7 @@ void testDeleteAccessGroup_NotFound_SkipsDeletion() { List hosts = new ArrayList<>(); HostVO host1 = mock(HostVO.class); - lenient().when(host1.getName()).thenReturn("host1"); + lenient().when(host1.getUuid()).thenReturn("host1"); hosts.add(host1); accessGroup.setHostsToConnect(hosts); @@ -1042,7 +1042,7 @@ void testDeleteAccessGroup_EmptyIgroupUuid_ThrowsException() { List hosts = new ArrayList<>(); HostVO host1 = mock(HostVO.class); - when(host1.getName()).thenReturn("host1"); + when(host1.getUuid()).thenReturn("host1"); hosts.add(host1); accessGroup.setHostsToConnect(hosts); @@ -1079,7 +1079,7 @@ void testDeleteAccessGroup_FeignExceptionNon404_ThrowsException() { List hosts = new ArrayList<>(); HostVO host1 = mock(HostVO.class); - when(host1.getName()).thenReturn("host1"); + when(host1.getUuid()).thenReturn("host1"); hosts.add(host1); accessGroup.setHostsToConnect(hosts); @@ -1728,7 +1728,7 @@ void testDeleteAccessGroup_FeignException404_SkipsDeletion() { accessGroup.setStoragePoolId(1L); List hosts = new ArrayList<>(); HostVO host1 = mock(HostVO.class); - when(host1.getName()).thenReturn("host1"); + when(host1.getUuid()).thenReturn("host1"); hosts.add(host1); accessGroup.setHostsToConnect(hosts); @@ -1754,7 +1754,7 @@ void testDeleteAccessGroup_NotFoundInResponse_SkipsDeletion() { accessGroup.setStoragePoolId(1L); List hosts = new ArrayList<>(); HostVO host1 = mock(HostVO.class); - when(host1.getName()).thenReturn("host1"); + when(host1.getUuid()).thenReturn("host1"); hosts.add(host1); accessGroup.setHostsToConnect(hosts); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java new file mode 100644 index 000000000000..1772b92ed179 --- /dev/null +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.storage.utils; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class OntapStorageUtilsTest { + + @Test + public void getIgroupName_returnsExpectedFormat_whenWithinLimit() { + String result = OntapStorageUtils.getIgroupName("svm1", "host-uuid-123"); + + assertEquals("cs_svm1_host-uuid-123", result); + assertTrue(result.length() <= OntapStorageConstants.IGROUP_NAME_MAX_LENGTH); + } + + @Test + public void getIgroupName_sanitizesInvalidCharacters() { + // Characters outside [a-zA-Z0-9_-] in the host uuid must be replaced with '_'. + String result = OntapStorageUtils.getIgroupName("svm1", "host.uuid:123/abc"); + + assertEquals("cs_svm1_host_uuid_123_abc", result); + } + + @Test + public void getIgroupName_doesNotTruncate_whenExactlyAtMaxLength() { + // Format: cs(2) + _(1) + svmName + _(1) + hostUuid + // For an overall length of 96 with a 4-char uuid, svmName must be 88 chars. + String svmName = "a".repeat(88); + String hostUuid = "uuid"; + + String result = OntapStorageUtils.getIgroupName(svmName, hostUuid); + + assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length()); + assertEquals("cs_" + svmName + "_" + hostUuid, result); + } + + @Test + public void getIgroupName_truncates_whenExceedingMaxLength() { + String svmName = "a".repeat(200); + String hostUuid = "host-uuid-123"; + + String result = OntapStorageUtils.getIgroupName(svmName, hostUuid); + + assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length()); + // The truncated value must still be a prefix of the full, untruncated name. + String fullName = "cs_" + svmName + "_" + hostUuid; + assertEquals(fullName.substring(0, OntapStorageConstants.IGROUP_NAME_MAX_LENGTH), result); + assertTrue(result.startsWith("cs_")); + } + + @Test + public void getIgroupName_truncates_whenOneCharOverMaxLength() { + // Build a name that is exactly one character over the limit (97 chars): + // svmName of 89 chars + 4-char uuid -> 2 + 1 + 89 + 1 + 4 = 97. + String svmName = "a".repeat(89); + String hostUuid = "uuid"; + + String result = OntapStorageUtils.getIgroupName(svmName, hostUuid); + + assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length()); + } +} From af4f23ed5afef119f1d186150b0a03c2344c80b6 Mon Sep 17 00:00:00 2001 From: piyush5netapp <91685498+piyush5netapp@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:29:06 +0530 Subject: [PATCH 4/7] Feature/cstackex 211: Add ONTAP provider check for Copy root volume template flow (#75) ### Description This PR... ### 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) - [x] 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): image ### How Has This Been Tested? #### How did you try to break this feature and the system with this change? Co-authored-by: Srivastava, Piyush --- .../storage/volume/VolumeServiceImpl.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 8a0f93fe5fc3..84b4a3e0c703 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -48,6 +48,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; @@ -1363,11 +1364,13 @@ private void createManagedVolumeCopyTemplateAsync(VolumeInfo volumeInfo, Primary primaryDataStore.setDetails(details); grantAccess(volumeInfo, destHost, primaryDataStore); - volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore); - // For Netapp ONTAP iscsiName or Lun path is available only after grantAccess - String managedStoreTarget = ObjectUtils.defaultIfNull(volumeInfo.get_iScsiName(), volumeInfo.getUuid()); - details.put(PrimaryDataStore.MANAGED_STORE_TARGET, managedStoreTarget); - primaryDataStore.setDetails(details); + if (DataStoreProvider.ONTAP_PLUGIN_NAME.equals(primaryDataStore.getStorageProviderName())) { + // For Netapp ONTAP iscsiName or Lun path is available only after grantAccess + volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore); + String managedStoreTarget = ObjectUtils.defaultIfNull(volumeInfo.get_iScsiName(), volumeInfo.getUuid()); + details.put(PrimaryDataStore.MANAGED_STORE_TARGET, managedStoreTarget); + primaryDataStore.setDetails(details); + } try { motionSrv.copyAsync(srcTemplateInfo, destTemplateInfo, destHost, caller); From c3a0117ef0cdd9d0e6399796d27aca82dd4f3624 Mon Sep 17 00:00:00 2001 From: piyush5netapp <91685498+piyush5netapp@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:32:21 +0530 Subject: [PATCH 5/7] Feature/cstackex 213: Upstream PR 13578 comments resolution (#78) ### Description This PR... ### 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 image VM instance image ONTAP flexvol image ONTAP Luns image ONTAP Igroup image ### How Has This Been Tested? #### How did you try to break this feature and the system with this change? --------- Co-authored-by: Srivastava, Piyush --- .../OntapPrimaryDatastoreLifecycle.java | 6 +---- .../storage/service/StorageStrategy.java | 2 +- .../OntapPrimaryDatastoreLifecycleTest.java | 26 +++++++++---------- .../cloud/storage/VolumeApiServiceImpl.java | 5 +++- 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java index a098c68c3d61..ab852971d956 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java @@ -341,10 +341,6 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper logger.error("attachZone : ONTAP primary storage is supported only for KVM hypervisor"); throw new CloudRuntimeException("ONTAP primary storage is supported only for KVM hypervisor"); } - storagePool.setHypervisor(hypervisorType); - storagePoolDao.update(storagePool.getId(),storagePool); - logger.debug("attachZone : Set Hypervisor type for storage pool {} to {}", storagePool.getName(), hypervisorType); - PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore; List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), hypervisorType); logger.debug(String.format("In createPool. Attaching the pool to each of the hosts in %s.", hostsToConnect)); @@ -371,7 +367,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper return false; } } - _dataStoreHelper.attachZone(dataStore); + _dataStoreHelper.attachZone(dataStore, hypervisorType); return true; } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index f4ab806d6885..c13b255c67ea 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -239,7 +239,7 @@ public Volume createStorageVolume(String volumeName, Long size) { final long availableBytes = aggrResp.getAvailableBlockStorageSpace().longValue(); logger.debug("Aggregate " + aggr.getName() + " available bytes=" + availableBytes + ", requested=" + size); - if (availableBytes <= size) { + if (availableBytes < size) { logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Required=" + size + " bytes, available=" + availableBytes + " bytes. Skipping this aggregate."); continue; diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java index 3b576df97e37..751b864ecfcc 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java @@ -612,7 +612,7 @@ public void testAttachZone_positive() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(mockHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -633,7 +633,7 @@ public void testAttachZone_positive() throws Exception { verify(storagePoolDetailsDao, times(1)).listDetailsKeyPairs(1L); verify(storageStrategy, times(1)).createAccessGroup(any(AccessGroup.class)); verify(_storageMgr, times(2)).connectHostToSharedPool(any(HostVO.class), eq(1L)); - verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -647,7 +647,7 @@ public void testAttachZone_withSingleHost() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(singleHost); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -662,7 +662,7 @@ public void testAttachZone_withSingleHost() throws Exception { // Verify assertTrue(result, "attachZone should return true with single host"); verify(_storageMgr, times(1)).connectHostToSharedPool(any(HostVO.class), eq(1L)); - verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -679,7 +679,7 @@ public void testAttachZone_withMultipleHosts() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(mockHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -694,7 +694,7 @@ public void testAttachZone_withMultipleHosts() throws Exception { // Verify assertTrue(result, "attachZone should return true with multiple hosts"); verify(_storageMgr, times(3)).connectHostToSharedPool(any(HostVO.class), eq(1L)); - verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -724,7 +724,7 @@ public void testAttachZone_hostConnectionFailure() throws Exception { verify(storageStrategy, times(1)).createAccessGroup(any(AccessGroup.class)); verify(_storageMgr, times(1)).connectHostToSharedPool(any(HostVO.class), eq(1L)); // _dataStoreHelper.attachZone should NOT be called due to early return - verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -737,7 +737,7 @@ public void testAttachZone_emptyHostList() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(emptyHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -751,7 +751,7 @@ public void testAttachZone_emptyHostList() throws Exception { // Verify assertTrue(result, "attachZone should return true even with no hosts"); verify(_storageMgr, times(0)).connectHostToSharedPool(any(HostVO.class), anyLong()); - verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -780,7 +780,7 @@ public void testAttachZone_secondHostConnectionFails() throws Exception { // Verify assertFalse(result, "attachZone should return false when any host connection fails"); verify(_storageMgr, times(2)).connectHostToSharedPool(any(HostVO.class), eq(1L)); - verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -791,7 +791,7 @@ public void testAttachZone_createAccessGroupCalled() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(mockHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -838,7 +838,7 @@ public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(mockHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -850,7 +850,7 @@ public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception { 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)); } } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 4555b4485970..78d35424e5aa 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -64,6 +64,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.engine.subsystem.api.storage.HostScope; @@ -5096,7 +5097,9 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L // Reload volume from DB after grantAccess — managed storage drivers (e.g. ONTAP) // may update the volume's path and iScsiName during grantAccess, so the local // volumeToAttach object can be stale. - volumeToAttach = _volsDao.findById(volumeToAttach.getId()); + if(DataStoreProvider.ONTAP_PLUGIN_NAME.equals(volumeToAttachStoragePool.getStorageProviderName())){ + volumeToAttach = _volsDao.findById(volumeToAttach.getId()); + } } if (sendCommand) { From e11a2ed7e3a563a7ce71247f4634b5df0182e3ab Mon Sep 17 00:00:00 2001 From: piyush5netapp <91685498+piyush5netapp@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:57:06 +0530 Subject: [PATCH 6/7] feature/CSTACKEX-217: Mockito upgrade for supporting java21,17 and 11 (#80) --- plugins/storage/volume/ontap/pom.xml | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/plugins/storage/volume/ontap/pom.xml b/plugins/storage/volume/ontap/pom.xml index ce9b8683ebaf..7af43a2325ff 100644 --- a/plugins/storage/volume/ontap/pom.xml +++ b/plugins/storage/volume/ontap/pom.xml @@ -37,9 +37,7 @@ 2.13.4 3.24.2 5.8.1 - 3.12.4 - 5.2.0 - 1.11.13 + 1.15.11 @@ -105,21 +103,13 @@ org.mockito mockito-core - ${mockito.version} + ${cs.mockito.version} test org.mockito mockito-junit-jupiter - ${mockito-junit-jupiter.version} - test - - - - - org.mockito - mockito-inline - ${mockito.version} + ${cs.mockito.version} test @@ -163,7 +153,6 @@ maven-surefire-plugin ${maven-surefire-plugin.version} - false **/*Test.java From 3425369e634c796c50e36f79ac70849abe96fcba Mon Sep 17 00:00:00 2001 From: Surya Gupta Date: Fri, 10 Jul 2026 21:48:18 +0530 Subject: [PATCH 7/7] [CSTACKEX-216] Review Comments for Igroup Name Length (#79) ### Description Issue: "getIgroupName() truncates the full "cs__" string as a whole. If svmName is long, this can truncate away most or all of the hostUuid portion, increasing the chance of igroup name collisions across different hosts (and also throws a NullPointerException if hostUuid is null). Consider validating inputs and truncating svmName first so the host UUID remains intact (or, if hostUuid itself is too long, truncate only the UUID)." Fix: In typical deployments, truncation likely never triggers bcz total length will remain less than 96. But to avoid any issues for future where svm limits get exceeded, will change the igroup format to cs_hostUuid_svmName Note: This is a breaking change for existing deployments. Igroups already on ONTAP use the old format (cs_svm1_hostname). After this change, CloudStack will look for cs__svm1 and may not find existing igroup ### 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): image image ### How Has This Been Tested? 1- Created 1st VM on ISCSI Storage pool - Igroup got create with new name format attached a screen shot and lun got mapped 2- Create 2nd VM on same storage pool - Igroup got reused and lun got mapped 3- Storage Pool to enter maintenance mode - All Luns got unmapped and igroup also got deleted 4- Storage Pool to cancel maintenance mode - New Igroup got created with same name format and All Luns got mapped again. 5-Create a new CloudStack volume and attched to the instance - used the existing igroup and mapped the lun 6- Delete All VMs - Igroup got deleted #### How did you try to break this feature and the system with this change? Co-authored-by: Gupta, Surya --- .../cloudstack/storage/utils/OntapStorageUtils.java | 4 ++-- .../storage/utils/OntapStorageUtilsTest.java | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java index 66fd41d5123d..8a74e77b3371 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java @@ -140,9 +140,9 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map OntapStorageConstants.IGROUP_NAME_MAX_LENGTH) { igroupName = igroupName.substring(0, OntapStorageConstants.IGROUP_NAME_MAX_LENGTH); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java index 1772b92ed179..372a75ad257d 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java @@ -29,7 +29,7 @@ public class OntapStorageUtilsTest { public void getIgroupName_returnsExpectedFormat_whenWithinLimit() { String result = OntapStorageUtils.getIgroupName("svm1", "host-uuid-123"); - assertEquals("cs_svm1_host-uuid-123", result); + assertEquals("cs_host-uuid-123_svm1", result); assertTrue(result.length() <= OntapStorageConstants.IGROUP_NAME_MAX_LENGTH); } @@ -38,12 +38,12 @@ public void getIgroupName_sanitizesInvalidCharacters() { // Characters outside [a-zA-Z0-9_-] in the host uuid must be replaced with '_'. String result = OntapStorageUtils.getIgroupName("svm1", "host.uuid:123/abc"); - assertEquals("cs_svm1_host_uuid_123_abc", result); + assertEquals("cs_host_uuid_123_abc_svm1", result); } @Test public void getIgroupName_doesNotTruncate_whenExactlyAtMaxLength() { - // Format: cs(2) + _(1) + svmName + _(1) + hostUuid + // Format: cs(2) + _(1) + hostUuid + _(1) + svmName // For an overall length of 96 with a 4-char uuid, svmName must be 88 chars. String svmName = "a".repeat(88); String hostUuid = "uuid"; @@ -51,7 +51,7 @@ public void getIgroupName_doesNotTruncate_whenExactlyAtMaxLength() { String result = OntapStorageUtils.getIgroupName(svmName, hostUuid); assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length()); - assertEquals("cs_" + svmName + "_" + hostUuid, result); + assertEquals("cs_" + hostUuid + "_" + svmName, result); } @Test @@ -63,7 +63,7 @@ public void getIgroupName_truncates_whenExceedingMaxLength() { assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length()); // The truncated value must still be a prefix of the full, untruncated name. - String fullName = "cs_" + svmName + "_" + hostUuid; + String fullName = "cs_" + hostUuid + "_" + svmName; assertEquals(fullName.substring(0, OntapStorageConstants.IGROUP_NAME_MAX_LENGTH), result); assertTrue(result.startsWith("cs_")); } @@ -71,7 +71,7 @@ public void getIgroupName_truncates_whenExceedingMaxLength() { @Test public void getIgroupName_truncates_whenOneCharOverMaxLength() { // Build a name that is exactly one character over the limit (97 chars): - // svmName of 89 chars + 4-char uuid -> 2 + 1 + 89 + 1 + 4 = 97. + // svmName of 89 chars + 4-char uuid -> 2 + 1 + 4 + 1 + 89 = 97. String svmName = "a".repeat(89); String hostUuid = "uuid";