From eafc1e7185a39743e88c6a5e8d28ed0f39fa00a0 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 5 Nov 2025 15:28:09 +0530 Subject: [PATCH 01/21] CSTACKEX-46 Create Async, Attach Cluster/Zone and Grant/Revoke Access --- .../driver/OntapPrimaryDatastoreDriver.java | 194 +++++++++++++++--- .../storage/feign/client/SANFeignClient.java | 26 +-- .../OntapPrimaryDatastoreLifecycle.java | 91 +++++++- .../storage/service/StorageStrategy.java | 20 +- .../storage/service/UnifiedNASStrategy.java | 8 +- .../storage/service/UnifiedSANStrategy.java | 118 +++++++++-- .../cloudstack/storage/utils/Constants.java | 11 +- .../cloudstack/storage/utils/Utility.java | 99 ++++++++- 8 files changed, 484 insertions(+), 83 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 e2eb6220230a..cfdd5a2ad0c8 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; @@ -44,9 +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.OntapStorage; -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.cloudstack.storage.utils.Constants; @@ -64,6 +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"); @@ -100,10 +103,15 @@ 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) { - path = createCloudStackVolumeForTypeVolume(dataStore, dataObject); + path = createCloudStackVolumeForTypeVolume(storagePool, dataObject); createCmdResult = new CreateCmdResult(path, new Answer(null, true, null)); } else { errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync"; @@ -120,14 +128,9 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } } - private String createCloudStackVolumeForTypeVolume(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()); - } - Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); - StorageStrategy storageStrategy = getStrategyByStoragePoolDetails(details); + private String createCloudStackVolumeForTypeVolume(StoragePoolVO storagePool, DataObject dataObject) { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.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, dataObject); CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); @@ -172,12 +175,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 @@ -269,24 +417,4 @@ public boolean isStorageSupportHA(Storage.StoragePoolType type) { public void detachVolumeFromAllStorageNodes(Volume volume) { } - - private StorageStrategy getStrategyByStoragePoolDetails(Map details) { - if (details == null || details.isEmpty()) { - s_logger.error("getStrategyByStoragePoolDetails: Storage pool details are null or empty"); - throw new CloudRuntimeException("getStrategyByStoragePoolDetails: Storage pool details are null or empty"); - } - String protocol = details.get(Constants.PROTOCOL); - OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD), - details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol), - Boolean.parseBoolean(details.get(Constants.IS_DISAGGREGATED))); - StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage); - boolean isValid = storageStrategy.connect(); - if (isValid) { - s_logger.info("Connection to Ontap SVM [{}] successful", details.get(Constants.SVM_NAME)); - return storageStrategy; - } else { - s_logger.error("getStrategyByStoragePoolDetails: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); - throw new CloudRuntimeException("getStrategyByStoragePoolDetails: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); - } - } } 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 dd2463d7f3bb..ed6f89c37904 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 @@ -18,6 +18,7 @@ */ package org.apache.cloudstack.storage.feign.client; +import feign.QueryMap; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunMap; @@ -26,6 +27,7 @@ import feign.Param; import feign.RequestLine; import java.net.URI; +import java.util.Map; //TODO: Proper URLs should be added in the RequestLine annotations below public interface SANFeignClient { @@ -54,15 +56,14 @@ OntapResponse createLun(@Param("authHeader") String authHeader, void deleteLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid); // iGroup Operation APIs - @RequestLine("POST /") + @RequestLine("POST /api/protocols/san/igroups") @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"}) OntapResponse createIgroup(@Param("authHeader") String authHeader, - @Param("returnRecords") boolean returnRecords, - Igroup igroupRequest); + @Param("returnRecords") boolean returnRecords, Igroup igroupRequest); - @RequestLine("GET /") - @Headers({"Authorization: {authHeader}"}) // TODO: Check this again, uuid should be part of the path? - OntapResponse getIgroupResponse(@Param("authHeader") String authHeader, @Param("uuid") String uuid); + @RequestLine("GET /api/protocols/san/igroups") + @Headers({"Authorization: {authHeader}"}) + OntapResponse getIgroupResponse(@Param("authHeader") String authHeader, @QueryMap Map queryMap); @RequestLine("GET /{uuid}") @Headers({"Authorization: {authHeader}"}) @@ -73,17 +74,18 @@ OntapResponse createIgroup(@Param("authHeader") String authHeader, void deleteIgroup(@Param("baseUri") URI baseUri, @Param("authHeader") String authHeader, @Param("uuid") String uuid); // LUN Maps Operation APIs - @RequestLine("POST /") - @Headers({"Authorization: {authHeader}"}) - OntapResponse createLunMap(@Param("authHeader") String authHeader, LunMap lunMap); + @RequestLine("POST /api/protocols/san/lun-maps") + @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"}) + OntapResponse createLunMap(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, LunMap lunMap); + @RequestLine("GET /") @Headers({"Authorization: {authHeader}"}) OntapResponse getLunMapResponse(@Param("authHeader") String authHeader); - @RequestLine("DELETE /{lunUuid}/{igroupUuid}") + @RequestLine("DELETE /api/protocols/san/lun-maps/{lunUuid}/{igroupUuid}") @Headers({"Authorization: {authHeader}"}) void deleteLunMap(@Param("authHeader") String authHeader, - @Param("lunUuid") String lunUuid, - @Param("igroupUuid") String igroupUuid); + @Param("lunUuid") String lunUUID, + @Param("igroupUuid") String igroupUUID); } 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 01b013f606dd..5fe69225aea0 100644 --- 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; @@ -38,17 +39,23 @@ import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreParameters; import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl; import org.apache.cloudstack.storage.feign.model.OntapStorage; 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.ProtocolType; import org.apache.cloudstack.storage.utils.Constants; +import org.apache.cloudstack.storage.utils.Utility; import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper; import org.apache.logging.log4j.LogManager; 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; @@ -59,6 +66,8 @@ 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; private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreLifecycle.class); // ONTAP minimum volume size is 1.56 GB (1677721600 bytes) @@ -241,16 +250,42 @@ 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); + 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())); + 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) { - // TODO: Fetch the host IQN and add to the initiator group on ONTAP cluster try { _storageMgr.connectHostToSharedPool(host, dataStore.getId()); } catch (Exception e) { - logger.warn("Unable to establish a connection between " + host + " and " + dataStore, e); + logger.warn("attachCluster: Unable to establish a connection between " + host + " and " + dataStore, e); } } _dataStoreHelper.attachCluster(dataStore); @@ -265,11 +300,35 @@ 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()); + } List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), Hypervisor.HypervisorType.KVM); + // TODO- need to check if no host to connect then throw exception or just continue + logger.debug("attachZone: Eligible Up and Enabled hosts: {}", hostsToConnect); - logger.debug(String.format("In createPool. Attaching the pool to each of the hosts in %s.", 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) { - // TODO: Fetch the host IQN and add to the initiator group on ONTAP cluster try { _storageMgr.connectHostToSharedPool(host, dataStore.getId()); } catch (Exception e) { @@ -280,6 +339,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) { return true; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index 0f9706335784..acd9e4c7b71d 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 @@ -274,10 +274,10 @@ public Volume getStorageVolume(Volume volume) * 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 + * @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 @@ -287,7 +287,7 @@ public Volume getStorageVolume(Volume volume) * @param accessGroup the access group to create * @return the created AccessGroup object */ - abstract AccessGroup createAccessGroup(AccessGroup accessGroup); + abstract public AccessGroup createAccessGroup(AccessGroup accessGroup); /** * Method encapsulates the behavior based on the opted protocol in subclasses @@ -310,13 +310,11 @@ public Volume getStorageVolume(Volume volume) /** * 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 @@ -324,7 +322,7 @@ public Volume getStorageVolume(Volume volume) * //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 @@ -332,5 +330,5 @@ public Volume getStorageVolume(Volume volume) * //TODO for Nvme/TCP and Nvme/FC protocols * @param values */ - abstract void disableLogicalAccess(Map values); + abstract public void disableLogicalAccess(Map values); } 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 cb3079691c94..9931d1ba09ce 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 @@ -67,7 +67,7 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) { + public CloudStackVolume getCloudStackVolume(Map cloudStackVolumeMap) { //TODO return null; } @@ -90,18 +90,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 0b47e1ff70a0..3e0f49d7a139 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,7 +22,9 @@ import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.storage.feign.FeignClientFactory; import org.apache.cloudstack.storage.feign.client.SANFeignClient; +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.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; @@ -95,15 +97,38 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) { + public CloudStackVolume getCloudStackVolume(Map cloudStackVolumeMap) { //TODO return null; } @Override public AccessGroup createAccessGroup(AccessGroup accessGroup) { - //TODO - return null; + s_logger.info("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 AuthHeader + String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // Create Igroup + OntapResponse createdIgroup = sanFeignClient.createIgroup(authHeader, true, accessGroup.getIgroup()); + if (createdIgroup == null || createdIgroup.getRecords() == null || createdIgroup.getRecords().size() == 0) { + s_logger.error("createAccessGroup: Igroup creation failed for Igroup Name {}", accessGroup.getIgroup().getName()); + throw new CloudRuntimeException("Failed to create Igroup: " + accessGroup.getIgroup().getName()); + } + Igroup igroup = createdIgroup.getRecords().get(0); + s_logger.debug("createAccessGroup: Igroup created successfully. Igroup: {}", igroup); + s_logger.info("createAccessGroup: Igroup created successfully. IgroupName: {}", igroup.getName()); + + AccessGroup createdAccessGroup = new AccessGroup(); + createdAccessGroup.setIgroup(igroup); + return createdAccessGroup; + } catch (Exception e) { + s_logger.error("Exception occurred while creating Igroup: {}. Exception: {}", accessGroup.getIgroup().getName(), e.getMessage()); + throw new CloudRuntimeException("Failed to create Igroup: " + e.getMessage()); + } } @Override @@ -117,19 +142,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/Constants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java index b58e8484cd48..c4496b269cba 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 @@ -49,7 +49,7 @@ public class Constants { public static final int JOB_MAX_RETRIES = 100; public static final int CREATE_VOLUME_CHECK_SLEEP_TIME = 2000; - public static final String PATH_SEPARATOR = "/"; + public static final String SLASH = "/"; public static final String EQUALS = "="; public static final String SEMICOLON = ";"; public static final String COMMA = ","; @@ -59,5 +59,12 @@ public class Constants { public static final String KVM = "KVM"; public static final String HTTPS = "https://"; - + public static final String SVM_DOT_NAME = "svm.name"; + 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 IGROUP_DOT_NAME = "igroup.name"; + public static final String IGROUP_DOT_UUID = "igroup.uuid"; + public static final String UNDERSCORE = "_"; + public static final String CS = "cs"; } 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 af48724f984c..829cf4aabc53 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,28 @@ package org.apache.cloudstack.storage.utils; +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.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 { @@ -68,7 +77,7 @@ public static CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePo lunSpace.setSize(dataObject.getSize()); lunRequest.setSpace(lunSpace); //Lun name is full path like in unified "/vol/VolumeName/LunName" - String lunFullName = Constants.VOLUME_PATH_PREFIX + storagePool.getName() + Constants.PATH_SEPARATOR + dataObject.getName(); + String lunFullName = getLunName(storagePool.getName(), dataObject.getName()); lunRequest.setName(lunFullName); String hypervisorType = storagePool.getHypervisor().name(); @@ -90,4 +99,92 @@ public static CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePo throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol); } } + + public static String getOSTypeFromHypervisor(String hypervisorType){ + switch (hypervisorType) { + case Constants.KVM: + return Lun.OsTypeEnum.LINUX.getValue(); + default: + String errMsg = "getOSTypeFromHypervisor : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage"; + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + } + + public static StorageStrategy getStrategyByStoragePoolDetails(Map details) { + if (details == null || details.isEmpty()) { + s_logger.error("getStrategyByStoragePoolDetails: Storage pool details are null or empty"); + throw new CloudRuntimeException("getStrategyByStoragePoolDetails: Storage pool details are null or empty"); + } + String protocol = details.get(Constants.PROTOCOL); + OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD), + details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol), + Boolean.parseBoolean(details.get(Constants.IS_DISAGGREGATED))); + StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage); + boolean isValid = storageStrategy.connect(); + if (isValid) { + s_logger.info("Connection to Ontap SVM [{}] successful", details.get(Constants.SVM_NAME)); + return storageStrategy; + } else { + s_logger.error("getStrategyByStoragePoolDetails: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); + throw new CloudRuntimeException("getStrategyByStoragePoolDetails: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); + } + } + + 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 f43783ca6fca4b1ecd0ec2a044cda436dd53eb09 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 5 Nov 2025 15:29:56 +0530 Subject: [PATCH 02/21] 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 829cf4aabc53..404dcc3c8cdf 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 @@ -184,7 +184,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 ad4e526707679829ca9219c3c92e92dff18e9d00 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Thu, 6 Nov 2025 10:17:52 +0530 Subject: [PATCH 03/21] CSTACKEX-46 Resolve Copilot review comments --- .../driver/OntapPrimaryDatastoreDriver.java | 20 +++++++-- .../storage/feign/client/SANFeignClient.java | 2 +- .../OntapPrimaryDatastoreLifecycle.java | 2 +- .../storage/service/UnifiedSANStrategy.java | 44 ++++++++++++++++--- 4 files changed, 55 insertions(+), 13 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 cfdd5a2ad0c8..ba12c5ea1b2b 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; @@ -224,7 +226,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); } @@ -240,6 +242,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) { @@ -291,9 +303,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/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index ed6f89c37904..3e72132c91ea 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 @@ -41,7 +41,7 @@ OntapResponse createLun(@Param("authHeader") String authHeader, @RequestLine("GET /") @Headers({"Authorization: {authHeader}"}) - OntapResponse getLunResponse(@Param("authHeader") String authHeader); + OntapResponse getLunResponse(@Param("authHeader") String authHeader, @QueryMap Map queryMap); @RequestLine("GET /{uuid}") @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 5fe69225aea0..fdeb3076fb60 100644 --- 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 @@ -321,7 +321,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 3e0f49d7a139..b1024d5c787b 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 @@ -97,9 +97,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 @@ -152,7 +182,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 @@ -166,7 +196,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); @@ -200,7 +230,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()); } } @@ -220,7 +250,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 54a7eef428b17b9d26972caa8818e951a576b972 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Thu, 6 Nov 2025 15:16:58 +0530 Subject: [PATCH 04/21] CSTACKEX-46 Resolve review comments --- .../driver/OntapPrimaryDatastoreDriver.java | 52 +++++++++-- .../storage/feign/client/SANFeignClient.java | 4 +- .../OntapPrimaryDatastoreLifecycle.java | 60 +++++++++++-- .../cloudstack/storage/utils/Utility.java | 87 +------------------ 4 files changed, 103 insertions(+), 100 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 ba12c5ea1b2b..a0a4abb6cf50 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; @@ -113,7 +112,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) { - path = createCloudStackVolumeForTypeVolume(storagePool, dataObject); + path = createCloudStackVolumeForTypeVolume(storagePool, (VolumeInfo)dataObject); createCmdResult = new CreateCmdResult(path, new Answer(null, true, null)); } else { errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync"; @@ -130,21 +129,59 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } } - private String createCloudStackVolumeForTypeVolume(StoragePoolVO storagePool, DataObject dataObject) { + private String createCloudStackVolumeForTypeVolume(StoragePoolVO storagePool, VolumeInfo volumeInfo) { Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.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, dataObject); + CloudStackVolume cloudStackVolumeRequest = createCloudStackVolumeRequestByProtocol(storagePool, details, volumeInfo); 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 { - String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + dataObject; + String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + volumeInfo; s_logger.error(errMsg); throw new CloudRuntimeException(errMsg); } } + 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) { @@ -219,6 +256,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); String svmName = details.get(Constants.SVM_NAME); + //TODO- verify scopeId DatacenterId is zoneId long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); if(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { @@ -305,7 +343,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 3e72132c91ea..c2f7986b55e8 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 @@ -33,13 +33,13 @@ public interface SANFeignClient { // LUN Operation APIs - @RequestLine("POST /") + @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 /") + @RequestLine("GET /api/storage/luns") @Headers({"Authorization: {authHeader}"}) OntapResponse getLunResponse(@Param("authHeader") String authHeader, @QueryMap Map queryMap); 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 fdeb3076fb60..e637f2e20d02 100644 --- 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,7 +43,10 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.lifecycle.BasePrimaryDataStoreLifeCycleImpl; +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.provider.StorageProviderFactory; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; @@ -271,13 +274,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()); @@ -320,12 +323,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) { @@ -339,7 +342,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; @@ -357,6 +360,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) { return true; 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 404dcc3c8cdf..99bd1b6aa88c 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 @@ -62,44 +62,6 @@ public static String generateAuthHeader (String username, String password) { return BASIC + StringUtils.SPACE + new String(encodedBytes); } - public static CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map details, DataObject dataObject) { - 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(dataObject.getSize()); - lunRequest.setSpace(lunSpace); - //Lun name is full path like in unified "/vol/VolumeName/LunName" - String lunFullName = getLunName(storagePool.getName(), dataObject.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); - } - } - public static String getOSTypeFromHypervisor(String hypervisorType){ switch (hypervisorType) { case Constants.KVM: @@ -131,54 +93,7 @@ public static StorageStrategy getStrategyByStoragePoolDetails(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 815af83a5cde5a544d23e4997fbbfb1184a19778 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Thu, 6 Nov 2025 15:49:37 +0530 Subject: [PATCH 05/21] CSTACKEX-46 Resolve review bot comments --- .../driver/OntapPrimaryDatastoreDriver.java | 2 +- .../storage/service/UnifiedSANStrategy.java | 28 +++++++++---------- 2 files changed, 15 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 a0a4abb6cf50..8bd9dbb2632a 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 @@ -281,7 +281,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 b1024d5c787b..e30402885ce7 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 @@ -80,7 +80,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()); } } @@ -98,16 +98,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 @@ -116,8 +116,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); @@ -127,7 +127,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()); } } @@ -156,7 +156,7 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { createdAccessGroup.setIgroup(igroup); return createdAccessGroup; } catch (Exception e) { - s_logger.error("Exception occurred while creating Igroup: {}. Exception: {}", accessGroup.getIgroup().getName(), e.getMessage()); + s_logger.error("Exception occurred while creating Igroup: {}, Exception: {}", accessGroup.getIgroup().getName(), e.getMessage()); throw new CloudRuntimeException("Failed to create Igroup: " + e.getMessage()); } } @@ -202,7 +202,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()); } } @@ -227,10 +227,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()); } } @@ -250,7 +250,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 15081b59409276321f81cbc164541d8edcd27988 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 10:57:50 +0530 Subject: [PATCH 06/21] 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 e637f2e20d02..c495edaffb22 100644 --- 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 @@ -274,7 +274,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()); } @@ -323,7 +323,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()); } @@ -342,7 +342,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; @@ -355,7 +355,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 65fa626a97100984714a5ab30d6de3dbdd0a60d1 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 11:42:03 +0530 Subject: [PATCH 07/21] CSTACKEX-46 Resolve check styling issues --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 6 +++++- .../org/apache/cloudstack/storage/utils/Utility.java | 11 ----------- 2 files changed, 5 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 8bd9dbb2632a..d442d7fe68c5 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 99bd1b6aa88c..82eed178a260 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,28 +19,17 @@ package org.apache.cloudstack.storage.utils; -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.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 { From 3a7d2f8b2be016bb300044a6b1fe4f54b5ea9258 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 12:50:46 +0530 Subject: [PATCH 08/21] 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 c495edaffb22..f8625baf5b59 100644 --- 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 @@ -270,7 +270,7 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { // 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()); - 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 acd9e4c7b71d..7a9fec70ce09 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 @@ -310,7 +310,7 @@ public Volume getStorageVolume(Volume volume) /** * 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 e30402885ce7..4546f39d9b02 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,7 +106,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 bc8e2168c9dec7e46e7294ccd7c0e8afbf4feb24 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 13:08:16 +0530 Subject: [PATCH 09/21] 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 f8625baf5b59..3586ed98f805 100644 --- 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 @@ -361,7 +361,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: @@ -404,6 +404,7 @@ private AccessGroup createSANAccessGroupRequest(String svmName, String igroupNam igroup.setInitiators(initiators); } accessGroupRequest.setIgroup(igroup); + s_logger.debug("createSANAccessGroupRequest: request: " + accessGroupRequest); return accessGroupRequest; } From cd876402ad8d7d79f6bfb7fae25eca936387de70 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 13:33:15 +0530 Subject: [PATCH 10/21] CSTACKEX-46 Testing: Added debug log --- .../apache/cloudstack/storage/feign/client/SANFeignClient.java | 3 +-- .../apache/cloudstack/storage/service/UnifiedSANStrategy.java | 2 ++ 2 files changed, 3 insertions(+), 2 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 c2f7986b55e8..e9a8740a5359 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 @@ -58,8 +58,7 @@ OntapResponse createLun(@Param("authHeader") String authHeader, // iGroup Operation APIs @RequestLine("POST /api/protocols/san/igroups") @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"}) - OntapResponse createIgroup(@Param("authHeader") String authHeader, - @Param("returnRecords") boolean returnRecords, Igroup igroupRequest); + OntapResponse createIgroup(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, Igroup igroupRequest); @RequestLine("GET /api/protocols/san/igroups") @Headers({"Authorization: {authHeader}"}) 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 4546f39d9b02..b9c6da59da3b 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 @@ -144,6 +144,8 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { String authHeader = Utility.generateAuthHeader(storage.getUsername(), storage.getPassword()); // Create Igroup OntapResponse createdIgroup = sanFeignClient.createIgroup(authHeader, true, accessGroup.getIgroup()); + s_logger.debug("createAccessGroup: createdIgroup: {}", createdIgroup); + s_logger.debug("createAccessGroup: createdIgroup Records: {}", createdIgroup.getRecords()); if (createdIgroup == null || createdIgroup.getRecords() == null || createdIgroup.getRecords().size() == 0) { s_logger.error("createAccessGroup: Igroup creation failed for Igroup Name {}", accessGroup.getIgroup().getName()); throw new CloudRuntimeException("Failed to create Igroup: " + accessGroup.getIgroup().getName()); From a10981acabdc50b457c06e112fc7286b772adad7 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 13:36:04 +0530 Subject: [PATCH 11/21] 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 3586ed98f805..014368b18610 100644 --- 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 @@ -280,8 +280,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 c4399c1e4fec1f40ea322be5d711184502024aed Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 13:50:48 +0530 Subject: [PATCH 12/21] CSTACKEX-46 Testing: Corrected the URL for Create Lun and Igroup --- .../storage/feign/client/SANFeignClient.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 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 e9a8740a5359..f1609f89c86d 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 @@ -33,11 +33,9 @@ public interface SANFeignClient { // LUN Operation APIs - @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}"}) @@ -56,8 +54,8 @@ OntapResponse createLun(@Param("authHeader") String authHeader, void deleteLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid); // iGroup Operation APIs - @RequestLine("POST /api/protocols/san/igroups") - @Headers({"Authorization: {authHeader}", "return_records: {returnRecords}"}) + @RequestLine("POST /api/protocols/san/igroups?return_records={returnRecords}") + @Headers({"Authorization: {authHeader}"}) OntapResponse createIgroup(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, Igroup igroupRequest); @RequestLine("GET /api/protocols/san/igroups") From 71daee2d43aeef8321befb88e3e34d140de000ff Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 7 Nov 2025 16:07:15 +0530 Subject: [PATCH 13/21] 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 d442d7fe68c5..3beb06583b4c 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 @@ -166,17 +166,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); @@ -270,7 +260,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 014368b18610..ce9958a2cc46 100644 --- 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 @@ -275,8 +275,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) { @@ -329,8 +330,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 7a9fec70ce09..db414ad6d1de 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 @@ -310,9 +310,10 @@ public Volume getStorageVolume(Volume volume) /** * 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 b9c6da59da3b..9d50e9a9229c 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 @@ -197,9 +197,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; @@ -226,7 +223,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 dfde3f8a8fb59d7daa2c8bbdb2c08c4e2444cd8e Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 12:35:15 +0530 Subject: [PATCH 14/21] 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 3beb06583b4c..0f9a595d8e4a 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 @@ -108,7 +108,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) { @@ -125,7 +125,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 { @@ -150,13 +150,12 @@ private String createCloudStackVolumeForTypeVolume(StoragePoolVO storagePool, Vo 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 ce9958a2cc46..515b40ce0e5e 100644 --- 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 @@ -364,6 +364,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 bce721d7bc766f98fd870fc3ed220c30fcd1473c Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 14:07:14 +0530 Subject: [PATCH 15/21] 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 0f9a595d8e4a..795eba915b98 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 @@ -340,7 +340,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 db414ad6d1de..beec70ec82da 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 @@ -310,8 +310,8 @@ public Volume getStorageVolume(Volume volume) /** * 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 82eed178a260..37c56764c84f 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 @@ -83,7 +83,7 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map Date: Mon, 10 Nov 2025 14:15:14 +0530 Subject: [PATCH 16/21] CSTACKEX-46 Change log level --- .../storage/service/UnifiedSANStrategy.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 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 9d50e9a9229c..ad6f06e9ec9a 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 @@ -98,7 +98,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"); @@ -134,7 +135,8 @@ public CloudStackVolume getCloudStackVolume(Map values) { @Override public AccessGroup createAccessGroup(AccessGroup accessGroup) { - s_logger.info("createAccessGroup : Creating Igroup with access group request {} ", accessGroup); + s_logger.info("createAccessGroup : Create Igroup"); + 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"); @@ -175,7 +177,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"); @@ -207,7 +210,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); @@ -235,7 +239,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 70c6b13479c32f4c80afa97cfbee99187f3ddc87 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 15:02:37 +0530 Subject: [PATCH 17/21] 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 795eba915b98..1dd427646abc 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,11 +107,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()); } @@ -133,6 +137,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(StoragePoolVO storagePool, VolumeInfo volumeInfo) { Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy storageStrategy = Utility.getStrategyByStoragePoolDetails(details); @@ -218,7 +232,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()); } @@ -229,7 +243,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()); } @@ -252,12 +266,12 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, //TODO- verify scopeId DatacenterId is zoneId 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 + "]"); } @@ -297,7 +311,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()); } @@ -308,7 +322,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()); } @@ -329,12 +343,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; } @@ -352,7 +366,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 515b40ce0e5e..783f6b58eace 100644 --- 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 @@ -261,7 +261,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()); } @@ -317,7 +317,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 ad6f06e9ec9a..26d8b366efdb 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 @@ -212,7 +212,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); @@ -224,6 +223,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); @@ -233,7 +236,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()); } } @@ -254,7 +257,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 63b484beec10cdcba3a2c2ff33e630b35ac54799 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 16:46:43 +0530 Subject: [PATCH 18/21] CSTACKEX-46 resolve license issue --- .../storage/feign/FeignConfiguration.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java index ce2783add228..fc4d3484506b 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java @@ -1,3 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.apache.cloudstack.storage.feign; import feign.RequestInterceptor; From 241d7f14d62902208555a7069e9875a692cb189e Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 18:03:08 +0530 Subject: [PATCH 19/21] CSTACKEX-46 resolve check style issues --- .../driver/OntapPrimaryDatastoreDriver.java | 3 +-- .../feign/client/VolumeFeignClient.java | 1 - .../cloudstack/storage/feign/model/Svm.java | 3 +-- .../OntapPrimaryDatastoreLifecycle.java | 14 +++++++++----- .../storage/service/UnifiedSANStrategy.java | 19 ++++++++++++------- .../cloudstack/storage/utils/Constants.java | 1 + .../cloudstack/storage/utils/Utility.java | 2 +- 7 files changed, 25 insertions(+), 18 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 1dd427646abc..1ef6a1ed6e15 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 @@ -143,8 +143,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(StoragePoolVO storagePool, VolumeInfo volumeInfo) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/VolumeFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/VolumeFeignClient.java index 9a2c76639221..bfa2a2ac0090 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/VolumeFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/VolumeFeignClient.java @@ -42,4 +42,3 @@ public interface VolumeFeignClient { @Headers({"Accept: {acceptHeader}", "Authorization: {authHeader}"}) JobResponse updateVolumeRebalancing(@Param("acceptHeader") String acceptHeader, @Param("uuid") String uuid, Volume volumeRequest); } - 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 f1a226739365..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 { } - -} \ No newline at end of file +} 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 783f6b58eace..645d879bd78d 100644 --- 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 @@ -285,8 +285,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()); @@ -335,8 +335,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 { @@ -461,4 +466,3 @@ public void changeStoragePoolScopeToCluster(DataStore store, ClusterScope cluste } } - 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 26d8b366efdb..b4b2bfad39ee 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,10 +22,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.storage.feign.FeignClientFactory; import org.apache.cloudstack.storage.feign.client.SANFeignClient; -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.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -224,9 +221,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/Constants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Constants.java index c4496b269cba..41c6f1205dbe 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 @@ -56,6 +56,7 @@ public class Constants { public static final String VOLUME_PATH_PREFIX = "/vol/"; + public static final String ONTAP_NAME_REGEX = "^[a-zA-Z][a-zA-Z0-9_]*$"; public static final String KVM = "KVM"; public static final String HTTPS = "https://"; 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 37c56764c84f..79e93485c3ac 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 @@ -83,7 +83,7 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map Date: Mon, 10 Nov 2025 18:49:57 +0530 Subject: [PATCH 20/21] CSTACKEX-46 Resolve import issues --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 2 +- .../cloudstack/storage/service/UnifiedSANStrategy.java | 6 +++++- 2 files changed, 6 insertions(+), 2 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 1ef6a1ed6e15..9e725d81dc9e 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 @@ -137,7 +137,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 b4b2bfad39ee..60e9b5b9c4c7 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,7 +22,11 @@ import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.storage.feign.FeignClientFactory; import org.apache.cloudstack.storage.feign.client.SANFeignClient; -import org.apache.cloudstack.storage.feign.model.*; +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 79a078f9170c447a4ee2dbf27ce4c9229066d4dd Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 10 Nov 2025 19:12:50 +0530 Subject: [PATCH 21/21] 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 645d879bd78d..31a5f6f9d75e 100644 --- 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 @@ -285,8 +285,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());