From 0cf8d60693d5e5f5e69012b9738bb30a330cd80f Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 29 Oct 2025 11:52:26 +0530 Subject: [PATCH 1/4] CSTACKEX-35 Create Async --- .../driver/OntapPrimaryDatastoreDriver.java | 75 ++++++++++++++++++- .../storage/service/StorageStrategy.java | 2 +- .../storage/service/UnifiedSANStrategy.java | 46 +++++++++++- .../cloudstack/storage/utils/Constants.java | 8 ++ .../cloudstack/storage/utils/Utility.java | 75 +++++++++++++++++-- 5 files changed, 194 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 3310064406fd..fe4e25d99f66 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 @@ -18,13 +18,17 @@ */ package org.apache.cloudstack.storage.driver; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.to.DataObjectType; import com.cloud.agent.api.to.DataStoreTO; import com.cloud.agent.api.to.DataTO; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.host.Host; import com.cloud.storage.Storage; import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; @@ -37,15 +41,27 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.command.CommandResult; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +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.CloudStackVolume; +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.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import javax.inject.Inject; import java.util.HashMap; import java.util.Map; public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { private static final Logger s_logger = (Logger)LogManager.getLogger(OntapPrimaryDatastoreDriver.class); + + @Inject private Utility utils; + @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @Override public Map getCapabilities() { s_logger.trace("OntapPrimaryDatastoreDriver: getCapabilities: Called"); @@ -68,9 +84,64 @@ public DataStoreTO getStoreTO(DataStore store) { } @Override - public void createAsync(DataStore store, DataObject data, AsyncCompletionCallback callback) { + public void createAsync(DataStore dataStore, DataObject dataObject, AsyncCompletionCallback callback) { + CreateCmdResult createCmdResult = null; + String path = null; + String errMsg = null; + if (dataStore == null) { + throw new InvalidParameterValueException("createAsync: dataStore should not be null"); + } + if (dataObject == null) { + throw new InvalidParameterValueException("createAsync: dataObject should not be null"); + } + if (callback == null) { + throw new InvalidParameterValueException("createAsync: callback should not be null"); + } + try { + s_logger.info("createAsync: Volume creation starting for data store [{}] and data object [{}] of type [{}]", + dataStore, dataObject, dataObject.getType()); + if (dataObject.getType() == DataObjectType.VOLUME) { + path = createCloudStackVolumeForTypeVolume(dataStore, dataObject); + createCmdResult = new CreateCmdResult(path, new Answer(null, true, null)); + } else { + errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync"; + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + } catch (Exception e) { + errMsg = e.getMessage(); + s_logger.error("createAsync: Volume creation failed for dataObject [{}]: {}", dataObject, errMsg); + createCmdResult = new CreateCmdResult(null, new Answer(null, false, errMsg)); + createCmdResult.setResult(e.toString()); + } finally { + callback.complete(createCmdResult); + } + } - s_logger.trace("OntapPrimaryDatastoreDriver: createAsync: Store: "+store+", data: "+data); + private String createCloudStackVolumeForTypeVolume(DataStore dataStore, DataObject dataObject) { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); + String protocol = details.get(Constants.PROTOCOL); + OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD), + details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol), + Boolean.parseBoolean(details.get(Constants.IS_DISAGGREGATED))); + StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage); + boolean isValid = storageStrategy.connect(); + if (isValid) { + s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME)); + CloudStackVolume cloudStackVolumeRequest = utils.createCloudStackVolumeRequestByProtocol(dataStore.getId(), details, dataObject); + CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); + if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) { + return cloudStackVolume.getLun().getName(); + } else { + String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + dataObject; + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + } else { + String errMsg = "createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"; + s_logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } } @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 dde5ab5b7a44..e3b15e81f210 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 @@ -64,7 +64,7 @@ public abstract class StorageStrategy { @Inject private JobFeignClient jobFeignClient; - private final OntapStorage storage; + protected final OntapStorage storage; /** * Presents aggregate object for the unified storage, not eligible for disaggregated 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 7b654d8d0e98..0ab601c04be4 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 @@ -19,21 +19,61 @@ package org.apache.cloudstack.storage.service; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.storage.feign.client.SANFeignClient; +import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.LunSpace; import org.apache.cloudstack.storage.feign.model.OntapStorage; +import org.apache.cloudstack.storage.feign.model.Svm; +import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; +import org.apache.cloudstack.storage.utils.Constants; +import org.apache.cloudstack.storage.utils.Utility; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import javax.inject.Inject; +import java.net.URI; import java.util.Map; -public class UnifiedSANStrategy extends SANStrategy{ +public class UnifiedSANStrategy extends SANStrategy { + + private static final Logger s_logger = (Logger) LogManager.getLogger(UnifiedSANStrategy.class); + @Inject private Utility utils; + @Inject private SANFeignClient sanFeignClient; public UnifiedSANStrategy(OntapStorage ontapStorage) { super(ontapStorage); } @Override public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume) { - //TODO - return null; + s_logger.info("createCloudStackVolume : Creating Lun with cloudstackVolume request {} ", cloudstackVolume); + if (cloudstackVolume == null || cloudstackVolume.getLun() == null) { + s_logger.error("createCloudStackVolume: LUN creation failed. Invalid cloudstackVolume request: {}", cloudstackVolume); + throw new CloudRuntimeException("createCloudStackVolume : Failed to create Lun, invalid cloudstackVolume request"); + } + try { + // Get AuthHeader + String authHeader = utils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + // Create URI for lun creation + URI url = utils.generateURI(Constants.CREATE_LUN); + OntapResponse createdLun = sanFeignClient.createLun(url, authHeader, true, cloudstackVolume.getLun()); + if (createdLun == null || createdLun.getRecords() == null || createdLun.getRecords().size() == 0) { + s_logger.error("createCloudStackVolume: LUN creation failed for Lun {}", cloudstackVolume.getLun().getName()); + throw new CloudRuntimeException("Failed to create Lun: " + cloudstackVolume.getLun().getName()); + } + Lun lun = createdLun.getRecords().get(0); + s_logger.debug("createCloudStackVolume: LUN created successfully. Lun: {}", lun); + s_logger.info("createCloudStackVolume: LUN created successfully. LunName: {}", lun.getName()); + + CloudStackVolume createdCloudStackVolume = new CloudStackVolume(); + createdCloudStackVolume.setLun(lun); + return createdCloudStackVolume; + } catch (Exception e) { + s_logger.error("Exception occurred while creating LUN: {}. Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); + throw new CloudRuntimeException("Failed to create Lun: " + e.getMessage()); + } } @Override 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 a5b1a45bbb74..61674db7c8b0 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 @@ -40,8 +40,16 @@ 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 VOLUME_PATH_PREFIX = "/vol/"; + + public static final String KVM = "KVM"; + 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"; + } 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 6fcf155e27b5..ba6c3de53379 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java @@ -20,33 +20,96 @@ package org.apache.cloudstack.storage.utils; 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.driver.OntapPrimaryDatastoreDriver; +import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.LunSpace; import org.apache.cloudstack.storage.feign.model.OntapStorage; +import org.apache.cloudstack.storage.feign.model.Svm; +import org.apache.cloudstack.storage.provider.StorageProviderFactory; +import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.service.model.CloudStackVolume; +import org.apache.cloudstack.storage.service.model.ProtocolType; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.springframework.stereotype.Component; import org.springframework.util.Base64Utils; import javax.inject.Inject; import java.net.URI; +import java.util.Map; @Component public class Utility { - @Inject - OntapStorage ontapStorage; + + private static final Logger s_logger = (Logger) LogManager.getLogger(Utility.class); + @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 = ":"; + /** * Method generates authentication headers using storage backend credentials passed as normal string - * @param username -->> username of the storage backend - * @param password -->> normal decoded password of the storage backend + * + * @param username -->> username of the storage backend + * @param password -->> normal decoded password of the storage backend * @return */ - public String generateAuthHeader(String username, String password) { + public String generateAuthHeader (String username, String password) { byte[] encodedBytes = Base64Utils.encode((username + AUTH_HEADER_COLON + password).getBytes()); return BASIC + StringUtils.SPACE + new String(encodedBytes); } - public URI generateURI(String path) { + public URI generateURI (String path) { String uriString = Constants.HTTPS + ontapStorage.getManagementLIF() + path; return URI.create(uriString); } + + public CloudStackVolume createCloudStackVolumeRequestByProtocol(Long storagePoolId, Map details, DataObject dataObject) { + StoragePoolVO storagePool = storagePoolDao.findById(storagePoolId); + if(storagePool == null) { + throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + storagePoolId); + } + 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); + + 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); + } + } } From c968cccc58b1aa371c78200def63adf084f7acc3 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 29 Oct 2025 12:20:15 +0530 Subject: [PATCH 2/4] CSTACKEX-35 Added Null and empty check --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 12 +++++++++++- .../storage/service/UnifiedSANStrategy.java | 2 -- .../org/apache/cloudstack/storage/utils/Utility.java | 9 +-------- 3 files changed, 12 insertions(+), 11 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 fe4e25d99f66..6cc69c57bb3c 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 @@ -41,7 +41,9 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.command.CommandResult; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.provider.StorageProviderFactory; import org.apache.cloudstack.storage.service.StorageStrategy; @@ -62,6 +64,7 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { @Inject private Utility utils; @Inject private StoragePoolDetailsDao storagePoolDetailsDao; + @Inject private PrimaryDataStoreDao storagePoolDao; @Override public Map getCapabilities() { s_logger.trace("OntapPrimaryDatastoreDriver: getCapabilities: Called"); @@ -119,7 +122,14 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } private String createCloudStackVolumeForTypeVolume(DataStore dataStore, DataObject dataObject) { + StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId()); + if(storagePool == null) { + throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); + } Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); + if(details == null || details.isEmpty()) { + throw new CloudRuntimeException("createCloudStackVolume : Storage Details not found for id: " + dataStore.getId()); + } String protocol = details.get(Constants.PROTOCOL); OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD), details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol), @@ -128,7 +138,7 @@ private String createCloudStackVolumeForTypeVolume(DataStore dataStore, DataObje boolean isValid = storageStrategy.connect(); if (isValid) { s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME)); - CloudStackVolume cloudStackVolumeRequest = utils.createCloudStackVolumeRequestByProtocol(dataStore.getId(), details, dataObject); + CloudStackVolume cloudStackVolumeRequest = utils.createCloudStackVolumeRequestByProtocol(storagePool, details, dataObject); CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) { return cloudStackVolume.getLun().getName(); 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 0ab601c04be4..e489d837fef4 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -22,9 +22,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.LunSpace; import org.apache.cloudstack.storage.feign.model.OntapStorage; -import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/Utility.java index ba6c3de53379..8b7edfd3b009 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 @@ -25,13 +25,10 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.driver.OntapPrimaryDatastoreDriver; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunSpace; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Svm; -import org.apache.cloudstack.storage.provider.StorageProviderFactory; -import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.logging.log4j.LogManager; @@ -71,11 +68,7 @@ public URI generateURI (String path) { return URI.create(uriString); } - public CloudStackVolume createCloudStackVolumeRequestByProtocol(Long storagePoolId, Map details, DataObject dataObject) { - StoragePoolVO storagePool = storagePoolDao.findById(storagePoolId); - if(storagePool == null) { - throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + storagePoolId); - } + public CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map details, DataObject dataObject) { CloudStackVolume cloudStackVolumeRequest = null; String protocol = details.get(Constants.PROTOCOL); From cc6af4af3a4867da44794d468d3443177ab40974 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 29 Oct 2025 14:34:22 +0530 Subject: [PATCH 3/4] CSTACKEX-35 Resolved review comments --- .../driver/OntapPrimaryDatastoreDriver.java | 33 +++++++------------ .../storage/service/UnifiedSANStrategy.java | 5 +-- .../cloudstack/storage/utils/Utility.java | 20 ++++++++++- 3 files changed, 33 insertions(+), 25 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 6cc69c57bb3c..becf50106663 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 @@ -44,8 +44,6 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.model.OntapStorage; -import org.apache.cloudstack.storage.provider.StorageProviderFactory; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; @@ -101,7 +99,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet throw new InvalidParameterValueException("createAsync: callback should not be null"); } try { - s_logger.info("createAsync: Volume creation starting for data store [{}] and data object [{}] of type [{}]", + s_logger.info("createAsync: Started for data store [{}] and data object [{}] of type [{}]", dataStore, dataObject, dataObject.getType()); if (dataObject.getType() == DataObjectType.VOLUME) { path = createCloudStackVolumeForTypeVolume(dataStore, dataObject); @@ -113,7 +111,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } } catch (Exception e) { errMsg = e.getMessage(); - s_logger.error("createAsync: Volume creation failed for dataObject [{}]: {}", dataObject, errMsg); + s_logger.error("createAsync: Failed for dataObject [{}]: {}", dataObject, errMsg); createCmdResult = new CreateCmdResult(null, new Answer(null, false, errMsg)); createCmdResult.setResult(e.toString()); } finally { @@ -124,31 +122,22 @@ 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()); if(details == null || details.isEmpty()) { + s_logger.error("createCloudStackVolume : Storage Details not found for id: " + dataStore.getId()); throw new CloudRuntimeException("createCloudStackVolume : Storage Details not found for id: " + dataStore.getId()); } - String protocol = details.get(Constants.PROTOCOL); - OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD), - details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol), - Boolean.parseBoolean(details.get(Constants.IS_DISAGGREGATED))); - StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage); - boolean isValid = storageStrategy.connect(); - if (isValid) { - s_logger.info("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [{}] successful, preparing CloudStackVolumeRequest", details.get(Constants.SVM_NAME)); - CloudStackVolume cloudStackVolumeRequest = utils.createCloudStackVolumeRequestByProtocol(storagePool, details, dataObject); - CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); - if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) { - return cloudStackVolume.getLun().getName(); - } else { - String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + dataObject; - s_logger.error(errMsg); - throw new CloudRuntimeException(errMsg); - } + 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); + CloudStackVolume cloudStackVolume = storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); + if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(Constants.PROTOCOL)) && cloudStackVolume.getLun() != null && cloudStackVolume.getLun().getName() != null) { + return cloudStackVolume.getLun().getName(); } else { - String errMsg = "createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"; + String errMsg = "createCloudStackVolumeForTypeVolume: Volume creation failed. Lun or Lun Path is null for dataObject: " + dataObject; s_logger.error(errMsg); throw new CloudRuntimeException(errMsg); } 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 e489d837fef4..9aaebdbd5e8b 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 @@ -48,14 +48,15 @@ public UnifiedSANStrategy(OntapStorage ontapStorage) { public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume) { s_logger.info("createCloudStackVolume : Creating Lun with cloudstackVolume request {} ", cloudstackVolume); if (cloudstackVolume == null || cloudstackVolume.getLun() == null) { - s_logger.error("createCloudStackVolume: LUN creation failed. Invalid cloudstackVolume request: {}", cloudstackVolume); - throw new CloudRuntimeException("createCloudStackVolume : Failed to create Lun, invalid cloudstackVolume request"); + s_logger.error("createCloudStackVolume: LUN creation failed. Invalid request: {}", cloudstackVolume); + throw new CloudRuntimeException("createCloudStackVolume : Failed to create Lun, 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); + //TODO: there is possible that Lun creation will take time and we may need to handle through async job. OntapResponse createdLun = sanFeignClient.createLun(url, authHeader, true, cloudstackVolume.getLun()); if (createdLun == null || createdLun.getRecords() == null || createdLun.getRecords().size() == 0) { s_logger.error("createCloudStackVolume: LUN creation failed for Lun {}", cloudstackVolume.getLun().getName()); 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 8b7edfd3b009..f1901d2de3d1 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 @@ -29,6 +29,8 @@ import org.apache.cloudstack.storage.feign.model.LunSpace; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Svm; +import org.apache.cloudstack.storage.provider.StorageProviderFactory; +import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.logging.log4j.LogManager; @@ -82,7 +84,7 @@ public CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO st 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); @@ -105,4 +107,20 @@ public CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO st throw new CloudRuntimeException("createCloudStackVolumeRequestByProtocol: Unsupported protocol " + protocol); } } + + public StorageStrategy getStrategyByStoragePoolDetails(Map details) { + String protocol = details.get(Constants.PROTOCOL); + OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD), + details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol), + Boolean.parseBoolean(details.get(Constants.IS_DISAGGREGATED))); + StorageStrategy storageStrategy = StorageProviderFactory.getStrategy(ontapStorage); + boolean isValid = storageStrategy.connect(); + if (isValid) { + s_logger.info("Connection to Ontap SVM [{}] successful", details.get(Constants.SVM_NAME)); + return storageStrategy; + } else { + s_logger.error("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); + throw new CloudRuntimeException("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); + } + } } From 199ac0fb50e45f050b07d56d833eba7a8fe64ae3 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 29 Oct 2025 15:00:05 +0530 Subject: [PATCH 4/4] CSTACKEX-35 Removed Type Casting for logger --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 6 +----- .../cloudstack/storage/service/UnifiedSANStrategy.java | 4 ++-- .../org/apache/cloudstack/storage/utils/Utility.java | 10 +++++++--- 3 files changed, 10 insertions(+), 10 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 becf50106663..6d850a97168f 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 @@ -58,7 +58,7 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { - private static final Logger s_logger = (Logger)LogManager.getLogger(OntapPrimaryDatastoreDriver.class); + private static final Logger s_logger = LogManager.getLogger(OntapPrimaryDatastoreDriver.class); @Inject private Utility utils; @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @@ -126,10 +126,6 @@ private String createCloudStackVolumeForTypeVolume(DataStore dataStore, DataObje throw new CloudRuntimeException("createCloudStackVolume : Storage Pool not found for id: " + dataStore.getId()); } Map details = storagePoolDetailsDao.listDetailsKeyPairs(dataStore.getId()); - if(details == null || details.isEmpty()) { - s_logger.error("createCloudStackVolume : Storage Details not found for id: " + dataStore.getId()); - throw new CloudRuntimeException("createCloudStackVolume : Storage Details not found for id: " + dataStore.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); 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 9aaebdbd5e8b..e0f1c16e788f 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 @@ -37,7 +37,7 @@ public class UnifiedSANStrategy extends SANStrategy { - private static final Logger s_logger = (Logger) LogManager.getLogger(UnifiedSANStrategy.class); + private static final Logger s_logger = LogManager.getLogger(UnifiedSANStrategy.class); @Inject private Utility utils; @Inject private SANFeignClient sanFeignClient; public UnifiedSANStrategy(OntapStorage ontapStorage) { @@ -56,7 +56,7 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume String authHeader = utils.generateAuthHeader(storage.getUsername(), storage.getPassword()); // Create URI for lun creation URI url = utils.generateURI(Constants.CREATE_LUN); - //TODO: there is possible that Lun creation will take time and we may need to handle through async job. + //TODO: It is possible that Lun creation will take time and we may need to handle through async job. OntapResponse createdLun = sanFeignClient.createLun(url, authHeader, true, cloudstackVolume.getLun()); if (createdLun == null || createdLun.getRecords() == null || createdLun.getRecords().size() == 0) { s_logger.error("createCloudStackVolume: LUN creation failed for Lun {}", cloudstackVolume.getLun().getName()); 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 f1901d2de3d1..c1fc47aef90b 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 @@ -45,7 +45,7 @@ @Component public class Utility { - private static final Logger s_logger = (Logger) LogManager.getLogger(Utility.class); + private static final Logger s_logger = LogManager.getLogger(Utility.class); @Inject private OntapStorage ontapStorage; @Inject private PrimaryDataStoreDao storagePoolDao; @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @@ -109,6 +109,10 @@ public CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO st } 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"); + } String protocol = details.get(Constants.PROTOCOL); OntapStorage ontapStorage = new OntapStorage(details.get(Constants.USERNAME), details.get(Constants.PASSWORD), details.get(Constants.MANAGEMENT_LIF), details.get(Constants.SVM_NAME), ProtocolType.valueOf(protocol), @@ -119,8 +123,8 @@ public StorageStrategy getStrategyByStoragePoolDetails(Map detai s_logger.info("Connection to Ontap SVM [{}] successful", details.get(Constants.SVM_NAME)); return storageStrategy; } else { - s_logger.error("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); - throw new CloudRuntimeException("createCloudStackVolumeForTypeVolume: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); + s_logger.error("getStrategyByStoragePoolDetails: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); + throw new CloudRuntimeException("getStrategyByStoragePoolDetails: Connection to Ontap SVM [" + details.get(Constants.SVM_NAME) + "] failed"); } } }