From 9c85c6b24e1fd9b8c9878edef6e757230461eed8 Mon Sep 17 00:00:00 2001 From: "Locharla, Sandeep" Date: Mon, 10 Nov 2025 18:35:00 +0530 Subject: [PATCH 01/29] CSTACKEX-46: Disable, Re-Enable, Delete Storage pool and Enter, Exit Storage pool workflows --- .../storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 2cdd7de0b7c5..5c0a8f05eee2 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 @@ -42,6 +42,7 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDetailsDao; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDetailsDao; import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl; import org.apache.cloudstack.storage.feign.model.ExportPolicy; import org.apache.cloudstack.storage.feign.model.OntapStorage; @@ -67,10 +68,10 @@ public class OntapPrimaryDatastoreLifecycle extends BasePrimaryDataStoreLifeCycl @Inject private StorageManager _storageMgr; @Inject private ResourceManager _resourceMgr; @Inject private PrimaryDataStoreHelper _dataStoreHelper; - @Inject private PrimaryDataStoreDao storagePoolDao; - @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @Inject private PrimaryDataStoreDetailsDao _datastoreDetailsDao; @Inject private StoragePoolAutomation _storagePoolAutomation; + @Inject private PrimaryDataStoreDao storagePoolDao; + @Inject private StoragePoolDetailsDao storagePoolDetailsDao; private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreLifecycle.class); // ONTAP minimum volume size is 1.56 GB (1677721600 bytes) @@ -411,7 +412,6 @@ public boolean deleteDataStore(DataStore store) { return _dataStoreHelper.deletePrimaryDataStore(store); } - @Override public boolean migrateToObjectStore(DataStore store) { return true; From 41e832491549d441ac6b49ad7cc4f5f77c3106da Mon Sep 17 00:00:00 2001 From: "Locharla, Sandeep" Date: Tue, 18 Nov 2025 11:31:54 +0530 Subject: [PATCH 02/29] CSTACKEX-50: Fixed some issues seen while testing --- .../main/java/org/apache/cloudstack/storage/utils/Constants.java | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java index 23425aa6b797..e71a26577fa9 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java @@ -55,6 +55,7 @@ public class Constants { public static final String FIELDS = "fields"; public static final String AGGREGATES = "aggregates"; public static final String STATE = "state"; + public static final String SVMDOTNAME = "svm.name"; public static final String DATA_NFS = "data_nfs"; public static final String DATA_ISCSI = "data_iscsi"; public static final String IP_ADDRESS = "ip.address"; From 7f47d930b606f99d17fc6f3d2a0e69762a1ab352 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 5 Nov 2025 15:28:09 +0530 Subject: [PATCH 03/29] CSTACKEX-46 Create Async, Attach Cluster/Zone and Grant/Revoke Access --- .../driver/OntapPrimaryDatastoreDriver.java | 161 +++++++++++++++++- .../OntapPrimaryDatastoreLifecycle.java | 78 +++++++++ .../storage/service/StorageStrategy.java | 59 +++---- .../storage/service/UnifiedNASStrategy.java | 8 +- .../storage/service/UnifiedSANStrategy.java | 91 ++++++++-- .../cloudstack/storage/utils/Utility.java | 65 +++++++ 6 files changed, 410 insertions(+), 52 deletions(-) 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 5e79aa2298da..f4322f27226f 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 @@ -27,6 +27,9 @@ import com.cloud.storage.Storage; import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.ScopeType; +import com.cloud.storage.dao.VolumeDao; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; @@ -45,6 +48,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.utils.Constants; @@ -62,7 +66,7 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @Inject private PrimaryDataStoreDao storagePoolDao; - + @Inject private VolumeDao volumeDao; @Override public Map getCapabilities() { s_logger.trace("OntapPrimaryDatastoreDriver: getCapabilities: Called"); @@ -98,8 +102,13 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet throw new InvalidParameterValueException("createAsync: callback should not be null"); } try { - s_logger.info("createAsync: Started for data store [{}] and data object [{}] of type [{}]", - dataStore, dataObject, dataObject.getType()); + s_logger.info("createAsync: Started for data store [{}] and data object [{}] of type [{}]", dataStore, dataObject, dataObject.getType()); + + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); + if(storagePool == null) { + s_logger.error("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); + throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); + } if (dataObject.getType() == DataObjectType.VOLUME) { VolumeInfo volumeInfo = (VolumeInfo) dataObject; path = createCloudStackVolumeForTypeVolume(dataStore, volumeInfo); @@ -197,11 +206,157 @@ public ChapInfo getChapInfo(DataObject dataObject) { @Override public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore) { + if (dataStore == null) { + throw new InvalidParameterValueException("grantAccess: dataStore should not be null"); + } + if (dataObject == null) { + throw new InvalidParameterValueException("grantAccess: dataObject should not be null"); + } + if (host == null) { + throw new InvalidParameterValueException("grantAccess: host should not be null"); + } + try { + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); + if(storagePool == null) { + s_logger.error("grantAccess : Storage Pool not found for id: " + dataStore.getId()); + throw new CloudRuntimeException("grantAccess : Storage Pool not found for id: " + dataStore.getId()); + } + if (storagePool.getScope() != ScopeType.CLUSTER && storagePool.getScope() != ScopeType.ZONE) { + s_logger.error("grantAccess: Only Cluster and Zone scoped primary storage is supported for storage Pool: " + storagePool.getName()); + throw new CloudRuntimeException("grantAccess: Only Cluster and Zone scoped primary storage is supported for Storage Pool: " + storagePool.getName()); + } + + if (dataObject.getType() == DataObjectType.VOLUME) { + VolumeVO volumeVO = volumeDao.findById(dataObject.getId()); + if(volumeVO == null) { + s_logger.error("grantAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); + throw new CloudRuntimeException("grantAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); + } + grantAccessForVolume(storagePool, volumeVO, host); + } else { + s_logger.error("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); + throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); + } + } catch(Exception e){ + s_logger.error("grantAccess: Failed for dataObject [{}]: {}", dataObject, e.getMessage()); + throw new CloudRuntimeException("grantAccess: Failed with error :" + e.getMessage()); + } return true; } + private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Host host) { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); + String svmName = details.get(Constants.SVM_NAME); + long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); + + if(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + String accessGroupName = Utility.getIgroupName(svmName, scopeId); + CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); + s_logger.info("grantAccessForVolume: Retrieved LUN [{}] details for volume [{}]", cloudStackVolume.getLun().getName(), volumeVO.getName()); + AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); + if(accessGroup.getIgroup().getInitiators() == null || accessGroup.getIgroup().getInitiators().size() == 0 || !accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) { + s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); + throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName); + } + + Map enableLogicalAccessMap = new HashMap<>(); + enableLogicalAccessMap.put(Constants.LUN_DOT_NAME, volumeVO.getPath()); + enableLogicalAccessMap.put(Constants.SVM_DOT_NAME, svmName); + enableLogicalAccessMap.put(Constants.IGROUP_DOT_NAME, accessGroupName); + storageStrategy.enableLogicalAccess(enableLogicalAccessMap); + } else { + String errMsg = "grantAccessForVolume: Unsupported protocol type for volume grantAccess: " + details.get(Constants.PROTOCOL); + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + } + @Override public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { + if (dataStore == null) { + throw new InvalidParameterValueException("revokeAccess: data store should not be null"); + } + if (dataObject == null) { + throw new InvalidParameterValueException("revokeAccess: data object should not be null"); + } + if (host == null) { + throw new InvalidParameterValueException("revokeAccess: host should not be null"); + } + try { + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); + if(storagePool == null) { + s_logger.error("revokeAccess : Storage Pool not found for id: " + dataStore.getId()); + throw new CloudRuntimeException("revokeAccess : Storage Pool not found for id: " + dataStore.getId()); + } + if (storagePool.getScope() != ScopeType.CLUSTER && storagePool.getScope() != ScopeType.ZONE) { + s_logger.error("revokeAccess: Only Cluster and Zone scoped primary storage is supported for storage Pool: " + storagePool.getName()); + throw new CloudRuntimeException("revokeAccess: Only Cluster and Zone scoped primary storage is supported for Storage Pool: " + storagePool.getName()); + } + + if (dataObject.getType() == DataObjectType.VOLUME) { + VolumeVO volumeVO = volumeDao.findById(dataObject.getId()); + if(volumeVO == null) { + s_logger.error("revokeAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); + throw new CloudRuntimeException("revokeAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); + } + revokeAccessForVolume(storagePool, volumeVO, host); + } else { + s_logger.error("revokeAccess: Invalid DataObjectType (" + dataObject.getType() + ") passed to revokeAccess"); + throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to revokeAccess"); + } + } catch(Exception e){ + s_logger.error("revokeAccess: Failed for dataObject [{}]: {}", dataObject, e.getMessage()); + throw new CloudRuntimeException("revokeAccess: Failed with error :" + e.getMessage()); + } + } + + private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Host host) { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); + String svmName = details.get(Constants.SVM_NAME); + long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); + + if(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + String accessGroupName = Utility.getIgroupName(svmName, scopeId); + CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); + AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); + //TODO check if initiator does exits in igroup, will throw the error ? + if(!accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) { + s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); + throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName); + } + + Map disableLogicalAccessMap = new HashMap<>(); + disableLogicalAccessMap.put(Constants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid().toString()); + disableLogicalAccessMap.put(Constants.IGROUP_DOT_UUID, accessGroup.getIgroup().getUuid()); + storageStrategy.disableLogicalAccess(disableLogicalAccessMap); + } + } + + + private CloudStackVolume getCloudStackVolumeByName(StorageStrategy storageStrategy, String svmName, String cloudStackVolumeName) { + Map getCloudStackVolumeMap = new HashMap<>(); + getCloudStackVolumeMap.put(Constants.NAME, cloudStackVolumeName); + getCloudStackVolumeMap.put(Constants.SVM_DOT_NAME, svmName); + CloudStackVolume cloudStackVolume = storageStrategy.getCloudStackVolume(getCloudStackVolumeMap); + if(cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getName() == null) { + s_logger.error("getCloudStackVolumeByName: Failed to get LUN details [{}]", cloudStackVolumeName); + throw new CloudRuntimeException("getCloudStackVolumeByName: Failed to get LUN [" + cloudStackVolumeName + "]"); + } + return cloudStackVolume; + } + + private AccessGroup getAccessGroupByName(StorageStrategy storageStrategy, String svmName, String accessGroupName) { + Map getAccessGroupMap = new HashMap<>(); + getAccessGroupMap.put(Constants.NAME, accessGroupName); + getAccessGroupMap.put(Constants.SVM_DOT_NAME, svmName); + AccessGroup accessGroup = storageStrategy.getAccessGroup(getAccessGroupMap); + if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getName() == null) { + s_logger.error("getAccessGroupByName: Failed to get iGroup details [{}]", accessGroupName); + throw new CloudRuntimeException("getAccessGroupByName: Failed to get iGroup details [" + accessGroupName + "]"); + } + return accessGroup; } @Override 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 5c0a8f05eee2..8ae2036f468a 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 @@ -23,6 +23,7 @@ import com.cloud.agent.api.StoragePoolInfo; import com.cloud.dc.ClusterVO; import com.cloud.dc.dao.ClusterDao; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor; import com.cloud.resource.ResourceManager; @@ -58,6 +59,7 @@ import org.apache.logging.log4j.Logger; import javax.inject.Inject; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -288,6 +290,22 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(primaryStore); logger.debug(" datastore object received is {} ",primaryStore ); + if (dataStore == null) { + throw new InvalidParameterValueException("attachCluster: dataStore should not be null"); + } + if (scope == null) { + throw new InvalidParameterValueException("attachCluster: scope should not be null"); + } + List hostsIdentifier = new ArrayList<>(); + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); + if(storagePool == null) { + s_logger.error("attachCluster : Storage Pool not found for id: " + dataStore.getId()); + throw new CloudRuntimeException("attachCluster : Storage Pool not found for id: " + dataStore.getId()); + } + PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore; + List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(primaryStore); + // TODO- need to check if no host to connect then throw exception or just continue + logger.debug("attachCluster: Eligible Up and Enabled hosts: {} in cluster {}", hostsToConnect, primaryStore.getClusterId()); logger.debug(String.format("Attaching the pool to each of the hosts %s in the cluster: %s", hostsToConnect, primaryStore.getClusterId())); @@ -302,6 +320,20 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { accessGroupRequest.setPolicy(exportPolicy); strategy.createAccessGroup(accessGroupRequest); + logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); + Map details = primaryStore.getDetails(); + StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); + ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); + //TODO- Check if we have to handle heterogeneous host within the cluster + if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) { + s_logger.error("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name()); + throw new CloudRuntimeException("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name()); + } + //TODO - check if no host to connect then also need to create access group without initiators + if (hostsIdentifier != null && hostsIdentifier.size() > 0) { + AccessGroup accessGroupRequest = Utility.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); + strategy.createAccessGroup(accessGroupRequest); + } logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); for (HostVO host : hostsToConnect) { try { @@ -309,6 +341,7 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { } catch (Exception e) { logger.warn("Unable to establish a connection between " + host + " and " + dataStore, e); return false; + logger.warn("attachCluster: Unable to establish a connection between " + host + " and " + dataStore, e); } } _dataStoreHelper.attachCluster(dataStore); @@ -323,6 +356,18 @@ public boolean attachHost(DataStore store, HostScope scope, StoragePoolInfo exis @Override public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.HypervisorType hypervisorType) { logger.debug("In attachZone for ONTAP primary storage"); + if (dataStore == null) { + throw new InvalidParameterValueException("attachZone: dataStore should not be null"); + } + if (scope == null) { + throw new InvalidParameterValueException("attachZone: scope should not be null"); + } + List hostsIdentifier = new ArrayList<>(); + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); + if(storagePool == null) { + s_logger.error("attachZone : Storage Pool not found for id: " + dataStore.getId()); + throw new CloudRuntimeException("attachZone : Storage Pool not found for id: " + dataStore.getId()); + } PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore; List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), Hypervisor.HypervisorType.KVM); @@ -339,6 +384,21 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper accessGroupRequest.setPolicy(exportPolicy); strategy.createAccessGroup(accessGroupRequest); + // TODO- need to check if no host to connect then throw exception or just continue + logger.debug("attachZone: Eligible Up and Enabled hosts: {}", hostsToConnect); + + Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); + StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); + ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); + //TODO- Check if we have to handle heterogeneous host within the zone + if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) { + s_logger.error("attachZone: Not all hosts in the cluster support the protocol: " + protocol.name()); + throw new CloudRuntimeException("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); + } + if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) { + AccessGroup accessGroupRequest = Utility.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); + strategy.createAccessGroup(accessGroupRequest); + } for (HostVO host : hostsToConnect) { try { _storageMgr.connectHostToSharedPool(host, dataStore.getId()); @@ -351,6 +411,24 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper return true; } + private boolean isProtocolSupportedByAllHosts(List hosts, ProtocolType protocolType, List hostIdentifiers) { + switch (protocolType) { + case ISCSI: + String protocolPrefix = Constants.IQN; + for (HostVO host : hosts) { + if (host == null || host.getStorageUrl() == null || host.getStorageUrl().trim().isEmpty() + || !host.getStorageUrl().startsWith(protocolPrefix)) { + return false; + } + hostIdentifiers.add(host.getStorageUrl()); + } + break; + default: + throw new CloudRuntimeException("isProtocolSupportedByAllHosts : Unsupported protocol: " + protocolType.name()); + } + return true; + } + @Override public boolean maintain(DataStore store) { _storagePoolAutomation.maintain(store); 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 822e09851f39..edfda1f4bd2d 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 @@ -147,7 +147,7 @@ public boolean connect() { * throw exception in case of disaggregated ONTAP storage * * @param volumeName the name of the volume to create - * @param size the size of the volume in bytes + * @param size the size of the volume in bytes * @return the created Volume object */ public Volume createStorageVolume(String volumeName, Long size) { @@ -477,21 +477,19 @@ public String getNetworkInterface() { /** * Method encapsulates the behavior based on the opted protocol in subclasses. * it is going to mimic - * getLun for iSCSI, FC protocols - * getFile for NFS3.0 and NFS4.1 protocols - * getNameSpace for Nvme/TCP and Nvme/FC protocol - * - * @param cloudstackVolume the CloudStack volume to retrieve + * getLun for iSCSI, FC protocols + * getFile for NFS3.0 and NFS4.1 protocols + * getNameSpace for Nvme/TCP and Nvme/FC protocol + * @param cloudStackVolumeMap the CloudStack volume to retrieve * @return the retrieved CloudStackVolume object */ - abstract CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume); + abstract public CloudStackVolume getCloudStackVolume(Map cloudStackVolumeMap); /** * Method encapsulates the behavior based on the opted protocol in subclasses - * createiGroup for iSCSI and FC protocols - * createExportPolicy for NFS 3.0 and NFS 4.1 protocols - * createSubsystem for Nvme/TCP and Nvme/FC protocols - * + * createiGroup for iSCSI and FC protocols + * createExportPolicy for NFS 3.0 and NFS 4.1 protocols + * createSubsystem for Nvme/TCP and Nvme/FC protocols * @param accessGroup the access group to create * @return the created AccessGroup object */ @@ -499,20 +497,18 @@ public String getNetworkInterface() { /** * Method encapsulates the behavior based on the opted protocol in subclasses - * deleteiGroup for iSCSI and FC protocols - * deleteExportPolicy for NFS 3.0 and NFS 4.1 protocols - * deleteSubsystem for Nvme/TCP and Nvme/FC protocols - * + * deleteiGroup for iSCSI and FC protocols + * deleteExportPolicy for NFS 3.0 and NFS 4.1 protocols + * deleteSubsystem for Nvme/TCP and Nvme/FC protocols * @param accessGroup the access group to delete */ abstract public void deleteAccessGroup(AccessGroup accessGroup); /** * Method encapsulates the behavior based on the opted protocol in subclasses - * updateiGroup example add/remove-Iqn for iSCSI and FC protocols - * updateExportPolicy example add/remove-Rule for NFS 3.0 and NFS 4.1 protocols - * //TODO for Nvme/TCP and Nvme/FC protocols - * + * updateiGroup example add/remove-Iqn for iSCSI and FC protocols + * updateExportPolicy example add/remove-Rule for NFS 3.0 and NFS 4.1 protocols + * //TODO for Nvme/TCP and Nvme/FC protocols * @param accessGroup the access group to update * @return the updated AccessGroup object */ @@ -520,32 +516,27 @@ public String getNetworkInterface() { /** * Method encapsulates the behavior based on the opted protocol in subclasses - * getiGroup for iSCSI and FC protocols - * getExportPolicy for NFS 3.0 and NFS 4.1 protocols - * getNameSpace for Nvme/TCP and Nvme/FC protocols - * - * @param accessGroup the access group to retrieve - * @return the retrieved AccessGroup object + @@ -306,22 +306,22 @@ public Volume getStorageVolume(Volume volume) + * getNameSpace for Nvme/TCP and Nvme/FC protocols + * @param values */ - abstract AccessGroup getAccessGroup(AccessGroup accessGroup); + abstract public AccessGroup getAccessGroup(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses - * lunMap for iSCSI and FC protocols - * //TODO for Nvme/TCP and Nvme/FC protocols - * + * lunMap for iSCSI and FC protocols + * //TODO for Nvme/TCP and Nvme/FC protocols * @param values */ - abstract void enableLogicalAccess(Map values); + abstract public void enableLogicalAccess(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses - * lunUnmap for iSCSI and FC protocols - * //TODO for Nvme/TCP and Nvme/FC protocols - * + * lunUnmap for iSCSI and FC protocols + * //TODO for Nvme/TCP and Nvme/FC protocols * @param values */ - abstract void disableLogicalAccess(Map values); + abstract public void disableLogicalAccess(Map values); private Boolean jobPollForSuccess(String jobUUID) { //Create URI for GET Job API diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java index b35bedf2ef3c..861d22ff68d9 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java @@ -115,7 +115,7 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) { + public CloudStackVolume getCloudStackVolume(Map cloudStackVolumeMap) { //TODO return null; } @@ -188,18 +188,18 @@ public AccessGroup updateAccessGroup(AccessGroup accessGroup) { } @Override - public AccessGroup getAccessGroup(AccessGroup accessGroup) { + public AccessGroup getAccessGroup(Map values) { //TODO return null; } @Override - void enableLogicalAccess(Map values) { + public void enableLogicalAccess(Map values) { //TODO } @Override - void disableLogicalAccess(Map values) { + public void disableLogicalAccess(Map values) { //TODO } 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 7b5372c69bdd..d594354bd143 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 @@ -27,7 +27,9 @@ import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Initiator; +import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.LunMap; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; @@ -104,7 +106,7 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) { + public CloudStackVolume getCloudStackVolume(Map cloudStackVolumeMap) { //TODO return null; } @@ -305,19 +307,86 @@ public AccessGroup updateAccessGroup(AccessGroup accessGroup) { return null; } - @Override - public AccessGroup getAccessGroup(AccessGroup accessGroup) { - //TODO - return null; + public AccessGroup getAccessGroup(Map values) { + s_logger.info("getAccessGroup : fetching Igroup with params {} ", values); + if (values == null || values.isEmpty()) { + s_logger.error("getAccessGroup: get Igroup failed. Invalid request: {}", values); + throw new CloudRuntimeException("getAccessGroup : get Igroup Failed, invalid request"); + } + String svmName = values.get(Constants.SVM_DOT_NAME); + String igroupName = values.get(Constants.IGROUP_DOT_NAME); + if(svmName == null || igroupName == null || svmName.isEmpty() || igroupName.isEmpty()) { + s_logger.error("getAccessGroup: get Igroup failed. Invalid svm:{} or igroup name: {}", svmName, igroupName); + throw new CloudRuntimeException("getAccessGroup : Fget Igroup failed, invalid request"); + } + try { + // Get AuthHeader + String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // get Igroup + Map queryParams = Map.of(Constants.SVM_DOT_NAME, svmName, Constants.IGROUP_DOT_NAME, igroupName); + OntapResponse igroupResponse = sanFeignClient.getIgroupResponse(authHeader, queryParams); + if (igroupResponse == null || igroupResponse.getRecords() == null || igroupResponse.getRecords().size() == 0) { + s_logger.error("getAccessGroup: Failed to fetch Igroup"); + throw new CloudRuntimeException("Failed to fetch Igroup"); + } + Igroup igroup = igroupResponse.getRecords().get(0); + s_logger.debug("getAccessGroup: Igroup Details : {}", igroup); + s_logger.info("getAccessGroup: Fetched the Igroup successfully. LunName: {}", igroup.getName()); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setIgroup(igroup); + return accessGroup; + } catch (Exception e) { + s_logger.error("Exception occurred while fetching Igroup. Exception: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Igroup details: " + e.getMessage()); + } } - @Override - void enableLogicalAccess(Map values) { - //TODO + public void enableLogicalAccess(Map values) { + s_logger.info("enableLogicalAccess : Creating LunMap with values {} ", values); + LunMap lunMapRequest = new LunMap(); + String svmName = values.get(Constants.SVM_DOT_NAME); + String lunName = values.get(Constants.LUN_DOT_NAME); + String igroupName = values.get(Constants.IGROUP_DOT_NAME); + if(svmName == null || lunName == null || igroupName == null || svmName.isEmpty() || lunName.isEmpty() || igroupName.isEmpty()) { + s_logger.error("enableLogicalAccess: LunMap creation failed. Invalid request values: {}", values); + throw new CloudRuntimeException("enableLogicalAccess : Failed to create LunMap, invalid request"); + } + try { + // Get AuthHeader + String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // Create LunMap + OntapResponse createdLunMap = sanFeignClient.createLunMap(authHeader, true, lunMapRequest); + if (createdLunMap == null || createdLunMap.getRecords() == null || createdLunMap.getRecords().size() == 0) { + s_logger.error("enableLogicalAccess: LunMap failed for Lun: {} and igroup: {}", lunName, igroupName); + throw new CloudRuntimeException("Failed to perform LunMap for Lun: " +lunName+ " and igroup: " + igroupName); + } + LunMap lunMap = createdLunMap.getRecords().get(0); + s_logger.debug("enableLogicalAccess: LunMap created successfully. LunMap: {}", lunMap); + s_logger.info("enableLogicalAccess: LunMap created successfully."); + } catch (Exception e) { + s_logger.error("Exception occurred while creating LunMap: {}. Exception: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage()); + } } - @Override - void disableLogicalAccess(Map values) { - //TODO + public void disableLogicalAccess(Map values) { + s_logger.info("disableLogicalAccess : Deleting LunMap with values {} ", values); + String lunUUID = values.get(Constants.LUN_DOT_UUID); + String igroupUUID = values.get(Constants.IGROUP_DOT_UUID); + if(lunUUID == null || igroupUUID == null || lunUUID.isEmpty() || igroupUUID.isEmpty()) { + s_logger.error("disableLogicalAccess: LunMap deletion failed. Invalid request values: {}", values); + throw new CloudRuntimeException("disableLogicalAccess : Failed to delete LunMap, invalid request"); + } + try { + // Get AuthHeader + String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // LunMap delete + sanFeignClient.deleteLunMap(authHeader, lunUUID, igroupUUID); + s_logger.info("disableLogicalAccess: LunMap deleted successfully."); + } catch (Exception e) { + s_logger.error("Exception occurred while deleting LunMap: {}. Exception: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to delete LunMap: " + e.getMessage()); + } } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index c20c9d6dd151..ebe7017b672b 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -20,6 +20,7 @@ package org.apache.cloudstack.storage.utils; import com.cloud.storage.ScopeType; +import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.StringUtils; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; @@ -30,12 +31,19 @@ import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.provider.StorageProviderFactory; import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.feign.model.Igroup; +import org.apache.cloudstack.storage.feign.model.Initiator; +import org.apache.cloudstack.storage.provider.StorageProviderFactory; +import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.util.Base64Utils; +import java.util.ArrayList; +import java.util.List; import java.util.Map; public class Utility { @@ -143,4 +151,61 @@ public static String getIgroupName(String svmName, ScopeType scopeType, Long sco public static String generateExportPolicyName(String svmName, String volumeName){ return Constants.EXPORT + Constants.HYPHEN + svmName + Constants.HYPHEN + volumeName; } + + public static AccessGroup createAccessGroupRequestByProtocol(StoragePoolVO storagePool, long scopeId, Map details, List hostsIdentifier) { + ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toLowerCase()); + String svmName = details.get(Constants.SVM_NAME); + switch (protocol) { + case ISCSI: + // Access group name format: cs_svmName_scopeId + String igroupName = getIgroupName(svmName, scopeId); + Hypervisor.HypervisorType hypervisorType = storagePool.getHypervisor(); + return createSANAccessGroupRequest(svmName, igroupName, hypervisorType, hostsIdentifier); + default: + s_logger.error("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol); + throw new CloudRuntimeException("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol); + } + } + + public static AccessGroup createSANAccessGroupRequest(String svmName, String igroupName, Hypervisor.HypervisorType hypervisorType, List hostsIdentifier) { + AccessGroup accessGroupRequest = new AccessGroup(); + Igroup igroup = new Igroup(); + + if (svmName != null && !svmName.isEmpty()) { + Svm svm = new Svm(); + svm.setName(svmName); + igroup.setSvm(svm); + } + + if (igroupName != null && !igroupName.isEmpty()) { + igroup.setName(igroupName); + } + + if (hypervisorType != null) { + String hypervisorName = hypervisorType.name(); + igroup.setOsType(Igroup.OsTypeEnum.valueOf(getOSTypeFromHypervisor(hypervisorName))); + } + + if (hostsIdentifier != null && hostsIdentifier.size() > 0) { + List initiators = new ArrayList<>(); + for (String hostIdentifier : hostsIdentifier) { + Initiator initiator = new Initiator(); + initiator.setName(hostIdentifier); + initiators.add(initiator); + } + igroup.setInitiators(initiators); + } + accessGroupRequest.setIgroup(igroup); + return accessGroupRequest; + } + + public static String getLunName(String volName, String lunName) { + //Lun name in unified "/vol/VolumeName/LunName" + return Constants.VOLUME_PATH_PREFIX + volName + Constants.SLASH + lunName; + } + + public static String getIgroupName(String svmName, long scopeId) { + // Igroup name format: cs_svmName_scopeId + return Constants.CS + Constants.UNDERSCORE + svmName + Constants.UNDERSCORE + scopeId; + } } From d6e1d70cc49b0a3a3a63a993ac1439987632c069 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 5 Nov 2025 15:29:56 +0530 Subject: [PATCH 04/29] CSTACKEX-46 Added Logging --- .../main/java/org/apache/cloudstack/storage/utils/Utility.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index ebe7017b672b..26420d79aee5 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -205,7 +205,7 @@ public static String getLunName(String volName, String lunName) { } public static String getIgroupName(String svmName, long scopeId) { - // Igroup name format: cs_svmName_scopeId + //Igroup name format: cs_svmName_scopeId return Constants.CS + Constants.UNDERSCORE + svmName + Constants.UNDERSCORE + scopeId; } } From eff6d97455b5c784f63eb740652960f7f0d28b3e Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Thu, 6 Nov 2025 10:17:52 +0530 Subject: [PATCH 05/29] CSTACKEX-46 Resolve Copilot review comments --- .../driver/OntapPrimaryDatastoreDriver.java | 20 +++++++-- .../OntapPrimaryDatastoreLifecycle.java | 2 +- .../storage/service/UnifiedSANStrategy.java | 44 ++++++++++++++++--- 3 files changed, 54 insertions(+), 12 deletions(-) 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 f4322f27226f..398d5f46d557 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 @@ -47,6 +47,8 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.feign.model.Igroup; +import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -255,7 +257,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); s_logger.info("grantAccessForVolume: Retrieved LUN [{}] details for volume [{}]", cloudStackVolume.getLun().getName(), volumeVO.getName()); AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); - if(accessGroup.getIgroup().getInitiators() == null || accessGroup.getIgroup().getInitiators().size() == 0 || !accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) { + if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName); } @@ -271,6 +273,16 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, throw new CloudRuntimeException(errMsg); } } + private boolean hostInitiatorFoundInIgroup(String hostInitiator, Igroup igroup) { + if(igroup != null || igroup.getInitiators() != null || hostInitiator != null || !hostInitiator.isEmpty()) { + for(Initiator initiator : igroup.getInitiators()) { + if(initiator.getName().equalsIgnoreCase(hostInitiator)) { + return true; + } + } + } + return false; + } @Override public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { @@ -322,9 +334,9 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); //TODO check if initiator does exits in igroup, will throw the error ? - if(!accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) { - s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); - throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName); + if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { + s_logger.error("revokeAccessForVolume: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); + throw new CloudRuntimeException("revokeAccessForVolume: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName); } Map disableLogicalAccessMap = new HashMap<>(); 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 8ae2036f468a..74d2a20771f5 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 @@ -392,7 +392,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the zone if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) { - s_logger.error("attachZone: Not all hosts in the cluster support the protocol: " + protocol.name()); + s_logger.error("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); throw new CloudRuntimeException("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); } if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) { 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 d594354bd143..64fe1dc8ef76 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 @@ -106,9 +106,39 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - public CloudStackVolume getCloudStackVolume(Map cloudStackVolumeMap) { - //TODO - return null; + public CloudStackVolume getCloudStackVolume(Map values) { + s_logger.info("getCloudStackVolume : fetching Igroup with params {} ", values); + if (values == null || values.isEmpty()) { + s_logger.error("getCloudStackVolume: get Igroup failed. Invalid request: {}", values); + throw new CloudRuntimeException("getCloudStackVolume : get Igroup Failed, invalid request"); + } + String svmName = values.get(Constants.SVM_DOT_NAME); + String lunName = values.get(Constants.NAME); + if(svmName == null || lunName == null || svmName.isEmpty() || lunName.isEmpty()) { + s_logger.error("getCloudStackVolume: get Igroup failed. Invalid svm:{} or igroup name: {}", svmName, lunName); + throw new CloudRuntimeException("getCloudStackVolume : Fget Igroup failed, invalid request"); + } + try { + // Get AuthHeader + String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // get Igroup + Map queryParams = Map.of(Constants.SVM_DOT_NAME, svmName, Constants.NAME, lunName); + OntapResponse lunResponse = sanFeignClient.getLunResponse(authHeader, queryParams); + if (lunResponse == null || lunResponse.getRecords() == null || lunResponse.getRecords().size() == 0) { + s_logger.error("getCloudStackVolume: Failed to fetch Igroup"); + throw new CloudRuntimeException("getCloudStackVolume: Failed to fetch Igroup"); + } + Lun lun = lunResponse.getRecords().get(0); + s_logger.debug("getCloudStackVolume: Lun Details : {}", lun); + s_logger.info("getCloudStackVolume: Fetched the Lun successfully. LunName: {}", lun.getName()); + + CloudStackVolume cloudStackVolume = new CloudStackVolume(); + cloudStackVolume.setLun(lun); + return cloudStackVolume; + } catch (Exception e) { + s_logger.error("Exception occurred while fetching Lun. Exception: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Lun details: " + e.getMessage()); + } } @Override @@ -317,7 +347,7 @@ public AccessGroup getAccessGroup(Map values) { String igroupName = values.get(Constants.IGROUP_DOT_NAME); if(svmName == null || igroupName == null || svmName.isEmpty() || igroupName.isEmpty()) { s_logger.error("getAccessGroup: get Igroup failed. Invalid svm:{} or igroup name: {}", svmName, igroupName); - throw new CloudRuntimeException("getAccessGroup : Fget Igroup failed, invalid request"); + throw new CloudRuntimeException("getAccessGroup : Failed to get Igroup, invalid request"); } try { // Get AuthHeader @@ -331,7 +361,7 @@ public AccessGroup getAccessGroup(Map values) { } Igroup igroup = igroupResponse.getRecords().get(0); s_logger.debug("getAccessGroup: Igroup Details : {}", igroup); - s_logger.info("getAccessGroup: Fetched the Igroup successfully. LunName: {}", igroup.getName()); + s_logger.info("getAccessGroup: Fetched the Igroup successfully. IgroupName: {}", igroup.getName()); AccessGroup accessGroup = new AccessGroup(); accessGroup.setIgroup(igroup); @@ -365,7 +395,7 @@ public void enableLogicalAccess(Map values) { s_logger.debug("enableLogicalAccess: LunMap created successfully. LunMap: {}", lunMap); s_logger.info("enableLogicalAccess: LunMap created successfully."); } catch (Exception e) { - s_logger.error("Exception occurred while creating LunMap: {}. Exception: {}", e.getMessage()); + s_logger.error("Exception occurred while creating LunMap: {}. Exception: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage()); } } @@ -385,7 +415,7 @@ public void disableLogicalAccess(Map values) { sanFeignClient.deleteLunMap(authHeader, lunUUID, igroupUUID); s_logger.info("disableLogicalAccess: LunMap deleted successfully."); } catch (Exception e) { - s_logger.error("Exception occurred while deleting LunMap: {}. Exception: {}", e.getMessage()); + s_logger.error("Exception occurred while deleting LunMap: {}. Exception: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to delete LunMap: " + e.getMessage()); } } From faf8dbf76044467ec8a8be63bd0590cf2a85dde1 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Thu, 6 Nov 2025 15:16:58 +0530 Subject: [PATCH 06/29] CSTACKEX-46 Resolve review comments --- .../driver/OntapPrimaryDatastoreDriver.java | 46 ++++++++++++-- .../storage/feign/client/SANFeignClient.java | 5 ++ .../OntapPrimaryDatastoreLifecycle.java | 60 +++++++++++++++++-- .../cloudstack/storage/utils/Utility.java | 49 +-------------- 4 files changed, 102 insertions(+), 58 deletions(-) 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 398d5f46d557..71e88541b5d3 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 @@ -47,8 +47,7 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.model.Igroup; -import org.apache.cloudstack.storage.feign.model.Initiator; +import org.apache.cloudstack.storage.feign.model.*; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -112,8 +111,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); } if (dataObject.getType() == DataObjectType.VOLUME) { - VolumeInfo volumeInfo = (VolumeInfo) dataObject; - path = createCloudStackVolumeForTypeVolume(dataStore, volumeInfo); + path = createCloudStackVolumeForTypeVolume(storagePool, (VolumeInfo)dataObject); createCmdResult = new CreateCmdResult(path, new Answer(null, true, null)); } else { errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync"; @@ -155,6 +153,44 @@ private String createCloudStackVolumeForTypeVolume(DataStore dataStore, VolumeIn } } + private CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map details, VolumeInfo volumeInfo) { + CloudStackVolume cloudStackVolumeRequest = null; + + String protocol = details.get(Constants.PROTOCOL); + if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { + cloudStackVolumeRequest = new CloudStackVolume(); + Lun lunRequest = new Lun(); + Svm svm = new Svm(); + svm.setName(details.get(Constants.SVM_NAME)); + lunRequest.setSvm(svm); + + LunSpace lunSpace = new LunSpace(); + lunSpace.setSize(volumeInfo.getSize()); + lunRequest.setSpace(lunSpace); + //Lun name is full path like in unified "/vol/VolumeName/LunName" + String lunFullName = Utility.getLunName(storagePool.getName(), volumeInfo.getName()); + lunRequest.setName(lunFullName); + + String hypervisorType = storagePool.getHypervisor().name(); + String osType = null; + switch (hypervisorType) { + case Constants.KVM: + osType = Lun.OsTypeEnum.LINUX.getValue(); + break; + default: + String errMsg = "createCloudStackVolume : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage"; + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType)); + + cloudStackVolumeRequest.setLun(lunRequest); + return cloudStackVolumeRequest; + } else { + throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol); + } + } + @Override public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallback callback) { CommandResult commandResult = new CommandResult(); @@ -336,7 +372,7 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, //TODO check if initiator does exits in igroup, will throw the error ? if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { s_logger.error("revokeAccessForVolume: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); - throw new CloudRuntimeException("revokeAccessForVolume: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName); + return; } Map disableLogicalAccessMap = new HashMap<>(); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index 868aab293518..d67c1f10c5a0 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java @@ -40,6 +40,11 @@ public interface SANFeignClient { @RequestLine("POST /api/storage/luns?return_records={returnRecords}") @Headers({"Authorization: {authHeader}"}) OntapResponse createLun(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, Lun lun); + @RequestLine("POST /api/storage/luns") + @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"}) + OntapResponse createLun(@Param("authHeader") String authHeader, + @Param("returnRecords") boolean returnRecords, + Lun lun); @RequestLine("GET /api/storage/luns") @Headers({"Authorization: {authHeader}"}) 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 74d2a20771f5..9cc5712ca450 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 @@ -46,7 +46,10 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDetailsDao; import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl; import org.apache.cloudstack.storage.feign.model.ExportPolicy; +import org.apache.cloudstack.storage.feign.model.Igroup; +import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.feign.model.OntapStorage; +import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.Volume; import org.apache.cloudstack.storage.provider.StorageProviderFactory; import org.apache.cloudstack.storage.service.StorageStrategy; @@ -325,13 +328,13 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the cluster - if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) { + if (!validateProtocolSupportAndFetchHostsIndentifier(hostsToConnect, protocol, hostsIdentifier)) { s_logger.error("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name()); throw new CloudRuntimeException("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name()); } //TODO - check if no host to connect then also need to create access group without initiators if (hostsIdentifier != null && hostsIdentifier.size() > 0) { - AccessGroup accessGroupRequest = Utility.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); + AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); strategy.createAccessGroup(accessGroupRequest); } logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); @@ -391,12 +394,12 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the zone - if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) { + if (!validateProtocolSupportAndFetchHostsIndentifier(hostsToConnect, protocol, hostsIdentifier)) { s_logger.error("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); throw new CloudRuntimeException("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); } if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) { - AccessGroup accessGroupRequest = Utility.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); + AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); strategy.createAccessGroup(accessGroupRequest); } for (HostVO host : hostsToConnect) { @@ -411,7 +414,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper return true; } - private boolean isProtocolSupportedByAllHosts(List hosts, ProtocolType protocolType, List hostIdentifiers) { + private boolean validateProtocolSupportAndFetchHostsIndentifier(List hosts, ProtocolType protocolType, List hostIdentifiers) { switch (protocolType) { case ISCSI: String protocolPrefix = Constants.IQN; @@ -429,6 +432,53 @@ private boolean isProtocolSupportedByAllHosts(List hosts, ProtocolType p return true; } + private AccessGroup createAccessGroupRequestByProtocol(StoragePoolVO storagePool, long scopeId, Map details, List hostsIdentifier) { + ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toLowerCase()); + String svmName = details.get(Constants.SVM_NAME); + switch (protocol) { + case ISCSI: + // Access group name format: cs_svmName_scopeId + String igroupName = Utility.getIgroupName(svmName, scopeId); + Hypervisor.HypervisorType hypervisorType = storagePool.getHypervisor(); + return createSANAccessGroupRequest(svmName, igroupName, hypervisorType, hostsIdentifier); + default: + s_logger.error("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol); + throw new CloudRuntimeException("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol); + } + } + + private AccessGroup createSANAccessGroupRequest(String svmName, String igroupName, Hypervisor.HypervisorType hypervisorType, List hostsIdentifier) { + AccessGroup accessGroupRequest = new AccessGroup(); + Igroup igroup = new Igroup(); + + if (svmName != null && !svmName.isEmpty()) { + Svm svm = new Svm(); + svm.setName(svmName); + igroup.setSvm(svm); + } + + if (igroupName != null && !igroupName.isEmpty()) { + igroup.setName(igroupName); + } + + if (hypervisorType != null) { + String hypervisorName = hypervisorType.name(); + igroup.setOsType(Igroup.OsTypeEnum.valueOf(Utility.getOSTypeFromHypervisor(hypervisorName))); + } + + if (hostsIdentifier != null && hostsIdentifier.size() > 0) { + List initiators = new ArrayList<>(); + for (String hostIdentifier : hostsIdentifier) { + Initiator initiator = new Initiator(); + initiator.setName(hostIdentifier); + initiators.add(initiator); + } + igroup.setInitiators(initiators); + } + accessGroupRequest.setIgroup(igroup); + return accessGroupRequest; + } + @Override public boolean maintain(DataStore store) { _storagePoolAutomation.maintain(store); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index 26420d79aee5..3cd4343b827a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -152,54 +152,7 @@ public static String generateExportPolicyName(String svmName, String volumeName) return Constants.EXPORT + Constants.HYPHEN + svmName + Constants.HYPHEN + volumeName; } - public static AccessGroup createAccessGroupRequestByProtocol(StoragePoolVO storagePool, long scopeId, Map details, List hostsIdentifier) { - ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toLowerCase()); - String svmName = details.get(Constants.SVM_NAME); - switch (protocol) { - case ISCSI: - // Access group name format: cs_svmName_scopeId - String igroupName = getIgroupName(svmName, scopeId); - Hypervisor.HypervisorType hypervisorType = storagePool.getHypervisor(); - return createSANAccessGroupRequest(svmName, igroupName, hypervisorType, hostsIdentifier); - default: - s_logger.error("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol); - throw new CloudRuntimeException("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol); - } - } - - public static AccessGroup createSANAccessGroupRequest(String svmName, String igroupName, Hypervisor.HypervisorType hypervisorType, List hostsIdentifier) { - AccessGroup accessGroupRequest = new AccessGroup(); - Igroup igroup = new Igroup(); - - if (svmName != null && !svmName.isEmpty()) { - Svm svm = new Svm(); - svm.setName(svmName); - igroup.setSvm(svm); - } - - if (igroupName != null && !igroupName.isEmpty()) { - igroup.setName(igroupName); - } - - if (hypervisorType != null) { - String hypervisorName = hypervisorType.name(); - igroup.setOsType(Igroup.OsTypeEnum.valueOf(getOSTypeFromHypervisor(hypervisorName))); - } - - if (hostsIdentifier != null && hostsIdentifier.size() > 0) { - List initiators = new ArrayList<>(); - for (String hostIdentifier : hostsIdentifier) { - Initiator initiator = new Initiator(); - initiator.setName(hostIdentifier); - initiators.add(initiator); - } - igroup.setInitiators(initiators); - } - accessGroupRequest.setIgroup(igroup); - return accessGroupRequest; - } - - public static String getLunName(String volName, String lunName) { + public static String getLunName(String volName, String lunName) { //Lun name in unified "/vol/VolumeName/LunName" return Constants.VOLUME_PATH_PREFIX + volName + Constants.SLASH + lunName; } From 8852bb4e5392ca0328ddabcdf4d56d5c45774efa Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Thu, 6 Nov 2025 15:49:37 +0530 Subject: [PATCH 07/29] CSTACKEX-46 Resolve review bot comments --- .../driver/OntapPrimaryDatastoreDriver.java | 2 +- .../storage/service/UnifiedSANStrategy.java | 26 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) 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 71e88541b5d3..025eb06ca866 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 @@ -310,7 +310,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, } } private boolean hostInitiatorFoundInIgroup(String hostInitiator, Igroup igroup) { - if(igroup != null || igroup.getInitiators() != null || hostInitiator != null || !hostInitiator.isEmpty()) { + if(igroup != null && igroup.getInitiators() != null && hostInitiator != null && !hostInitiator.isEmpty()) { for(Initiator initiator : igroup.getInitiators()) { if(initiator.getName().equalsIgnoreCase(hostInitiator)) { return true; 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 64fe1dc8ef76..754f902c587b 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 @@ -89,7 +89,7 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume createdCloudStackVolume.setLun(lun); return createdCloudStackVolume; } catch (Exception e) { - s_logger.error("Exception occurred while creating LUN: {}. Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); + s_logger.error("Exception occurred while creating LUN: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); throw new CloudRuntimeException("Failed to create Lun: " + e.getMessage()); } } @@ -107,16 +107,16 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { @Override public CloudStackVolume getCloudStackVolume(Map values) { - s_logger.info("getCloudStackVolume : fetching Igroup with params {} ", values); + s_logger.info("getCloudStackVolume : fetching Lun with params {} ", values); if (values == null || values.isEmpty()) { - s_logger.error("getCloudStackVolume: get Igroup failed. Invalid request: {}", values); - throw new CloudRuntimeException("getCloudStackVolume : get Igroup Failed, invalid request"); + s_logger.error("getCloudStackVolume: get Lun failed. Invalid request: {}", values); + throw new CloudRuntimeException("getCloudStackVolume : get Lun Failed, invalid request"); } String svmName = values.get(Constants.SVM_DOT_NAME); String lunName = values.get(Constants.NAME); if(svmName == null || lunName == null || svmName.isEmpty() || lunName.isEmpty()) { - s_logger.error("getCloudStackVolume: get Igroup failed. Invalid svm:{} or igroup name: {}", svmName, lunName); - throw new CloudRuntimeException("getCloudStackVolume : Fget Igroup failed, invalid request"); + s_logger.error("getCloudStackVolume: get Lun failed. Invalid svm:{} or igroup name: {}", svmName, lunName); + throw new CloudRuntimeException("getCloudStackVolume : Failed to get Lun, invalid request"); } try { // Get AuthHeader @@ -125,8 +125,8 @@ public CloudStackVolume getCloudStackVolume(Map values) { Map queryParams = Map.of(Constants.SVM_DOT_NAME, svmName, Constants.NAME, lunName); OntapResponse lunResponse = sanFeignClient.getLunResponse(authHeader, queryParams); if (lunResponse == null || lunResponse.getRecords() == null || lunResponse.getRecords().size() == 0) { - s_logger.error("getCloudStackVolume: Failed to fetch Igroup"); - throw new CloudRuntimeException("getCloudStackVolume: Failed to fetch Igroup"); + s_logger.error("getCloudStackVolume: Failed to fetch Lun"); + throw new CloudRuntimeException("getCloudStackVolume: Failed to fetch Lun"); } Lun lun = lunResponse.getRecords().get(0); s_logger.debug("getCloudStackVolume: Lun Details : {}", lun); @@ -136,7 +136,7 @@ public CloudStackVolume getCloudStackVolume(Map values) { cloudStackVolume.setLun(lun); return cloudStackVolume; } catch (Exception e) { - s_logger.error("Exception occurred while fetching Lun. Exception: {}", e.getMessage()); + s_logger.error("Exception occurred while fetching Lun, Exception: {}", e.getMessage()); throw new CloudRuntimeException("Failed to fetch Lun details: " + e.getMessage()); } } @@ -367,7 +367,7 @@ public AccessGroup getAccessGroup(Map values) { accessGroup.setIgroup(igroup); return accessGroup; } catch (Exception e) { - s_logger.error("Exception occurred while fetching Igroup. Exception: {}", e.getMessage()); + s_logger.error("Exception occurred while fetching Igroup, Exception: {}", e.getMessage()); throw new CloudRuntimeException("Failed to fetch Igroup details: " + e.getMessage()); } } @@ -392,10 +392,10 @@ public void enableLogicalAccess(Map values) { throw new CloudRuntimeException("Failed to perform LunMap for Lun: " +lunName+ " and igroup: " + igroupName); } LunMap lunMap = createdLunMap.getRecords().get(0); - s_logger.debug("enableLogicalAccess: LunMap created successfully. LunMap: {}", lunMap); + s_logger.debug("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMap); s_logger.info("enableLogicalAccess: LunMap created successfully."); } catch (Exception e) { - s_logger.error("Exception occurred while creating LunMap: {}. Exception: {}", e.getMessage(), e); + s_logger.error("Exception occurred while creating LunMap: {}, Exception: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage()); } } @@ -415,7 +415,7 @@ public void disableLogicalAccess(Map values) { sanFeignClient.deleteLunMap(authHeader, lunUUID, igroupUUID); s_logger.info("disableLogicalAccess: LunMap deleted successfully."); } catch (Exception e) { - s_logger.error("Exception occurred while deleting LunMap: {}. Exception: {}", e.getMessage(), e); + s_logger.error("Exception occurred while deleting LunMap: {}, Exception: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to delete LunMap: " + e.getMessage()); } } From dc5f01a6605d8bf70ea281885e15362393b51d54 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 10:57:50 +0530 Subject: [PATCH 08/29] CSTACKEX-46 Change method name --- .../storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 9cc5712ca450..b83fcdd77a3d 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 @@ -328,7 +328,7 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the cluster - if (!validateProtocolSupportAndFetchHostsIndentifier(hostsToConnect, protocol, hostsIdentifier)) { + if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) { s_logger.error("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name()); throw new CloudRuntimeException("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name()); } @@ -394,7 +394,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the zone - if (!validateProtocolSupportAndFetchHostsIndentifier(hostsToConnect, protocol, hostsIdentifier)) { + if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) { s_logger.error("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); throw new CloudRuntimeException("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); } @@ -414,7 +414,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper return true; } - private boolean validateProtocolSupportAndFetchHostsIndentifier(List hosts, ProtocolType protocolType, List hostIdentifiers) { + private boolean validateProtocolSupportAndFetchHostsIdentifier(List hosts, ProtocolType protocolType, List hostIdentifiers) { switch (protocolType) { case ISCSI: String protocolPrefix = Constants.IQN; @@ -427,7 +427,7 @@ private boolean validateProtocolSupportAndFetchHostsIndentifier(List hos } break; default: - throw new CloudRuntimeException("isProtocolSupportedByAllHosts : Unsupported protocol: " + protocolType.name()); + throw new CloudRuntimeException("validateProtocolSupportAndFetchHostsIdentifier : Unsupported protocol: " + protocolType.name()); } return true; } From 14ec034feaf9315ee7ca7d549ead8565c3bdc2ca Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 11:42:03 +0530 Subject: [PATCH 09/29] CSTACKEX-46 Resolve check styling issues --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 6 +++++- .../java/org/apache/cloudstack/storage/utils/Utility.java | 7 ------- 2 files changed, 5 insertions(+), 8 deletions(-) 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 025eb06ca866..f9750215cab2 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 @@ -47,7 +47,11 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.model.*; +import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.LunSpace; +import org.apache.cloudstack.storage.feign.model.Svm; +import org.apache.cloudstack.storage.feign.model.Igroup; +import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index 3cd4343b827a..134b928dc26e 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -23,10 +23,7 @@ import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.StringUtils; import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.LunSpace; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.provider.StorageProviderFactory; @@ -35,15 +32,11 @@ import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.provider.StorageProviderFactory; import org.apache.cloudstack.storage.service.StorageStrategy; -import org.apache.cloudstack.storage.service.model.AccessGroup; -import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.util.Base64Utils; -import java.util.ArrayList; -import java.util.List; import java.util.Map; public class Utility { From 423aee2f364782eeca6d59628ea79c462e18cf0b Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 12:50:46 +0530 Subject: [PATCH 10/29] CSTACKEX-46 Testing: Resolve fetch Storage details issue --- .../storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | 2 +- .../org/apache/cloudstack/storage/service/StorageStrategy.java | 2 +- .../apache/cloudstack/storage/service/UnifiedSANStrategy.java | 2 +- 3 files changed, 3 insertions(+), 3 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 b83fcdd77a3d..e6f4cc0fad00 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 @@ -324,7 +324,7 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { strategy.createAccessGroup(accessGroupRequest); logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); - Map details = primaryStore.getDetails(); + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the cluster 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 edfda1f4bd2d..bd86a7e591d2 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 @@ -516,7 +516,7 @@ public String getNetworkInterface() { /** * Method encapsulates the behavior based on the opted protocol in subclasses - @@ -306,22 +306,22 @@ public Volume getStorageVolume(Volume volume) + @@ -306,22 +306,22 @@ public AccessGroup getAccessGroup(Map values) * getNameSpace for Nvme/TCP and Nvme/FC protocols * @param values */ 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 754f902c587b..822ba19b41bf 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 @@ -115,7 +115,7 @@ public CloudStackVolume getCloudStackVolume(Map values) { String svmName = values.get(Constants.SVM_DOT_NAME); String lunName = values.get(Constants.NAME); if(svmName == null || lunName == null || svmName.isEmpty() || lunName.isEmpty()) { - s_logger.error("getCloudStackVolume: get Lun failed. Invalid svm:{} or igroup name: {}", svmName, lunName); + s_logger.error("getCloudStackVolume: get Lun failed. Invalid svm:{} or Lun name: {}", svmName, lunName); throw new CloudRuntimeException("getCloudStackVolume : Failed to get Lun, invalid request"); } try { From 905ae4ad89223e253497ed1af4b595a736ee31c9 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 13:08:16 +0530 Subject: [PATCH 11/29] CSTACKEX-46 Testing: Protocol Type fetch issue --- .../storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 e6f4cc0fad00..9de19303d6db 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 @@ -433,7 +433,7 @@ private boolean validateProtocolSupportAndFetchHostsIdentifier(List host } private AccessGroup createAccessGroupRequestByProtocol(StoragePoolVO storagePool, long scopeId, Map details, List hostsIdentifier) { - ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toLowerCase()); + ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toUpperCase()); String svmName = details.get(Constants.SVM_NAME); switch (protocol) { case ISCSI: @@ -476,6 +476,7 @@ private AccessGroup createSANAccessGroupRequest(String svmName, String igroupNam igroup.setInitiators(initiators); } accessGroupRequest.setIgroup(igroup); + s_logger.debug("createSANAccessGroupRequest: request: " + accessGroupRequest); return accessGroupRequest; } From 3ee5ead3be3ae788d40016966de6335e6b71f79b Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 13:33:15 +0530 Subject: [PATCH 12/29] CSTACKEX-46 Testing: Added debug log --- .../apache/cloudstack/storage/service/UnifiedSANStrategy.java | 2 ++ 1 file changed, 2 insertions(+) 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 822ba19b41bf..95a04adc1434 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 @@ -222,6 +222,8 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { throw feignEx; } + s_logger.debug("createAccessGroup: createdIgroup: {}", createdIgroup); + s_logger.debug("createAccessGroup: createdIgroup Records: {}", createdIgroup.getRecords()); if (createdIgroup == null || createdIgroup.getRecords() == null || createdIgroup.getRecords().isEmpty()) { s_logger.error("createAccessGroup: Igroup creation failed for Igroup Name {}", igroupName); throw new CloudRuntimeException("Failed to create Igroup: " + igroupName); From dc0e84e046c7c84da19c8dd4575f29bc8c3b754b Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 13:36:04 +0530 Subject: [PATCH 13/29] CSTACKEX-46 Testing: Added debug log --- .../lifecycle/OntapPrimaryDatastoreLifecycle.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 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 9de19303d6db..baf12d3b0d00 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 @@ -334,8 +334,13 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { } //TODO - check if no host to connect then also need to create access group without initiators if (hostsIdentifier != null && hostsIdentifier.size() > 0) { - AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); - strategy.createAccessGroup(accessGroupRequest); + try { + AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); + strategy.createAccessGroup(accessGroupRequest); + } catch (Exception e) { + s_logger.error("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId(), e); + throw new CloudRuntimeException("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId(), e); + } } logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); for (HostVO host : hostsToConnect) { From b2056ff73a7a58763a04d3696846fc381adfe2ca Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 13:50:48 +0530 Subject: [PATCH 14/29] CSTACKEX-46 Testing: Corrected the URL for Create Lun and Igroup --- .../cloudstack/storage/feign/client/SANFeignClient.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index d67c1f10c5a0..0da826f04fd2 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java @@ -40,11 +40,9 @@ public interface SANFeignClient { @RequestLine("POST /api/storage/luns?return_records={returnRecords}") @Headers({"Authorization: {authHeader}"}) OntapResponse createLun(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, Lun lun); - @RequestLine("POST /api/storage/luns") - @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"}) - OntapResponse createLun(@Param("authHeader") String authHeader, - @Param("returnRecords") boolean returnRecords, - Lun lun); + @RequestLine("POST /api/storage/luns?return_records={returnRecords}") + @Headers({"Authorization: {authHeader}"}) + OntapResponse createLun(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, Lun lun); @RequestLine("GET /api/storage/luns") @Headers({"Authorization: {authHeader}"}) From 67bf958ab86930c122124301739717b5dad06ea5 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 16:07:15 +0530 Subject: [PATCH 15/29] CSTACKEX-46 Resolve bot review comments --- .../driver/OntapPrimaryDatastoreDriver.java | 14 ++------------ .../lifecycle/OntapPrimaryDatastoreLifecycle.java | 10 ++++++---- .../storage/service/StorageStrategy.java | 7 ++++--- .../storage/service/UnifiedSANStrategy.java | 5 +---- 4 files changed, 13 insertions(+), 23 deletions(-) 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 f9750215cab2..233815eb305e 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 @@ -175,17 +175,7 @@ private CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO s String lunFullName = Utility.getLunName(storagePool.getName(), volumeInfo.getName()); lunRequest.setName(lunFullName); - String hypervisorType = storagePool.getHypervisor().name(); - String osType = null; - switch (hypervisorType) { - case Constants.KVM: - osType = Lun.OsTypeEnum.LINUX.getValue(); - break; - default: - String errMsg = "createCloudStackVolume : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage"; - s_logger.error(errMsg); - throw new CloudRuntimeException(errMsg); - } + String osType = Utility.getOSTypeFromHypervisor(storagePool.getHypervisor().name()); lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType)); cloudStackVolumeRequest.setLun(lunRequest); @@ -299,7 +289,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); - throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName); + throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName + "]"); } Map enableLogicalAccessMap = new HashMap<>(); 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 baf12d3b0d00..83b564158af9 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 @@ -329,8 +329,9 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the cluster if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) { - s_logger.error("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name()); - throw new CloudRuntimeException("attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name()); + String errMsg = "attachCluster: Not all hosts in the cluster support the protocol: " + protocol.name(); + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); } //TODO - check if no host to connect then also need to create access group without initiators if (hostsIdentifier != null && hostsIdentifier.size() > 0) { @@ -400,8 +401,9 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the zone if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) { - s_logger.error("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); - throw new CloudRuntimeException("attachZone: Not all hosts in the zone support the protocol: " + protocol.name()); + String errMsg = "attachZone: Not all hosts in the zone support the protocol: " + protocol.name(); + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); } if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) { AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); 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 bd86a7e591d2..c5df71ebd652 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 @@ -516,9 +516,10 @@ public String getNetworkInterface() { /** * Method encapsulates the behavior based on the opted protocol in subclasses - @@ -306,22 +306,22 @@ public AccessGroup getAccessGroup(Map values) - * getNameSpace for Nvme/TCP and Nvme/FC protocols - * @param values + * getIGroup example getIgroup for iSCSI and FC protocols + * getExportPolicy example getExportPolicy for NFS 3.0 and NFS 4.1 protocols + * //TODO for Nvme/TCP and Nvme/FC protocols + * @param values map to get access group values like name, svm name etc. */ abstract public AccessGroup getAccessGroup(Map values); 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 95a04adc1434..62e59cf61105 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 @@ -362,9 +362,6 @@ public AccessGroup getAccessGroup(Map values) { throw new CloudRuntimeException("Failed to fetch Igroup"); } Igroup igroup = igroupResponse.getRecords().get(0); - s_logger.debug("getAccessGroup: Igroup Details : {}", igroup); - s_logger.info("getAccessGroup: Fetched the Igroup successfully. IgroupName: {}", igroup.getName()); - AccessGroup accessGroup = new AccessGroup(); accessGroup.setIgroup(igroup); return accessGroup; @@ -391,7 +388,7 @@ public void enableLogicalAccess(Map values) { OntapResponse createdLunMap = sanFeignClient.createLunMap(authHeader, true, lunMapRequest); if (createdLunMap == null || createdLunMap.getRecords() == null || createdLunMap.getRecords().size() == 0) { s_logger.error("enableLogicalAccess: LunMap failed for Lun: {} and igroup: {}", lunName, igroupName); - throw new CloudRuntimeException("Failed to perform LunMap for Lun: " +lunName+ " and igroup: " + igroupName); + throw new CloudRuntimeException("Failed to perform LunMap for Lun: " + lunName + " and igroup: " + igroupName); } LunMap lunMap = createdLunMap.getRecords().get(0); s_logger.debug("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMap); From 8732f7ff062a7094dfe6ef21d7cb1e2c6d058281 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 12:35:15 +0530 Subject: [PATCH 16/29] CSTACKEX-46 Resolve review comments --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 9 ++++----- .../lifecycle/OntapPrimaryDatastoreLifecycle.java | 1 + 2 files changed, 5 insertions(+), 5 deletions(-) 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 233815eb305e..733296fc3395 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 @@ -107,7 +107,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet throw new InvalidParameterValueException("createAsync: callback should not be null"); } try { - s_logger.info("createAsync: Started for data store [{}] and data object [{}] of type [{}]", dataStore, dataObject, dataObject.getType()); + s_logger.info("createAsync: Started for data store name [{}] and data object name [{}] of type [{}]", dataStore.getName(), dataObject.getName(), dataObject.getType()); StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if(storagePool == null) { @@ -124,7 +124,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } } catch (Exception e) { errMsg = e.getMessage(); - s_logger.error("createAsync: Failed for dataObject [{}]: {}", dataObject, errMsg); + s_logger.error("createAsync: Failed for dataObject name [{}]: {}", dataObject.getName(), errMsg); createCmdResult = new CreateCmdResult(null, new Answer(null, false, errMsg)); createCmdResult.setResult(e.toString()); } finally { @@ -159,13 +159,12 @@ private String createCloudStackVolumeForTypeVolume(DataStore dataStore, VolumeIn private CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map details, VolumeInfo volumeInfo) { CloudStackVolume cloudStackVolumeRequest = null; - + Svm svm = new Svm(); + svm.setName(details.get(Constants.SVM_NAME)); String protocol = details.get(Constants.PROTOCOL); if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { cloudStackVolumeRequest = new CloudStackVolume(); Lun lunRequest = new Lun(); - Svm svm = new Svm(); - svm.setName(details.get(Constants.SVM_NAME)); lunRequest.setSvm(svm); LunSpace lunSpace = new LunSpace(); 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 83b564158af9..a8cb81514aba 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 @@ -436,6 +436,7 @@ private boolean validateProtocolSupportAndFetchHostsIdentifier(List host default: throw new CloudRuntimeException("validateProtocolSupportAndFetchHostsIdentifier : Unsupported protocol: " + protocolType.name()); } + logger.info("validateProtocolSupportAndFetchHostsIdentifier: All hosts support the protocol: " + protocolType.name()); return true; } From f1b7dbd99504d6e97bc1ece6e27b68240595644a Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 14:07:14 +0530 Subject: [PATCH 17/29] CSTACKEX-46 Resolve review comments again --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 2 +- .../apache/cloudstack/storage/service/StorageStrategy.java | 4 ++-- .../java/org/apache/cloudstack/storage/utils/Utility.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) 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 733296fc3395..2e3d9438e21f 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 @@ -369,7 +369,7 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, } Map disableLogicalAccessMap = new HashMap<>(); - disableLogicalAccessMap.put(Constants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid().toString()); + disableLogicalAccessMap.put(Constants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid()); disableLogicalAccessMap.put(Constants.IGROUP_DOT_UUID, accessGroup.getIgroup().getUuid()); storageStrategy.disableLogicalAccess(disableLogicalAccessMap); } 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 c5df71ebd652..bb25928438c6 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 @@ -516,8 +516,8 @@ public String getNetworkInterface() { /** * Method encapsulates the behavior based on the opted protocol in subclasses - * getIGroup example getIgroup for iSCSI and FC protocols - * getExportPolicy example getExportPolicy for NFS 3.0 and NFS 4.1 protocols + * e.g., getIGroup for iSCSI and FC protocols + * e.g., getExportPolicy for NFS 3.0 and NFS 4.1 protocols * //TODO for Nvme/TCP and Nvme/FC protocols * @param values map to get access group values like name, svm name etc. */ diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index 134b928dc26e..09529117cfc9 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -146,7 +146,7 @@ public static String generateExportPolicyName(String svmName, String volumeName) } public static String getLunName(String volName, String lunName) { - //Lun name in unified "/vol/VolumeName/LunName" + //LUN name in ONTAP unified : "/vol/VolumeName/LunName" return Constants.VOLUME_PATH_PREFIX + volName + Constants.SLASH + lunName; } From e9ba8b34907b00759b72eb1a1f6b38d75f70d7f5 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 14:15:14 +0530 Subject: [PATCH 18/29] CSTACKEX-46 Change log level --- .../storage/service/UnifiedSANStrategy.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) 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 62e59cf61105..6ecac4338a79 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 @@ -107,7 +107,8 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { @Override public CloudStackVolume getCloudStackVolume(Map values) { - s_logger.info("getCloudStackVolume : fetching Lun with params {} ", values); + s_logger.info("getCloudStackVolume : fetching Lun"); + s_logger.debug("getCloudStackVolume : fetching Lun with params {} ", values); if (values == null || values.isEmpty()) { s_logger.error("getCloudStackVolume: get Lun failed. Invalid request: {}", values); throw new CloudRuntimeException("getCloudStackVolume : get Lun Failed, invalid request"); @@ -144,9 +145,10 @@ public CloudStackVolume getCloudStackVolume(Map values) { @Override public AccessGroup createAccessGroup(AccessGroup accessGroup) { s_logger.info("createAccessGroup : Create Igroup"); - String igroupName = "unknown"; - if (accessGroup == null) { - throw new CloudRuntimeException("createAccessGroup : Failed to create Igroup, invalid accessGroup object passed"); + s_logger.debug("createAccessGroup : Creating Igroup with access group request {} ", accessGroup); + if (accessGroup == null || accessGroup.getIgroup() == null) { + s_logger.error("createAccessGroup: Igroup creation failed. Invalid request: {}", accessGroup); + throw new CloudRuntimeException("createAccessGroup : Failed to create Igroup, invalid request"); } try { // Get StoragePool details @@ -340,7 +342,8 @@ public AccessGroup updateAccessGroup(AccessGroup accessGroup) { } public AccessGroup getAccessGroup(Map values) { - s_logger.info("getAccessGroup : fetching Igroup with params {} ", values); + s_logger.info("getAccessGroup : fetch Igroup"); + s_logger.debug("getAccessGroup : fetching Igroup with params {} ", values); if (values == null || values.isEmpty()) { s_logger.error("getAccessGroup: get Igroup failed. Invalid request: {}", values); throw new CloudRuntimeException("getAccessGroup : get Igroup Failed, invalid request"); @@ -372,7 +375,8 @@ public AccessGroup getAccessGroup(Map values) { } public void enableLogicalAccess(Map values) { - s_logger.info("enableLogicalAccess : Creating LunMap with values {} ", values); + s_logger.info("enableLogicalAccess : Create LunMap"); + s_logger.debug("enableLogicalAccess : Creating LunMap with values {} ", values); LunMap lunMapRequest = new LunMap(); String svmName = values.get(Constants.SVM_DOT_NAME); String lunName = values.get(Constants.LUN_DOT_NAME); @@ -400,7 +404,8 @@ public void enableLogicalAccess(Map values) { } public void disableLogicalAccess(Map values) { - s_logger.info("disableLogicalAccess : Deleting LunMap with values {} ", values); + s_logger.info("disableLogicalAccess : Delete LunMap"); + s_logger.debug("disableLogicalAccess : Deleting LunMap with values {} ", values); String lunUUID = values.get(Constants.LUN_DOT_UUID); String igroupUUID = values.get(Constants.IGROUP_DOT_UUID); if(lunUUID == null || igroupUUID == null || lunUUID.isEmpty() || igroupUUID.isEmpty()) { From f067b500572d1bee76c75ece7ce8f69557b388f8 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 15:02:37 +0530 Subject: [PATCH 19/29] CSTACKEX-46 added valiodation for lun name --- .../driver/OntapPrimaryDatastoreDriver.java | 36 +++++++++++++------ .../OntapPrimaryDatastoreLifecycle.java | 4 +-- .../storage/service/UnifiedSANStrategy.java | 9 +++-- 3 files changed, 33 insertions(+), 16 deletions(-) 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 2e3d9438e21f..46e1306543e8 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 @@ -106,11 +106,15 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet if (callback == null) { throw new InvalidParameterValueException("createAsync: callback should not be null"); } + if(!isValidName(dataObject.getName())) { + errMsg = "createAsync: Invalid dataObject name [" + dataObject.getName() + "]. It must start with a letter and can only contain letters, digits, and underscores, and be up to 200 characters long."; + s_logger.error(errMsg); + throw new InvalidParameterValueException(errMsg); + } try { s_logger.info("createAsync: Started for data store name [{}] and data object name [{}] of type [{}]", dataStore.getName(), dataObject.getName(), dataObject.getType()); - StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); - if(storagePool == null) { + if (storagePool == null) { s_logger.error("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); } @@ -135,6 +139,16 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } } + public boolean isValidName(String name) { + // Check for null and length constraint first + if (name == null || name.length() > 200) { + return false; + } + // Regex: Starts with a letter, followed by letters, digits, or underscores + String regex = "^[a-zA-Z][a-zA-Z0-9_]*$"; + return name.matches(regex); + } + private String createCloudStackVolumeForTypeVolume(DataStore dataStore, VolumeInfo volumeObject) { StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if(storagePool == null) { @@ -248,7 +262,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore } try { StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); - if(storagePool == null) { + if (storagePool == null) { s_logger.error("grantAccess : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("grantAccess : Storage Pool not found for id: " + dataStore.getId()); } @@ -259,7 +273,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore if (dataObject.getType() == DataObjectType.VOLUME) { VolumeVO volumeVO = volumeDao.findById(dataObject.getId()); - if(volumeVO == null) { + if (volumeVO == null) { s_logger.error("grantAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); throw new CloudRuntimeException("grantAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); } @@ -281,12 +295,12 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, String svmName = details.get(Constants.SVM_NAME); long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); - if(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { String accessGroupName = Utility.getIgroupName(svmName, scopeId); CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); s_logger.info("grantAccessForVolume: Retrieved LUN [{}] details for volume [{}]", cloudStackVolume.getLun().getName(), volumeVO.getName()); AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); - if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { + if (!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName + "]"); } @@ -326,7 +340,7 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) } try { StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); - if(storagePool == null) { + if (storagePool == null) { s_logger.error("revokeAccess : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("revokeAccess : Storage Pool not found for id: " + dataStore.getId()); } @@ -337,7 +351,7 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) if (dataObject.getType() == DataObjectType.VOLUME) { VolumeVO volumeVO = volumeDao.findById(dataObject.getId()); - if(volumeVO == null) { + if (volumeVO == null) { s_logger.error("revokeAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); throw new CloudRuntimeException("revokeAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); } @@ -358,12 +372,12 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, String svmName = details.get(Constants.SVM_NAME); long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); - if(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { String accessGroupName = Utility.getIgroupName(svmName, scopeId); CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); //TODO check if initiator does exits in igroup, will throw the error ? - if(!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { + if (!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { s_logger.error("revokeAccessForVolume: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); return; } @@ -381,7 +395,7 @@ private CloudStackVolume getCloudStackVolumeByName(StorageStrategy storageStrate getCloudStackVolumeMap.put(Constants.NAME, cloudStackVolumeName); getCloudStackVolumeMap.put(Constants.SVM_DOT_NAME, svmName); CloudStackVolume cloudStackVolume = storageStrategy.getCloudStackVolume(getCloudStackVolumeMap); - if(cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getName() == null) { + if (cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getName() == null) { s_logger.error("getCloudStackVolumeByName: Failed to get LUN details [{}]", cloudStackVolumeName); throw new CloudRuntimeException("getCloudStackVolumeByName: Failed to get LUN [" + cloudStackVolumeName + "]"); } 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 a8cb81514aba..ff233d1fac5e 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 @@ -301,7 +301,7 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { } List hostsIdentifier = new ArrayList<>(); StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); - if(storagePool == null) { + if (storagePool == null) { s_logger.error("attachCluster : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("attachCluster : Storage Pool not found for id: " + dataStore.getId()); } @@ -373,7 +373,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper } List hostsIdentifier = new ArrayList<>(); StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); - if(storagePool == null) { + if (storagePool == null) { s_logger.error("attachZone : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("attachZone : Storage Pool not found for id: " + dataStore.getId()); } 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 6ecac4338a79..2291728b92cc 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 @@ -377,7 +377,6 @@ public AccessGroup getAccessGroup(Map values) { public void enableLogicalAccess(Map values) { s_logger.info("enableLogicalAccess : Create LunMap"); s_logger.debug("enableLogicalAccess : Creating LunMap with values {} ", values); - LunMap lunMapRequest = new LunMap(); String svmName = values.get(Constants.SVM_DOT_NAME); String lunName = values.get(Constants.LUN_DOT_NAME); String igroupName = values.get(Constants.IGROUP_DOT_NAME); @@ -389,6 +388,10 @@ public void enableLogicalAccess(Map values) { // Get AuthHeader String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); // Create LunMap + LunMap lunMapRequest = new LunMap(); + lunMapRequest.getSvm().setName(svmName); + lunMapRequest.getLun().setName(lunName); + lunMapRequest.getIgroup().setName(igroupName); OntapResponse createdLunMap = sanFeignClient.createLunMap(authHeader, true, lunMapRequest); if (createdLunMap == null || createdLunMap.getRecords() == null || createdLunMap.getRecords().size() == 0) { s_logger.error("enableLogicalAccess: LunMap failed for Lun: {} and igroup: {}", lunName, igroupName); @@ -398,7 +401,7 @@ public void enableLogicalAccess(Map values) { s_logger.debug("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMap); s_logger.info("enableLogicalAccess: LunMap created successfully."); } catch (Exception e) { - s_logger.error("Exception occurred while creating LunMap: {}, Exception: {}", e.getMessage(), e); + s_logger.error("Exception occurred while creating LunMap, Exception: {}", e); throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage()); } } @@ -419,7 +422,7 @@ public void disableLogicalAccess(Map values) { sanFeignClient.deleteLunMap(authHeader, lunUUID, igroupUUID); s_logger.info("disableLogicalAccess: LunMap deleted successfully."); } catch (Exception e) { - s_logger.error("Exception occurred while deleting LunMap: {}, Exception: {}", e.getMessage(), e); + s_logger.error("Exception occurred while deleting LunMap, Exception: {}", e); throw new CloudRuntimeException("Failed to delete LunMap: " + e.getMessage()); } } From dac753f6583ecbfbef2edabadee1f537c81dd13a Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 18:03:08 +0530 Subject: [PATCH 20/29] CSTACKEX-46 resolve check style issues --- .../driver/OntapPrimaryDatastoreDriver.java | 3 +-- .../cloudstack/storage/feign/model/Svm.java | 1 - .../OntapPrimaryDatastoreLifecycle.java | 13 +++++++++---- .../storage/service/UnifiedSANStrategy.java | 19 ++++++++++++------- .../cloudstack/storage/utils/Utility.java | 2 +- 5 files changed, 23 insertions(+), 15 deletions(-) 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 46e1306543e8..c06762fc0659 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 @@ -145,8 +145,7 @@ public boolean isValidName(String name) { return false; } // Regex: Starts with a letter, followed by letters, digits, or underscores - String regex = "^[a-zA-Z][a-zA-Z0-9_]*$"; - return name.matches(regex); + return name.matches(Constants.ONTAP_NAME_REGEX); } private String createCloudStackVolumeForTypeVolume(DataStore dataStore, VolumeInfo volumeObject) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Svm.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Svm.java index 65821739f1b2..b1462c593863 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Svm.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Svm.java @@ -143,5 +143,4 @@ public int hashCode() { @JsonInclude(JsonInclude.Include.NON_NULL) public static class Links { } - } 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 ff233d1fac5e..edd804bd32a2 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 @@ -339,8 +339,8 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); strategy.createAccessGroup(accessGroupRequest); } catch (Exception e) { - s_logger.error("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId(), e); - throw new CloudRuntimeException("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId(), e); + s_logger.error("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage(), e); + throw new CloudRuntimeException("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage(), e); } } logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); @@ -406,8 +406,13 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper throw new CloudRuntimeException(errMsg); } if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) { - AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); - strategy.createAccessGroup(accessGroupRequest); + try { + AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); + strategy.createAccessGroup(accessGroupRequest); + } catch (Exception e) { + s_logger.error("attachZone: Failed to create access group on storage system for zone with Exception: " + e.getMessage()); + throw new CloudRuntimeException("attachZone: Failed to create access group on storage system for zone with Exception: " + e.getMessage()); + } } for (HostVO host : hostsToConnect) { try { 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 2291728b92cc..84c1b0b01d7e 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 @@ -27,10 +27,7 @@ import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Initiator; -import org.apache.cloudstack.storage.feign.model.Igroup; -import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.LunMap; -import org.apache.cloudstack.storage.feign.model.OntapStorage; +import org.apache.cloudstack.storage.feign.model.*; import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; @@ -389,9 +386,17 @@ public void enableLogicalAccess(Map values) { String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); // Create LunMap LunMap lunMapRequest = new LunMap(); - lunMapRequest.getSvm().setName(svmName); - lunMapRequest.getLun().setName(lunName); - lunMapRequest.getIgroup().setName(igroupName); + Svm svm = new Svm(); + svm.setName(svmName); + lunMapRequest.setSvm(svm); + //Set Lun name + Lun lun = new Lun(); + lun.setName(lunName); + lunMapRequest.setLun(lun); + //Set Igroup name + Igroup igroup = new Igroup(); + igroup.setName(igroupName); + lunMapRequest.setIgroup(igroup); OntapResponse createdLunMap = sanFeignClient.createLunMap(authHeader, true, lunMapRequest); if (createdLunMap == null || createdLunMap.getRecords() == null || createdLunMap.getRecords().size() == 0) { s_logger.error("enableLogicalAccess: LunMap failed for Lun: {} and igroup: {}", lunName, igroupName); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index 09529117cfc9..9532bf36fd9a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -146,7 +146,7 @@ public static String generateExportPolicyName(String svmName, String volumeName) } public static String getLunName(String volName, String lunName) { - //LUN name in ONTAP unified : "/vol/VolumeName/LunName" + //LUN name in ONTAP unified format: "/vol/VolumeName/LunName" return Constants.VOLUME_PATH_PREFIX + volName + Constants.SLASH + lunName; } From 716fcaf146b6516cadcc5e00ed60ce38f4b3384a Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 18:49:57 +0530 Subject: [PATCH 21/29] CSTACKEX-46 Resolve import issues --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 2 +- .../cloudstack/storage/service/UnifiedSANStrategy.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) 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 c06762fc0659..1f1591e8ad5e 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 @@ -139,7 +139,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } } - public boolean isValidName(String name) { + public boolean isValidName(String name) { // Check for null and length constraint first if (name == null || name.length() > 200) { return false; 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 84c1b0b01d7e..47f529968deb 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 @@ -29,6 +29,11 @@ import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.feign.model.*; import org.apache.cloudstack.storage.feign.model.Svm; +import org.apache.cloudstack.storage.feign.model.OntapStorage; +import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.Igroup; +import org.apache.cloudstack.storage.feign.model.LunMap; +import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; From e6ece89d05c605526b91b9329abda3f539595986 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 19:12:50 +0530 Subject: [PATCH 22/29] CSTACKEX-46 Resolve copilot comments --- .../storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 edd804bd32a2..9a37c8e65d44 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 @@ -339,8 +339,8 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); strategy.createAccessGroup(accessGroupRequest); } catch (Exception e) { - s_logger.error("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage(), e); - throw new CloudRuntimeException("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage(), e); + s_logger.error("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage()); + throw new CloudRuntimeException("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage()); } } logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); From d111436649252294f05efc580a1f738f52716906 Mon Sep 17 00:00:00 2001 From: "Locharla, Sandeep" Date: Wed, 21 Jan 2026 21:33:13 +0530 Subject: [PATCH 23/29] CSTACKEX-46: Create, Delete iSCSI type Cloudstack volumes, Enter, Cancel Maintenance mode --- .../kvm/storage/IscsiAdmStorageAdaptor.java | 61 +++- .../driver/OntapPrimaryDatastoreDriver.java | 302 ++++++++++++------ .../storage/feign/client/SANFeignClient.java | 7 +- .../storage/feign/model/Igroup.java | 2 +- .../cloudstack/storage/feign/model/Lun.java | 43 +++ .../OntapPrimaryDatastoreLifecycle.java | 135 +++----- .../storage/service/StorageStrategy.java | 64 +++- .../storage/service/UnifiedNASStrategy.java | 16 +- .../storage/service/UnifiedSANStrategy.java | 190 ++++++++--- .../cloudstack/storage/utils/Constants.java | 3 +- .../cloudstack/storage/utils/Utility.java | 50 +-- 11 files changed, 595 insertions(+), 278 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java index ba689d5107f7..155e97b90558 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java @@ -19,6 +19,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.nio.file.Files; +import java.nio.file.Paths; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; @@ -35,6 +37,7 @@ import com.cloud.storage.Storage.StoragePoolType; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.OutputInterpreter; +import com.cloud.utils.script.OutputInterpreter.AllLinesParser; import com.cloud.utils.script.Script; public class IscsiAdmStorageAdaptor implements StorageAdaptor { @@ -96,10 +99,15 @@ public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map 0) { @@ -238,6 +268,15 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) { } private long getDeviceSize(String deviceByPath) { + try { + if (!Files.exists(Paths.get(deviceByPath))) { + logger.debug("Device by-path does not exist yet: " + deviceByPath); + return 0L; + } + } catch (Exception ignore) { + // If FS check fails for any reason, fall back to blockdev call + } + Script iScsiAdmCmd = new Script(true, "blockdev", 0, logger); iScsiAdmCmd.add("--getsize64", deviceByPath); 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 1f1591e8ad5e..04a2300b5578 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 @@ -30,8 +30,14 @@ import com.cloud.storage.VolumeVO; import com.cloud.storage.ScopeType; import com.cloud.storage.dao.VolumeDao; +import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.storage.dao.VMTemplateDao; +//import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; @@ -47,11 +53,9 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.LunSpace; -import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Initiator; +import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -62,6 +66,7 @@ import org.apache.logging.log4j.Logger; import javax.inject.Inject; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -71,7 +76,9 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @Inject private PrimaryDataStoreDao storagePoolDao; + @Inject private VMInstanceDao vmDao; @Inject private VolumeDao volumeDao; + @Inject private VolumeDetailsDao volumeDetailsDao; @Override public Map getCapabilities() { s_logger.trace("OntapPrimaryDatastoreDriver: getCapabilities: Called"); @@ -95,7 +102,6 @@ public DataTO getTO(DataObject data) { @Override public void createAsync(DataStore dataStore, DataObject dataObject, AsyncCompletionCallback callback) { CreateCmdResult createCmdResult = null; - String path = null; String errMsg = null; if (dataStore == null) { throw new InvalidParameterValueException("createAsync: dataStore should not be null"); @@ -106,11 +112,6 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet if (callback == null) { throw new InvalidParameterValueException("createAsync: callback should not be null"); } - if(!isValidName(dataObject.getName())) { - errMsg = "createAsync: Invalid dataObject name [" + dataObject.getName() + "]. It must start with a letter and can only contain letters, digits, and underscores, and be up to 200 characters long."; - s_logger.error(errMsg); - throw new InvalidParameterValueException(errMsg); - } try { s_logger.info("createAsync: Started for data store name [{}] and data object name [{}] of type [{}]", dataStore.getName(), dataObject.getName(), dataObject.getType()); StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); @@ -118,9 +119,49 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet s_logger.error("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); } + Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); + if (dataObject.getType() == DataObjectType.VOLUME) { - path = createCloudStackVolumeForTypeVolume(storagePool, (VolumeInfo)dataObject); - createCmdResult = new CreateCmdResult(path, new Answer(null, true, null)); + VolumeInfo volInfo = (VolumeInfo) dataObject; + // Create LUN/backing for volume and record relevant details + CloudStackVolume created = createCloudStackVolume(dataStore, volInfo); + + // Immediately ensure LUN-map exists and update VolumeVO path + if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + String svmName = details.get(Constants.SVM_NAME); + String lunName = volumeDetailsDao.findDetail(volInfo.getId(), Constants.LUN_DOT_NAME) != null ? + volumeDetailsDao.findDetail(volInfo.getId(), Constants.LUN_DOT_NAME).getValue() : null; + if (lunName == null) { + // Fallback from returned LUN + lunName = created != null && created.getLun() != null ? created.getLun().getName() : null; + } + if (lunName == null) { + throw new CloudRuntimeException("createAsync: Missing LUN name for volume " + volInfo.getId()); + } + long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? storagePool.getClusterId() : storagePool.getDataCenterId(); + String lunNumber = ensureLunMapped(storagePool, svmName, lunName, scopeId); + + VolumeVO volumeVO = volumeDao.findById(volInfo.getId()); + if (volumeVO != null) { + String iscsiPath = Constants.SLASH + storagePool.getPath() + Constants.SLASH + lunNumber; + volumeVO.set_iScsiName(iscsiPath); + volumeVO.setPath(iscsiPath); + volumeVO.setPoolType(storagePool.getPoolType()); + volumeVO.setPoolId(storagePool.getId()); + volumeDao.update(volumeVO.getId(), volumeVO); + s_logger.info("createAsync: Volume [{}] iSCSI path set to {}", volumeVO.getId(), iscsiPath); + } + } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + // Ensure pool fields are recorded for managed NFS as well + VolumeVO volumeVO = volumeDao.findById(volInfo.getId()); + if (volumeVO != null) { + volumeVO.setPoolType(storagePool.getPoolType()); + volumeVO.setPoolId(storagePool.getId()); + volumeDao.update(volumeVO.getId(), volumeVO); + s_logger.info("createAsync: Managed NFS volume [{}] associated with pool {}", volumeVO.getId(), storagePool.getId()); + } + } + createCmdResult = new CreateCmdResult(null, new Answer(null, true, null)); } else { errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync"; s_logger.error(errMsg); @@ -133,68 +174,86 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet createCmdResult.setResult(e.toString()); } finally { if (createCmdResult != null && createCmdResult.isSuccess()) { - s_logger.info("createAsync: Volume created successfully. Path: {}", path); + s_logger.info("createAsync: Operation completed successfully for {}", dataObject.getType()); } callback.complete(createCmdResult); } } - public boolean isValidName(String name) { - // Check for null and length constraint first - if (name == null || name.length() > 200) { - return false; - } - // Regex: Starts with a letter, followed by letters, digits, or underscores - return name.matches(Constants.ONTAP_NAME_REGEX); - } - - private String createCloudStackVolumeForTypeVolume(DataStore dataStore, VolumeInfo volumeObject) { + private CloudStackVolume createCloudStackVolume(DataStore dataStore, DataObject dataObject) { StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); - if(storagePool == null) { - s_logger.error("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); - throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); + if (storagePool == null) { + s_logger.error("createCloudStackVolume: Storage Pool not found for id: {}", dataStore.getId()); + throw new CloudRuntimeException("createCloudStackVolume: Storage Pool not found for id: " + dataStore.getId()); } Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); - s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME)); - CloudStackVolume cloudStackVolumeRequest = Utility.createCloudStackVolumeRequestByProtocol(storagePool, details, volumeObject); - CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); - if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL)) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) { - return cloudStackVolume.getLun().getName(); - } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - return volumeObject.getUuid(); // return the volume UUID for agent as path for mounting + + if (dataObject.getType() == DataObjectType.VOLUME) { + VolumeInfo volumeObject = (VolumeInfo) dataObject; + + CloudStackVolume cloudStackVolumeRequest = Utility.createCloudStackVolumeRequestByProtocol(storagePool, details, volumeObject); + CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); + if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL)) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) { + s_logger.info("createCloudStackVolume: iSCSI LUN object created for volume [{}]", volumeObject.getId()); + volumeDetailsDao.addDetail(volumeObject.getId(), Constants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid(), false); + volumeDetailsDao.addDetail(volumeObject.getId(), Constants.LUN_DOT_NAME, cloudStackVolume.getLun().getName(), false); + VolumeVO volumeVO = volumeDao.findById(volumeObject.getId()); + if (volumeVO != null) { + volumeVO.setPath(null); + if (cloudStackVolume.getLun().getUuid() != null) { + volumeVO.setFolder(cloudStackVolume.getLun().getUuid()); + } + volumeVO.setPoolType(storagePool.getPoolType()); + volumeVO.setPoolId(storagePool.getId()); + volumeDao.update(volumeVO.getId(), volumeVO); + } + } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + s_logger.info("createCloudStackVolume: Managed NFS object created for volume [{}]", volumeObject.getId()); + // For Managed NFS, set pool fields on Volume + VolumeVO volumeVO = volumeDao.findById(volumeObject.getId()); + if (volumeVO != null) { + volumeVO.setPoolType(storagePool.getPoolType()); + volumeVO.setPoolId(storagePool.getId()); + volumeDao.update(volumeVO.getId(), volumeVO); + } + } else { + String errMsg = "createCloudStackVolume: Volume creation failed for dataObject: " + volumeObject; + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + return cloudStackVolume; } else { - String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + volumeObject; - s_logger.error(errMsg); - throw new CloudRuntimeException(errMsg); + throw new CloudRuntimeException("createCloudStackVolume: Unsupported DataObjectType: " + dataObject.getType()); } } - private CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map details, VolumeInfo volumeInfo) { - CloudStackVolume cloudStackVolumeRequest = null; - Svm svm = new Svm(); - svm.setName(details.get(Constants.SVM_NAME)); - String protocol = details.get(Constants.PROTOCOL); - if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { - cloudStackVolumeRequest = new CloudStackVolume(); - Lun lunRequest = new Lun(); - lunRequest.setSvm(svm); - - LunSpace lunSpace = new LunSpace(); - lunSpace.setSize(volumeInfo.getSize()); - lunRequest.setSpace(lunSpace); - //Lun name is full path like in unified "/vol/VolumeName/LunName" - String lunFullName = Utility.getLunName(storagePool.getName(), volumeInfo.getName()); - lunRequest.setName(lunFullName); - - String osType = Utility.getOSTypeFromHypervisor(storagePool.getHypervisor().name()); - lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType)); - - cloudStackVolumeRequest.setLun(lunRequest); - return cloudStackVolumeRequest; - } else { - throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol); + private String ensureLunMapped(StoragePoolVO storagePool, String svmName, String lunName, long scopeId) { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); + String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + + // Check existing map first. getLogicalAccess returns null (no exception) when map doesn't exist. + Map getMap = new HashMap<>(); + getMap.put(Constants.LUN_DOT_NAME, lunName); + getMap.put(Constants.SVM_DOT_NAME, svmName); + getMap.put(Constants.IGROUP_DOT_NAME, accessGroupName); + Map mapResp = storageStrategy.getLogicalAccess(getMap); + if (mapResp != null && mapResp.containsKey(Constants.LOGICAL_UNIT_NUMBER)) { + String lunNumber = mapResp.get(Constants.LOGICAL_UNIT_NUMBER); + s_logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}] with LUN number [{}]", lunName, accessGroupName, lunNumber); + return lunNumber; } + // Create if not exists + Map enableMap = new HashMap<>(); + enableMap.put(Constants.LUN_DOT_NAME, lunName); + enableMap.put(Constants.SVM_DOT_NAME, svmName); + enableMap.put(Constants.IGROUP_DOT_NAME, accessGroupName); + Map response = storageStrategy.enableLogicalAccess(enableMap); + if (response == null || !response.containsKey(Constants.LOGICAL_UNIT_NUMBER)) { + throw new CloudRuntimeException("ensureLunMapped: Failed to map LUN [" + lunName + "] to iGroup [" + accessGroupName + "]"); + } + return response.get(Constants.LOGICAL_UNIT_NUMBER); } @Override @@ -214,9 +273,32 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { // ManagedNFS qcow2 backing file deletion handled by KVM host/libvirt; nothing to do via ONTAP REST. s_logger.info("deleteAsync: ManagedNFS volume {} no-op ONTAP deletion", data.getId()); + } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); + VolumeInfo volumeObject = (VolumeInfo) data; + s_logger.info("deleteAsync: Deleting volume & LUN for volume id [{}]", volumeObject.getId()); + String lunName = volumeDetailsDao.findDetail(volumeObject.getId(), Constants.LUN_DOT_NAME).getValue(); + String lunUUID = volumeDetailsDao.findDetail(volumeObject.getId(), Constants.LUN_DOT_UUID).getValue(); + if (lunName == null) { + throw new CloudRuntimeException("deleteAsync: Missing LUN name for volume " + volumeObject.getId()); + } + CloudStackVolume delRequest = new CloudStackVolume(); + Lun lun = new Lun(); + lun.setName(lunName); + lun.setUuid(lunUUID); + delRequest.setLun(lun); + storageStrategy.deleteCloudStackVolume(delRequest); + // Set the result + commandResult.setResult(null); + commandResult.setSuccess(true); + s_logger.info("deleteAsync: Volume LUN [{}] deleted successfully", lunName); + } else { + throw new CloudRuntimeException("deleteAsync: Unsupported protocol for deletion: " + details.get(Constants.PROTOCOL)); } } } catch (Exception e) { + s_logger.error("deleteAsync: Failed for data object [{}]: {}", data, e.getMessage()); + commandResult.setSuccess(false); commandResult.setResult(e.getMessage()); } finally { callback.complete(commandResult); @@ -225,7 +307,6 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac @Override public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback callback) { - } @Override @@ -262,7 +343,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore try { StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if (storagePool == null) { - s_logger.error("grantAccess : Storage Pool not found for id: " + dataStore.getId()); + s_logger.error("grantAccess: Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("grantAccess : Storage Pool not found for id: " + dataStore.getId()); } if (storagePool.getScope() != ScopeType.CLUSTER && storagePool.getScope() != ScopeType.ZONE) { @@ -276,7 +357,24 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore s_logger.error("grantAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); throw new CloudRuntimeException("grantAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); } - grantAccessForVolume(storagePool, volumeVO, host); + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + String svmName = details.get(Constants.SVM_NAME); + String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue(); + long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); + // Validate initiator membership + validateHostInitiatorInIgroup(storagePool, svmName, scopeId, host); + // Ensure mapping exists + String lunNumber = ensureLunMapped(storagePool, svmName, cloudStackVolumeName, scopeId); + // Update Volume path if missing or changed + String iscsiPath = Constants.SLASH + storagePool.getPath() + Constants.SLASH + lunNumber; + if (volumeVO.getPath() == null || !volumeVO.getPath().equals(iscsiPath)) { + volumeVO.set_iScsiName(iscsiPath); + volumeVO.setPath(iscsiPath); + } + // Ensure pool fields are set (align with SolidFire) + volumeVO.setPoolType(storagePool.getPoolType()); + volumeVO.setPoolId(storagePool.getId()); + volumeDao.update(volumeVO.getId(), volumeVO); } else { s_logger.error("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); @@ -288,33 +386,20 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore return true; } - private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Host host) { + private void validateHostInitiatorInIgroup(StoragePoolVO storagePool, String svmName, long scopeId, Host host) { Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); - String svmName = details.get(Constants.SVM_NAME); - long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); - - if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - String accessGroupName = Utility.getIgroupName(svmName, scopeId); - CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); - s_logger.info("grantAccessForVolume: Retrieved LUN [{}] details for volume [{}]", cloudStackVolume.getLun().getName(), volumeVO.getName()); - AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); - if (!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { - s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); - throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName + "]"); - } - - Map enableLogicalAccessMap = new HashMap<>(); - enableLogicalAccessMap.put(Constants.LUN_DOT_NAME, volumeVO.getPath()); - enableLogicalAccessMap.put(Constants.SVM_DOT_NAME, svmName); - enableLogicalAccessMap.put(Constants.IGROUP_DOT_NAME, accessGroupName); - storageStrategy.enableLogicalAccess(enableLogicalAccessMap); - } else { - String errMsg = "grantAccessForVolume: Unsupported protocol type for volume grantAccess: " + details.get(Constants.PROTOCOL); - s_logger.error(errMsg); - throw new CloudRuntimeException(errMsg); + String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); + if (host == null || host.getStorageUrl() == null) { + throw new CloudRuntimeException("validateHostInitiatorInIgroup: host/initiator required but not provided"); + } + if (!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { + s_logger.error("validateHostInitiatorInIgroup: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); + throw new CloudRuntimeException("validateHostInitiatorInIgroup: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName + "]"); } } + private boolean hostInitiatorFoundInIgroup(String hostInitiator, Igroup igroup) { if(igroup != null && igroup.getInitiators() != null && hostInitiator != null && !hostInitiator.isEmpty()) { for(Initiator initiator : igroup.getInitiators()) { @@ -337,10 +422,21 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) if (host == null) { throw new InvalidParameterValueException("revokeAccess: host should not be null"); } + if (dataObject.getType() == DataObjectType.VOLUME) { + Volume volume = volumeDao.findById(dataObject.getId()); + if (volume.getInstanceId() != null) { + VirtualMachine vm = vmDao.findById(volume.getInstanceId()); + if (vm != null && !Arrays.asList(VirtualMachine.State.Destroyed, VirtualMachine.State.Expunging, VirtualMachine.State.Error).contains(vm.getState())) { + s_logger.debug("revokeAccess: Volume [{}] is still attached to VM [{}] in state [{}], skipping revokeAccess", + dataObject.getId(), vm.getInstanceName(), vm.getState()); + return; + } + } + } try { StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if (storagePool == null) { - s_logger.error("revokeAccess : Storage Pool not found for id: " + dataStore.getId()); + s_logger.error("revokeAccess: Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("revokeAccess : Storage Pool not found for id: " + dataStore.getId()); } if (storagePool.getScope() != ScopeType.CLUSTER && storagePool.getScope() != ScopeType.ZONE) { @@ -351,7 +447,7 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) if (dataObject.getType() == DataObjectType.VOLUME) { VolumeVO volumeVO = volumeDao.findById(dataObject.getId()); if (volumeVO == null) { - s_logger.error("revokeAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); + s_logger.error("revokeAccess: Cloud Stack Volume not found for id: " + dataObject.getId()); throw new CloudRuntimeException("revokeAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); } revokeAccessForVolume(storagePool, volumeVO, host); @@ -366,16 +462,34 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) } private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Host host) { + s_logger.info("revokeAccessForVolume: Revoking access to volume [{}] for host [{}]", volumeVO.getName(), host.getName()); Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); String svmName = details.get(Constants.SVM_NAME); long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - String accessGroupName = Utility.getIgroupName(svmName, scopeId); - CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); + String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + + String lunName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME) != null ? + volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue() : null; + if (lunName == null) { + s_logger.warn("revokeAccessForVolume: No LUN name detail found for volume [{}]; assuming no backend LUN to revoke", volumeVO.getId()); + return; + } + + CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, lunName); + if (cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getUuid() == null) { + s_logger.warn("revokeAccessForVolume: LUN for volume [{}] not found on ONTAP, assuming already deleted", volumeVO.getId()); + return; + } + AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); - //TODO check if initiator does exits in igroup, will throw the error ? + if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getUuid() == null) { + s_logger.warn("revokeAccessForVolume: iGroup [{}] not found on ONTAP, assuming already deleted", accessGroupName); + return; + } + if (!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { s_logger.error("revokeAccessForVolume: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); return; @@ -395,8 +509,8 @@ private CloudStackVolume getCloudStackVolumeByName(StorageStrategy storageStrate getCloudStackVolumeMap.put(Constants.SVM_DOT_NAME, svmName); CloudStackVolume cloudStackVolume = storageStrategy.getCloudStackVolume(getCloudStackVolumeMap); if (cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getName() == null) { - s_logger.error("getCloudStackVolumeByName: Failed to get LUN details [{}]", cloudStackVolumeName); - throw new CloudRuntimeException("getCloudStackVolumeByName: Failed to get LUN [" + cloudStackVolumeName + "]"); + s_logger.error("getCloudStackVolumeByName: LUN [{}] not found on ONTAP; returning null", cloudStackVolumeName); + return null; } return cloudStackVolume; } @@ -407,8 +521,8 @@ private AccessGroup getAccessGroupByName(StorageStrategy storageStrategy, String getAccessGroupMap.put(Constants.SVM_DOT_NAME, svmName); AccessGroup accessGroup = storageStrategy.getAccessGroup(getAccessGroupMap); if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getName() == null) { - s_logger.error("getAccessGroupByName: Failed to get iGroup details [{}]", accessGroupName); - throw new CloudRuntimeException("getAccessGroupByName: Failed to get iGroup details [" + accessGroupName + "]"); + s_logger.error("getAccessGroupByName: iGroup [{}] not found on ONTAP; returning null", accessGroupName); + return null; } return accessGroup; } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index 0da826f04fd2..45a20fe876fe 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java @@ -40,9 +40,6 @@ public interface SANFeignClient { @RequestLine("POST /api/storage/luns?return_records={returnRecords}") @Headers({"Authorization: {authHeader}"}) OntapResponse createLun(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, Lun lun); - @RequestLine("POST /api/storage/luns?return_records={returnRecords}") - @Headers({"Authorization: {authHeader}"}) - OntapResponse createLun(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, Lun lun); @RequestLine("GET /api/storage/luns") @Headers({"Authorization: {authHeader}"}) @@ -56,9 +53,9 @@ public interface SANFeignClient { @Headers({"Authorization: {authHeader}"}) void updateLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid, Lun lun); - @RequestLine("DELETE /{uuid}") + @RequestLine("DELETE /api/storage/luns/{uuid}") @Headers({"Authorization: {authHeader}"}) - void deleteLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid); + void deleteLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid, @QueryMap Map queryMap); // iGroup Operation APIs @RequestLine("POST /api/protocols/san/igroups?return_records={returnRecords}") diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Igroup.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Igroup.java index 877d60de830c..4dc07e349fad 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Igroup.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Igroup.java @@ -48,7 +48,7 @@ public class Igroup { private String name = null; @JsonProperty("protocol") - private ProtocolEnum protocol = ProtocolEnum.mixed; + private ProtocolEnum protocol = null; @JsonProperty("svm") private Svm svm = null; @JsonProperty("uuid") diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java index 48ebc9c739cb..364790958c8a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java @@ -83,6 +83,9 @@ public static PropertyClassEnum fromValue(String value) { @JsonProperty("name") private String name = null; + @JsonProperty("clone") + private Clone clone = null; + /** * The operating system type of the LUN.<br/> Required in POST when creating a LUN that is not a clone of another. Disallowed in POST when creating a LUN clone. */ @@ -249,6 +252,14 @@ public void setUuid(String uuid) { this.uuid = uuid; } + public Clone getClone() { + return clone; + } + + public void setClone(Clone clone) { + this.clone = clone; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -295,4 +306,36 @@ private String toIndentedString(Object o) { } return o.toString().replace("\n", "\n "); } + + + public static class Clone { + @JsonProperty("source") + private Source source = null; + public Source getSource() { + return source; + } + public void setSource(Source source) { + this.source = source; + } + } + + public static class Source { + @JsonProperty("name") + private String name = null; + @JsonProperty("uuid") + private String uuid = null; + + public String getName() { + return name; + } + public void setName(String name) { + this.name = name; + } + public String getUuid() { + return uuid; + } + public void setUuid(String uuid) { + this.uuid = uuid; + } + } } 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 9a37c8e65d44..6bbd88db14b0 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 @@ -43,13 +43,9 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDetailsDao; -import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl; -import org.apache.cloudstack.storage.feign.model.ExportPolicy; -import org.apache.cloudstack.storage.feign.model.Igroup; -import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.feign.model.OntapStorage; -import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.Volume; import org.apache.cloudstack.storage.provider.StorageProviderFactory; import org.apache.cloudstack.storage.service.StorageStrategy; @@ -289,10 +285,6 @@ public DataStore initialize(Map dsInfos) { @Override public boolean attachCluster(DataStore dataStore, ClusterScope scope) { logger.debug("In attachCluster for ONTAP primary storage"); - PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore; - List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(primaryStore); - - logger.debug(" datastore object received is {} ",primaryStore ); if (dataStore == null) { throw new InvalidParameterValueException("attachCluster: dataStore should not be null"); } @@ -307,25 +299,12 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { } PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore; List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInClusterForStorageConnection(primaryStore); - // TODO- need to check if no host to connect then throw exception or just continue + // TODO- need to check if no host to connect then throw exception or just continue? logger.debug("attachCluster: Eligible Up and Enabled hosts: {} in cluster {}", hostsToConnect, primaryStore.getClusterId()); - logger.debug(String.format("Attaching the pool to each of the hosts %s in the cluster: %s", hostsToConnect, primaryStore.getClusterId())); - Map details = storagePoolDetailsDao.listDetailsKeyPairs(primaryStore.getId()); StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); - ExportPolicy exportPolicy = new ExportPolicy(); - AccessGroup accessGroupRequest = new AccessGroup(); - accessGroupRequest.setHostsToConnect(hostsToConnect); - accessGroupRequest.setScope(scope); - primaryStore.setDetails(details);// setting details as it does not come from cloudstack - accessGroupRequest.setPrimaryDataStoreInfo(primaryStore); - accessGroupRequest.setPolicy(exportPolicy); - strategy.createAccessGroup(accessGroupRequest); - logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); - Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); - StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the cluster if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) { @@ -333,10 +312,16 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { s_logger.error(errMsg); throw new CloudRuntimeException(errMsg); } + + logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); //TODO - check if no host to connect then also need to create access group without initiators if (hostsIdentifier != null && hostsIdentifier.size() > 0) { try { - AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); + AccessGroup accessGroupRequest = new AccessGroup(); + accessGroupRequest.setHostsToConnect(hostsToConnect); + accessGroupRequest.setScope(scope); + primaryStore.setDetails(details);// setting details as it does not come from cloudstack + accessGroupRequest.setPrimaryDataStoreInfo(primaryStore); strategy.createAccessGroup(accessGroupRequest); } catch (Exception e) { s_logger.error("attachCluster: Failed to create access group on storage system for cluster: " + primaryStore.getClusterId() + ". Exception: " + e.getMessage()); @@ -348,9 +333,8 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { try { _storageMgr.connectHostToSharedPool(host, dataStore.getId()); } catch (Exception e) { - logger.warn("Unable to establish a connection between " + host + " and " + dataStore, e); - return false; logger.warn("attachCluster: Unable to establish a connection between " + host + " and " + dataStore, e); + return false; } } _dataStoreHelper.attachCluster(dataStore); @@ -384,20 +368,9 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper Map details = storagePoolDetailsDao.listDetailsKeyPairs(primaryStore.getId()); StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); - ExportPolicy exportPolicy = new ExportPolicy(); - AccessGroup accessGroupRequest = new AccessGroup(); - accessGroupRequest.setHostsToConnect(hostsToConnect); - accessGroupRequest.setScope(scope); - primaryStore.setDetails(details); // setting details as it does not come from cloudstack - accessGroupRequest.setPrimaryDataStoreInfo(primaryStore); - accessGroupRequest.setPolicy(exportPolicy); - strategy.createAccessGroup(accessGroupRequest); // TODO- need to check if no host to connect then throw exception or just continue logger.debug("attachZone: Eligible Up and Enabled hosts: {}", hostsToConnect); - - Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); - StorageStrategy strategy = Utility.getStrategyByStoragePoolDetails(details); ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); //TODO- Check if we have to handle heterogeneous host within the zone if (!validateProtocolSupportAndFetchHostsIdentifier(hostsToConnect, protocol, hostsIdentifier)) { @@ -407,7 +380,11 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper } if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) { try { - AccessGroup accessGroupRequest = createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); + AccessGroup accessGroupRequest = new AccessGroup(); + accessGroupRequest.setHostsToConnect(hostsToConnect); + accessGroupRequest.setScope(scope); + primaryStore.setDetails(details); // setting details as it does not come from cloudstack + accessGroupRequest.setPrimaryDataStoreInfo(primaryStore); strategy.createAccessGroup(accessGroupRequest); } catch (Exception e) { s_logger.error("attachZone: Failed to create access group on storage system for zone with Exception: " + e.getMessage()); @@ -445,64 +422,24 @@ private boolean validateProtocolSupportAndFetchHostsIdentifier(List host return true; } - private AccessGroup createAccessGroupRequestByProtocol(StoragePoolVO storagePool, long scopeId, Map details, List hostsIdentifier) { - ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toUpperCase()); - String svmName = details.get(Constants.SVM_NAME); - switch (protocol) { - case ISCSI: - // Access group name format: cs_svmName_scopeId - String igroupName = Utility.getIgroupName(svmName, scopeId); - Hypervisor.HypervisorType hypervisorType = storagePool.getHypervisor(); - return createSANAccessGroupRequest(svmName, igroupName, hypervisorType, hostsIdentifier); - default: - s_logger.error("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol); - throw new CloudRuntimeException("createAccessGroupRequestByProtocol: Unsupported protocol " + protocol); - } - } - - private AccessGroup createSANAccessGroupRequest(String svmName, String igroupName, Hypervisor.HypervisorType hypervisorType, List hostsIdentifier) { - AccessGroup accessGroupRequest = new AccessGroup(); - Igroup igroup = new Igroup(); - - if (svmName != null && !svmName.isEmpty()) { - Svm svm = new Svm(); - svm.setName(svmName); - igroup.setSvm(svm); - } - - if (igroupName != null && !igroupName.isEmpty()) { - igroup.setName(igroupName); - } - - if (hypervisorType != null) { - String hypervisorName = hypervisorType.name(); - igroup.setOsType(Igroup.OsTypeEnum.valueOf(Utility.getOSTypeFromHypervisor(hypervisorName))); - } - - if (hostsIdentifier != null && hostsIdentifier.size() > 0) { - List initiators = new ArrayList<>(); - for (String hostIdentifier : hostsIdentifier) { - Initiator initiator = new Initiator(); - initiator.setName(hostIdentifier); - initiators.add(initiator); - } - igroup.setInitiators(initiators); - } - accessGroupRequest.setIgroup(igroup); - s_logger.debug("createSANAccessGroupRequest: request: " + accessGroupRequest); - return accessGroupRequest; - } - @Override public boolean maintain(DataStore store) { - _storagePoolAutomation.maintain(store); - return _dataStoreHelper.maintain(store); + logger.info("Placing storage pool {} in maintenance mode", store); + if (_storagePoolAutomation.maintain(store)) { + return _dataStoreHelper.maintain(store); + } else { + return false; + } } @Override public boolean cancelMaintain(DataStore store) { - _storagePoolAutomation.cancelMaintain(store); - return _dataStoreHelper.cancelMaintain(store); + logger.info("Cancelling storage pool maintenance for {}", store); + if (_dataStoreHelper.cancelMaintain(store)) { + return _storagePoolAutomation.cancelMaintain(store); + } else { + return false; + } } @Override @@ -543,6 +480,24 @@ public boolean deleteDataStore(DataStore store) { s_logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName()); + // Call deleteStorageVolume to delete the underlying ONTAP volume + s_logger.info("deleteDataStore: Deleting ONTAP volume for storage pool '{}'", storagePool.getName()); + Volume volume = new Volume(); + volume.setUuid(details.get(Constants.VOLUME_UUID)); + volume.setName(details.get(Constants.VOLUME_NAME)); + try { + if (volume.getUuid() == null || volume.getUuid().isEmpty() || volume.getName() == null || volume.getName().isEmpty()) { + s_logger.error("deleteDataStore: Volume UUID/Name not found in details for storage pool id: {}, cannot delete volume", storagePoolId); + throw new CloudRuntimeException("Volume UUID/Name not found in details, cannot delete ONTAP volume"); + } + storageStrategy.deleteStorageVolume(volume); + s_logger.info("deleteDataStore: Successfully deleted ONTAP volume '{}' (UUID: {}) for storage pool '{}'", + volume.getName(), volume.getUuid(), storagePool.getName()); + } catch (Exception e) { + s_logger.error("deleteDataStore: Exception while retrieving volume UUID for storage pool id: {}. Error: {}", + storagePoolId, e.getMessage(), e); + } + } catch (Exception e) { s_logger.error("deleteDataStore: Failed to delete access groups for storage pool id: {}. Error: {}", storagePoolId, e.getMessage(), e); 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 bb25928438c6..7b20c946d6af 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 @@ -108,33 +108,55 @@ public boolean connect() { if (svms != null && svms.getRecords() != null && !svms.getRecords().isEmpty()) { svm = svms.getRecords().get(0); } else { - throw new CloudRuntimeException("No SVM found on the ONTAP cluster by the name" + svmName + "."); + s_logger.error("No SVM found on the ONTAP cluster by the name" + svmName + "."); + return false; } // Validations s_logger.info("Validating SVM state and protocol settings..."); if (!Objects.equals(svm.getState(), Constants.RUNNING)) { s_logger.error("SVM " + svmName + " is not in running state."); - throw new CloudRuntimeException("SVM " + svmName + " is not in running state."); + return false; } if (Objects.equals(storage.getProtocol(), Constants.NFS) && !svm.getNfsEnabled()) { s_logger.error("NFS protocol is not enabled on SVM " + svmName); - throw new CloudRuntimeException("NFS protocol is not enabled on SVM " + svmName); + return false; } else if (Objects.equals(storage.getProtocol(), Constants.ISCSI) && !svm.getIscsiEnabled()) { s_logger.error("iSCSI protocol is not enabled on SVM " + svmName); - throw new CloudRuntimeException("iSCSI protocol is not enabled on SVM " + svmName); + return false; } - + // TODO: Implement logic to select appropriate aggregate based on storage requirements List aggrs = svm.getAggregates(); if (aggrs == null || aggrs.isEmpty()) { s_logger.error("No aggregates are assigned to SVM " + svmName); - throw new CloudRuntimeException("No aggregates are assigned to SVM " + svmName); + return false; + } + // Set the aggregates which are according to the storage requirements + for (Aggregate aggr : aggrs) { + s_logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid()); + Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid()); + if (!Objects.equals(aggrResp.getState(), Aggregate.StateEnum.ONLINE)) { + s_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()) { + s_logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Skipping this aggregate."); + continue; + } + s_logger.info("Selected aggregate: " + aggr.getName() + " for volume operations."); + this.aggregates = List.of(aggr); + break; + } + if (this.aggregates == null || this.aggregates.isEmpty()) { + s_logger.error("No suitable aggregates found on SVM " + svmName + " for volume creation."); + return false; } this.aggregates = aggrs; s_logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided"); } catch (Exception e) { - throw new CloudRuntimeException("Failed to connect to ONTAP cluster: " + e.getMessage(), e); + s_logger.error("Failed to connect to ONTAP cluster: " + e.getMessage(), e); + return false; } return true; } @@ -472,7 +494,17 @@ public String getNetworkInterface() { * * @param cloudstackVolume the CloudStack volume to delete */ - abstract void deleteCloudStackVolume(CloudStackVolume cloudstackVolume); + abstract public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume); + + /** + * Method encapsulates the behavior based on the opted protocol in subclasses. + * it is going to mimic + * cloneLun for iSCSI, FC protocols + * cloneFile for NFS3.0 and NFS4.1 protocols + * cloneNameSpace for Nvme/TCP and Nvme/FC protocol + * @param cloudstackVolume the CloudStack volume to copy + */ + abstract public void copyCloudStackVolume(CloudStackVolume cloudstackVolume); /** * Method encapsulates the behavior based on the opted protocol in subclasses. @@ -527,18 +559,28 @@ public String getNetworkInterface() { * Method encapsulates the behavior based on the opted protocol in subclasses * lunMap for iSCSI and FC protocols * //TODO for Nvme/TCP and Nvme/FC protocols - * @param values + * @param values map including SVM name, LUN name, and igroup name + * @return map containing logical unit number for the new/existing mapping */ - abstract public void enableLogicalAccess(Map values); + abstract public Map enableLogicalAccess(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses * lunUnmap for iSCSI and FC protocols * //TODO for Nvme/TCP and Nvme/FC protocols - * @param values + * @param values map including LUN UUID and iGroup UUID */ abstract public void disableLogicalAccess(Map values); + /** + * Method encapsulates the behavior based on the opted protocol in subclasses + * lunMap lookup for iSCSI/FC protocols (GET-only, no side-effects) + * //TODO for Nvme/TCP and Nvme/FC protocols + * @param values map with SVM name, LUN name, and igroup name + * @return map containing logical unit number if mapping exists; otherwise null + */ + abstract public Map getLogicalAccess(Map values); + private Boolean jobPollForSuccess(String jobUUID) { //Create URI for GET Job API int jobRetryCount = 0; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java index 861d22ff68d9..9657edc614fa 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java @@ -110,10 +110,15 @@ CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { + public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { //TODO } + @Override + public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) { + + } + @Override public CloudStackVolume getCloudStackVolume(Map cloudStackVolumeMap) { //TODO @@ -194,8 +199,9 @@ public AccessGroup getAccessGroup(Map values) { } @Override - public void enableLogicalAccess(Map values) { + public Map enableLogicalAccess(Map values) { //TODO + return null; } @Override @@ -203,6 +209,12 @@ public void disableLogicalAccess(Map values) { //TODO } + @Override + public Map getLogicalAccess(Map values) { + // NAS does not use LUN mapping; nothing to fetch + return null; + } + private ExportPolicy createExportPolicy(String svmName, ExportPolicy policy) { s_logger.info("Creating export policy: {} for SVM: {}", policy, svmName); 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 47f529968deb..fd1f2bb6b84a 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 @@ -27,13 +27,10 @@ import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Initiator; -import org.apache.cloudstack.storage.feign.model.*; import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.LunMap; -import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -103,8 +100,60 @@ CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { - //TODO + public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { + if (cloudstackVolume == null || cloudstackVolume.getLun() == null) { + s_logger.error("deleteCloudStackVolume: Lun deletion failed. Invalid request: {}", cloudstackVolume); + throw new CloudRuntimeException("deleteCloudStackVolume : Failed to delete Lun, invalid request"); + } + s_logger.info("deleteCloudStackVolume : Deleting Lun: {}", cloudstackVolume.getLun().getName()); + try { + String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); + Map queryParams = Map.of("allow_delete_while_mapped", "true"); + try { + sanFeignClient.deleteLun(authHeader, cloudstackVolume.getLun().getUuid(), queryParams); + } catch (Exception ex) { + String errMsg = ex.getMessage(); + if (errMsg != null && (errMsg.contains("entry doesn't exist") + || errMsg.contains("does not exist") + || errMsg.contains("not found") + || errMsg.contains("status 404"))) { + s_logger.warn("deleteCloudStackVolume: Lun {} does not exist ({}), skipping deletion", cloudstackVolume.getLun().getName(), errMsg); + return; + } + throw ex; + } + s_logger.info("deleteCloudStackVolume: Lun deleted successfully. LunName: {}", cloudstackVolume.getLun().getName()); + } catch (Exception e) { + s_logger.error("Exception occurred while deleting Lun: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); + throw new CloudRuntimeException("Failed to delete Lun: " + e.getMessage()); + } + } + + @Override + public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) { + s_logger.debug("copyCloudStackVolume: Creating clone of the cloudstack volume: {}", cloudstackVolume.getLun().getName()); + if (cloudstackVolume == null || cloudstackVolume.getLun() == null) { + s_logger.error("copyCloudStackVolume: Lun clone creation failed. Invalid request: {}", cloudstackVolume); + throw new CloudRuntimeException("copyCloudStackVolume : Failed to create Lun clone, invalid request"); + } + + try { + // Get AuthHeader + String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // Create URI for lun clone creation + Lun lunCloneRequest = cloudstackVolume.getLun(); + Lun.Clone clone = new Lun.Clone(); + Lun.Source source = new Lun.Source(); + source.setName(cloudstackVolume.getLun().getName()); + clone.setSource(source); + lunCloneRequest.setClone(clone); + String lunCloneName = cloudstackVolume.getLun().getName() + "_clone"; + lunCloneRequest.setName(lunCloneName); + sanFeignClient.createLun(authHeader, true, lunCloneRequest); + } catch (Exception e) { + s_logger.error("Exception occurred while creating Lun clone: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); + throw new CloudRuntimeException("Failed to create Lun clone: " + e.getMessage()); + } } @Override @@ -117,19 +166,17 @@ public CloudStackVolume getCloudStackVolume(Map values) { } String svmName = values.get(Constants.SVM_DOT_NAME); String lunName = values.get(Constants.NAME); - if(svmName == null || lunName == null || svmName.isEmpty() || lunName.isEmpty()) { + if (svmName == null || lunName == null || svmName.isEmpty() || lunName.isEmpty()) { s_logger.error("getCloudStackVolume: get Lun failed. Invalid svm:{} or Lun name: {}", svmName, lunName); throw new CloudRuntimeException("getCloudStackVolume : Failed to get Lun, invalid request"); } try { - // Get AuthHeader String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); - // get Igroup Map queryParams = Map.of(Constants.SVM_DOT_NAME, svmName, Constants.NAME, lunName); OntapResponse lunResponse = sanFeignClient.getLunResponse(authHeader, queryParams); - if (lunResponse == null || lunResponse.getRecords() == null || lunResponse.getRecords().size() == 0) { - s_logger.error("getCloudStackVolume: Failed to fetch Lun"); - throw new CloudRuntimeException("getCloudStackVolume: Failed to fetch Lun"); + if (lunResponse == null || lunResponse.getRecords() == null || lunResponse.getRecords().isEmpty()) { + s_logger.warn("getCloudStackVolume: Lun '{}' on SVM '{}' not found. Returning null.", lunName, svmName); + return null; } Lun lun = lunResponse.getRecords().get(0); s_logger.debug("getCloudStackVolume: Lun Details : {}", lun); @@ -139,16 +186,22 @@ public CloudStackVolume getCloudStackVolume(Map values) { cloudStackVolume.setLun(lun); return cloudStackVolume; } catch (Exception e) { - s_logger.error("Exception occurred while fetching Lun, Exception: {}", e.getMessage()); - throw new CloudRuntimeException("Failed to fetch Lun details: " + e.getMessage()); + String errMsg = e.getMessage(); + if (errMsg != null && errMsg.contains("not found")) { + s_logger.warn("getCloudStackVolume: Lun '{}' on SVM '{}' not found ({}). Returning null.", lunName, svmName, errMsg); + return null; + } + s_logger.error("Exception occurred while fetching Lun, Exception: {}", errMsg); + throw new CloudRuntimeException("Failed to fetch Lun details: " + errMsg); } } @Override public AccessGroup createAccessGroup(AccessGroup accessGroup) { s_logger.info("createAccessGroup : Create Igroup"); + String igroupName = "unknown"; s_logger.debug("createAccessGroup : Creating Igroup with access group request {} ", accessGroup); - if (accessGroup == null || accessGroup.getIgroup() == null) { + if (accessGroup == null) { s_logger.error("createAccessGroup: Igroup creation failed. Invalid request: {}", accessGroup); throw new CloudRuntimeException("createAccessGroup : Failed to create Igroup, invalid request"); } @@ -351,38 +404,42 @@ public AccessGroup getAccessGroup(Map values) { throw new CloudRuntimeException("getAccessGroup : get Igroup Failed, invalid request"); } String svmName = values.get(Constants.SVM_DOT_NAME); - String igroupName = values.get(Constants.IGROUP_DOT_NAME); - if(svmName == null || igroupName == null || svmName.isEmpty() || igroupName.isEmpty()) { + String igroupName = values.get(Constants.NAME); + if (svmName == null || igroupName == null || svmName.isEmpty() || igroupName.isEmpty()) { s_logger.error("getAccessGroup: get Igroup failed. Invalid svm:{} or igroup name: {}", svmName, igroupName); throw new CloudRuntimeException("getAccessGroup : Failed to get Igroup, invalid request"); } try { - // Get AuthHeader String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); - // get Igroup - Map queryParams = Map.of(Constants.SVM_DOT_NAME, svmName, Constants.IGROUP_DOT_NAME, igroupName); + Map queryParams = Map.of(Constants.SVM_DOT_NAME, svmName, Constants.NAME, igroupName, Constants.FIELDS, Constants.INITIATORS); OntapResponse igroupResponse = sanFeignClient.getIgroupResponse(authHeader, queryParams); - if (igroupResponse == null || igroupResponse.getRecords() == null || igroupResponse.getRecords().size() == 0) { - s_logger.error("getAccessGroup: Failed to fetch Igroup"); - throw new CloudRuntimeException("Failed to fetch Igroup"); + if (igroupResponse == null || igroupResponse.getRecords() == null || igroupResponse.getRecords().isEmpty()) { + s_logger.warn("getAccessGroup: Igroup '{}' not found on SVM '{}'. Returning null.", igroupName, svmName); + return null; } Igroup igroup = igroupResponse.getRecords().get(0); AccessGroup accessGroup = new AccessGroup(); accessGroup.setIgroup(igroup); return accessGroup; } catch (Exception e) { - s_logger.error("Exception occurred while fetching Igroup, Exception: {}", e.getMessage()); - throw new CloudRuntimeException("Failed to fetch Igroup details: " + e.getMessage()); + String errMsg = e.getMessage(); + if (errMsg != null && errMsg.contains("not found")) { + s_logger.warn("getAccessGroup: Igroup '{}' not found on SVM '{}' ({}). Returning null.", igroupName, svmName, errMsg); + return null; + } + s_logger.error("Exception occurred while fetching Igroup, Exception: {}", errMsg); + throw new CloudRuntimeException("Failed to fetch Igroup details: " + errMsg); } } - public void enableLogicalAccess(Map values) { + public Map enableLogicalAccess(Map values) { s_logger.info("enableLogicalAccess : Create LunMap"); s_logger.debug("enableLogicalAccess : Creating LunMap with values {} ", values); + Map response = null; String svmName = values.get(Constants.SVM_DOT_NAME); String lunName = values.get(Constants.LUN_DOT_NAME); String igroupName = values.get(Constants.IGROUP_DOT_NAME); - if(svmName == null || lunName == null || igroupName == null || svmName.isEmpty() || lunName.isEmpty() || igroupName.isEmpty()) { + if (svmName == null || lunName == null || igroupName == null || svmName.isEmpty() || lunName.isEmpty() || igroupName.isEmpty()) { s_logger.error("enableLogicalAccess: LunMap creation failed. Invalid request values: {}", values); throw new CloudRuntimeException("enableLogicalAccess : Failed to create LunMap, invalid request"); } @@ -402,18 +459,41 @@ public void enableLogicalAccess(Map values) { Igroup igroup = new Igroup(); igroup.setName(igroupName); lunMapRequest.setIgroup(igroup); - OntapResponse createdLunMap = sanFeignClient.createLunMap(authHeader, true, lunMapRequest); - if (createdLunMap == null || createdLunMap.getRecords() == null || createdLunMap.getRecords().size() == 0) { - s_logger.error("enableLogicalAccess: LunMap failed for Lun: {} and igroup: {}", lunName, igroupName); - throw new CloudRuntimeException("Failed to perform LunMap for Lun: " + lunName + " and igroup: " + igroupName); + try { + sanFeignClient.createLunMap(authHeader, true, lunMapRequest); + } catch (Exception feignEx) { + String errMsg = feignEx.getMessage(); + if (errMsg != null && errMsg.contains(("LUN already mapped to this group"))) { + s_logger.warn("enableLogicalAccess: LunMap for Lun: {} and igroup: {} already exists.", lunName, igroupName); + } else { + s_logger.error("enableLogicalAccess: Exception during Feign call: {}", feignEx.getMessage(), feignEx); + throw feignEx; + } + } + // Get the LunMap details + OntapResponse lunMapResponse = null; + try { + lunMapResponse = sanFeignClient.getLunMapResponse(authHeader, + Map.of( + Constants.SVM_DOT_NAME, svmName, + Constants.LUN_DOT_NAME, lunName, + Constants.IGROUP_DOT_NAME, igroupName, + Constants.FIELDS, Constants.LOGICAL_UNIT_NUMBER + )); + response = Map.of( + Constants.LOGICAL_UNIT_NUMBER, lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString() + ); + } catch (Exception e) { + s_logger.error("enableLogicalAccess: Failed to fetch LunMap details for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e); + throw new CloudRuntimeException("Failed to fetch LunMap details for Lun: " + lunName + " and igroup: " + igroupName); } - LunMap lunMap = createdLunMap.getRecords().get(0); - s_logger.debug("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMap); + s_logger.debug("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMapResponse.getRecords().get(0)); s_logger.info("enableLogicalAccess: LunMap created successfully."); } catch (Exception e) { - s_logger.error("Exception occurred while creating LunMap, Exception: {}", e); + s_logger.error("Exception occurred while creating LunMap", e); throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage()); } + return response; } public void disableLogicalAccess(Map values) { @@ -421,19 +501,53 @@ public void disableLogicalAccess(Map values) { s_logger.debug("disableLogicalAccess : Deleting LunMap with values {} ", values); String lunUUID = values.get(Constants.LUN_DOT_UUID); String igroupUUID = values.get(Constants.IGROUP_DOT_UUID); - if(lunUUID == null || igroupUUID == null || lunUUID.isEmpty() || igroupUUID.isEmpty()) { + if (lunUUID == null || igroupUUID == null || lunUUID.isEmpty() || igroupUUID.isEmpty()) { s_logger.error("disableLogicalAccess: LunMap deletion failed. Invalid request values: {}", values); throw new CloudRuntimeException("disableLogicalAccess : Failed to delete LunMap, invalid request"); } try { - // Get AuthHeader String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); - // LunMap delete sanFeignClient.deleteLunMap(authHeader, lunUUID, igroupUUID); s_logger.info("disableLogicalAccess: LunMap deleted successfully."); } catch (Exception e) { - s_logger.error("Exception occurred while deleting LunMap, Exception: {}", e); - throw new CloudRuntimeException("Failed to delete LunMap: " + e.getMessage()); + String errMsg = e.getMessage(); + if (errMsg != null && errMsg.contains("not found")) { + s_logger.warn("disableLogicalAccess: LunMap with Lun UUID: {} and igroup UUID: {} does not exist ({}), skipping deletion", lunUUID, igroupUUID, errMsg); + return; + } + s_logger.error("Exception occurred while deleting LunMap", e); + throw new CloudRuntimeException("Failed to delete LunMap: " + errMsg); + } + } + + // GET-only helper: fetch LUN-map and return logical unit number if it exists; otherwise return null + public Map getLogicalAccess(Map values) { + s_logger.info("getLogicalAccess : Fetch LunMap"); + s_logger.debug("getLogicalAccess : Fetching LunMap with values {} ", values); + String svmName = values.get(Constants.SVM_DOT_NAME); + String lunName = values.get(Constants.LUN_DOT_NAME); + String igroupName = values.get(Constants.IGROUP_DOT_NAME); + if (svmName == null || lunName == null || igroupName == null || svmName.isEmpty() || lunName.isEmpty() || igroupName.isEmpty()) { + s_logger.error("getLogicalAccess: Invalid request values: {}", values); + throw new CloudRuntimeException("getLogicalAccess : Invalid request"); } + try { + String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); + OntapResponse lunMapResponse = sanFeignClient.getLunMapResponse(authHeader, + Map.of( + Constants.SVM_DOT_NAME, svmName, + Constants.LUN_DOT_NAME, lunName, + Constants.IGROUP_DOT_NAME, igroupName, + Constants.FIELDS, Constants.LOGICAL_UNIT_NUMBER + )); + if (lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty()) { + String lunNumber = lunMapResponse.getRecords().get(0).getLogicalUnitNumber() != null ? + lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString() : null; + return lunNumber != null ? Map.of(Constants.LOGICAL_UNIT_NUMBER, lunNumber) : null; + } + } catch (Exception e) { + s_logger.warn("getLogicalAccess: LunMap not found for Lun: {} and igroup: {} ({}).", lunName, igroupName, e.getMessage()); + } + return null; } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java index e71a26577fa9..43f8511967e7 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java @@ -53,9 +53,9 @@ public class Constants { // Query params public static final String NAME = "name"; public static final String FIELDS = "fields"; + public static final String INITIATORS = "initiators"; public static final String AGGREGATES = "aggregates"; public static final String STATE = "state"; - public static final String SVMDOTNAME = "svm.name"; public static final String DATA_NFS = "data_nfs"; public static final String DATA_ISCSI = "data_iscsi"; public static final String IP_ADDRESS = "ip.address"; @@ -81,6 +81,7 @@ public class Constants { public static final String LUN_DOT_NAME = "lun.name"; public static final String IQN = "iqn"; public static final String LUN_DOT_UUID = "lun.uuid"; + public static final String LOGICAL_UNIT_NUMBER = "logical_unit_number"; public static final String IGROUP_DOT_NAME = "igroup.name"; public static final String IGROUP_DOT_UUID = "igroup.uuid"; public static final String UNDERSCORE = "_"; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index 9532bf36fd9a..9d5eac8b2cea 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -19,19 +19,19 @@ package org.apache.cloudstack.storage.utils; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.storage.ScopeType; -import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.StringUtils; import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.LunSpace; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.provider.StorageProviderFactory; import org.apache.cloudstack.storage.service.StorageStrategy; -import org.apache.cloudstack.storage.feign.model.Igroup; -import org.apache.cloudstack.storage.feign.model.Initiator; -import org.apache.cloudstack.storage.provider.StorageProviderFactory; -import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -70,31 +70,27 @@ public static CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePo cloudStackVolumeRequest.setVolumeInfo(volumeObject); break; case ISCSI: - cloudStackVolumeRequest = new CloudStackVolume(); - Lun lunRequest = new Lun(); Svm svm = new Svm(); svm.setName(details.get(Constants.SVM_NAME)); + cloudStackVolumeRequest = new CloudStackVolume(); + Lun lunRequest = new Lun(); lunRequest.setSvm(svm); LunSpace lunSpace = new LunSpace(); lunSpace.setSize(volumeObject.getSize()); lunRequest.setSpace(lunSpace); //Lun name is full path like in unified "/vol/VolumeName/LunName" - String lunFullName = Constants.VOLUME_PATH_PREFIX + storagePool.getName() + Constants.SLASH + volumeObject.getName(); + String lunName = volumeObject.getName().replace(Constants.HYPHEN, Constants.UNDERSCORE); + if(!isValidName(lunName)) { + String errMsg = "createAsync: Invalid dataObject name [" + lunName + "]. It must start with a letter and can only contain letters, digits, and underscores, and be up to 200 characters long."; + throw new InvalidParameterValueException(errMsg); + } + String lunFullName = getLunName(storagePool.getName(), lunName); lunRequest.setName(lunFullName); - String hypervisorType = storagePool.getHypervisor().name(); - String osType = null; - switch (hypervisorType) { - case Constants.KVM: - osType = Lun.OsTypeEnum.LINUX.getValue(); - break; - default: - String errMsg = "createCloudStackVolume : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage"; - s_logger.error(errMsg); - throw new CloudRuntimeException(errMsg); - } + String osType = getOSTypeFromHypervisor(storagePool.getHypervisor().name()); lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType)); + cloudStackVolumeRequest.setLun(lunRequest); break; default: @@ -104,6 +100,15 @@ public static CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePo return cloudStackVolumeRequest; } + public static boolean isValidName(String name) { + // Check for null and length constraint first + if (name == null || name.length() > 200) { + return false; + } + // Regex: Starts with a letter, followed by letters, digits, or underscores + return name.matches(Constants.ONTAP_NAME_REGEX); + } + public static String getOSTypeFromHypervisor(String hypervisorType){ switch (hypervisorType) { case Constants.KVM: @@ -137,7 +142,7 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map Date: Thu, 22 Jan 2026 13:24:08 +0530 Subject: [PATCH 24/29] CSTACKEX-46: Fixed a check style issue which got introduced during rebase and renamed plugin name --- .../cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java | 3 --- .../storage/provider/OntapPrimaryDatastoreProvider.java | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) 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 04a2300b5578..98c34ef94b72 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 @@ -31,9 +31,6 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; -import com.cloud.storage.dao.VMTemplateDao; -//import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachine; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java index 91bfd0a8584c..1aadec79b3ab 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java @@ -65,7 +65,7 @@ public HypervisorHostListener getHostListener() { @Override public String getName() { s_logger.trace("OntapPrimaryDatastoreProvider: getName: Called"); - return "ONTAP Primary Datastore Provider"; + return "ONTAP"; } @Override From 3eea5c442e3f4416562131975c0dd9b7cd614614 Mon Sep 17 00:00:00 2001 From: "Locharla, Sandeep" Date: Thu, 22 Jan 2026 15:04:51 +0530 Subject: [PATCH 25/29] CSTACKEX-46: Fixed a check style issue which got introduced during rebase in agent code --- .../com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java index 155e97b90558..433e173fbbf9 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java @@ -37,7 +37,6 @@ import com.cloud.storage.Storage.StoragePoolType; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.OutputInterpreter; -import com.cloud.utils.script.OutputInterpreter.AllLinesParser; import com.cloud.utils.script.Script; public class IscsiAdmStorageAdaptor implements StorageAdaptor { From 63addcd3e7046f5b99b3564f2d3b233128be1814 Mon Sep 17 00:00:00 2001 From: "Locharla, Sandeep" Date: Tue, 27 Jan 2026 12:45:14 +0530 Subject: [PATCH 26/29] CSTACKEX-46: Fixed a couple of issues observed while testing NFS3 storagePool create and delete --- .../OntapPrimaryDatastoreLifecycle.java | 28 +++++++++++++------ .../storage/service/StorageStrategy.java | 15 ++++++++-- 2 files changed, 32 insertions(+), 11 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 6bbd88db14b0..eabd6482572c 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 @@ -415,6 +415,20 @@ private boolean validateProtocolSupportAndFetchHostsIdentifier(List host hostIdentifiers.add(host.getStorageUrl()); } break; + case NFS3: + String ip = ""; + for (HostVO host : hosts) { + if (host != null) { + ip = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : ""; + if (ip.isEmpty() && host.getPrivateIpAddress() != null || host.getPrivateIpAddress().trim().isEmpty()) { + return false; + } else { + ip = ip.isEmpty() ? host.getPrivateIpAddress().trim() : ip; + } + } + hostIdentifiers.add(ip); + } + break; default: throw new CloudRuntimeException("validateProtocolSupportAndFetchHostsIdentifier : Unsupported protocol: " + protocolType.name()); } @@ -471,15 +485,6 @@ public boolean deleteDataStore(DataStore store) { PrimaryDataStoreInfo primaryDataStoreInfo = (PrimaryDataStoreInfo) store; primaryDataStoreInfo.setDetails(details); - // Create AccessGroup object with PrimaryDataStoreInfo - AccessGroup accessGroup = new AccessGroup(); - accessGroup.setPrimaryDataStoreInfo(primaryDataStoreInfo); - - // Call deleteAccessGroup - it will figure out scope, protocol, and all details internally - storageStrategy.deleteAccessGroup(accessGroup); - - s_logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName()); - // Call deleteStorageVolume to delete the underlying ONTAP volume s_logger.info("deleteDataStore: Deleting ONTAP volume for storage pool '{}'", storagePool.getName()); Volume volume = new Volume(); @@ -497,6 +502,11 @@ public boolean deleteDataStore(DataStore store) { s_logger.error("deleteDataStore: Exception while retrieving volume UUID for storage pool id: {}. Error: {}", storagePoolId, e.getMessage(), e); } + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setPrimaryDataStoreInfo(primaryDataStoreInfo); + // Delete access groups associated with this storage pool + storageStrategy.deleteAccessGroup(accessGroup); + s_logger.info("deleteDataStore: Successfully deleted access groups for storage pool '{}'", storagePool.getName()); } catch (Exception e) { s_logger.error("deleteDataStore: Failed to delete access groups for storage pool id: {}. Error: {}", 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 7b20c946d6af..89ad712e9665 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 @@ -447,8 +447,19 @@ public String getNetworkInterface() { OntapResponse response = networkFeignClient.getNetworkIpInterfaces(authHeader, queryParams); if (response != null && response.getRecords() != null && !response.getRecords().isEmpty()) { - // For simplicity, return the first interface's name - IpInterface ipInterface = response.getRecords().get(0); + IpInterface ipInterface = null; + // For simplicity, return the first interface's name (Of IPv4 type for NFS3) + if (storage.getProtocol() == ProtocolType.ISCSI) { + ipInterface = response.getRecords().get(0); + } else if (storage.getProtocol() == ProtocolType.NFS3) { + for (IpInterface iface : response.getRecords()) { + if (iface.getIp().getAddress().contains(".")) { + ipInterface = iface; + break; + } + } + } + s_logger.info("Retrieved network interface: " + ipInterface.getIp().getAddress()); return ipInterface.getIp().getAddress(); } else { From dfc1ee97bf85dedf06c4005f0fa3854d26ef2d88 Mon Sep 17 00:00:00 2001 From: "Locharla, Sandeep" Date: Wed, 28 Jan 2026 20:27:55 +0530 Subject: [PATCH 27/29] CSTACKEX-46: Refactored driver class --- .../driver/OntapPrimaryDatastoreDriver.java | 460 ++++++++++-------- .../storage/service/SANStrategy.java | 21 + .../storage/service/StorageStrategy.java | 11 +- .../storage/service/UnifiedSANStrategy.java | 63 +++ 4 files changed, 359 insertions(+), 196 deletions(-) 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 98c34ef94b72..ea3b8a89673a 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 @@ -50,10 +50,10 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.model.Igroup; -import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.service.SANStrategy; import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.service.UnifiedSANStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; @@ -67,6 +67,19 @@ import java.util.HashMap; import java.util.Map; +/** + * Primary datastore driver for NetApp ONTAP storage systems. + * This driver handles volume lifecycle operations (create, delete, grant/revoke access) + * for both iSCSI (LUN-based) and NFS protocols against ONTAP storage backends. + * + * For iSCSI protocol: + * - Creates LUNs on ONTAP and maps them to initiator groups (igroups) + * - Manages LUN mappings for host access control + * + * For NFS protocol: + * - Delegates file operations to KVM host/libvirt + * - ONTAP volume/export management handled at storage pool creation time + */ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreDriver.class); @@ -84,7 +97,6 @@ public Map getCapabilities() { // TODO Set it to false once we start supporting snapshot feature mapCapabilities.put(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString(), Boolean.FALSE.toString()); mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_SNAPSHOT.toString(), Boolean.FALSE.toString()); - return mapCapabilities; } @@ -94,12 +106,31 @@ public DataTO getTO(DataObject data) { } @Override - public DataStoreTO getStoreTO(DataStore store) { return null; } + public DataStoreTO getStoreTO(DataStore store) { + return null; + } + /** + * Asynchronously creates a volume on the ONTAP storage system. + * + * For iSCSI protocol: + * - Creates a LUN on ONTAP via the SAN strategy + * - Stores LUN UUID and name in volume_details table for later reference + * - Creates a LUN mapping to the appropriate igroup (based on cluster/zone scope) + * - Sets the iSCSI path on the volume for host attachment + * + * For NFS protocol: + * - Associates the volume with the storage pool (actual file creation handled by hypervisor) + * + * @param dataStore The target data store (storage pool) + * @param dataObject The volume to create + * @param callback Callback to notify completion + */ @Override public void createAsync(DataStore dataStore, DataObject dataObject, AsyncCompletionCallback callback) { CreateCmdResult createCmdResult = null; - String errMsg = null; + String errMsg; + if (dataStore == null) { throw new InvalidParameterValueException("createAsync: dataStore should not be null"); } @@ -109,54 +140,68 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet if (callback == null) { throw new InvalidParameterValueException("createAsync: callback should not be null"); } + try { - s_logger.info("createAsync: Started for data store name [{}] and data object name [{}] of type [{}]", dataStore.getName(), dataObject.getName(), dataObject.getType()); + s_logger.info("createAsync: Started for data store name [{}] and data object name [{}] of type [{}]", + dataStore.getName(), dataObject.getName(), dataObject.getType()); + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if (storagePool == null) { - s_logger.error("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); - throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); + s_logger.error("createAsync: Storage Pool not found for id: " + dataStore.getId()); + throw new CloudRuntimeException("createAsync: Storage Pool not found for id: " + dataStore.getId()); } + Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); if (dataObject.getType() == DataObjectType.VOLUME) { VolumeInfo volInfo = (VolumeInfo) dataObject; - // Create LUN/backing for volume and record relevant details - CloudStackVolume created = createCloudStackVolume(dataStore, volInfo); - // Immediately ensure LUN-map exists and update VolumeVO path - if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - String svmName = details.get(Constants.SVM_NAME); - String lunName = volumeDetailsDao.findDetail(volInfo.getId(), Constants.LUN_DOT_NAME) != null ? - volumeDetailsDao.findDetail(volInfo.getId(), Constants.LUN_DOT_NAME).getValue() : null; - if (lunName == null) { - // Fallback from returned LUN - lunName = created != null && created.getLun() != null ? created.getLun().getName() : null; - } - if (lunName == null) { - throw new CloudRuntimeException("createAsync: Missing LUN name for volume " + volInfo.getId()); - } - long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? storagePool.getClusterId() : storagePool.getDataCenterId(); - String lunNumber = ensureLunMapped(storagePool, svmName, lunName, scopeId); + // Create the backend storage object (LUN for iSCSI, no-op for NFS) + CloudStackVolume created = createCloudStackVolume(dataStore, volInfo, details); - VolumeVO volumeVO = volumeDao.findById(volInfo.getId()); - if (volumeVO != null) { + // Update CloudStack volume record with storage pool association and protocol-specific details + VolumeVO volumeVO = volumeDao.findById(volInfo.getId()); + if (volumeVO != null) { + volumeVO.setPoolType(storagePool.getPoolType()); + volumeVO.setPoolId(storagePool.getId()); + + if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + String svmName = details.get(Constants.SVM_NAME); + String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null; + if (lunName == null) { + throw new CloudRuntimeException("createAsync: Missing LUN name for volume " + volInfo.getId()); + } + + // Determine scope ID based on storage pool scope (cluster or zone level igroup) + long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) + ? storagePool.getClusterId() + : storagePool.getDataCenterId(); + + // Persist LUN details for future operations (delete, grant/revoke access) + volumeDetailsDao.addDetail(volInfo.getId(), Constants.LUN_DOT_UUID, created.getLun().getUuid(), false); + volumeDetailsDao.addDetail(volInfo.getId(), Constants.LUN_DOT_NAME, lunName, false); + if (created.getLun().getUuid() != null) { + volumeVO.setFolder(created.getLun().getUuid()); + } + + // Create LUN-to-igroup mapping and retrieve the assigned LUN ID + UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details); + String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName); + + // Construct iSCSI path: // format for KVM/libvirt attachment String iscsiPath = Constants.SLASH + storagePool.getPath() + Constants.SLASH + lunNumber; volumeVO.set_iScsiName(iscsiPath); volumeVO.setPath(iscsiPath); - volumeVO.setPoolType(storagePool.getPoolType()); - volumeVO.setPoolId(storagePool.getId()); - volumeDao.update(volumeVO.getId(), volumeVO); s_logger.info("createAsync: Volume [{}] iSCSI path set to {}", volumeVO.getId(), iscsiPath); + + } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + // For NFS, the hypervisor handles file creation; we only track pool association + s_logger.info("createAsync: Managed NFS volume [{}] associated with pool {}", + volumeVO.getId(), storagePool.getId()); } - } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - // Ensure pool fields are recorded for managed NFS as well - VolumeVO volumeVO = volumeDao.findById(volInfo.getId()); - if (volumeVO != null) { - volumeVO.setPoolType(storagePool.getPoolType()); - volumeVO.setPoolId(storagePool.getId()); - volumeDao.update(volumeVO.getId(), volumeVO); - s_logger.info("createAsync: Managed NFS volume [{}] associated with pool {}", volumeVO.getId(), storagePool.getId()); - } + + volumeDao.update(volumeVO.getId(), volumeVO); } createCmdResult = new CreateCmdResult(null, new Answer(null, true, null)); } else { @@ -177,82 +222,46 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } } - private CloudStackVolume createCloudStackVolume(DataStore dataStore, DataObject dataObject) { + /** + * Creates a CloudStack volume on the ONTAP backend using the appropriate storage strategy. + * + * @param dataStore The target data store + * @param dataObject The volume to create + * @param details Storage pool configuration details + * @return CloudStackVolume containing the created backend object (LUN for iSCSI) + */ + private CloudStackVolume createCloudStackVolume(DataStore dataStore, DataObject dataObject, Map details) { StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if (storagePool == null) { s_logger.error("createCloudStackVolume: Storage Pool not found for id: {}", dataStore.getId()); throw new CloudRuntimeException("createCloudStackVolume: Storage Pool not found for id: " + dataStore.getId()); } - Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); + StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); if (dataObject.getType() == DataObjectType.VOLUME) { VolumeInfo volumeObject = (VolumeInfo) dataObject; - CloudStackVolume cloudStackVolumeRequest = Utility.createCloudStackVolumeRequestByProtocol(storagePool, details, volumeObject); - CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); - if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL)) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) { - s_logger.info("createCloudStackVolume: iSCSI LUN object created for volume [{}]", volumeObject.getId()); - volumeDetailsDao.addDetail(volumeObject.getId(), Constants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid(), false); - volumeDetailsDao.addDetail(volumeObject.getId(), Constants.LUN_DOT_NAME, cloudStackVolume.getLun().getName(), false); - VolumeVO volumeVO = volumeDao.findById(volumeObject.getId()); - if (volumeVO != null) { - volumeVO.setPath(null); - if (cloudStackVolume.getLun().getUuid() != null) { - volumeVO.setFolder(cloudStackVolume.getLun().getUuid()); - } - volumeVO.setPoolType(storagePool.getPoolType()); - volumeVO.setPoolId(storagePool.getId()); - volumeDao.update(volumeVO.getId(), volumeVO); - } - } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - s_logger.info("createCloudStackVolume: Managed NFS object created for volume [{}]", volumeObject.getId()); - // For Managed NFS, set pool fields on Volume - VolumeVO volumeVO = volumeDao.findById(volumeObject.getId()); - if (volumeVO != null) { - volumeVO.setPoolType(storagePool.getPoolType()); - volumeVO.setPoolId(storagePool.getId()); - volumeDao.update(volumeVO.getId(), volumeVO); - } - } else { - String errMsg = "createCloudStackVolume: Volume creation failed for dataObject: " + volumeObject; - s_logger.error(errMsg); - throw new CloudRuntimeException(errMsg); - } - return cloudStackVolume; + return storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); } else { throw new CloudRuntimeException("createCloudStackVolume: Unsupported DataObjectType: " + dataObject.getType()); } } - private String ensureLunMapped(StoragePoolVO storagePool, String svmName, String lunName, long scopeId) { - Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); - StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); - String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); - - // Check existing map first. getLogicalAccess returns null (no exception) when map doesn't exist. - Map getMap = new HashMap<>(); - getMap.put(Constants.LUN_DOT_NAME, lunName); - getMap.put(Constants.SVM_DOT_NAME, svmName); - getMap.put(Constants.IGROUP_DOT_NAME, accessGroupName); - Map mapResp = storageStrategy.getLogicalAccess(getMap); - if (mapResp != null && mapResp.containsKey(Constants.LOGICAL_UNIT_NUMBER)) { - String lunNumber = mapResp.get(Constants.LOGICAL_UNIT_NUMBER); - s_logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}] with LUN number [{}]", lunName, accessGroupName, lunNumber); - return lunNumber; - } - // Create if not exists - Map enableMap = new HashMap<>(); - enableMap.put(Constants.LUN_DOT_NAME, lunName); - enableMap.put(Constants.SVM_DOT_NAME, svmName); - enableMap.put(Constants.IGROUP_DOT_NAME, accessGroupName); - Map response = storageStrategy.enableLogicalAccess(enableMap); - if (response == null || !response.containsKey(Constants.LOGICAL_UNIT_NUMBER)) { - throw new CloudRuntimeException("ensureLunMapped: Failed to map LUN [" + lunName + "] to iGroup [" + accessGroupName + "]"); - } - return response.get(Constants.LOGICAL_UNIT_NUMBER); - } - + /** + * Asynchronously deletes a volume from the ONTAP storage system. + * + * For iSCSI protocol: + * - Retrieves LUN details from volume_details table + * - Deletes the LUN from ONTAP (LUN mappings are automatically removed) + * + * For NFS protocol: + * - No ONTAP operation needed; file deletion handled by KVM host/libvirt + * + * @param store The data store containing the volume + * @param data The volume to delete + * @param callback Callback to notify completion + */ @Override public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallback callback) { CommandResult commandResult = new CommandResult(); @@ -260,37 +269,45 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac if (store == null || data == null) { throw new CloudRuntimeException("deleteAsync: store or data is null"); } + if (data.getType() == DataObjectType.VOLUME) { StoragePoolVO storagePool = storagePoolDao.findById(store.getId()); - if(storagePool == null) { - s_logger.error("deleteAsync : Storage Pool not found for id: " + store.getId()); - throw new CloudRuntimeException("deleteAsync : Storage Pool not found for id: " + store.getId()); + if (storagePool == null) { + s_logger.error("deleteAsync: Storage Pool not found for id: " + store.getId()); + throw new CloudRuntimeException("deleteAsync: Storage Pool not found for id: " + store.getId()); } + Map details = storagePoolDetailsDao.listDetailsKeyPairs(store.getId()); + if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - // ManagedNFS qcow2 backing file deletion handled by KVM host/libvirt; nothing to do via ONTAP REST. - s_logger.info("deleteAsync: ManagedNFS volume {} no-op ONTAP deletion", data.getId()); + // NFS file deletion is handled by the hypervisor; no ONTAP REST call needed + s_logger.info("deleteAsync: ManagedNFS volume {} - file deletion handled by hypervisor", data.getId()); + } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); VolumeInfo volumeObject = (VolumeInfo) data; - s_logger.info("deleteAsync: Deleting volume & LUN for volume id [{}]", volumeObject.getId()); + s_logger.info("deleteAsync: Deleting LUN for volume id [{}]", volumeObject.getId()); + + // Retrieve LUN identifiers stored during volume creation String lunName = volumeDetailsDao.findDetail(volumeObject.getId(), Constants.LUN_DOT_NAME).getValue(); String lunUUID = volumeDetailsDao.findDetail(volumeObject.getId(), Constants.LUN_DOT_UUID).getValue(); if (lunName == null) { throw new CloudRuntimeException("deleteAsync: Missing LUN name for volume " + volumeObject.getId()); } + CloudStackVolume delRequest = new CloudStackVolume(); Lun lun = new Lun(); lun.setName(lunName); lun.setUuid(lunUUID); delRequest.setLun(lun); storageStrategy.deleteCloudStackVolume(delRequest); - // Set the result + commandResult.setResult(null); commandResult.setSuccess(true); - s_logger.info("deleteAsync: Volume LUN [{}] deleted successfully", lunName); + s_logger.info("deleteAsync: LUN [{}] deleted successfully", lunName); + } else { - throw new CloudRuntimeException("deleteAsync: Unsupported protocol for deletion: " + details.get(Constants.PROTOCOL)); + throw new CloudRuntimeException("deleteAsync: Unsupported protocol: " + details.get(Constants.PROTOCOL)); } } } catch (Exception e) { @@ -326,23 +343,42 @@ public ChapInfo getChapInfo(DataObject dataObject) { return null; } + /** + * Grants a host access to a volume on the ONTAP storage system. + * + * For iSCSI protocol: + * - Validates that the host's iSCSI initiator (IQN) is present in the target igroup + * - Ensures the LUN is mapped to the igroup (creates mapping if not exists) + * - Updates the volume's iSCSI path with the assigned LUN ID + * + * For NFS protocol: + * - No explicit grant needed; NFS exports are configured at storage pool level + * + * @param dataObject The volume to grant access to + * @param host The host requesting access + * @param dataStore The data store containing the volume + * @return true if access was granted successfully + */ @Override public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore) { - if (dataStore == null) { - throw new InvalidParameterValueException("grantAccess: dataStore should not be null"); - } - if (dataObject == null) { - throw new InvalidParameterValueException("grantAccess: dataObject should not be null"); - } - if (host == null) { - throw new InvalidParameterValueException("grantAccess: host should not be null"); - } try { + if (dataStore == null) { + throw new InvalidParameterValueException("grantAccess: dataStore should not be null"); + } + if (dataObject == null) { + throw new InvalidParameterValueException("grantAccess: dataObject should not be null"); + } + if (host == null) { + throw new InvalidParameterValueException("grantAccess: host should not be null"); + } + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if (storagePool == null) { s_logger.error("grantAccess: Storage Pool not found for id: " + dataStore.getId()); - throw new CloudRuntimeException("grantAccess : Storage Pool not found for id: " + dataStore.getId()); + throw new CloudRuntimeException("grantAccess: Storage Pool not found for id: " + dataStore.getId()); } + + // ONTAP managed storage only supports cluster and zone scoped pools if (storagePool.getScope() != ScopeType.CLUSTER && storagePool.getScope() != ScopeType.ZONE) { s_logger.error("grantAccess: Only Cluster and Zone scoped primary storage is supported for storage Pool: " + storagePool.getName()); throw new CloudRuntimeException("grantAccess: Only Cluster and Zone scoped primary storage is supported for Storage Pool: " + storagePool.getName()); @@ -351,24 +387,36 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore if (dataObject.getType() == DataObjectType.VOLUME) { VolumeVO volumeVO = volumeDao.findById(dataObject.getId()); if (volumeVO == null) { - s_logger.error("grantAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); - throw new CloudRuntimeException("grantAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); + s_logger.error("grantAccess: CloudStack Volume not found for id: " + dataObject.getId()); + throw new CloudRuntimeException("grantAccess: CloudStack Volume not found for id: " + dataObject.getId()); } + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); String svmName = details.get(Constants.SVM_NAME); String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue(); long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); - // Validate initiator membership - validateHostInitiatorInIgroup(storagePool, svmName, scopeId, host); - // Ensure mapping exists - String lunNumber = ensureLunMapped(storagePool, svmName, cloudStackVolumeName, scopeId); - // Update Volume path if missing or changed - String iscsiPath = Constants.SLASH + storagePool.getPath() + Constants.SLASH + lunNumber; - if (volumeVO.getPath() == null || !volumeVO.getPath().equals(iscsiPath)) { - volumeVO.set_iScsiName(iscsiPath); - volumeVO.setPath(iscsiPath); + + if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details); + String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + + // Verify host initiator is registered in the igroup before allowing access + if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroupName)) { + throw new CloudRuntimeException("grantAccess: Host initiator [" + host.getStorageUrl() + + "] is not present in iGroup [" + accessGroupName + "]"); + } + + // Create or retrieve existing LUN mapping + String lunNumber = sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName); + + // Update volume path if changed (e.g., after migration or re-mapping) + String iscsiPath = Constants.SLASH + storagePool.getPath() + Constants.SLASH + lunNumber; + if (volumeVO.getPath() == null || !volumeVO.getPath().equals(iscsiPath)) { + volumeVO.set_iScsiName(iscsiPath); + volumeVO.setPath(iscsiPath); + } } - // Ensure pool fields are set (align with SolidFire) + volumeVO.setPoolType(storagePool.getPoolType()); volumeVO.setPoolId(storagePool.getId()); volumeDao.update(volumeVO.getId(), volumeVO); @@ -376,66 +424,62 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore s_logger.error("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); } - } catch(Exception e){ + return true; + } catch (Exception e) { s_logger.error("grantAccess: Failed for dataObject [{}]: {}", dataObject, e.getMessage()); - throw new CloudRuntimeException("grantAccess: Failed with error :" + e.getMessage()); - } - return true; - } - - private void validateHostInitiatorInIgroup(StoragePoolVO storagePool, String svmName, long scopeId, Host host) { - Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); - StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); - String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); - AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); - if (host == null || host.getStorageUrl() == null) { - throw new CloudRuntimeException("validateHostInitiatorInIgroup: host/initiator required but not provided"); - } - if (!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { - s_logger.error("validateHostInitiatorInIgroup: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); - throw new CloudRuntimeException("validateHostInitiatorInIgroup: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + accessGroupName + "]"); - } - } - - private boolean hostInitiatorFoundInIgroup(String hostInitiator, Igroup igroup) { - if(igroup != null && igroup.getInitiators() != null && hostInitiator != null && !hostInitiator.isEmpty()) { - for(Initiator initiator : igroup.getInitiators()) { - if(initiator.getName().equalsIgnoreCase(hostInitiator)) { - return true; - } - } + throw new CloudRuntimeException("grantAccess: Failed with error: " + e.getMessage(), e); } - return false; } + /** + * Revokes a host's access to a volume on the ONTAP storage system. + * + * For iSCSI protocol: + * - Validates the volume is not attached to an active VM + * - Removes the LUN mapping from the igroup + * + * For NFS protocol: + * - No explicit revoke needed; NFS exports remain at storage pool level + * + * @param dataObject The volume to revoke access from + * @param host The host losing access + * @param dataStore The data store containing the volume + */ @Override public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { - if (dataStore == null) { - throw new InvalidParameterValueException("revokeAccess: data store should not be null"); - } - if (dataObject == null) { - throw new InvalidParameterValueException("revokeAccess: data object should not be null"); - } - if (host == null) { - throw new InvalidParameterValueException("revokeAccess: host should not be null"); - } - if (dataObject.getType() == DataObjectType.VOLUME) { - Volume volume = volumeDao.findById(dataObject.getId()); - if (volume.getInstanceId() != null) { - VirtualMachine vm = vmDao.findById(volume.getInstanceId()); - if (vm != null && !Arrays.asList(VirtualMachine.State.Destroyed, VirtualMachine.State.Expunging, VirtualMachine.State.Error).contains(vm.getState())) { - s_logger.debug("revokeAccess: Volume [{}] is still attached to VM [{}] in state [{}], skipping revokeAccess", - dataObject.getId(), vm.getInstanceName(), vm.getState()); - return; + try { + if (dataStore == null) { + throw new InvalidParameterValueException("revokeAccess: dataStore should not be null"); + } + if (dataObject == null) { + throw new InvalidParameterValueException("revokeAccess: dataObject should not be null"); + } + if (host == null) { + throw new InvalidParameterValueException("revokeAccess: host should not be null"); + } + + // Safety check: don't revoke access if volume is still attached to an active VM + if (dataObject.getType() == DataObjectType.VOLUME) { + Volume volume = volumeDao.findById(dataObject.getId()); + if (volume.getInstanceId() != null) { + VirtualMachine vm = vmDao.findById(volume.getInstanceId()); + if (vm != null && !Arrays.asList( + VirtualMachine.State.Destroyed, + VirtualMachine.State.Expunging, + VirtualMachine.State.Error).contains(vm.getState())) { + s_logger.debug("revokeAccess: Volume [{}] is still attached to VM [{}] in state [{}], skipping revokeAccess", + dataObject.getId(), vm.getInstanceName(), vm.getState()); + return; + } } } - } - try { + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if (storagePool == null) { s_logger.error("revokeAccess: Storage Pool not found for id: " + dataStore.getId()); - throw new CloudRuntimeException("revokeAccess : Storage Pool not found for id: " + dataStore.getId()); + throw new CloudRuntimeException("revokeAccess: Storage Pool not found for id: " + dataStore.getId()); } + if (storagePool.getScope() != ScopeType.CLUSTER && storagePool.getScope() != ScopeType.ZONE) { s_logger.error("revokeAccess: Only Cluster and Zone scoped primary storage is supported for storage Pool: " + storagePool.getName()); throw new CloudRuntimeException("revokeAccess: Only Cluster and Zone scoped primary storage is supported for Storage Pool: " + storagePool.getName()); @@ -444,22 +488,31 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) if (dataObject.getType() == DataObjectType.VOLUME) { VolumeVO volumeVO = volumeDao.findById(dataObject.getId()); if (volumeVO == null) { - s_logger.error("revokeAccess: Cloud Stack Volume not found for id: " + dataObject.getId()); - throw new CloudRuntimeException("revokeAccess : Cloud Stack Volume not found for id: " + dataObject.getId()); + s_logger.error("revokeAccess: CloudStack Volume not found for id: " + dataObject.getId()); + throw new CloudRuntimeException("revokeAccess: CloudStack Volume not found for id: " + dataObject.getId()); } revokeAccessForVolume(storagePool, volumeVO, host); } else { s_logger.error("revokeAccess: Invalid DataObjectType (" + dataObject.getType() + ") passed to revokeAccess"); throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to revokeAccess"); } - } catch(Exception e){ + } catch (Exception e) { s_logger.error("revokeAccess: Failed for dataObject [{}]: {}", dataObject, e.getMessage()); - throw new CloudRuntimeException("revokeAccess: Failed with error :" + e.getMessage()); + throw new CloudRuntimeException("revokeAccess: Failed with error: " + e.getMessage(), e); } } + /** + * Revokes volume access by removing the LUN mapping from the igroup. + * This method handles the iSCSI-specific logic for access revocation. + * + * @param storagePool The storage pool containing the volume + * @param volumeVO The volume to revoke access from + * @param host The host losing access + */ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Host host) { s_logger.info("revokeAccessForVolume: Revoking access to volume [{}] for host [{}]", volumeVO.getName(), host.getName()); + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); String svmName = details.get(Constants.SVM_NAME); @@ -468,57 +521,84 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + // Retrieve LUN name from volume details; if missing, volume may not have been fully created String lunName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME) != null ? volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME).getValue() : null; if (lunName == null) { - s_logger.warn("revokeAccessForVolume: No LUN name detail found for volume [{}]; assuming no backend LUN to revoke", volumeVO.getId()); + s_logger.warn("revokeAccessForVolume: No LUN name found for volume [{}]; skipping revoke", volumeVO.getId()); return; } + // Verify LUN still exists on ONTAP (may have been manually deleted) CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, lunName); if (cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getUuid() == null) { - s_logger.warn("revokeAccessForVolume: LUN for volume [{}] not found on ONTAP, assuming already deleted", volumeVO.getId()); + s_logger.warn("revokeAccessForVolume: LUN for volume [{}] not found on ONTAP, skipping revoke", volumeVO.getId()); return; } + // Verify igroup still exists on ONTAP AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getUuid() == null) { - s_logger.warn("revokeAccessForVolume: iGroup [{}] not found on ONTAP, assuming already deleted", accessGroupName); + s_logger.warn("revokeAccessForVolume: iGroup [{}] not found on ONTAP, skipping revoke", accessGroupName); return; } - if (!hostInitiatorFoundInIgroup(host.getStorageUrl(), accessGroup.getIgroup())) { - s_logger.error("revokeAccessForVolume: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), accessGroupName); + // Verify host initiator is in the igroup before attempting to remove mapping + SANStrategy sanStrategy = (UnifiedSANStrategy) storageStrategy; + if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup().getName())) { + s_logger.warn("revokeAccessForVolume: Initiator [{}] is not in iGroup [{}], skipping revoke", + host.getStorageUrl(), accessGroupName); return; } + // Remove the LUN mapping from the igroup Map disableLogicalAccessMap = new HashMap<>(); disableLogicalAccessMap.put(Constants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid()); disableLogicalAccessMap.put(Constants.IGROUP_DOT_UUID, accessGroup.getIgroup().getUuid()); storageStrategy.disableLogicalAccess(disableLogicalAccessMap); + + s_logger.info("revokeAccessForVolume: Successfully revoked access to LUN [{}] for host [{}]", + lunName, host.getName()); } } - + /** + * Retrieves a CloudStack volume (LUN) from ONTAP by name. + * + * @param storageStrategy The storage strategy to use for the lookup + * @param svmName The SVM name containing the LUN + * @param cloudStackVolumeName The LUN name to look up + * @return CloudStackVolume if found, null otherwise + */ private CloudStackVolume getCloudStackVolumeByName(StorageStrategy storageStrategy, String svmName, String cloudStackVolumeName) { Map getCloudStackVolumeMap = new HashMap<>(); getCloudStackVolumeMap.put(Constants.NAME, cloudStackVolumeName); getCloudStackVolumeMap.put(Constants.SVM_DOT_NAME, svmName); + CloudStackVolume cloudStackVolume = storageStrategy.getCloudStackVolume(getCloudStackVolumeMap); if (cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getName() == null) { - s_logger.error("getCloudStackVolumeByName: LUN [{}] not found on ONTAP; returning null", cloudStackVolumeName); + s_logger.warn("getCloudStackVolumeByName: LUN [{}] not found on ONTAP", cloudStackVolumeName); return null; } return cloudStackVolume; } + /** + * Retrieves an access group (igroup) from ONTAP by name. + * + * @param storageStrategy The storage strategy to use for the lookup + * @param svmName The SVM name containing the igroup + * @param accessGroupName The igroup name to look up + * @return AccessGroup if found, null otherwise + */ private AccessGroup getAccessGroupByName(StorageStrategy storageStrategy, String svmName, String accessGroupName) { Map getAccessGroupMap = new HashMap<>(); getAccessGroupMap.put(Constants.NAME, accessGroupName); getAccessGroupMap.put(Constants.SVM_DOT_NAME, svmName); + AccessGroup accessGroup = storageStrategy.getAccessGroup(getAccessGroupMap); if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getName() == null) { - s_logger.error("getAccessGroupByName: iGroup [{}] not found on ONTAP; returning null", accessGroupName); + s_logger.warn("getAccessGroupByName: iGroup [{}] not found on ONTAP", accessGroupName); return null; } return accessGroup; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java index ce3b2806ef75..6be5ecfaf3f2 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/SANStrategy.java @@ -26,4 +26,25 @@ public SANStrategy(OntapStorage ontapStorage) { super(ontapStorage); } + /** + * Ensures the LUN is mapped to the specified access group (igroup). + * If a mapping already exists, returns the existing LUN number. + * If not, creates a new mapping and returns the assigned LUN number. + * + * @param svmName the SVM name + * @param lunName the LUN name + * @param accessGroupName the igroup name + * @return the logical unit number as a String + */ + public abstract String ensureLunMapped(String svmName, String lunName, String accessGroupName); + + /** + * Validates that the host initiator is present in the access group (igroup). + * + * @param hostInitiator the host initiator IQN + * @param svmName the SVM name + * @param accessGroupName the igroup name + * @return true if the initiator is found in the igroup, false otherwise + */ + public abstract boolean validateInitiatorInAccessGroup(String hostInitiator, String svmName, String accessGroupName); } 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 89ad712e9665..5bdebc5d716f 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 @@ -569,25 +569,24 @@ public String getNetworkInterface() { /** * Method encapsulates the behavior based on the opted protocol in subclasses * lunMap for iSCSI and FC protocols + * //TODO for NFS 3.0 and NFS 4.1 protocols (e.g., export rule management) * //TODO for Nvme/TCP and Nvme/FC protocols - * @param values map including SVM name, LUN name, and igroup name - * @return map containing logical unit number for the new/existing mapping + * @param values map including SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS + * @return map containing logical unit number for the new/existing mapping (SAN) or relevant info for NAS */ abstract public Map enableLogicalAccess(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses * lunUnmap for iSCSI and FC protocols - * //TODO for Nvme/TCP and Nvme/FC protocols - * @param values map including LUN UUID and iGroup UUID + * @param values map including LUN UUID and iGroup UUID (for SAN) or equivalent for NAS */ abstract public void disableLogicalAccess(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses * lunMap lookup for iSCSI/FC protocols (GET-only, no side-effects) - * //TODO for Nvme/TCP and Nvme/FC protocols - * @param values map with SVM name, LUN name, and igroup name + * @param values map with SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS * @return map containing logical unit number if mapping exists; otherwise null */ abstract public Map getLogicalAccess(Map values); 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 fd1f2bb6b84a..204249f1d16a 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 @@ -550,4 +550,67 @@ public Map getLogicalAccess(Map values) { } return null; } + + @Override + public String ensureLunMapped(String svmName, String lunName, String accessGroupName) { + s_logger.info("ensureLunMapped: Ensuring LUN [{}] is mapped to igroup [{}] on SVM [{}]", lunName, accessGroupName, svmName); + + // Check existing map first + Map getMap = Map.of( + Constants.LUN_DOT_NAME, lunName, + Constants.SVM_DOT_NAME, svmName, + Constants.IGROUP_DOT_NAME, accessGroupName + ); + Map mapResp = getLogicalAccess(getMap); + if (mapResp != null && mapResp.containsKey(Constants.LOGICAL_UNIT_NUMBER)) { + String lunNumber = mapResp.get(Constants.LOGICAL_UNIT_NUMBER); + s_logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}] with LUN number [{}]", lunName, accessGroupName, lunNumber); + return lunNumber; + } + + // Create if not exists + Map enableMap = Map.of( + Constants.LUN_DOT_NAME, lunName, + Constants.SVM_DOT_NAME, svmName, + Constants.IGROUP_DOT_NAME, accessGroupName + ); + Map response = enableLogicalAccess(enableMap); + if (response == null || !response.containsKey(Constants.LOGICAL_UNIT_NUMBER)) { + throw new CloudRuntimeException("ensureLunMapped: Failed to map LUN [" + lunName + "] to iGroup [" + accessGroupName + "]"); + } + s_logger.info("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response.get(Constants.LOGICAL_UNIT_NUMBER)); + return response.get(Constants.LOGICAL_UNIT_NUMBER); + } + + @Override + public boolean validateInitiatorInAccessGroup(String hostInitiator, String svmName, String accessGroupName) { + s_logger.info("validateInitiatorInAccessGroup: Validating initiator [{}] is in igroup [{}] on SVM [{}]", hostInitiator, accessGroupName, svmName); + + if (hostInitiator == null || hostInitiator.isEmpty()) { + s_logger.warn("validateInitiatorInAccessGroup: host initiator is null or empty"); + return false; + } + + Map getAccessGroupMap = Map.of( + Constants.NAME, accessGroupName, + Constants.SVM_DOT_NAME, svmName + ); + AccessGroup accessGroup = getAccessGroup(getAccessGroupMap); + if (accessGroup == null || accessGroup.getIgroup() == null) { + s_logger.warn("validateInitiatorInAccessGroup: iGroup [{}] not found on SVM [{}]", accessGroupName, svmName); + return false; + } + + Igroup igroup = accessGroup.getIgroup(); + if (igroup.getInitiators() != null) { + for (Initiator initiator : igroup.getInitiators()) { + if (initiator.getName().equalsIgnoreCase(hostInitiator)) { + s_logger.info("validateInitiatorInAccessGroup: Initiator [{}] validated successfully in igroup [{}]", hostInitiator, accessGroupName); + return true; + } + } + } + s_logger.warn("validateInitiatorInAccessGroup: Initiator [{}] NOT found in igroup [{}]", hostInitiator, accessGroupName); + return false; + } } From 5895ae3feef7de9ba622e02f78bd52ec26e60c9e Mon Sep 17 00:00:00 2001 From: "Locharla, Sandeep" Date: Thu, 5 Feb 2026 11:35:10 +0530 Subject: [PATCH 28/29] CSTACKEX-46: Fixed code as per comments received --- plugins/storage/volume/ontap/README.md | 123 ++++++++++ .../driver/OntapPrimaryDatastoreDriver.java | 109 ++------- .../OntapPrimaryDatastoreLifecycle.java | 13 +- .../OntapPrimaryDatastoreProvider.java | 3 +- .../storage/service/StorageStrategy.java | 22 -- .../storage/service/UnifiedSANStrategy.java | 99 ++++---- .../cloudstack/storage/utils/Constants.java | 4 + .../cloudstack/storage/utils/Utility.java | 6 +- .../OntapPrimaryDatastoreProviderTest.java | 216 ++++++++++++++++++ 9 files changed, 434 insertions(+), 161 deletions(-) create mode 100644 plugins/storage/volume/ontap/README.md create mode 100644 plugins/storage/volume/ontap/src/test/java/provider/OntapPrimaryDatastoreProviderTest.java diff --git a/plugins/storage/volume/ontap/README.md b/plugins/storage/volume/ontap/README.md new file mode 100644 index 000000000000..e7e066aafb55 --- /dev/null +++ b/plugins/storage/volume/ontap/README.md @@ -0,0 +1,123 @@ + + +# Apache CloudStack - NetApp ONTAP Storage Plugin + +## Overview + +The NetApp ONTAP Storage Plugin provides integration between Apache CloudStack and NetApp ONTAP storage systems. This plugin enables CloudStack to provision and manage primary storage on ONTAP clusters, supporting both NAS (NFS) and SAN (iSCSI) protocols. + +## Features + +- **Primary Storage Support**: Provision and manage primary storage pools on NetApp ONTAP +- **Multiple Protocols**: Support for NFS 3.0 and iSCSI protocols +- **Unified Storage**: Integration with traditional ONTAP unified storage architecture +- **KVM Hypervisor Support**: Supports KVM hypervisor environments +- **Managed Storage**: Operates as managed storage with full lifecycle management +- **Flexible Scoping**: Support for Zone-wide and Cluster-scoped storage pools + +## Architecture + +### Component Structure + +| Package | Description | +|---------|-------------------------------------------------------| +| `driver` | Primary datastore driver implementation | +| `feign` | REST API clients and data models for ONTAP operations | +| `lifecycle` | Storage pool lifecycle management | +| `listener` | Host connection event handlers | +| `provider` | Main provider and strategy factory | +| `service` | ONTAP Storage strategy implementations (NAS/SAN) | +| `utils` | Constants and helper utilities | + +## Requirements + +### ONTAP Requirements + +- NetApp ONTAP 9.15.1 or higher +- Storage Virtual Machine (SVM) configured with appropriate protocols enabled +- Management LIF accessible from CloudStack management server +- Data LIF(s) accessible from hypervisor hosts and are of IPv4 type +- Aggregates assigned to the SVM with sufficient capacity + +### CloudStack Requirements + +- Apache CloudStack current version or higher +- KVM hypervisor hosts +- For iSCSI: Hosts must have iSCSI initiator configured with valid IQN +- For NFS: Hosts must have NFS client packages installed + +### Minimum Volume Size + +ONTAP requires a minimum volume size of **1.56 GB** (1,677,721,600 bytes). The plugin will automatically adjust requested sizes below this threshold. + +## Configuration + +### Storage Pool Creation Parameters + +When creating an ONTAP primary storage pool, provide the following details in the URL field (semicolon-separated key=value pairs): + +| Parameter | Required | Description | +|-----------|----------|-------------| +| `username` | Yes | ONTAP cluster admin username | +| `password` | Yes | ONTAP cluster admin password | +| `svmName` | Yes | Storage Virtual Machine name | +| `protocol` | Yes | Storage protocol (`NFS3` or `ISCSI`) | +| `managementLIF` | Yes | ONTAP cluster management LIF IP address | + +### Example URL Format + +``` +username=admin;password=secretpass;svmName=svm1;protocol=ISCSI;managementLIF=192.168.1.100 +``` + +## Port Configuration + +| Protocol | Default Port | +|----------|--------------| +| NFS | 2049 | +| iSCSI | 3260 | +| ONTAP Management API | 443 (HTTPS) | + +## Limitations + +- Supports only **KVM** hypervisor +- Supports only **Unified ONTAP** storage (disaggregated not supported) +- Supports only **NFS3** and **iSCSI** protocols +- IPv6 type and FQDN LIFs are not supported + +## Troubleshooting + +### Common Issues + +1. **Connection Failures** + - Verify management LIF is reachable from CloudStack management server + - Check firewall rules for port 443 + +2. **Protocol Errors** + - Ensure the protocol (NFS/iSCSI) is enabled on the SVM + - Verify Data LIFs are configured for the protocol + +3. **Capacity Errors** + - Check aggregate space availability + - Ensure requested volume size meets minimum requirements (1.56 GB) + +4. **Host Connection Issues** + - For iSCSI: Verify host IQN is properly configured in host's storage URL + - For NFS: Ensure NFS client is installed and running 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 ea3b8a89673a..f912449f269e 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 @@ -69,16 +69,7 @@ /** * Primary datastore driver for NetApp ONTAP storage systems. - * This driver handles volume lifecycle operations (create, delete, grant/revoke access) - * for both iSCSI (LUN-based) and NFS protocols against ONTAP storage backends. - * - * For iSCSI protocol: - * - Creates LUNs on ONTAP and maps them to initiator groups (igroups) - * - Manages LUN mappings for host access control - * - * For NFS protocol: - * - Delegates file operations to KVM host/libvirt - * - ONTAP volume/export management handled at storage pool creation time + * Handles volume lifecycle operations for iSCSI and NFS protocols. */ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { @@ -111,26 +102,16 @@ public DataStoreTO getStoreTO(DataStore store) { } /** - * Asynchronously creates a volume on the ONTAP storage system. - * - * For iSCSI protocol: - * - Creates a LUN on ONTAP via the SAN strategy - * - Stores LUN UUID and name in volume_details table for later reference - * - Creates a LUN mapping to the appropriate igroup (based on cluster/zone scope) - * - Sets the iSCSI path on the volume for host attachment - * - * For NFS protocol: - * - Associates the volume with the storage pool (actual file creation handled by hypervisor) - * - * @param dataStore The target data store (storage pool) - * @param dataObject The volume to create - * @param callback Callback to notify completion + * Creates a volume on the ONTAP storage system. */ @Override public void createAsync(DataStore dataStore, DataObject dataObject, AsyncCompletionCallback callback) { CreateCmdResult createCmdResult = null; String errMsg; + if (dataObject == null) { + throw new InvalidParameterValueException("createAsync: dataObject should not be null"); + } if (dataStore == null) { throw new InvalidParameterValueException("createAsync: dataStore should not be null"); } @@ -150,6 +131,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet s_logger.error("createAsync: Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("createAsync: Storage Pool not found for id: " + dataStore.getId()); } + String storagePoolUuid = dataStore.getUuid(); Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); @@ -186,7 +168,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet // Create LUN-to-igroup mapping and retrieve the assigned LUN ID UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details); - String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid); String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName); // Construct iSCSI path: // format for KVM/libvirt attachment @@ -223,12 +205,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } /** - * Creates a CloudStack volume on the ONTAP backend using the appropriate storage strategy. - * - * @param dataStore The target data store - * @param dataObject The volume to create - * @param details Storage pool configuration details - * @return CloudStackVolume containing the created backend object (LUN for iSCSI) + * Creates a volume on the ONTAP backend. */ private CloudStackVolume createCloudStackVolume(DataStore dataStore, DataObject dataObject, Map details) { StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); @@ -249,18 +226,7 @@ private CloudStackVolume createCloudStackVolume(DataStore dataStore, DataObject } /** - * Asynchronously deletes a volume from the ONTAP storage system. - * - * For iSCSI protocol: - * - Retrieves LUN details from volume_details table - * - Deletes the LUN from ONTAP (LUN mappings are automatically removed) - * - * For NFS protocol: - * - No ONTAP operation needed; file deletion handled by KVM host/libvirt - * - * @param store The data store containing the volume - * @param data The volume to delete - * @param callback Callback to notify completion + * Deletes a volume from the ONTAP storage system. */ @Override public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallback callback) { @@ -344,20 +310,7 @@ public ChapInfo getChapInfo(DataObject dataObject) { } /** - * Grants a host access to a volume on the ONTAP storage system. - * - * For iSCSI protocol: - * - Validates that the host's iSCSI initiator (IQN) is present in the target igroup - * - Ensures the LUN is mapped to the igroup (creates mapping if not exists) - * - Updates the volume's iSCSI path with the assigned LUN ID - * - * For NFS protocol: - * - No explicit grant needed; NFS exports are configured at storage pool level - * - * @param dataObject The volume to grant access to - * @param host The host requesting access - * @param dataStore The data store containing the volume - * @return true if access was granted successfully + * Grants a host access to a volume. */ @Override public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore) { @@ -377,6 +330,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore s_logger.error("grantAccess: Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("grantAccess: Storage Pool not found for id: " + dataStore.getId()); } + String storagePoolUuid = dataStore.getUuid(); // ONTAP managed storage only supports cluster and zone scoped pools if (storagePool.getScope() != ScopeType.CLUSTER && storagePool.getScope() != ScopeType.ZONE) { @@ -398,7 +352,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) Utility.getStrategyByStoragePoolDetails(details); - String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid); // Verify host initiator is registered in the igroup before allowing access if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroupName)) { @@ -432,18 +386,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore } /** - * Revokes a host's access to a volume on the ONTAP storage system. - * - * For iSCSI protocol: - * - Validates the volume is not attached to an active VM - * - Removes the LUN mapping from the igroup - * - * For NFS protocol: - * - No explicit revoke needed; NFS exports remain at storage pool level - * - * @param dataObject The volume to revoke access from - * @param host The host losing access - * @param dataStore The data store containing the volume + * Revokes a host's access to a volume. */ @Override public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { @@ -467,7 +410,7 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) VirtualMachine.State.Destroyed, VirtualMachine.State.Expunging, VirtualMachine.State.Error).contains(vm.getState())) { - s_logger.debug("revokeAccess: Volume [{}] is still attached to VM [{}] in state [{}], skipping revokeAccess", + s_logger.warn("revokeAccess: Volume [{}] is still attached to VM [{}] in state [{}], skipping revokeAccess", dataObject.getId(), vm.getInstanceName(), vm.getState()); return; } @@ -503,12 +446,7 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) } /** - * Revokes volume access by removing the LUN mapping from the igroup. - * This method handles the iSCSI-specific logic for access revocation. - * - * @param storagePool The storage pool containing the volume - * @param volumeVO The volume to revoke access from - * @param host The host losing access + * Revokes volume access for the specified host. */ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Host host) { s_logger.info("revokeAccessForVolume: Revoking access to volume [{}] for host [{}]", volumeVO.getName(), host.getName()); @@ -516,10 +454,11 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); String svmName = details.get(Constants.SVM_NAME); + String storagePoolUuid = storagePool.getUuid(); long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { - String accessGroupName = Utility.getIgroupName(svmName, storagePool.getScope(), scopeId); + String accessGroupName = Utility.getIgroupName(svmName, storagePoolUuid); // Retrieve LUN name from volume details; if missing, volume may not have been fully created String lunName = volumeDetailsDao.findDetail(volumeVO.getId(), Constants.LUN_DOT_NAME) != null ? @@ -563,12 +502,7 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, } /** - * Retrieves a CloudStack volume (LUN) from ONTAP by name. - * - * @param storageStrategy The storage strategy to use for the lookup - * @param svmName The SVM name containing the LUN - * @param cloudStackVolumeName The LUN name to look up - * @return CloudStackVolume if found, null otherwise + * Retrieves a volume from ONTAP by name. */ private CloudStackVolume getCloudStackVolumeByName(StorageStrategy storageStrategy, String svmName, String cloudStackVolumeName) { Map getCloudStackVolumeMap = new HashMap<>(); @@ -584,12 +518,7 @@ private CloudStackVolume getCloudStackVolumeByName(StorageStrategy storageStrate } /** - * Retrieves an access group (igroup) from ONTAP by name. - * - * @param storageStrategy The storage strategy to use for the lookup - * @param svmName The SVM name containing the igroup - * @param accessGroupName The igroup name to look up - * @return AccessGroup if found, null otherwise + * Retrieves an access group from ONTAP by name. */ private AccessGroup getAccessGroupByName(StorageStrategy storageStrategy, String svmName, String accessGroupName) { Map getAccessGroupMap = new HashMap<>(); 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 eabd6482572c..ff767fb81c63 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 @@ -152,16 +152,21 @@ public DataStore initialize(Map dsInfos) { throw new CloudRuntimeException("ONTAP primary storage must be managed"); } - // Required ONTAP detail keys Set requiredKeys = Set.of( Constants.USERNAME, Constants.PASSWORD, Constants.SVM_NAME, Constants.PROTOCOL, - Constants.MANAGEMENT_LIF, + Constants.MANAGEMENT_LIF + ); + + Set optionalKeys = Set.of( Constants.IS_DISAGGREGATED ); + Set allowedKeys = new java.util.HashSet<>(requiredKeys); + allowedKeys.addAll(optionalKeys); + // Parse key=value pairs from URL into details (skip empty segments) if (url != null && !url.isEmpty()) { for (String segment : url.split(Constants.SEMICOLON)) { @@ -249,13 +254,13 @@ public DataStore initialize(Map dsInfos) { case NFS3: parameters.setType(Storage.StoragePoolType.NetworkFilesystem); path = Constants.SLASH + storagePoolName; - port = 2049; + port = Constants.NFS3_PORT; s_logger.info("Setting NFS path for storage pool: " + path + ", port: " + port); break; case ISCSI: parameters.setType(Storage.StoragePoolType.Iscsi); path = storageStrategy.getStoragePath(); - port = 3260; + port = Constants.ISCSI_PORT; s_logger.info("Setting iSCSI path for storage pool: " + path + ", port: " + port); break; default: diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java index 1aadec79b3ab..5b44c951a5fa 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java @@ -28,6 +28,7 @@ import org.apache.cloudstack.storage.driver.OntapPrimaryDatastoreDriver; import org.apache.cloudstack.storage.lifecycle.OntapPrimaryDatastoreLifecycle; import org.apache.cloudstack.storage.listener.OntapHostListener; +import org.apache.cloudstack.storage.utils.Constants; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.stereotype.Component; @@ -65,7 +66,7 @@ public HypervisorHostListener getHostListener() { @Override public String getName() { s_logger.trace("OntapPrimaryDatastoreProvider: getName: Called"); - return "ONTAP"; + return Constants.ONTAP_PLUGIN_NAME; } @Override 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 5bdebc5d716f..565cf0399663 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 @@ -125,33 +125,11 @@ public boolean connect() { s_logger.error("iSCSI protocol is not enabled on SVM " + svmName); return false; } - // TODO: Implement logic to select appropriate aggregate based on storage requirements List aggrs = svm.getAggregates(); if (aggrs == null || aggrs.isEmpty()) { s_logger.error("No aggregates are assigned to SVM " + svmName); return false; } - // Set the aggregates which are according to the storage requirements - for (Aggregate aggr : aggrs) { - s_logger.debug("Found aggregate: " + aggr.getName() + " with UUID: " + aggr.getUuid()); - Aggregate aggrResp = aggregateFeignClient.getAggregateByUUID(authHeader, aggr.getUuid()); - if (!Objects.equals(aggrResp.getState(), Aggregate.StateEnum.ONLINE)) { - s_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()) { - s_logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Skipping this aggregate."); - continue; - } - s_logger.info("Selected aggregate: " + aggr.getName() + " for volume operations."); - this.aggregates = List.of(aggr); - break; - } - if (this.aggregates == null || this.aggregates.isEmpty()) { - s_logger.error("No suitable aggregates found on SVM " + svmName + " for volume creation."); - return false; - } - this.aggregates = aggrs; s_logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided"); } catch (Exception e) { 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 204249f1d16a..cab598702ecb 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 @@ -22,6 +22,7 @@ import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.exception.CloudRuntimeException; +import feign.FeignException; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo; import org.apache.cloudstack.storage.feign.FeignClientFactory; import org.apache.cloudstack.storage.feign.client.SANFeignClient; @@ -87,6 +88,10 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume CloudStackVolume createdCloudStackVolume = new CloudStackVolume(); createdCloudStackVolume.setLun(lun); return createdCloudStackVolume; + } catch (FeignException e) { + s_logger.error("FeignException occurred while creating LUN: {}, Status: {}, Exception: {}", + cloudstackVolume.getLun().getName(), e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to create Lun: " + e.getMessage()); } catch (Exception e) { s_logger.error("Exception occurred while creating LUN: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); throw new CloudRuntimeException("Failed to create Lun: " + e.getMessage()); @@ -111,16 +116,12 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { Map queryParams = Map.of("allow_delete_while_mapped", "true"); try { sanFeignClient.deleteLun(authHeader, cloudstackVolume.getLun().getUuid(), queryParams); - } catch (Exception ex) { - String errMsg = ex.getMessage(); - if (errMsg != null && (errMsg.contains("entry doesn't exist") - || errMsg.contains("does not exist") - || errMsg.contains("not found") - || errMsg.contains("status 404"))) { - s_logger.warn("deleteCloudStackVolume: Lun {} does not exist ({}), skipping deletion", cloudstackVolume.getLun().getName(), errMsg); + } catch (FeignException feignEx) { + if (feignEx.status() == 404) { + s_logger.warn("deleteCloudStackVolume: Lun {} does not exist (status 404), skipping deletion", cloudstackVolume.getLun().getName()); return; } - throw ex; + throw feignEx; } s_logger.info("deleteCloudStackVolume: Lun deleted successfully. LunName: {}", cloudstackVolume.getLun().getName()); } catch (Exception e) { @@ -150,6 +151,9 @@ public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) { String lunCloneName = cloudstackVolume.getLun().getName() + "_clone"; lunCloneRequest.setName(lunCloneName); sanFeignClient.createLun(authHeader, true, lunCloneRequest); + } catch (FeignException e) { + s_logger.error("FeignException occurred while creating Lun clone: {}, Status: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to create Lun clone: " + e.getMessage()); } catch (Exception e) { s_logger.error("Exception occurred while creating Lun clone: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); throw new CloudRuntimeException("Failed to create Lun clone: " + e.getMessage()); @@ -185,14 +189,16 @@ public CloudStackVolume getCloudStackVolume(Map values) { CloudStackVolume cloudStackVolume = new CloudStackVolume(); cloudStackVolume.setLun(lun); return cloudStackVolume; - } catch (Exception e) { - String errMsg = e.getMessage(); - if (errMsg != null && errMsg.contains("not found")) { - s_logger.warn("getCloudStackVolume: Lun '{}' on SVM '{}' not found ({}). Returning null.", lunName, svmName, errMsg); + } catch (FeignException e) { + if (e.status() == 404) { + s_logger.warn("getCloudStackVolume: Lun '{}' on SVM '{}' not found (status 404). Returning null.", lunName, svmName); return null; } - s_logger.error("Exception occurred while fetching Lun, Exception: {}", errMsg); - throw new CloudRuntimeException("Failed to fetch Lun details: " + errMsg); + s_logger.error("FeignException occurred while fetching Lun, Status: {}, Exception: {}", e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Lun details: " + e.getMessage()); + } catch (Exception e) { + s_logger.error("Exception occurred while fetching Lun, Exception: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Lun details: " + e.getMessage()); } } @@ -221,7 +227,8 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { Igroup igroupRequest = new Igroup(); List hostsIdentifier = new ArrayList<>(); String svmName = dataStoreDetails.get(Constants.SVM_NAME); - igroupName = Utility.getIgroupName(svmName, accessGroup.getScope().getScopeType(), accessGroup.getScope().getScopeId()); + String storagePoolUuid = accessGroup.getPrimaryDataStoreInfo().getUuid(); + igroupName = Utility.getIgroupName(svmName, storagePoolUuid); Hypervisor.HypervisorType hypervisorType = accessGroup.getPrimaryDataStoreInfo().getHypervisor(); ProtocolType protocol = ProtocolType.valueOf(dataStoreDetails.get(Constants.PROTOCOL)); @@ -261,21 +268,20 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { } igroupRequest.setInitiators(initiators); } - igroupRequest.setProtocol(Igroup.ProtocolEnum.valueOf("iscsi")); + igroupRequest.setProtocol(Igroup.ProtocolEnum.valueOf(Constants.ISCSI)); // Create Igroup s_logger.debug("createAccessGroup: About to call sanFeignClient.createIgroup with igroupName: {}", igroupName); AccessGroup createdAccessGroup = new AccessGroup(); OntapResponse createdIgroup = null; try { createdIgroup = sanFeignClient.createIgroup(authHeader, true, igroupRequest); - } catch (Exception feignEx) { - String errMsg = feignEx.getMessage(); - if (errMsg != null && errMsg.contains(("5374023"))) { - s_logger.warn("createAccessGroup: Igroup with name {} already exists. Fetching existing Igroup.", igroupName); + } catch (FeignException feignEx) { + if (feignEx.status() == 409) { + s_logger.warn("createAccessGroup: Igroup with name {} already exists (status 409). Fetching existing Igroup.", igroupName); // TODO: Currently we aren't doing anything with the returned AccessGroup object, so, haven't added code here to fetch the existing Igroup and set it in AccessGroup. return createdAccessGroup; } - s_logger.error("createAccessGroup: Exception during Feign call: {}", feignEx.getMessage(), feignEx); + s_logger.error("createAccessGroup: FeignException during Igroup creation: Status: {}, Exception: {}", feignEx.status(), feignEx.getMessage(), feignEx); throw feignEx; } @@ -317,14 +323,16 @@ public void deleteAccessGroup(AccessGroup accessGroup) { // Extract SVM name from storage (already initialized in constructor via OntapStorage) String svmName = storage.getSvmName(); + String storagePoolUuid = primaryDataStoreInfo.getUuid(); // Determine scope and generate iGroup name - String igroupName; + String igroupName = Utility.getIgroupName(svmName, storagePoolUuid); + s_logger.info("deleteAccessGroup: Generated iGroup name '{}'", igroupName); if (primaryDataStoreInfo.getClusterId() != null) { - igroupName = Utility.getIgroupName(svmName, com.cloud.storage.ScopeType.CLUSTER, primaryDataStoreInfo.getClusterId()); + igroupName = Utility.getIgroupName(svmName, storagePoolUuid); s_logger.info("deleteAccessGroup: Deleting cluster-scoped iGroup '{}'", igroupName); } else { - igroupName = Utility.getIgroupName(svmName, com.cloud.storage.ScopeType.ZONE, primaryDataStoreInfo.getDataCenterId()); + igroupName = Utility.getIgroupName(svmName, storagePoolUuid); s_logger.info("deleteAccessGroup: Deleting zone-scoped iGroup '{}'", igroupName); } @@ -355,16 +363,21 @@ public void deleteAccessGroup(AccessGroup accessGroup) { s_logger.info("deleteAccessGroup: Successfully deleted iGroup '{}'", igroupName); - } catch (Exception e) { - String errorMsg = e.getMessage(); - // Check if iGroup doesn't exist (ONTAP error code: 5374852 - "The initiator group does not exist.") - if (errorMsg != null && (errorMsg.contains("5374852") || errorMsg.contains("not found"))) { - s_logger.warn("deleteAccessGroup: iGroup '{}' does not exist, skipping deletion", igroupName); + } catch (FeignException e) { + if (e.status() == 404) { + s_logger.warn("deleteAccessGroup: iGroup '{}' does not exist (status 404), skipping deletion", igroupName); } else { + s_logger.error("deleteAccessGroup: FeignException occurred: Status: {}, Exception: {}", e.status(), e.getMessage(), e); throw e; } + } catch (Exception e) { + s_logger.error("deleteAccessGroup: Exception occurred: {}", e.getMessage(), e); + throw e; } + } catch (FeignException e) { + s_logger.error("deleteAccessGroup: FeignException occurred while deleting iGroup. Status: {}, Exception: {}", e.status(), e.getMessage(), e); + throw new CloudRuntimeException("Failed to delete iGroup: " + e.getMessage(), e); } catch (Exception e) { s_logger.error("deleteAccessGroup: Failed to delete iGroup. Exception: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to delete iGroup: " + e.getMessage(), e); @@ -421,14 +434,16 @@ public AccessGroup getAccessGroup(Map values) { AccessGroup accessGroup = new AccessGroup(); accessGroup.setIgroup(igroup); return accessGroup; - } catch (Exception e) { - String errMsg = e.getMessage(); - if (errMsg != null && errMsg.contains("not found")) { - s_logger.warn("getAccessGroup: Igroup '{}' not found on SVM '{}' ({}). Returning null.", igroupName, svmName, errMsg); + } catch (FeignException e) { + if (e.status() == 404) { + s_logger.warn("getAccessGroup: Igroup '{}' not found on SVM '{}' (status 404). Returning null.", igroupName, svmName); return null; } - s_logger.error("Exception occurred while fetching Igroup, Exception: {}", errMsg); - throw new CloudRuntimeException("Failed to fetch Igroup details: " + errMsg); + s_logger.error("FeignException occurred while fetching Igroup, Status: {}, Exception: {}", e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Igroup details: " + e.getMessage()); + } catch (Exception e) { + s_logger.error("Exception occurred while fetching Igroup, Exception: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Igroup details: " + e.getMessage()); } } @@ -509,14 +524,16 @@ public void disableLogicalAccess(Map values) { String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); sanFeignClient.deleteLunMap(authHeader, lunUUID, igroupUUID); s_logger.info("disableLogicalAccess: LunMap deleted successfully."); - } catch (Exception e) { - String errMsg = e.getMessage(); - if (errMsg != null && errMsg.contains("not found")) { - s_logger.warn("disableLogicalAccess: LunMap with Lun UUID: {} and igroup UUID: {} does not exist ({}), skipping deletion", lunUUID, igroupUUID, errMsg); + } catch (FeignException e) { + if (e.status() == 404) { + s_logger.warn("disableLogicalAccess: LunMap with Lun UUID: {} and igroup UUID: {} does not exist, skipping deletion", lunUUID, igroupUUID); return; } - s_logger.error("Exception occurred while deleting LunMap", e); - throw new CloudRuntimeException("Failed to delete LunMap: " + errMsg); + s_logger.error("FeignException occurred while deleting LunMap, Status: {}, Exception: {}", e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to delete LunMap: " + e.getMessage()); + } catch (Exception e) { + s_logger.error("Exception occurred while deleting LunMap, Exception: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to delete LunMap: " + e.getMessage()); } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java index 43f8511967e7..920bd45fd0d7 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java @@ -22,6 +22,10 @@ public class Constants { + public static final String ONTAP_PLUGIN_NAME = "ONTAP"; + public static final int NFS3_PORT = 2049; + public static final int ISCSI_PORT = 3260; + public static final String NFS = "nfs"; public static final String ISCSI = "iscsi"; public static final String SIZE = "size"; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index 9d5eac8b2cea..75890ae2e2cf 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -141,9 +141,9 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map types = provider.getTypes(); + assertNotNull(types); + assertEquals(1, types.size()); + assertTrue(types.contains(DataStoreProviderType.PRIMARY)); + } + + @Test + public void testGetDataStoreLifeCycle_beforeConfigure() { + DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle(); + assertNull(lifeCycle); + } + + @Test + public void testGetDataStoreDriver_beforeConfigure() { + DataStoreDriver driver = provider.getDataStoreDriver(); + assertNull(driver); + } + + @Test + public void testGetHostListener_beforeConfigure() { + HypervisorHostListener listener = provider.getHostListener(); + assertNull(listener); + } + + @Test + public void testConfigure() { + OntapPrimaryDatastoreDriver mockDriver = mock(OntapPrimaryDatastoreDriver.class); + OntapPrimaryDatastoreLifecycle mockLifecycle = mock(OntapPrimaryDatastoreLifecycle.class); + OntapHostListener mockListener = mock(OntapHostListener.class); + + try (MockedStatic componentContext = Mockito.mockStatic(ComponentContext.class)) { + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreDriver.class)) + .thenReturn(mockDriver); + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreLifecycle.class)) + .thenReturn(mockLifecycle); + componentContext.when(() -> ComponentContext.inject(OntapHostListener.class)) + .thenReturn(mockListener); + + Map params = new HashMap<>(); + boolean result = provider.configure(params); + + assertTrue(result); + } + } + + @Test + public void testGetDataStoreLifeCycle_afterConfigure() { + OntapPrimaryDatastoreDriver mockDriver = mock(OntapPrimaryDatastoreDriver.class); + OntapPrimaryDatastoreLifecycle mockLifecycle = mock(OntapPrimaryDatastoreLifecycle.class); + OntapHostListener mockListener = mock(OntapHostListener.class); + + try (MockedStatic componentContext = Mockito.mockStatic(ComponentContext.class)) { + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreDriver.class)) + .thenReturn(mockDriver); + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreLifecycle.class)) + .thenReturn(mockLifecycle); + componentContext.when(() -> ComponentContext.inject(OntapHostListener.class)) + .thenReturn(mockListener); + + provider.configure(new HashMap<>()); + + DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle(); + assertNotNull(lifeCycle); + assertEquals(mockLifecycle, lifeCycle); + } + } + + @Test + public void testGetDataStoreDriver_afterConfigure() { + OntapPrimaryDatastoreDriver mockDriver = mock(OntapPrimaryDatastoreDriver.class); + OntapPrimaryDatastoreLifecycle mockLifecycle = mock(OntapPrimaryDatastoreLifecycle.class); + OntapHostListener mockListener = mock(OntapHostListener.class); + + try (MockedStatic componentContext = Mockito.mockStatic(ComponentContext.class)) { + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreDriver.class)) + .thenReturn(mockDriver); + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreLifecycle.class)) + .thenReturn(mockLifecycle); + componentContext.when(() -> ComponentContext.inject(OntapHostListener.class)) + .thenReturn(mockListener); + + provider.configure(new HashMap<>()); + + DataStoreDriver driver = provider.getDataStoreDriver(); + assertNotNull(driver); + assertEquals(mockDriver, driver); + } + } + + @Test + public void testGetHostListener_afterConfigure() { + OntapPrimaryDatastoreDriver mockDriver = mock(OntapPrimaryDatastoreDriver.class); + OntapPrimaryDatastoreLifecycle mockLifecycle = mock(OntapPrimaryDatastoreLifecycle.class); + OntapHostListener mockListener = mock(OntapHostListener.class); + + try (MockedStatic componentContext = Mockito.mockStatic(ComponentContext.class)) { + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreDriver.class)) + .thenReturn(mockDriver); + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreLifecycle.class)) + .thenReturn(mockLifecycle); + componentContext.when(() -> ComponentContext.inject(OntapHostListener.class)) + .thenReturn(mockListener); + + provider.configure(new HashMap<>()); + + HypervisorHostListener listener = provider.getHostListener(); + assertNotNull(listener); + assertEquals(mockListener, listener); + } + } + + @Test + public void testConfigure_withNullParams() { + OntapPrimaryDatastoreDriver mockDriver = mock(OntapPrimaryDatastoreDriver.class); + OntapPrimaryDatastoreLifecycle mockLifecycle = mock(OntapPrimaryDatastoreLifecycle.class); + OntapHostListener mockListener = mock(OntapHostListener.class); + + try (MockedStatic componentContext = Mockito.mockStatic(ComponentContext.class)) { + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreDriver.class)) + .thenReturn(mockDriver); + componentContext.when(() -> ComponentContext.inject(OntapPrimaryDatastoreLifecycle.class)) + .thenReturn(mockLifecycle); + componentContext.when(() -> ComponentContext.inject(OntapHostListener.class)) + .thenReturn(mockListener); + + boolean result = provider.configure(null); + + assertTrue(result); + assertNotNull(provider.getDataStoreDriver()); + assertNotNull(provider.getDataStoreLifeCycle()); + assertNotNull(provider.getHostListener()); + } + } + + @Test + public void testGetTypes_returnsOnlyPrimaryType() { + Set types = provider.getTypes(); + + assertNotNull(types); + assertEquals(1, types.size()); + assertTrue(types.contains(DataStoreProviderType.PRIMARY)); + + // Verify it doesn't contain other types + for (DataStoreProviderType type : types) { + assertEquals(DataStoreProviderType.PRIMARY, type); + } + } +} From 887e5891085a897e1409b4732d84ae561a50dce3 Mon Sep 17 00:00:00 2001 From: "Locharla, Sandeep" Date: Thu, 5 Feb 2026 16:27:28 +0530 Subject: [PATCH 29/29] CSTACKEX-46: Added some code that was added to community PR for PrimaryStoragePool --- .../OntapPrimaryDatastoreLifecycle.java | 2 +- .../cloudstack/storage/utils/Utility.java | 1 - .../OntapPrimaryDatastoreLifecycleTest.java | 40 +++++++++++++++---- 3 files changed, 33 insertions(+), 10 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 ff767fb81c63..a7df490b9b95 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 @@ -184,7 +184,7 @@ public DataStore initialize(Map dsInfos) { for (Map.Entry e : details.entrySet()) { String key = e.getKey(); String val = e.getValue(); - if (!requiredKeys.contains(key)) { + if (!allowedKeys.contains(key)) { throw new CloudRuntimeException("Unexpected ONTAP detail key in URL: " + key); } if (val == null || val.isEmpty()) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index 75890ae2e2cf..2f805c1784d6 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -20,7 +20,6 @@ package org.apache.cloudstack.storage.utils; import com.cloud.exception.InvalidParameterValueException; -import com.cloud.storage.ScopeType; import com.cloud.utils.StringUtils; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; 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 af8aac84f490..789615a9f43b 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 @@ -78,6 +78,30 @@ void setUp() { @Test public void testInitialize_positive() { + Map dsInfos = new HashMap<>(); + dsInfos.put("username", "testUser"); + dsInfos.put("password", "testPassword"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1"); + dsInfos.put("zoneId",1L); + dsInfos.put("podId",1L); + dsInfos.put("clusterId", 1L); + dsInfos.put("name", "testStoragePool"); + dsInfos.put("providerName", "testProvider"); + dsInfos.put("capacityBytes",200000L); + dsInfos.put("managed",true); + dsInfos.put("tags", "testTag"); + dsInfos.put("isTagARule", false); + dsInfos.put("details", new HashMap()); + + try(MockedStatic storageProviderFactory = Mockito.mockStatic(StorageProviderFactory.class)) { + storageProviderFactory.when(() -> StorageProviderFactory.getStrategy(any())).thenReturn(storageStrategy); + ontapPrimaryDatastoreLifecycle.initialize(dsInfos); + } + } + + @Test + public void testInitialize_positiveWithIsDisaggregated() { + Map dsInfos = new HashMap<>(); dsInfos.put("username", "testUser"); dsInfos.put("password", "testPassword"); @@ -109,7 +133,7 @@ public void testInitialize_null_Arg() { @Test public void testInitialize_missingRequiredDetailKey() { Map dsInfos = new HashMap<>(); - dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3"); dsInfos.put("zoneId",1L); dsInfos.put("podId",1L); dsInfos.put("clusterId", 1L); @@ -131,7 +155,7 @@ public void testInitialize_missingRequiredDetailKey() { @Test public void testInitialize_invalidCapacityBytes() { Map dsInfos = new HashMap<>(); - dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1;isDisaggregated=false"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1"); dsInfos.put("zoneId",1L); dsInfos.put("podId",1L); dsInfos.put("clusterId", 1L); @@ -152,7 +176,7 @@ public void testInitialize_invalidCapacityBytes() { @Test public void testInitialize_unmanagedStorage() { Map dsInfos = new HashMap<>(); - dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1;isDisaggregated=false"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1"); dsInfos.put("zoneId",1L); dsInfos.put("podId",1L); dsInfos.put("clusterId", 1L); @@ -176,7 +200,7 @@ public void testInitialize_unmanagedStorage() { @Test public void testInitialize_nullStoragePoolName() { Map dsInfos = new HashMap<>(); - dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1;isDisaggregated=false"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1"); dsInfos.put("zoneId",1L); dsInfos.put("podId",1L); dsInfos.put("clusterId", 1L); @@ -200,7 +224,7 @@ public void testInitialize_nullStoragePoolName() { @Test public void testInitialize_nullProviderName() { Map dsInfos = new HashMap<>(); - dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1;isDisaggregated=false"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1"); dsInfos.put("zoneId",1L); dsInfos.put("podId",1L); dsInfos.put("clusterId", 1L); @@ -224,7 +248,7 @@ public void testInitialize_nullProviderName() { @Test public void testInitialize_nullPodAndClusterAndZone() { Map dsInfos = new HashMap<>(); - dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1;isDisaggregated=false"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1"); dsInfos.put("zoneId",null); dsInfos.put("podId",null); dsInfos.put("clusterId", null); @@ -252,7 +276,7 @@ public void testInitialize_clusterNotKVM() { when(_clusterDao.findById(2L)).thenReturn(clusterVO); Map dsInfos = new HashMap<>(); - dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1;isDisaggregated=false"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1"); dsInfos.put("zoneId",1L); dsInfos.put("podId",1L); dsInfos.put("clusterId", 2L); @@ -276,7 +300,7 @@ public void testInitialize_clusterNotKVM() { @Test public void testInitialize_unexpectedDetailKey() { Map dsInfos = new HashMap<>(); - dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1;isDisaggregated=false;unexpectedKey=unexpectedValue"); + dsInfos.put("url", "username=testUser;password=testPassword;svmName=testSVM;protocol=NFS3;managementLIF=192.168.1.1;unexpectedKey=unexpectedValue"); dsInfos.put("zoneId",1L); dsInfos.put("podId",1L); dsInfos.put("clusterId", 1L);