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); 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 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/lifecycle/OntapPrimaryDatastoreLifecycle.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java index 9854f02e6960..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 @@ -337,9 +337,12 @@ 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"); + } 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()); @@ -364,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 bd808a26d6f8..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 @@ -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)) { @@ -227,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; @@ -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..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 @@ -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/lifecycle/OntapPrimaryDatastoreLifecycleTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java index 604ab400474c..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 @@ -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)) { @@ -605,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())) @@ -626,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)); } } @@ -640,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())) @@ -655,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)); } } @@ -672,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())) @@ -687,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)); } } @@ -717,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)); } } @@ -730,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())) @@ -744,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)); } } @@ -773,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)); } } @@ -784,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())) @@ -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), eq(Hypervisor.HypervisorType.KVM))).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(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); + } + } + } 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..372a75ad257d --- /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_host-uuid-123_svm1", 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_host_uuid_123_abc_svm1", result); + } + + @Test + public void getIgroupName_doesNotTruncate_whenExactlyAtMaxLength() { + // 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"; + + String result = OntapStorageUtils.getIgroupName(svmName, hostUuid); + + assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length()); + assertEquals("cs_" + hostUuid + "_" + svmName, 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_" + hostUuid + "_" + svmName; + 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 + 4 + 1 + 89 = 97. + String svmName = "a".repeat(89); + String hostUuid = "uuid"; + + String result = OntapStorageUtils.getIgroupName(svmName, hostUuid); + + assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length()); + } +} diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 39686b4d3843..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; @@ -5092,6 +5093,13 @@ 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. + if(DataStoreProvider.ONTAP_PLUGIN_NAME.equals(volumeToAttachStoragePool.getStorageProviderName())){ + volumeToAttach = _volsDao.findById(volumeToAttach.getId()); + } } if (sendCommand) {