From 02445699848a0de0a7ccb58963cbef3bb9b4f032 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Thu, 30 Oct 2025 13:41:24 +0530 Subject: [PATCH 1/7] CSTACKEX-36 Attach Cluster and Grant Access --- .../driver/OntapPrimaryDatastoreDriver.java | 104 ++++++++-- .../storage/feign/client/SANFeignClient.java | 10 +- .../OntapPrimaryDatastoreLifecycle.java | 82 +++++++- .../storage/service/StorageStrategy.java | 8 +- .../storage/service/UnifiedNASStrategy.java | 6 +- .../storage/service/UnifiedSANStrategy.java | 126 ++++++++++-- .../cloudstack/storage/utils/Constants.java | 10 + .../cloudstack/storage/utils/Utility.java | 181 ++++++++++++++---- 8 files changed, 444 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 6d850a97168f..f5431f749668 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 @@ -25,8 +25,11 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.host.Host; import com.cloud.storage.Storage; -import com.cloud.storage.StoragePool; +import com.cloud.storage.ScopeType; +import com.cloud.storage.VolumeVO; import com.cloud.storage.Volume; +import com.cloud.storage.StoragePool; +import com.cloud.storage.dao.VolumeDao; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; @@ -45,6 +48,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.utils.Constants; @@ -56,13 +60,14 @@ import java.util.HashMap; import java.util.Map; -public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { +public abstract class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreDriver.class); @Inject private Utility utils; @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @Inject private PrimaryDataStoreDao storagePoolDao; + @Inject private VolumeDao volumeDao; @Override public Map getCapabilities() { s_logger.trace("OntapPrimaryDatastoreDriver: getCapabilities: Called"); @@ -99,10 +104,16 @@ 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"; @@ -119,13 +130,8 @@ 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()); + private String createCloudStackVolumeForTypeVolume(StoragePoolVO storagePool, DataObject dataObject) { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy storageStrategy = utils.getStrategyByStoragePoolDetails(details); s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME)); CloudStackVolume cloudStackVolumeRequest = utils.createCloudStackVolumeRequestByProtocol(storagePool, details, dataObject); @@ -171,9 +177,83 @@ 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. Storage Pool: " + storagePool.getName()); + throw new CloudRuntimeException("grantAccess: Only Cluster and ZONE scoped primary storage is supported. Storage Pool: " + storagePool.getName()); + } + + 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()); + } + + if (dataObject.getType() == DataObjectType.VOLUME) { + 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 = utils.getStrategyByStoragePoolDetails(details); + String svmName = details.get(Constants.SVM_NAME); + + if(ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL))) { + Map getCloudStackVolumeMap = new HashMap<>(); + getCloudStackVolumeMap.put(Constants.NAME, volumeVO.getPath()); + 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("grantAccess: Failed to get LUN details [{}]", volumeVO.getName()); + throw new CloudRuntimeException("grantAccess: Failed to get LUN [" + volumeVO.getName() + "]"); + } + + long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); + String igroupName = utils.getIgroupName(storagePool.getName(), scopeId); + Map getAccessGroupMap = new HashMap<>(); + getAccessGroupMap.put(Constants.NAME, igroupName); + 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("grantAccess: Failed to get iGroup details for host [{}]", host.getName()); + throw new CloudRuntimeException("grantAccess: Failed to get iGroup details for host [" + host.getName() + "]"); + } + if(!accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) { + s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), igroupName); + throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + igroupName); + } + + 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, igroupName); + storageStrategy.enableLogicalAccess(enableLogicalAccessMap); + } + } + @Override public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { 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 325823f8515c..51b7faf8436d 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 @@ -65,21 +65,17 @@ OntapResponse createIgroup(URI uri, @RequestHeader("Authorization") Stri //this method to get all igroups and also filtered igroups based on query params as a part of URL @RequestMapping(method = RequestMethod.GET) - OntapResponse getIgroupResponse(URI baseURL, @RequestHeader("Authorization") String header, @PathVariable(name = "uuid", required = true) String uuid); + OntapResponse getIgroupResponse(URI baseURL, @RequestHeader("Authorization") String header); @RequestMapping(method = RequestMethod.GET, value = "/{uuid}") Igroup getIgroupByUUID(URI baseURL, @RequestHeader("Authorization") String header, @PathVariable(name = "uuid", required = true) String uuid); @RequestMapping(method = RequestMethod.DELETE, value = "/{uuid}") void deleteIgroup(URI baseUri, @RequestHeader("Authorization") String authHeader, @PathVariable(name = "uuid", required = true) String uuid); - @RequestMapping(method = RequestMethod.POST, value = "/{uuid}/igroups") - OntapResponse addNestedIgroups(URI uri, @RequestHeader("Authorization") String header, @PathVariable(name = "uuid", required = true) String uuid, - @RequestBody Igroup igroupNestedRequest, @RequestHeader(value="return_records", defaultValue = "true") boolean value); - - //Lun Maps Operation APIs @RequestMapping(method = RequestMethod.POST) - OntapResponse createLunMap(URI baseURL, @RequestHeader("Authorization") String authHeader, @RequestBody LunMap lunMap); + OntapResponse createLunMap(URI baseURL, @RequestHeader("Authorization") String authHeader, @RequestHeader("return_records") boolean value, + @RequestBody LunMap lunMap); @RequestMapping(method = RequestMethod.GET) OntapResponse getLunMapResponse(URI baseURL, @RequestHeader("Authorization") String 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 1538cebfd373..0c595f14398e 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.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; 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.UUID; @@ -58,7 +65,10 @@ public class OntapPrimaryDatastoreLifecycle extends BasePrimaryDataStoreLifeCycl @Inject private StorageManager _storageMgr; @Inject private ResourceManager _resourceMgr; @Inject private PrimaryDataStoreHelper _dataStoreHelper; - private static final Logger s_logger = (Logger)LogManager.getLogger(OntapPrimaryDatastoreLifecycle.class); + @Inject private Utility utils; + @Inject private PrimaryDataStoreDao storagePoolDao; + @Inject private StoragePoolDetailsDao storagePoolDetailsDao; + private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreLifecycle.class); /** * Creates primary storage on NetApp storage @@ -167,12 +177,34 @@ public DataStore initialize(Map dsInfos) { @Override public boolean attachCluster(DataStore dataStore, ClusterScope scope) { logger.debug("In attachCluster for ONTAP primary storage"); + if (dataStore == null) { + throw new InvalidParameterValueException("attachCluster: dataStore should not be null"); + } + if (scope == null) { + throw new InvalidParameterValueException("attachCluster: clusterScope 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(String.format("Attaching the pool to each of the hosts %s in the cluster: %s", hostsToConnect, primarystore.getClusterId())); + + Map details = primarystore.getDetails(); + StorageStrategy strategy = utils.getStrategyByStoragePoolDetails(details); + ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); + if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) { + throw new CloudRuntimeException("Not all hosts in the cluster support the protocol: " + protocol.toString()); + } + if (hostsIdentifier != null && hostsIdentifier.size() > 0) { + AccessGroup accessGroupRequest = utils.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) { @@ -183,6 +215,25 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { return true; } + private boolean isProtocolSupportedByAllHosts(List hosts, ProtocolType protocolType, List hostIdentifiers) { + String protocolPrefix; + switch (protocolType) { + case ISCSI: + 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("Unsupported protocol: " + protocolType.toString()); + } + return true; + } + @Override public boolean attachHost(DataStore store, HostScope scope, StoragePoolInfo existingInfo) { return false; @@ -191,9 +242,32 @@ 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: clusterScope 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()); + } 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(String.format("In createPool. Attaching the pool to each of the hosts in %s.", hostsToConnect)); + + Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); + StorageStrategy strategy = utils.getStrategyByStoragePoolDetails(details); + ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL)); + if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) { + throw new CloudRuntimeException("Not all hosts in the zone support the protocol: " + protocol.toString()); + } + if (hostsIdentifier != null && !hostsIdentifier.isEmpty()) { + AccessGroup accessGroupRequest = utils.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 { 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 e3b15e81f210..1f8b904f384a 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 @@ -270,7 +270,7 @@ public Volume getStorageVolume(Volume volume) * getNameSpace for Nvme/TCP and Nvme/FC protocol * @param values */ - abstract CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume); + abstract public CloudStackVolume getCloudStackVolume(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses @@ -279,7 +279,7 @@ public Volume getStorageVolume(Volume volume) * createSubsystem for Nvme/TCP and Nvme/FC protocols * @param values */ - abstract AccessGroup createAccessGroup(AccessGroup accessGroup); + abstract public AccessGroup createAccessGroup(AccessGroup accessGroup); /** * Method encapsulates the behavior based on the opted protocol in subclasses @@ -306,7 +306,7 @@ 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 @@ -314,7 +314,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 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 865a5925168f..1ddf62b35e7f 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 @@ -48,7 +48,7 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - public CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) { + public CloudStackVolume getCloudStackVolume(Map values) { //TODO return null; } @@ -71,13 +71,13 @@ 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) { } 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 e0f1c16e788f..b6d836ba9ec9 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 @@ -21,7 +21,9 @@ import com.cloud.utils.exception.CloudRuntimeException; 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; @@ -87,15 +89,66 @@ void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - CloudStackVolume getCloudStackVolume(CloudStackVolume cloudstackVolume) { - //TODO - return null; + public CloudStackVolume getCloudStackVolume(Map values) { + s_logger.info("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"); + } + try { + // Get AuthHeader + String authHeader = utils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // Create URI for lun creation + URI url = utils.generateURI(Constants.CREATE_LUN); + url = utils.addQueryParamsToURI(url, values); + s_logger.info("getCloudStackVolume: URL: {}", url); + //TODO: It is possible that Lun creation will take time and we may need to handle through async job. + OntapResponse lunResponse = sanFeignClient.getLunResponse(url, authHeader); + if (lunResponse == null || lunResponse.getRecords() == null || lunResponse.getRecords().size() == 0) { + s_logger.error("getCloudStackVolume: Failed to fetch lun"); + throw new CloudRuntimeException("Failed to fetch Lun"); + } + 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 createdCloudStackVolume = new CloudStackVolume(); + createdCloudStackVolume.setLun(lun); + return createdCloudStackVolume; + } catch (Exception e) { + s_logger.error("Exception occurred while fetching LUN. Exception: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Lun: " + e.getMessage()); + } } @Override public AccessGroup createAccessGroup(AccessGroup accessGroup) { - //TODO - return null; + s_logger.info("createAccessGroup : Creating Igroup with accessGroup 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 Lun, invalid request"); + } + try { + // Get AuthHeader + String authHeader = utils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // Create URI for Igroup creation + URI url = utils.generateURI(Constants.CREATE_IGROUP); + OntapResponse createdIgroup = sanFeignClient.createIgroup(url, 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 @@ -110,19 +163,70 @@ public AccessGroup updateAccessGroup(AccessGroup accessGroup) { } @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"); + } + try { + // Get AuthHeader + String authHeader = utils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // Create URI for lun creation + URI url = utils.generateURI(Constants.CREATE_IGROUP); + url = utils.addQueryParamsToURI(url, values); + s_logger.info("getAccessGroup: URL: {}", url); + //TODO: It is possible that Lun creation will take time and we may need to handle through async job. + OntapResponse igroupResponse = sanFeignClient.getIgroupResponse(url, authHeader); + 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) { - + 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 = utils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // Create URI for Igroup creation + URI url = utils.generateURI(Constants.CREATE_LUNMAP); + OntapResponse createdLunMap = sanFeignClient.createLunMap(url, authHeader, true, lunMapRequest); + if (createdLunMap == null || createdLunMap.getRecords() == null || createdLunMap.getRecords().size() == 0) { + s_logger.error("enableLogicalAccess: LunMap creation failed for Lun {} and igroup ", lunName, igroupName); + throw new CloudRuntimeException("Failed to create LunMap: creation failed 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) { } - } 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 61674db7c8b0..2042c0ff4ff0 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 @@ -31,6 +31,11 @@ public class Constants { public static final String IS_DISAGGREGATED = "isDisaggregated"; public static final String RUNNING = "running"; + public static final String SVM_DOT_NAME = "svm.name"; + public static final String LUN_DOT_NAME = "lun.name"; + public static final String IGROUP_DOT_NAME = "igroup.name"; + public static final String NAME = "name"; + public static final String JOB_RUNNING = "running"; public static final String JOB_QUEUE = "queued"; public static final String JOB_PAUSED = "paused"; @@ -41,15 +46,20 @@ public class Constants { public static final int CREATE_VOLUME_CHECK_SLEEP_TIME = 2000; public static final String PATH_SEPARATOR = "/"; + public static final String UNDERSCORE = "_"; public static final String VOLUME_PATH_PREFIX = "/vol/"; public static final String KVM = "KVM"; + public static final String IQN = "iqn"; + public static final String HTTPS = "https://"; public static final String GET_SVMs = "/api/svm/svms"; public static final String CREATE_VOLUME = "/api/storage/volumes"; public static final String GET_JOB_BY_UUID = "/api/cluster/jobs"; public static final String CREATE_LUN = "/api/storage/luns"; + public static final String CREATE_IGROUP = "/api/protocols/san/igroups"; + public static final String CREATE_LUNMAP = "/api/protocols/san/lun-maps"; } 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 c1fc47aef90b..167d83c097a1 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,20 +19,25 @@ 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.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.LunSpace; +import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.feign.model.OntapStorage; +import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.Svm; +import org.apache.cloudstack.storage.feign.model.LunSpace; +import org.apache.cloudstack.storage.feign.model.Igroup; 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.http.client.utils.URIBuilder; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.stereotype.Component; @@ -40,15 +45,21 @@ import javax.inject.Inject; import java.net.URI; +import java.util.ArrayList; +import java.util.List; import java.util.Map; +import java.util.Set; @Component public class Utility { private static final Logger s_logger = LogManager.getLogger(Utility.class); - @Inject private OntapStorage ontapStorage; - @Inject private PrimaryDataStoreDao storagePoolDao; - @Inject private StoragePoolDetailsDao storagePoolDetailsDao; + @Inject + private OntapStorage ontapStorage; + @Inject + private PrimaryDataStoreDao storagePoolDao; + @Inject + private StoragePoolDetailsDao storagePoolDetailsDao; private static final String BASIC = "Basic"; private static final String AUTH_HEADER_COLON = ":"; @@ -70,45 +81,72 @@ public URI generateURI (String path) { return URI.create(uriString); } - public 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 = Constants.VOLUME_PATH_PREFIX + storagePool.getName() + Constants.PATH_SEPARATOR + 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 URI addQueryParamsToURI(URI baseUri, Map params) throws Exception { + URIBuilder builder = new URIBuilder(baseUri); + for (Map.Entry entry : params.entrySet()) { + builder.addParameter(entry.getKey(), entry.getValue()); + } + return builder.build(); + } + + public CloudStackVolume createCloudStackVolumeRequestByProtocol (StoragePoolVO storagePool, Map details, DataObject dataObject) { + ProtocolType protocol = ProtocolType.valueOf(details.get(Constants.PROTOCOL).toLowerCase()); + String svmName = details.get(Constants.SVM_NAME); + switch (protocol) { + case ISCSI: + String volumeName = storagePool.getName(); + String lunName = dataObject.getName(); + long lunSize = dataObject.getSize(); + Hypervisor.HypervisorType hypervisorType = storagePool.getHypervisor(); + return createSANCloudStackVolumeRequest(svmName, volumeName, lunName, lunSize, hypervisorType); + default: + s_logger.error("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol); + throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol); + } + } + + public CloudStackVolume createSANCloudStackVolumeRequest(String svmName, String volName, String lunName, long lunSize, Hypervisor.HypervisorType hypervisorType) { + CloudStackVolume cloudStackVolumeRequest = new CloudStackVolume(); + Lun lunRequest = new Lun(); + + if(svmName != null || !svmName.isEmpty()) { + Svm svm = new Svm(); + svm.setName(svmName); + lunRequest.setSvm(svm); + } + + if(lunSize > 0L) { + LunSpace lunSpace = new LunSpace(); + lunSpace.setSize(lunSize); + lunRequest.setSpace(lunSpace); + } + + //Lun name is full path like in unified "/vol/VolumeName/LunName" + if(lunName != null || !lunName.isEmpty()) { + String lunFullName = getLunName(volName, lunName); + lunRequest.setName(lunFullName); + } + + if(hypervisorType != null) { + String hypervisorName = hypervisorType.name(); + lunRequest.setOsType(Lun.OsTypeEnum.valueOf(getOSTypeFromHypervisor(hypervisorName))); + } + cloudStackVolumeRequest.setLun(lunRequest); + return cloudStackVolumeRequest; } - public StorageStrategy getStrategyByStoragePoolDetails(Map details) { + public String getOSTypeFromHypervisor(String hypervisorType){ + switch (hypervisorType) { + case Constants.KVM: + return Lun.OsTypeEnum.LINUX.getValue(); + default: + String errMsg = "getOStypeFromHypervisorType : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage"; + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + } + + public 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"); @@ -127,4 +165,63 @@ public StorageStrategy getStrategyByStoragePoolDetails(Map detai throw new CloudRuntimeException("getStrategyByStoragePoolDetails: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); } } + + public 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: + String storagePoolName = storagePool.getName(); + //TODO: Check if we have to change the access group name format + // Access group name format: StoragePoolName_ScopeId + String igroupName = getIgroupName(storagePoolName, 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 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); + } + + return accessGroupRequest; + } + + public String getLunName(String volName, String lunName) { + //Lun name in unified "/vol/VolumeName/LunName" + return Constants.VOLUME_PATH_PREFIX + volName + Constants.PATH_SEPARATOR + lunName; + } + + public String getIgroupName(String storagePoolName, long scopeId) { + // Igroup name format: StoragePoolName_ScopeId + return storagePoolName + Constants.UNDERSCORE + scopeId; + } } From d57ed078c063762b1786d9ffdcb398d869b177e4 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Thu, 30 Oct 2025 13:48:29 +0530 Subject: [PATCH 2/7] CSTACKEX-36 Added TODOs --- .../cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java | 2 +- .../storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index f5431f749668..cf2d902aabae 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 @@ -60,7 +60,7 @@ import java.util.HashMap; import java.util.Map; -public abstract class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { +public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreDriver.class); 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 0c595f14398e..286131002eaa 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 @@ -200,6 +200,7 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { if (!isProtocolSupportedByAllHosts(hostsToConnect, protocol, hostsIdentifier)) { throw new CloudRuntimeException("Not all hosts in the cluster support the protocol: " + protocol.toString()); } + //TODO - check if no host to connect then also need to create access group without initiators if (hostsIdentifier != null && hostsIdentifier.size() > 0) { AccessGroup accessGroupRequest = utils.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); strategy.createAccessGroup(accessGroupRequest); From 256827c2f96b4b9147701b3aff239924700cdb67 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 31 Oct 2025 11:10:46 +0530 Subject: [PATCH 3/7] CSTACKEX-36 Correct the logger --- .../driver/OntapPrimaryDatastoreDriver.java | 17 +++-- .../OntapPrimaryDatastoreLifecycle.java | 65 ++++++++++--------- .../storage/service/UnifiedSANStrategy.java | 10 +-- .../cloudstack/storage/utils/Constants.java | 1 + .../cloudstack/storage/utils/Utility.java | 13 ++-- 5 files changed, 53 insertions(+), 53 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 cf2d902aabae..063ceeb1bbbc 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 @@ -192,18 +192,17 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore s_logger.error("grantAccess : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("grantAccess : Storage Pool not found for id: " + dataStore.getId()); } - if (storagePool.getScope() != ScopeType.CLUSTER || storagePool.getScope() != ScopeType.ZONE) { - s_logger.error("grantAccess: Only Cluster and ZONE scoped primary storage is supported. Storage Pool: " + storagePool.getName()); - throw new CloudRuntimeException("grantAccess: Only Cluster and ZONE scoped primary storage is supported. Storage Pool: " + storagePool.getName()); - } - - 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()); + 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"); 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 286131002eaa..cf5def5a3117 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 @@ -181,7 +181,7 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { throw new InvalidParameterValueException("attachCluster: dataStore should not be null"); } if (scope == null) { - throw new InvalidParameterValueException("attachCluster: clusterScope should not be null"); + throw new InvalidParameterValueException("attachCluster: scope should not be null"); } List hostsIdentifier = new ArrayList<>(); StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); @@ -189,52 +189,36 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { 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); + 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(String.format("Attaching the pool to each of the hosts %s in the cluster: %s", hostsToConnect, primarystore.getClusterId())); + logger.debug("attachCluster: Eligible Up and Enabled hosts: {} in cluster {}", hostsToConnect, primaryStore.getClusterId()); - Map details = primarystore.getDetails(); + Map details = primaryStore.getDetails(); StorageStrategy strategy = utils.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)) { - throw new CloudRuntimeException("Not all hosts in the cluster support the protocol: " + protocol.toString()); + 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 = utils.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); strategy.createAccessGroup(accessGroupRequest); } + logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: %s", primaryStore.getClusterId()); for (HostVO host : hostsToConnect) { 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); return true; } - private boolean isProtocolSupportedByAllHosts(List hosts, ProtocolType protocolType, List hostIdentifiers) { - String protocolPrefix; - switch (protocolType) { - case ISCSI: - 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("Unsupported protocol: " + protocolType.toString()); - } - return true; - } - @Override public boolean attachHost(DataStore store, HostScope scope, StoragePoolInfo existingInfo) { return false; @@ -247,30 +231,31 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper throw new InvalidParameterValueException("attachZone: dataStore should not be null"); } if (scope == null) { - throw new InvalidParameterValueException("attachZone: clusterScope should not be 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("attachCluster : Storage Pool not found for id: " + dataStore.getId()); + s_logger.error("attachZone : Storage Pool not found for id: " + dataStore.getId()); throw new CloudRuntimeException("attachCluster : 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(String.format("In createPool. Attaching the pool to each of the hosts in %s.", hostsToConnect)); + logger.debug("attachZone: Eligible Up and Enabled hosts: {}", hostsToConnect); Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); StorageStrategy strategy = utils.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)) { - throw new CloudRuntimeException("Not all hosts in the zone support the protocol: " + protocol.toString()); + 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 = utils.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) { @@ -281,6 +266,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/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index b6d836ba9ec9..84bcaf11543c 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 @@ -123,10 +123,10 @@ public CloudStackVolume getCloudStackVolume(Map values) { @Override public AccessGroup createAccessGroup(AccessGroup accessGroup) { - s_logger.info("createAccessGroup : Creating Igroup with accessGroup request {} ", accessGroup); + 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 Lun, invalid request"); + throw new CloudRuntimeException("createAccessGroup : Failed to create Igroup, invalid request"); } try { // Get AuthHeader @@ -176,7 +176,7 @@ public AccessGroup getAccessGroup(Map values) { URI url = utils.generateURI(Constants.CREATE_IGROUP); url = utils.addQueryParamsToURI(url, values); s_logger.info("getAccessGroup: URL: {}", url); - //TODO: It is possible that Lun creation will take time and we may need to handle through async job. + OntapResponse igroupResponse = sanFeignClient.getIgroupResponse(url, authHeader); if (igroupResponse == null || igroupResponse.getRecords() == null || igroupResponse.getRecords().size() == 0) { s_logger.error("getAccessGroup: Failed to fetch Igroup"); @@ -213,8 +213,8 @@ public void enableLogicalAccess(Map values) { URI url = utils.generateURI(Constants.CREATE_LUNMAP); OntapResponse createdLunMap = sanFeignClient.createLunMap(url, authHeader, true, lunMapRequest); if (createdLunMap == null || createdLunMap.getRecords() == null || createdLunMap.getRecords().size() == 0) { - s_logger.error("enableLogicalAccess: LunMap creation failed for Lun {} and igroup ", lunName, igroupName); - throw new CloudRuntimeException("Failed to create LunMap: creation failed for Lun: " +lunName+ "and igroup: " + igroupName); + 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); 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 2042c0ff4ff0..4b7f7ee1d162 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 @@ -47,6 +47,7 @@ public class Constants { public static final String PATH_SEPARATOR = "/"; public static final String UNDERSCORE = "_"; + public static final String CS = "cs"; public static final String VOLUME_PATH_PREFIX = "/vol/"; 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 167d83c097a1..5345174fab9c 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 @@ -171,10 +171,8 @@ public AccessGroup createAccessGroupRequestByProtocol(StoragePoolVO storagePool, String svmName = details.get(Constants.SVM_NAME); switch (protocol) { case ISCSI: - String storagePoolName = storagePool.getName(); - //TODO: Check if we have to change the access group name format - // Access group name format: StoragePoolName_ScopeId - String igroupName = getIgroupName(storagePoolName, scopeId); + // Access group name format: cs_svmName_scopeId + String igroupName = getIgroupName(svmName, scopeId); Hypervisor.HypervisorType hypervisorType = storagePool.getHypervisor(); return createSANAccessGroupRequest(svmName, igroupName, hypervisorType, hostsIdentifier); default: @@ -193,7 +191,7 @@ public AccessGroup createSANAccessGroupRequest(String svmName, String igroupName igroup.setSvm(svm); } - if(igroupName == null || igroupName.isEmpty()) { + if(igroupName != null || !igroupName.isEmpty()) { igroup.setName(igroupName); } @@ -211,7 +209,6 @@ public AccessGroup createSANAccessGroupRequest(String svmName, String igroupName } igroup.setInitiators(initiators); } - return accessGroupRequest; } @@ -220,8 +217,8 @@ public String getLunName(String volName, String lunName) { return Constants.VOLUME_PATH_PREFIX + volName + Constants.PATH_SEPARATOR + lunName; } - public String getIgroupName(String storagePoolName, long scopeId) { + public String getIgroupName(String svmName, long scopeId) { // Igroup name format: StoragePoolName_ScopeId - return storagePoolName + Constants.UNDERSCORE + scopeId; + return Constants.CS + Constants.UNDERSCORE + svmName + Constants.UNDERSCORE + scopeId; } } From a3a147db9aadbc0722e260383f25806055e1be38 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 31 Oct 2025 11:56:16 +0530 Subject: [PATCH 4/7] CSTACKEX-36 Fix Check Styling issue --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 2 +- .../storage/provider/OntapPrimaryDatastoreProvider.java | 4 ++-- .../apache/cloudstack/storage/service/StorageStrategy.java | 2 +- .../java/org/apache/cloudstack/storage/utils/Utility.java | 1 - 4 files changed, 4 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 063ceeb1bbbc..1103ac49e646 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 @@ -231,7 +231,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, } long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); - String igroupName = utils.getIgroupName(storagePool.getName(), scopeId); + String igroupName = utils.getIgroupName(svmName, scopeId); Map getAccessGroupMap = new HashMap<>(); getAccessGroupMap.put(Constants.NAME, igroupName); getAccessGroupMap.put(Constants.SVM_DOT_NAME, svmName); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java index 0240201b1057..fa2f14692c77 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java @@ -28,7 +28,7 @@ import org.apache.cloudstack.storage.driver.OntapPrimaryDatastoreDriver; import org.apache.cloudstack.storage.lifecycle.OntapPrimaryDatastoreLifecycle; import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.Logger; import org.springframework.stereotype.Component; import java.util.HashSet; @@ -38,7 +38,7 @@ @Component public class OntapPrimaryDatastoreProvider implements PrimaryDataStoreProvider { - private static final Logger s_logger = (Logger)LogManager.getLogger(OntapPrimaryDatastoreProvider.class); + private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreProvider.class); private OntapPrimaryDatastoreDriver primaryDatastoreDriver; private OntapPrimaryDatastoreLifecycle primaryDatastoreLifecycle; 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 1f8b904f384a..51c9c35c5ebe 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 @@ -71,7 +71,7 @@ public abstract class StorageStrategy { */ private List aggregates; - private static final Logger s_logger = (Logger) LogManager.getLogger(StorageStrategy.class); + private static final Logger s_logger = LogManager.getLogger(StorageStrategy.class); public StorageStrategy(OntapStorage ontapStorage) { storage = ontapStorage; 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 5345174fab9c..6a4ce8036fd3 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 @@ -48,7 +48,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Set; @Component public class Utility { From def2cff00dff50a44337c26e80cf41adda9e5dc7 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 31 Oct 2025 15:31:24 +0530 Subject: [PATCH 5/7] CSTACKEX-36 Revoke Access --- .../driver/OntapPrimaryDatastoreDriver.java | 101 ++++++++++++++---- .../storage/service/StorageStrategy.java | 2 +- .../storage/service/UnifiedNASStrategy.java | 2 +- .../storage/service/UnifiedSANStrategy.java | 21 +++- .../cloudstack/storage/utils/Constants.java | 6 +- 5 files changed, 105 insertions(+), 27 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 1103ac49e646..1e82fd50c68a 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 @@ -219,43 +219,100 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); StorageStrategy storageStrategy = utils.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))) { - Map getCloudStackVolumeMap = new HashMap<>(); - getCloudStackVolumeMap.put(Constants.NAME, volumeVO.getPath()); - 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("grantAccess: Failed to get LUN details [{}]", volumeVO.getName()); - throw new CloudRuntimeException("grantAccess: Failed to get LUN [" + volumeVO.getName() + "]"); - } - - long scopeId = (storagePool.getScope() == ScopeType.CLUSTER) ? host.getClusterId() : host.getDataCenterId(); - String igroupName = utils.getIgroupName(svmName, scopeId); - Map getAccessGroupMap = new HashMap<>(); - getAccessGroupMap.put(Constants.NAME, igroupName); - 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("grantAccess: Failed to get iGroup details for host [{}]", host.getName()); - throw new CloudRuntimeException("grantAccess: Failed to get iGroup details for host [" + host.getName() + "]"); - } + String accessGroupName = utils.getIgroupName(svmName, scopeId); + CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); + AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); if(!accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) { - s_logger.error("grantAccess: initiator [{}] is not present in iGroup [{}]", host.getStorageUrl(), igroupName); - throw new CloudRuntimeException("grantAccess: initiator [" + host.getStorageUrl() + "] is not present in iGroup [" + igroupName); + 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, igroupName); + enableLogicalAccessMap.put(Constants.IGROUP_DOT_NAME, accessGroupName); storageStrategy.enableLogicalAccess(enableLogicalAccessMap); } } @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 (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 grantAccess"); + throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); + } + } 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 = utils.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 = utils.getIgroupName(svmName, scopeId); + CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); + AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); + + Map enableLogicalAccessMap = new HashMap<>(); + enableLogicalAccessMap.put(Constants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid().toString()); + enableLogicalAccessMap.put(Constants.IGROUP_DOT_UUID, accessGroup.getIgroup().getUuid()); + storageStrategy.disableLogicalAccess(enableLogicalAccessMap); + } + } + + 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("revokeAccessForVolume: Failed to get LUN details [{}]", cloudStackVolumeName); + throw new CloudRuntimeException("revokeAccessForVolume: 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("revokeAccessForVolume: Failed to get iGroup details [{}]", accessGroupName); + throw new CloudRuntimeException("revokeAccessForVolume: Failed to get iGroup details [" + accessGroupName + "]"); + } + return accessGroup; } @Override diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index 51c9c35c5ebe..1e66c2adfb69 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 @@ -322,6 +322,6 @@ 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 1ddf62b35e7f..fcc5d6c3c599 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 @@ -82,7 +82,7 @@ public void enableLogicalAccess(Map values) { } @Override - void disableLogicalAccess(Map values) { + public void disableLogicalAccess(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 84bcaf11543c..1c0fdec39947 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 @@ -226,7 +226,24 @@ public void enableLogicalAccess(Map values) { } @Override - void disableLogicalAccess(Map values) { - + 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 = utils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // URI for Igroup delete + URI url = utils.generateURI(Constants.CREATE_LUNMAP); + sanFeignClient.deleteLunMap(url, 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 4b7f7ee1d162..a59dc8d9ad5a 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 @@ -33,7 +33,11 @@ public class Constants { public static final String SVM_DOT_NAME = "svm.name"; public static final String LUN_DOT_NAME = "lun.name"; + + 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 NAME = "name"; public static final String JOB_RUNNING = "running"; @@ -45,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 UNDERSCORE = "_"; public static final String CS = "cs"; From da8a7f2317f6dec1ae02f3b9e6f60e919fb0ffd1 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 3 Nov 2025 21:50:30 +0530 Subject: [PATCH 6/7] CSTACKEX-36 Revoke Access --- .../driver/OntapPrimaryDatastoreDriver.java | 29 ++++++++++++------- .../cloudstack/storage/utils/Utility.java | 4 +-- 2 files changed, 21 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 1e82fd50c68a..f2528819e591 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 @@ -193,8 +193,8 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore 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()); + 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) { @@ -255,6 +255,10 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) 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()); @@ -283,11 +287,16 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, String accessGroupName = utils.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 enableLogicalAccessMap = new HashMap<>(); - enableLogicalAccessMap.put(Constants.LUN_DOT_UUID, cloudStackVolume.getLun().getUuid().toString()); - enableLogicalAccessMap.put(Constants.IGROUP_DOT_UUID, accessGroup.getIgroup().getUuid()); - storageStrategy.disableLogicalAccess(enableLogicalAccessMap); + 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); } } @@ -297,8 +306,8 @@ private CloudStackVolume getCloudStackVolumeByName(StorageStrategy storageStrate getCloudStackVolumeMap.put(Constants.SVM_DOT_NAME, svmName); CloudStackVolume cloudStackVolume = storageStrategy.getCloudStackVolume(getCloudStackVolumeMap); if(cloudStackVolume == null ||cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getName() == null) { - s_logger.error("revokeAccessForVolume: Failed to get LUN details [{}]", cloudStackVolumeName); - throw new CloudRuntimeException("revokeAccessForVolume: Failed to get LUN [" + cloudStackVolumeName + "]"); + s_logger.error("getCloudStackVolumeByName: Failed to get LUN details [{}]", cloudStackVolumeName); + throw new CloudRuntimeException("getCloudStackVolumeByName: Failed to get LUN [" + cloudStackVolumeName + "]"); } return cloudStackVolume; } @@ -309,8 +318,8 @@ private AccessGroup getAccessGroupByName(StorageStrategy storageStrategy, String getAccessGroupMap.put(Constants.SVM_DOT_NAME, svmName); AccessGroup accessGroup = storageStrategy.getAccessGroup(getAccessGroupMap); if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getName() == null) { - s_logger.error("revokeAccessForVolume: Failed to get iGroup details [{}]", accessGroupName); - throw new CloudRuntimeException("revokeAccessForVolume: Failed to get iGroup details [" + accessGroupName + "]"); + s_logger.error("getAccessGroupByName: Failed to get iGroup details [{}]", accessGroupName); + throw new CloudRuntimeException("getAccessGroupByName: Failed to get iGroup details [" + accessGroupName + "]"); } return accessGroup; } 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 6a4ce8036fd3..e752df263e91 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 @@ -213,11 +213,11 @@ public AccessGroup createSANAccessGroupRequest(String svmName, String igroupName public String getLunName(String volName, String lunName) { //Lun name in unified "/vol/VolumeName/LunName" - return Constants.VOLUME_PATH_PREFIX + volName + Constants.PATH_SEPARATOR + lunName; + return Constants.VOLUME_PATH_PREFIX + volName + Constants.SLASH + lunName; } public String getIgroupName(String svmName, long scopeId) { - // Igroup name format: StoragePoolName_ScopeId + // Igroup name format: cs_svmName_scopeId return Constants.CS + Constants.UNDERSCORE + svmName + Constants.UNDERSCORE + scopeId; } } From 4f6092fa453769e3b0bcbf94e365f565b5620a9f Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 3 Nov 2025 22:19:40 +0530 Subject: [PATCH 7/7] CSTACKEX-36 Resolved Review Comments --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 8 ++++---- .../lifecycle/OntapPrimaryDatastoreLifecycle.java | 4 ++-- .../storage/service/UnifiedSANStrategy.java | 4 ++-- .../org/apache/cloudstack/storage/utils/Utility.java | 11 ++++++----- 4 files changed, 14 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 f2528819e591..288243f9e26c 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 @@ -225,7 +225,7 @@ private void grantAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, String accessGroupName = utils.getIgroupName(svmName, scopeId); CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, volumeVO.getPath()); AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); - if(!accessGroup.getIgroup().getInitiators().contains(host.getStorageUrl())) { + 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); } @@ -268,8 +268,8 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) } revokeAccessForVolume(storagePool, volumeVO, host); } else { - s_logger.error("revokeAccess: Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); - throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); + 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()); @@ -305,7 +305,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 cf5def5a3117..0df3f02c829b 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 @@ -207,7 +207,7 @@ public boolean attachCluster(DataStore dataStore, ClusterScope scope) { AccessGroup accessGroupRequest = utils.createAccessGroupRequestByProtocol(storagePool, scope.getScopeId(), details, hostsIdentifier); strategy.createAccessGroup(accessGroupRequest); } - logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: %s", primaryStore.getClusterId()); + logger.debug("attachCluster: Attaching the pool to each of the host in the cluster: {}", primaryStore.getClusterId()); for (HostVO host : hostsToConnect) { try { _storageMgr.connectHostToSharedPool(host, dataStore.getId()); @@ -237,7 +237,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); if(storagePool == null) { s_logger.error("attachZone : Storage Pool not found for id: " + dataStore.getId()); - throw new CloudRuntimeException("attachCluster : 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 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 1c0fdec39947..05d56666afa3 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 @@ -213,8 +213,8 @@ public void enableLogicalAccess(Map values) { URI url = utils.generateURI(Constants.CREATE_LUNMAP); OntapResponse createdLunMap = sanFeignClient.createLunMap(url, 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); + 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); 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 e752df263e91..d999eec12877 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 @@ -108,7 +108,7 @@ public CloudStackVolume createSANCloudStackVolumeRequest(String svmName, String CloudStackVolume cloudStackVolumeRequest = new CloudStackVolume(); Lun lunRequest = new Lun(); - if(svmName != null || !svmName.isEmpty()) { + if(svmName != null && !svmName.isEmpty()) { Svm svm = new Svm(); svm.setName(svmName); lunRequest.setSvm(svm); @@ -121,7 +121,7 @@ public CloudStackVolume createSANCloudStackVolumeRequest(String svmName, String } //Lun name is full path like in unified "/vol/VolumeName/LunName" - if(lunName != null || !lunName.isEmpty()) { + if(lunName != null && !lunName.isEmpty()) { String lunFullName = getLunName(volName, lunName); lunRequest.setName(lunFullName); } @@ -139,7 +139,7 @@ public String getOSTypeFromHypervisor(String hypervisorType){ case Constants.KVM: return Lun.OsTypeEnum.LINUX.getValue(); default: - String errMsg = "getOStypeFromHypervisorType : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage"; + String errMsg = "getOSTypeFromHypervisor : Unsupported hypervisor type " + hypervisorType + " for ONTAP storage"; s_logger.error(errMsg); throw new CloudRuntimeException(errMsg); } @@ -184,13 +184,13 @@ public AccessGroup createSANAccessGroupRequest(String svmName, String igroupName AccessGroup accessGroupRequest = new AccessGroup(); Igroup igroup = new Igroup(); - if(svmName != null || !svmName.isEmpty()) { + if(svmName != null && !svmName.isEmpty()) { Svm svm = new Svm(); svm.setName(svmName); igroup.setSvm(svm); } - if(igroupName != null || !igroupName.isEmpty()) { + if(igroupName != null && !igroupName.isEmpty()) { igroup.setName(igroupName); } @@ -208,6 +208,7 @@ public AccessGroup createSANAccessGroupRequest(String svmName, String igroupName } igroup.setInitiators(initiators); } + accessGroupRequest.setIgroup(igroup); return accessGroupRequest; }