From 7ff499770465b00d9cbb41763b76bec8d37ee64f Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Fri, 21 Aug 2026 12:32:34 +0530 Subject: [PATCH 01/15] CSTACKEX-259: Local template for the VM image in primary while creating a VM instances --- .../driver/OntapPrimaryDatastoreDriver.java | 698 +++++++++++++++--- .../storage/feign/client/NASFeignClient.java | 11 + .../storage/feign/client/SANFeignClient.java | 4 +- .../storage/feign/model/FileCloneRequest.java | 132 ++++ .../storage/service/StorageStrategy.java | 20 +- .../storage/service/UnifiedNASStrategy.java | 294 +++++++- .../storage/service/UnifiedSANStrategy.java | 76 +- .../storage/utils/OntapStorageConstants.java | 11 + .../storage/utils/OntapStorageUtils.java | 176 +++-- .../OntapPrimaryDatastoreDriverTest.java | 448 ++++++++++- .../storage/service/StorageStrategyTest.java | 6 +- .../service/UnifiedNASStrategyTest.java | 457 ++++++++++++ .../service/UnifiedSANStrategyTest.java | 71 +- 13 files changed, 2174 insertions(+), 230 deletions(-) create mode 100644 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index ece29f7cd0ac..b9eaeb2d5424 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 @@ -32,8 +32,12 @@ import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.ScopeType; +import com.cloud.storage.SnapshotVO; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.SnapshotDetailsDao; import com.cloud.storage.dao.SnapshotDetailsVO; +import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; import com.cloud.utils.Pair; @@ -44,6 +48,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; @@ -55,19 +60,22 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; +import org.apache.cloudstack.storage.feign.model.FileInfo; import org.apache.cloudstack.storage.feign.model.FlexVolSnapshot; import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.LunSpace; +import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.SANStrategy; import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.service.UnifiedNASStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.utils.OntapStorageUtils; -import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; @@ -91,6 +99,8 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver { @Inject private VolumeDao volumeDao; @Inject private VolumeDetailsDao volumeDetailsDao; @Inject private SnapshotDetailsDao snapshotDetailsDao; + @Inject private SnapshotDao snapshotDao; + @Inject private VMTemplatePoolDao vmTemplatePoolDao; @Override public Map getCapabilities() { @@ -98,6 +108,10 @@ public Map getCapabilities() { Map mapCapabilities = new HashMap<>(); mapCapabilities.put(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString(), Boolean.TRUE.toString()); mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_SNAPSHOT.toString(), Boolean.TRUE.toString()); + mapCapabilities.put(DataStoreCapabilities.CAN_REVERT_VOLUME_TO_SNAPSHOT.toString(), Boolean.TRUE.toString()); + // Enables the framework to cache a template on the FlexVolume once and serve every later + // deployment with an array-side clone instead of another copy from secondary storage. + mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_VOLUME.toString(), Boolean.TRUE.toString()); return mapCapabilities; } @@ -151,11 +165,17 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet // Update CloudStack volume record with storage pool association and protocol-specific details VolumeVO volumeVO = volumeDao.findById(volInfo.getId()); if (volumeVO != null) { - // Create the backend storage object (LUN for iSCSI, no-op for NFS) - CloudStackVolume created = createCloudStackVolume(storagePool, volInfo, details); + // Create the backend storage object: a clone of the cached template when the + // orchestrator asked for one, otherwise a blank LUN (iSCSI) or qcow2 file (NFS). + Long cloneOfTemplateId = getTemplateIdForCloning(volInfo.getId()); + CloudStackVolume created = cloneOfTemplateId != null + ? cloneCloudStackVolumeFromTemplate(storagePool, volInfo, details, cloneOfTemplateId) + : createCloudStackVolume(storagePool, volInfo, details); volumeVO.setPoolType(storagePool.getPoolType()); volumeVO.setPoolId(storagePool.getId()); + volumeVO.setFormat(getImageFormatByHypervisor(storagePool.getHypervisor())); + logger.info("createAsync: Volume format set to [{}] for hypervisor [{}]", volumeVO.getFormat(), storagePool.getHypervisor()); if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null; @@ -180,6 +200,8 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } volumeDao.update(volumeVO.getId(), volumeVO); } + } else if (dataObject.getType() == DataObjectType.TEMPLATE) { + createCmdResult = createTemplateOnPrimary(storagePool, (TemplateInfo) dataObject, details); } else { errMsg = "Invalid DataObjectType (" + dataObject.getType() + ") passed to createAsync"; logger.error(errMsg); @@ -203,15 +225,196 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet */ private CloudStackVolume createCloudStackVolume(StoragePoolVO storagePool, VolumeInfo volumeObject, Map details) { StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); - CloudStackVolume cloudStackVolumeRequest = OntapStorageUtils.createCloudStackVolumeRequestByProtocol(storagePool, details, volumeObject); - return storageStrategy.createCloudStackVolume(cloudStackVolumeRequest); + return storageStrategy.createCloudStackVolume(createVolumeRequest(storagePool, details, volumeObject)); + } + + /** + * Creates the backend object that caches a template on this pool's FlexVolume. + * + *

This is the first half of the cache-and-clone flow driven by + * {@code VolumeServiceImpl.createManagedStorageVolumeFromTemplateAsync}. Only the empty + * container is created here; the framework then sends a {@code CopyCommand} to a KVM host + * which writes the image content into it.

+ * + *

For iSCSI the ONTAP identity of the cache is recorded on {@code template_spool_ref}: + * {@code local_download_path} holds the LUN uuid, which is the clone source later on. + * {@code install_path} is deliberately left for {@code grantAccess} to fill, because it must + * be {@code //} and the LUN number does not exist until the LUN is + * mapped to an igroup.

+ * + *

For NFS nothing is pre-created on the array: the KVM agent writes the qcow2 into the + * mounted FlexVolume and reports the path, which the framework stores as {@code install_path}.

+ */ + private CreateCmdResult createTemplateOnPrimary(StoragePoolVO storagePool, TemplateInfo templateInfo, Map details) { + if (!isIscsi(details)) { + logger.info("createTemplateOnPrimary: NFS pool [{}], template [{}] will be written directly to the mounted FlexVolume", + storagePool.getId(), templateInfo.getId()); + return new CreateCmdResult(templateInfo.getUuid(), new Answer(null, true, null)); + } + + VMTemplateStoragePoolVO templatePoolRef = findTemplatePoolRef(storagePool.getId(), templateInfo.getId()); + + long sizeInBytes = getDataObjectSizeIncludingHypervisorSnapshotReserve(templateInfo, storagePool); + if (sizeInBytes <= 0) { + throw new CloudRuntimeException("Unknown virtual size for template [" + templateInfo.getId() + + "]; cannot size the template LUN on pool [" + storagePool.getId() + "]"); + } + + StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); + CloudStackVolume created = storageStrategy.createCloudStackVolume( + createTemplateLunRequest(storagePool, details, templateInfo.getId(), sizeInBytes)); + + if (created == null || created.getLun() == null || created.getLun().getName() == null) { + throw new CloudRuntimeException("ONTAP returned no LUN for the cache of template [" + templateInfo.getId() + "]"); + } + + Lun lun = created.getLun(); + templatePoolRef.setLocalDownloadPath(lun.getUuid()); + templatePoolRef.setTemplateSize(sizeInBytes); + vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); + + logger.info("createTemplateOnPrimary: Created template cache LUN [{}] (uuid [{}], {} bytes) on pool [{}] for template [{}]", + lun.getName(), lun.getUuid(), sizeInBytes, storagePool.getId(), templateInfo.getId()); + + return new CreateCmdResult(lun.getName(), new Answer(null, true, null)); + } + + /** + * Clones the cached template into a new volume on the same FlexVolume. + * + *

Invoked when {@code StorageSystemDataMotionStrategy} has recorded a + * {@code cloneOfTemplate} detail on the volume.

+ */ + private CloudStackVolume cloneCloudStackVolumeFromTemplate(StoragePoolVO storagePool, VolumeInfo volumeInfo, + Map details, long templateId) { + VMTemplateStoragePoolVO templatePoolRef = findTemplatePoolRef(storagePool.getId(), templateId); + StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); + boolean iscsi = isIscsi(details); + + CloudStackVolume request = iscsi + ? createCloneLunRequest(storagePool, details, volumeInfo, templatePoolRef, templateId) + : createCloneFileRequest(storagePool, volumeInfo, templatePoolRef, templateId); + + CloudStackVolume cloned = storageStrategy.cloneCloudStackVolume(request); + if (cloned == null || (iscsi && (cloned.getLun() == null || cloned.getLun().getName() == null))) { + throw new CloudRuntimeException("ONTAP returned nothing when cloning template [" + templateId + + "] for volume [" + volumeInfo.getId() + "]"); + } + + logger.info("cloneCloudStackVolumeFromTemplate: Cloned template [{}] for volume [{}] on pool [{}]", + templateId, volumeInfo.getId(), storagePool.getId()); + + long requestedSize = getDataObjectSizeIncludingHypervisorSnapshotReserve(volumeInfo, storagePool); + if (requestedSize > templatePoolRef.getTemplateSize()) { + logger.info("cloneCloudStackVolumeFromTemplate: Growing clone of template [{}] from {} to {} bytes for volume [{}]", + templateId, templatePoolRef.getTemplateSize(), requestedSize, volumeInfo.getId()); + storageStrategy.resizeCloudStackVolume(cloned, requestedSize); + } + + return cloned; + } + + /** + * Returns the CloudStack template id the volume should be cloned from, or null for a blank volume. + * + *

{@code StorageSystemDataMotionStrategy} persists this detail immediately before calling + * {@code createAsync} and removes it right after, so it is only visible during creation.

+ */ + private Long getTemplateIdForCloning(long volumeId) { + VolumeDetailVO detail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.CLONE_OF_TEMPLATE); + if (detail == null || detail.getValue() == null || detail.getValue().isEmpty()) { + return null; + } + return Long.valueOf(detail.getValue()); + } + + private VMTemplateStoragePoolVO findTemplatePoolRef(long poolId, long templateId) { + VMTemplateStoragePoolVO templatePoolRef = vmTemplatePoolDao.findByPoolTemplate(poolId, templateId, null); + if (templatePoolRef == null) { + throw new CloudRuntimeException("No template_spool_ref row for template [" + templateId + "] on pool [" + poolId + "]"); + } + return templatePoolRef; + } + + /** + * Deletes the LUN caching a template on this pool, invoked by template eviction + * ({@code TemplateManagerImpl.evictTemplateFromStoragePool}). + * + *

Volumes previously cloned from this LUN are unaffected: an ONTAP sis-clone shares blocks + * with its source through reference counting rather than depending on it, so the source can be + * removed while its clones stay online.

+ * + *

{@code deleteCloudStackVolume} already unmaps as it deletes ({@code allow_delete_while_mapped}) + * and treats a missing LUN as success.

+ */ + private void deleteTemplateOnPrimary(DataStore store, TemplateInfo templateInfo) { + StoragePoolVO storagePool = storagePoolDao.findById(store.getId()); + if (storagePool == null) { + throw new CloudRuntimeException("Storage Pool not found for id: " + store.getId()); + } + + Map details = storagePoolDetailsDao.listDetailsKeyPairs(store.getId()); + VMTemplateStoragePoolVO templatePoolRef = vmTemplatePoolDao.findByPoolTemplate(storagePool.getId(), templateInfo.getId(), null); + if (templatePoolRef == null) { + logger.warn("deleteTemplateOnPrimary: No template_spool_ref for template [{}] on pool [{}]; nothing to delete", + templateInfo.getId(), storagePool.getId()); + return; + } + + StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); + if (isIscsi(details)) { + deleteIscsiTemplateCache(storagePool, templateInfo, templatePoolRef, storageStrategy); + } else { + deleteNfsTemplateCache(details, templateInfo, templatePoolRef, storageStrategy); + } + } + + private void deleteIscsiTemplateCache(StoragePoolVO storagePool, TemplateInfo templateInfo, + VMTemplateStoragePoolVO templatePoolRef, StorageStrategy storageStrategy) { + String lunUuid = templatePoolRef.getLocalDownloadPath(); + if (lunUuid == null || lunUuid.isEmpty()) { + logger.warn("deleteTemplateOnPrimary: No cached LUN recorded for template [{}] on pool [{}]; nothing to delete", + templateInfo.getId(), storagePool.getId()); + return; + } + + Lun lun = new Lun(); + lun.setUuid(lunUuid); + lun.setName(getTemplateLunName(storagePool, templateInfo.getId())); + + CloudStackVolume deleteRequest = new CloudStackVolume(); + deleteRequest.setLun(lun); + storageStrategy.deleteCloudStackVolume(deleteRequest); + + logger.info("deleteTemplateOnPrimary: Deleted template cache LUN [{}] for template [{}] on pool [{}]", + lun.getName(), templateInfo.getId(), storagePool.getId()); + } + + private void deleteNfsTemplateCache(Map details, TemplateInfo templateInfo, + VMTemplateStoragePoolVO templatePoolRef, StorageStrategy storageStrategy) { + String filePath = templatePoolRef.getInstallPath(); + if (filePath == null || filePath.isEmpty()) { + logger.warn("deleteTemplateOnPrimary: No install_path recorded for template [{}]; nothing to delete", + templateInfo.getId()); + return; + } + String flexVolUuid = details.get(OntapStorageConstants.VOLUME_UUID); + ((UnifiedNASStrategy) storageStrategy).deleteFileByPath(flexVolUuid, filePath); + logger.info("deleteTemplateOnPrimary: Deleted template cache file [{}] for template [{}]", + filePath, templateInfo.getId()); } /** * Deletes a volume or snapshot from the ONTAP storage system. * - *

For volumes, deletes the backend storage object (LUN for iSCSI, no-op for NFS). - * For snapshots, deletes the FlexVolume snapshot from ONTAP that was created by takeSnapshot.

+ *

For volumes, deletes the backend storage object (LUN for iSCSI, file for NFS) via + * {@link StorageStrategy#deleteCloudStackVolume}.

+ * + *

For volume snapshots, this driver is invoked by the standard CloudStack delete chain + * ({@code StorageSystemSnapshotStrategy} → {@code SnapshotServiceImpl.deleteSnapshot} → + * {@code deleteAsync}). It reads ONTAP metadata from {@code snapshot_details} and delegates + * the actual FlexVol snapshot delete to {@link StorageStrategy} (NFS or iSCSI implementation). + * ONTAP REST/delete-job logic must not live here — keep it in the storage-strategy layer.

*/ @Override public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallback callback) { @@ -236,9 +439,14 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac logger.info("deleteAsync: Volume deleted: " + volumeInfo.getId()); commandResult.setResult(null); commandResult.setSuccess(true); + } else if (data.getType() == DataObjectType.TEMPLATE) { + deleteTemplateOnPrimary(store, (TemplateInfo) data); + commandResult.setResult(null); + commandResult.setSuccess(true); } else if (data.getType() == DataObjectType.SNAPSHOT) { - // Delete the ONTAP FlexVolume snapshot that was created by takeSnapshot - deleteOntapSnapshot((SnapshotInfo) data, commandResult); + logger.info("deleteAsync: volume-snapshot delete for CloudStack snapshot [{}] on primary pool [{}] — " + + "delegating ONTAP FlexVol cleanup to StorageStrategy", data.getId(), store.getId()); + deleteCloudStackVolumeSnapshot((SnapshotInfo) data, commandResult); } else { throw new CloudRuntimeException("Unsupported data object type: " + data.getType()); } @@ -252,81 +460,90 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac } /** - * Deletes an ONTAP FlexVolume snapshot. + * Orchestrates CloudStack volume-snapshot delete on ONTAP. * - *

Retrieves the snapshot details stored during takeSnapshot and calls the ONTAP - * REST API to delete the FlexVolume snapshot.

+ *

This method is intentionally thin: it resolves identifiers persisted during + * {@link #takeSnapshot} into {@code snapshot_details} and delegates to the protocol + * {@link StorageStrategy} selected from pool details (NFS → {@code UnifiedNASStrategy}, + * iSCSI → {@code UnifiedSANStrategy}). Both protocols share the same FlexVol-level + * snapshot delete REST API.

* - * @param snapshotInfo The CloudStack snapshot to delete - * @param commandResult Result object to populate with success/failure + *

Required {@code snapshot_details} keys (see {@link OntapStorageConstants}):

+ *
    + *
  • {@code base_ontap_fv_id} — FlexVol UUID
    • + *
  • {@code ontap_snap_id} — ONTAP snapshot UUID
    • + *
  • {@code ontap_snap_name} — snapshot name (logging)
    • + *
  • {@code primary_pool_id} — pool used to obtain credentials/protocol strategy
    • + *
*/ - private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult commandResult) { + private void deleteCloudStackVolumeSnapshot(SnapshotInfo snapshotInfo, CommandResult commandResult) { long snapshotId = snapshotInfo.getId(); - logger.info("deleteOntapSnapshot: Deleting ONTAP FlexVolume snapshot for CloudStack snapshot [{}]", snapshotId); + logger.info("deleteCloudStackVolumeSnapshot: starting ONTAP delete for CloudStack volume snapshot [{}]", snapshotId); try { - // Retrieve snapshot details stored during takeSnapshot String flexVolUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.BASE_ONTAP_FV_ID); String ontapSnapshotUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_ID); String snapshotName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_NAME); String poolIdStr = getSnapshotDetail(snapshotId, OntapStorageConstants.PRIMARY_POOL_ID); if (flexVolUuid == null || ontapSnapshotUuid == null) { - logger.warn("deleteOntapSnapshot: Missing ONTAP snapshot details for snapshot [{}]. " + - "flexVolUuid={}, ontapSnapshotUuid={}. Snapshot may have been created by a different method or already deleted.", + logger.warn("deleteCloudStackVolumeSnapshot: missing ONTAP identity for snapshot [{}] " + + "(flexVolUuid={}, ontapSnapshotUuid={}). Cannot call ONTAP delete; " + + "treating as no-op — verify snapshot_details were written during takeSnapshot", snapshotId, flexVolUuid, ontapSnapshotUuid); - // Consider this a success since there's nothing to delete on ONTAP commandResult.setSuccess(true); commandResult.setResult(null); return; } - long poolId = Long.parseLong(poolIdStr); + long poolId = resolveSnapshotPoolId(poolIdStr, snapshotId); Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(poolId); - + String protocol = poolDetails.get(OntapStorageConstants.PROTOCOL); StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient(); - String authHeader = storageStrategy.getAuthHeader(); - logger.info("deleteOntapSnapshot: Deleting ONTAP snapshot [{}] (uuid={}) from FlexVol [{}]", - snapshotName, ontapSnapshotUuid, flexVolUuid); + logger.info("deleteCloudStackVolumeSnapshot: snapshot [{}] — protocol [{}], pool [{}], " + + "flexVol [{}], ontapSnapshot [{}] (name [{}])", + snapshotId, protocol, poolId, flexVolUuid, ontapSnapshotUuid, snapshotName); - // Call ONTAP REST API to delete the snapshot - JobResponse jobResponse = snapshotClient.deleteSnapshot(authHeader, flexVolUuid, ontapSnapshotUuid); - - if (jobResponse != null && jobResponse.getJob() != null) { - // Poll for job completion - Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); - if (!jobSucceeded) { - throw new CloudRuntimeException("Delete job failed for snapshot [" + - snapshotName + "] on FlexVol [" + flexVolUuid + "]"); - } - } - - logger.info("deleteOntapSnapshot: Successfully deleted ONTAP snapshot [{}] (uuid={}) for CloudStack snapshot [{}]", - snapshotName, ontapSnapshotUuid, snapshotId); + storageStrategy.deleteFlexVolSnapshotForCloudStackVolume(flexVolUuid, ontapSnapshotUuid, snapshotName); + logger.info("deleteCloudStackVolumeSnapshot: completed ONTAP delete for CloudStack volume snapshot [{}]", snapshotId); commandResult.setSuccess(true); commandResult.setResult(null); - } catch (Exception e) { - // Check if the error indicates snapshot doesn't exist (already deleted) - String errorMsg = e.getMessage(); - if (errorMsg != null && (errorMsg.contains("404") || errorMsg.contains("not found") || - errorMsg.contains("does not exist"))) { - logger.warn("deleteOntapSnapshot: ONTAP snapshot for CloudStack snapshot [{}] not found, " + - "may have been already deleted. Treating as success.", snapshotId); + if (OntapStorageUtils.isOntapObjectNotFoundError(e)) { + logger.warn("deleteCloudStackVolumeSnapshot: ONTAP snapshot for CloudStack snapshot [{}] " + + "already absent (idempotent success): {}", snapshotId, e.getMessage()); commandResult.setSuccess(true); commandResult.setResult(null); - } else { - logger.error("deleteOntapSnapshot: Failed to delete ONTAP snapshot for CloudStack snapshot [{}]: {}", - snapshotId, e.getMessage(), e); - commandResult.setSuccess(false); - commandResult.setResult(e.getMessage()); + return; } + logger.error("deleteCloudStackVolumeSnapshot: ONTAP delete failed for CloudStack snapshot [{}]: {}", + snapshotId, e.getMessage(), e); + commandResult.setSuccess(false); + commandResult.setResult(e.getMessage()); } } + private long resolveSnapshotPoolId(String poolIdStr, long snapshotId) { + if (poolIdStr != null && !poolIdStr.isEmpty()) { + return Long.parseLong(poolIdStr); + } + SnapshotVO snapshotVO = snapshotDao.findById(snapshotId); + if (snapshotVO == null) { + throw new CloudRuntimeException("Snapshot not found for snapshot [" + snapshotId + "]"); + } + VolumeVO volumeVO = volumeDao.findByIdIncludingRemoved(snapshotVO.getVolumeId()); + if (volumeVO == null) { + throw new CloudRuntimeException("CloudStack Volume not found for snapshot [" + snapshotId + "]"); + } + Long poolId = volumeVO.getPoolId() != null ? volumeVO.getPoolId() : volumeVO.getLastPoolId(); + if (poolId == null || poolId <= 0) { + throw new CloudRuntimeException("Cannot resolve storage pool for snapshot [" + snapshotId + "]"); + } + return poolId; + } + @Override public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback callback) { throw new UnsupportedOperationException("Copy operation is not supported for ONTAP primary storage."); @@ -399,6 +616,8 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore volumeVO.setPoolType(storagePool.getPoolType()); volumeVO.setPoolId(storagePool.getId()); volumeDao.update(volumeVO.getId(), volumeVO); + } else if (dataObject.getType() == DataObjectType.TEMPLATE) { + grantAccessTemplate((TemplateInfo) dataObject, host, dataStore, storagePool); } else { logger.error("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to grantAccess"); @@ -415,23 +634,78 @@ private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid()); - // Validate if Igroup exist ONTAP for this host as we may be using delete_on_unmap= true and igroup may be deleted by ONTAP automatically + ensureAccessGroupForHost(sanStrategy, host, storagePool, svmName, accessGroupName); + + // Create or retrieve existing LUN mapping + String lunNumber = sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName); + + // Update volume path if changed (e.g., after migration or re-mapping) + String iscsiPath = buildIscsiPath(storagePool, lunNumber); + if (volumeVO.getPath() == null || !volumeVO.getPath().equals(iscsiPath)) { + volumeVO.set_iScsiName(iscsiPath); + volumeVO.setPath(iscsiPath); + } + } + + /** + * Maps the cached template LUN to the host so the KVM agent can write the image into it. + * + *

Called by the framework from {@code copyTemplateToManagedTemplateVolume} just before it + * issues the {@code CopyCommand}. That method reads {@code managedStoreTarget} from the pool + * details before this call, when the LUN number does not exist yet, so the stale + * value is corrected here. The datastore details are re-read when the command is built, so the + * update lands in time.

+ */ + private void grantAccessTemplate(TemplateInfo templateInfo, Host host, DataStore dataStore, StoragePoolVO storagePool) { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + if (!ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { + logger.debug("grantAccessTemplate: NFS template [{}], no igroup mapping required", templateInfo.getUuid()); + return; + } + + String svmName = details.get(OntapStorageConstants.SVM_NAME); + VMTemplateStoragePoolVO templatePoolRef = findTemplatePoolRef(storagePool.getId(), templateInfo.getId()); + String lunName = getTemplateLunName(storagePool, templateInfo.getId()); + + UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); + String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid()); + + ensureAccessGroupForHost(sanStrategy, host, storagePool, svmName, accessGroupName); + + String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName); + String iscsiPath = buildIscsiPath(storagePool, lunNumber); + + templatePoolRef.setInstallPath(iscsiPath); + vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); + refreshManagedStoreTarget(dataStore, iscsiPath); + + logger.info("grantAccessTemplate: Mapped template cache LUN [{}] to igroup [{}] as [{}] for template [{}]", + lunName, accessGroupName, iscsiPath, templateInfo.getId()); + } + + /** + * Ensures an igroup containing this host's initiator exists on the SVM. + * + *

The igroup may be absent even for a host that used the pool before, because LUN maps are + * created with {@code delete_on_unmap}, which lets ONTAP remove the igroup on its own.

+ */ + private void ensureAccessGroupForHost(UnifiedSANStrategy sanStrategy, Host host, StoragePoolVO storagePool, + String svmName, String accessGroupName) { Map getAccessGroupMap = Map.of( OntapStorageConstants.NAME, accessGroupName, OntapStorageConstants.SVM_DOT_NAME, svmName ); AccessGroup accessGroup = sanStrategy.getAccessGroup(getAccessGroupMap); - if(accessGroup == null || accessGroup.getIgroup() == null) { - logger.info("grantAccess: Igroup {} does not exist for the host {} : Need to create Igroup for the host ", accessGroupName, host.getName()); - // create the igroup for the host and perform lun-mapping + if (accessGroup == null || accessGroup.getIgroup() == null) { + logger.info("ensureAccessGroupForHost: Igroup {} does not exist for the host {} : Need to create Igroup for the host ", accessGroupName, host.getName()); accessGroup = new AccessGroup(); List hosts = new ArrayList<>(); hosts.add((HostVO) host); accessGroup.setHostsToConnect(hosts); accessGroup.setStoragePoolId(storagePool.getId()); accessGroup = sanStrategy.createAccessGroup(accessGroup); - }else{ - logger.info("grantAccess: Igroup {} already exist for the host {}: ", accessGroup.getIgroup().getName() , host.getName()); + } else { + logger.info("ensureAccessGroupForHost: Igroup {} already exist for the host {}: ", accessGroup.getIgroup().getName(), host.getName()); /* TODO Below cases will be covered later, for now they will be a pre-requisite on customer side 1. Igroup exist with the same name but host initiator has been removed 2. Igroup exist with the same name but host initiator has been changed may be due to new NIC or new adapter @@ -439,16 +713,28 @@ private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map Incase it is not , add it and proceed for lun-mapping */ } - logger.info("grantAccess: Igroup {} is present now with initiators {} ", accessGroup.getIgroup().getName(), accessGroup.getIgroup().getInitiators()); - // Create or retrieve existing LUN mapping - String lunNumber = sanStrategy.ensureLunMapped(svmName, cloudStackVolumeName, accessGroupName); + logger.info("ensureAccessGroupForHost: Igroup {} is present now with initiators {} ", accessGroup.getIgroup().getName(), accessGroup.getIgroup().getInitiators()); + } - // Update volume path if changed (e.g., after migration or re-mapping) - String iscsiPath = OntapStorageConstants.SLASH + storagePool.getPath() + OntapStorageConstants.SLASH + lunNumber; - if (volumeVO.getPath() == null || !volumeVO.getPath().equals(iscsiPath)) { - volumeVO.set_iScsiName(iscsiPath); - volumeVO.setPath(iscsiPath); + /** + * Builds the volume path the KVM agent expects for managed iSCSI: {@code //}. + */ + private String buildIscsiPath(StoragePoolVO storagePool, String lunNumber) { + return OntapStorageConstants.SLASH + storagePool.getPath() + OntapStorageConstants.SLASH + lunNumber; + } + + private void refreshManagedStoreTarget(DataStore dataStore, String iscsiPath) { + if (!(dataStore instanceof PrimaryDataStore)) { + return; + } + PrimaryDataStore primaryDataStore = (PrimaryDataStore) dataStore; + Map storeDetails = primaryDataStore.getDetails(); + if (storeDetails == null) { + return; } + Map updated = new HashMap<>(storeDetails); + updated.put(PrimaryDataStore.MANAGED_STORE_TARGET, iscsiPath); + primaryDataStore.setDetails(updated); } /** @@ -485,6 +771,8 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) throw new CloudRuntimeException("CloudStack Volume not found for id: " + dataObject.getId()); } revokeAccessForVolume(storagePool, volumeVO, host); + } else if (dataObject.getType() == DataObjectType.TEMPLATE) { + revokeAccessForTemplate(storagePool, (TemplateInfo) dataObject, host); } else { logger.error("revokeAccess: Invalid DataObjectType (" + dataObject.getType() + ") passed to revokeAccess"); throw new CloudRuntimeException("Invalid DataObjectType (" + dataObject.getType() + ") passed to revokeAccess"); @@ -510,46 +798,73 @@ private void revokeAccessForVolume(StoragePoolVO storagePool, VolumeVO volumeVO, // Retrieve LUN name from volume details; if missing, volume may not have been fully created VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME); - ValidateRevoke result = getValidateRevoke(volumeVO, host, lunDetail, storageStrategy, svmName, accessGroupName); - if (result == null) return; - - // Remove the LUN mapping from the igroup - Map disableLogicalAccessMap = new HashMap<>(); - disableLogicalAccessMap.put(OntapStorageConstants.LUN_DOT_UUID, result.cloudStackVolume.getLun().getUuid()); - disableLogicalAccessMap.put(OntapStorageConstants.IGROUP_DOT_UUID, result.accessGroup.getIgroup().getUuid()); - storageStrategy.disableLogicalAccess(disableLogicalAccessMap); + String lunName = lunDetail != null ? lunDetail.getValue() : null; + if (lunName == null) { + logger.warn("revokeAccessForVolume: No LUN name found for volume [{}]; skipping revoke", volumeVO.getId()); + return; + } + unmapLunFromHost(storageStrategy, svmName, lunName, accessGroupName, host); + } + } - logger.info("revokeAccessForVolume: Successfully revoked access to LUN [{}] for host [{}]", - result.lunName, host.getName()); + /** + * Unmaps the cached template LUN once the framework has finished writing the image into it. + */ + private void revokeAccessForTemplate(StoragePoolVO storagePool, TemplateInfo templateInfo, Host host) { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + if (!ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { + logger.debug("revokeAccessForTemplate: NFS template [{}], no igroup mapping to remove", templateInfo.getUuid()); + return; } + + String svmName = details.get(OntapStorageConstants.SVM_NAME); + StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); + String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid()); + String lunName = getTemplateLunName(storagePool, templateInfo.getId()); + + logger.info("revokeAccessForTemplate: Revoking access to template cache LUN [{}] for host [{}]", lunName, host.getName()); + unmapLunFromHost(storageStrategy, svmName, lunName, accessGroupName, host); } - @Nullable - private ValidateRevoke getValidateRevoke(VolumeVO volumeVO, Host host, VolumeDetailVO lunDetail, StorageStrategy storageStrategy, String svmName, String accessGroupName) { - String lunName = lunDetail != null ? lunDetail.getValue() : null; - if (lunName == null) { - logger.warn("revokeAccessForVolume: No LUN name found for volume [{}]; skipping revoke", volumeVO.getId()); - return null; + /** + * Removes a LUN-to-igroup mapping, skipping quietly when the LUN, the igroup or the host + * initiator is already gone. + */ + private void unmapLunFromHost(StorageStrategy storageStrategy, String svmName, String lunName, + String accessGroupName, Host host) { + ValidateRevoke result = getValidateRevoke(lunName, host, storageStrategy, svmName, accessGroupName); + if (result == null) { + return; } + Map disableLogicalAccessMap = new HashMap<>(); + disableLogicalAccessMap.put(OntapStorageConstants.LUN_DOT_UUID, result.cloudStackVolume.getLun().getUuid()); + disableLogicalAccessMap.put(OntapStorageConstants.IGROUP_DOT_UUID, result.accessGroup.getIgroup().getUuid()); + storageStrategy.disableLogicalAccess(disableLogicalAccessMap); + + logger.info("unmapLunFromHost: Successfully revoked access to LUN [{}] for host [{}]", result.lunName, host.getName()); + } + + @Nullable + private ValidateRevoke getValidateRevoke(String lunName, Host host, StorageStrategy storageStrategy, String svmName, String accessGroupName) { // Verify LUN still exists on ONTAP (may have been manually deleted) CloudStackVolume cloudStackVolume = getCloudStackVolumeByName(storageStrategy, svmName, lunName); if (cloudStackVolume == null || cloudStackVolume.getLun() == null || cloudStackVolume.getLun().getUuid() == null) { - logger.warn("revokeAccessForVolume: LUN for volume [{}] not found on ONTAP, skipping revoke", volumeVO.getId()); + logger.warn("getValidateRevoke: LUN [{}] not found on ONTAP, skipping revoke", lunName); return null; } // Verify igroup still exists on ONTAP AccessGroup accessGroup = getAccessGroupByName(storageStrategy, svmName, accessGroupName); if (accessGroup == null || accessGroup.getIgroup() == null || accessGroup.getIgroup().getUuid() == null) { - logger.warn("revokeAccessForVolume: iGroup [{}] not found on ONTAP, skipping revoke", accessGroupName); + logger.warn("getValidateRevoke: iGroup [{}] not found on ONTAP, skipping revoke", accessGroupName); return null; } // Verify host initiator is in the igroup before attempting to remove mapping SANStrategy sanStrategy = (UnifiedSANStrategy) storageStrategy; if (!sanStrategy.validateInitiatorInAccessGroup(host.getStorageUrl(), svmName, accessGroup.getIgroup())) { - logger.warn("revokeAccessForVolume: Initiator [{}] is not in iGroup [{}], skipping revoke", + logger.warn("getValidateRevoke: Initiator [{}] is not in iGroup [{}], skipping revoke", host.getStorageUrl(), accessGroupName); return null; } @@ -600,14 +915,34 @@ private AccessGroup getAccessGroupByName(StorageStrategy storageStrategy, String return accessGroup; } + /** + * ONTAP is only supported with KVM, which does not take hypervisor-side snapshots into the + * volume itself, so no reserve is added on top of the requested size. + * + *

For a template this returns the virtual size ({@code VMTemplateVO.size}), not the + * compressed size on secondary storage. The cached template LUN is written by the KVM agent + * with {@code qemu-img convert} from QCOW2 to RAW, so it must be able to hold the fully + * expanded image.

+ */ @Override public long getDataObjectSizeIncludingHypervisorSnapshotReserve(DataObject dataObject, StoragePool storagePool) { - return 0; + if (dataObject == null) { + return 0; + } + Long size = dataObject.getSize(); + return size != null && size > 0 ? size : 0; } @Override public long getBytesRequiredForTemplate(TemplateInfo templateInfo, StoragePool storagePool) { - return 0; + if (templateInfo == null || storagePool == null) { + return 0; + } + // Already cached on this pool, so deploying from it costs no additional space. + if (vmTemplatePoolDao.findByPoolTemplate(storagePool.getId(), templateInfo.getId(), null) != null) { + return 0; + } + return getDataObjectSizeIncludingHypervisorSnapshotReserve(templateInfo, storagePool); } @Override @@ -647,7 +982,7 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback details) { + return ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL)); + } + + /** + * Builds the request that creates a blank volume (LUN for iSCSI, qcow2 file for NFS). + */ + private CloudStackVolume createVolumeRequest(StoragePoolVO storagePool, Map details, DataObject volumeObject) { + CloudStackVolume request = new CloudStackVolume(); + String protocol = details.get(OntapStorageConstants.PROTOCOL); + if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { + request.setDatastoreId(String.valueOf(storagePool.getId())); + request.setVolumeInfo(volumeObject); + } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { + Lun lunRequest = new Lun(); + Svm svm = new Svm(); + svm.setName(details.get(OntapStorageConstants.SVM_NAME)); + String lunName = volumeObject.getName().replace(OntapStorageConstants.HYPHEN, OntapStorageConstants.UNDERSCORE); + if (!OntapStorageUtils.isValidName(lunName)) { + throw new InvalidParameterValueException("Invalid dataObject name [" + lunName + + "]. It must start with a letter and can only contain letters, digits, and underscores, and be up to 200 characters long."); + } + lunRequest.setSvm(svm); + lunRequest.setName(OntapStorageUtils.getLunName(storagePool.getName(), lunName)); + lunRequest.setOsType(Lun.OsTypeEnum.valueOf(OntapStorageUtils.getOSTypeFromHypervisor(storagePool.getHypervisor().name()))); + LunSpace lunSpace = new LunSpace(); + lunSpace.setSize(volumeObject.getSize()); + lunRequest.setSpace(lunSpace); + request.setLun(lunRequest); + } else { + throw new CloudRuntimeException("Unsupported protocol " + protocol); + } + return request; + } + + /** + * LUN path used to cache a template on this pool: {@code /vol//cs_tmpl_}. + */ + private String getTemplateLunName(StoragePoolVO storagePool, long templateId) { + return OntapStorageUtils.getLunName(storagePool.getName(), OntapStorageConstants.TEMPLATE_LUN_PREFIX + templateId); + } + + /** + * Builds the request that creates an empty LUN to cache a template. + * + *

The LUN is sized to the template's virtual disk size. That is the size KVM writes + * after {@code qemu-img convert} to RAW, so it must not be the compressed QCOW2 physical size.

+ */ + private CloudStackVolume createTemplateLunRequest(StoragePoolVO storagePool, Map details, + long templateId, long sizeInBytes) { + Svm svm = new Svm(); + svm.setName(details.get(OntapStorageConstants.SVM_NAME)); + + Lun lunRequest = new Lun(); + lunRequest.setSvm(svm); + lunRequest.setName(getTemplateLunName(storagePool, templateId)); + lunRequest.setOsType(Lun.OsTypeEnum.valueOf( + OntapStorageUtils.getOSTypeFromHypervisor(storagePool.getHypervisor().name()))); + LunSpace lunSpace = new LunSpace(); + lunSpace.setSize(sizeInBytes); + lunRequest.setSpace(lunSpace); + + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lunRequest); + return request; + } + + /** + * Builds the request that clones {@code local_download_path} (LUN uuid) into a new LUN. + * + *

Size is omitted: ONTAP rejects a size on a clone create, and the clone inherits the + * source size. Growing to the requested volume size is a separate PATCH.

+ */ + private CloudStackVolume createCloneLunRequest(StoragePoolVO storagePool, Map details, + VolumeInfo volumeObject, VMTemplateStoragePoolVO templatePoolRef, + long templateId) { + String sourceLunUuid = templatePoolRef.getLocalDownloadPath(); + if (sourceLunUuid == null || sourceLunUuid.isEmpty()) { + throw new CloudRuntimeException("Template [" + templateId + "] has no cached LUN on pool [" + + storagePool.getId() + "]; cannot clone volume [" + volumeObject.getId() + "]"); + } + + Svm svm = new Svm(); + svm.setName(details.get(OntapStorageConstants.SVM_NAME)); + + String lunName = volumeObject.getName().replace(OntapStorageConstants.HYPHEN, OntapStorageConstants.UNDERSCORE); + if (!OntapStorageUtils.isValidName(lunName)) { + throw new InvalidParameterValueException("Invalid dataObject name [" + lunName + + "]. It must start with a letter and can only contain letters, digits, and underscores, and be up to 200 characters long."); + } + + Lun.Source source = new Lun.Source(); + source.setUuid(sourceLunUuid); + Lun.Clone clone = new Lun.Clone(); + clone.setSource(source); + + Lun lunRequest = new Lun(); + lunRequest.setSvm(svm); + lunRequest.setName(OntapStorageUtils.getLunName(storagePool.getName(), lunName)); + lunRequest.setClone(clone); + + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lunRequest); + return request; + } + + /** + * Builds the request that clones the cached qcow2 ({@code install_path}) into a new file + * named after the volume uuid, inside the same FlexVolume. + */ + private CloudStackVolume createCloneFileRequest(StoragePoolVO storagePool, VolumeInfo volumeInfo, + VMTemplateStoragePoolVO templatePoolRef, long templateId) { + String sourcePath = templatePoolRef.getInstallPath(); + if (sourcePath == null || sourcePath.isEmpty()) { + throw new CloudRuntimeException("Template [" + templateId + "] has no cached file on pool [" + + storagePool.getId() + "]; cannot clone volume [" + volumeInfo.getId() + "]"); + } + + FileInfo file = new FileInfo(); + file.setPath(sourcePath); + + CloudStackVolume request = new CloudStackVolume(); + request.setDatastoreId(String.valueOf(storagePool.getId())); + request.setVolumeInfo(volumeInfo); + request.setFile(file); + request.setDestinationPath(volumeInfo.getUuid()); + return request; + } + // ────────────────────────────────────────────────────────────────────────── // Snapshot Helper Methods // ────────────────────────────────────────────────────────────────────────── /** - * Builds a snapshot name with proper length constraints. - * Format: {@code -} + * Builds an ONTAP-safe snapshot name from the CloudStack UI name with uniqueness suffix. */ - private String buildSnapshotName(String volumeName, String snapshotUuid) { - String name = volumeName + "-" + snapshotUuid; - int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH; - int trimRequired = name.length() - maxLength; + private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId) { + return OntapStorageUtils.buildOntapSnapshotName(cloudStackSnapshotName, OntapStorageConstants.CS + snapshotId); + } + - if (trimRequired > 0) { - name = StringUtils.left(volumeName, volumeName.length() - trimRequired) + "-" + snapshotUuid; + private Storage.ImageFormat getImageFormatByHypervisor(HypervisorType hypervisorType) { + if (HypervisorType.KVM.equals(hypervisorType)) { + return Storage.ImageFormat.QCOW2; } - return name; + throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution"); } - /** * Persists snapshot metadata in snapshot_details table. * + * Persists ONTAP snapshot metadata in {@code snapshot_details} for revert and delete. + * + *

Volume-snapshot delete reads {@code base_ontap_fv_id} and {@code ontap_snap_id} here + * during {@link #deleteCloudStackVolumeSnapshot}; missing rows prevent ONTAP cleanup.

+ * * @param csSnapshotId CloudStack snapshot ID * @param csVolumeId Source CloudStack volume ID * @param flexVolUuid ONTAP FlexVolume UUID diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java index 8cf21b94b2f1..ba38f57ef1c7 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java @@ -21,7 +21,9 @@ import feign.QueryMap; import org.apache.cloudstack.storage.feign.model.ExportPolicy; +import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.FileInfo; +import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import feign.Headers; import feign.Param; @@ -58,6 +60,15 @@ void createFile(@Param("authHeader") String authHeader, @Param("path") String filePath, FileInfo file); + /** + * Creates a space-efficient clone of a file within a FlexVolume. + * + *

ONTAP REST: {@code POST /api/storage/file/clone}

+ */ + @RequestLine("POST /api/storage/file/clone") + @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) + JobResponse cloneFile(@Param("authHeader") String authHeader, FileCloneRequest request); + // Export Policy Operations @RequestLine("POST /api/protocols/nfs/export-policies") @Headers({"Authorization: {authHeader}"}) 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 7281dc2ecbeb..d365468cee10 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 @@ -50,8 +50,8 @@ public interface SANFeignClient { @Headers({"Authorization: {authHeader}"}) Lun getLunByUUID(@Param("authHeader") String authHeader, @Param("uuid") String uuid); - @RequestLine("PATCH /{uuid}") - @Headers({"Authorization: {authHeader}"}) + @RequestLine("PATCH /api/storage/luns/{uuid}") + @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) void updateLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid, Lun lun); @RequestLine("DELETE /api/storage/luns/{uuid}") diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java new file mode 100644 index 000000000000..7ac5214f986c --- /dev/null +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.storage.feign.model; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + * Request body for the ONTAP file clone API. + * + *

ONTAP REST endpoint: {@code POST /api/storage/file/clone}

+ * + *

Creates a space-efficient copy of a file. Source and destination paths are relative to the + * root of {@code volume}, and both must live in that same FlexVolume.

+ */ +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) +public class FileCloneRequest { + + @JsonProperty("svm") + private Svm svm; + + @JsonProperty("volume") + private VolumeRef volume; + + @JsonProperty("source_path") + private String sourcePath; + + @JsonProperty("destination_path") + private String destinationPath; + + @JsonProperty("overwrite_destination") + private Boolean overwriteDestination; + + public FileCloneRequest() { + } + + public FileCloneRequest(String svmName, String flexVolName, String sourcePath, String destinationPath) { + this.svm = new Svm(); + this.svm.setName(svmName); + this.volume = new VolumeRef(flexVolName); + this.sourcePath = sourcePath; + this.destinationPath = destinationPath; + } + + public Svm getSvm() { + return svm; + } + + public void setSvm(Svm svm) { + this.svm = svm; + } + + public VolumeRef getVolume() { + return volume; + } + + public void setVolume(VolumeRef volume) { + this.volume = volume; + } + + public String getSourcePath() { + return sourcePath; + } + + public void setSourcePath(String sourcePath) { + this.sourcePath = sourcePath; + } + + public String getDestinationPath() { + return destinationPath; + } + + public void setDestinationPath(String destinationPath) { + this.destinationPath = destinationPath; + } + + public Boolean getOverwriteDestination() { + return overwriteDestination; + } + + public void setOverwriteDestination(Boolean overwriteDestination) { + this.overwriteDestination = overwriteDestination; + } + + @JsonIgnoreProperties(ignoreUnknown = true) + @JsonInclude(JsonInclude.Include.NON_NULL) + public static class VolumeRef { + + @JsonProperty("name") + private String name; + + public VolumeRef() { + } + + public VolumeRef(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + @Override + public String toString() { + return "FileCloneRequest{volume=" + (volume != null ? volume.getName() : null) + + ", sourcePath=" + sourcePath + + ", destinationPath=" + destinationPath + "}"; + } +} 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 c13b255c67ea..3d20036a493f 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 @@ -519,14 +519,28 @@ public String getNetworkInterface() { abstract public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume); /** - * Method encapsulates the behavior based on the opted protocol in subclasses. + * Creates a space-efficient clone of an existing object inside the same FlexVolume. * it is going to mimic * cloneLun for iSCSI, FC protocols * cloneFile for NFS3.0 and NFS4.1 protocols * cloneNameSpace for Nvme/TCP and Nvme/FC protocol - * @param cloudstackVolume the CloudStack volume to copy + * + *

ONTAP requires the source and the destination to live in the same FlexVolume, which + * holds because a CloudStack primary storage pool maps one-to-one onto a FlexVolume.

+ * + * @param cloudstackVolume describes the clone to create; the source is carried in the + * protocol-specific clone reference (for SAN, {@code lun.clone.source}) + * @return the created CloudStackVolume, populated with the backend identity of the clone + */ + abstract public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume); + + /** + * Grows an existing backend object to {@code sizeInBytes}. + * + *

Needed after cloning a cached template, because a clone inherits the size of its source + * while the service offering may ask for a larger disk.

*/ - abstract public void copyCloudStackVolume(CloudStackVolume cloudstackVolume); + abstract public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeInBytes); /** * 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 198957ca5db8..98656c7d8be2 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 @@ -19,21 +19,26 @@ package org.apache.cloudstack.storage.service; -import com.cloud.agent.api.Answer; -import com.cloud.host.HostVO; -import com.cloud.storage.Storage; -import com.cloud.storage.VolumeVO; -import com.cloud.storage.dao.VolumeDao; -import com.cloud.utils.exception.CloudRuntimeException; -import feign.FeignException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import javax.inject.Inject; + import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.storage.command.CreateObjectCommand; import org.apache.cloudstack.storage.command.DeleteCommand; +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.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.feign.model.ExportPolicy; import org.apache.cloudstack.storage.feign.model.ExportRule; +import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.FileInfo; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.Nas; @@ -42,25 +47,31 @@ import org.apache.cloudstack.storage.feign.model.Volume; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; -import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; -import org.apache.cloudstack.storage.volume.VolumeObject; import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; +import org.apache.cloudstack.storage.volume.VolumeObject; 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 com.cloud.agent.api.Answer; +import com.cloud.agent.api.storage.ResizeVolumeCommand; +import com.cloud.agent.api.to.StorageFilerTO; +import com.cloud.host.HostVO; +import com.cloud.storage.Storage; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.utils.exception.CloudRuntimeException; + +import feign.FeignException; public class UnifiedNASStrategy extends NASStrategy { private static final Logger logger = LogManager.getLogger(UnifiedNASStrategy.class); @Inject private VolumeDao volumeDao; @Inject private EndPointSelector epSelector; @Inject private StoragePoolDetailsDao storagePoolDetailsDao; + @Inject private PrimaryDataStoreDao primaryDataStoreDao; public UnifiedNASStrategy(OntapStorage ontapStorage) { super(ontapStorage); @@ -112,9 +123,100 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } } + /** + * Clones a file inside the FlexVolume using ONTAP's file clone API. + * + *

The source is taken from {@code file.path} and the destination from + * {@code destinationPath}, both relative to the root of the FlexVolume backing the pool.

+ */ + @Override + public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) { + if (cloudstackVolume == null || cloudstackVolume.getFile() == null + || cloudstackVolume.getFile().getPath() == null || cloudstackVolume.getDestinationPath() == null) { + logger.error("cloneCloudStackVolume: File clone failed. Invalid request: {}", cloudstackVolume); + throw new CloudRuntimeException("Failed to clone file, invalid request"); + } + if (cloudstackVolume.getDatastoreId() == null) { + throw new CloudRuntimeException("Failed to clone file, no datastore id in the request"); + } + + Map details = storagePoolDetailsDao.listDetailsKeyPairs(Long.parseLong(cloudstackVolume.getDatastoreId())); + String svmName = details.get(OntapStorageConstants.SVM_NAME); + String flexVolName = details.get(OntapStorageConstants.VOLUME_NAME); + String sourcePath = cloudstackVolume.getFile().getPath(); + String destinationPath = cloudstackVolume.getDestinationPath(); + + logger.info("cloneCloudStackVolume: Cloning file [{}] to [{}] in FlexVol [{}]", sourcePath, destinationPath, flexVolName); + try { + FileCloneRequest request = new FileCloneRequest(svmName, flexVolName, sourcePath, destinationPath); + JobResponse jobResponse = nasFeignClient.cloneFile(getAuthHeader(), request); + pollJobIfPresent(jobResponse, "clone file [" + sourcePath + "] to [" + destinationPath + "]"); + + updateCloudStackVolumeMetadata(cloudstackVolume.getDatastoreId(), cloudstackVolume.getVolumeInfo()); + + FileInfo clonedFile = new FileInfo(); + clonedFile.setPath(destinationPath); + + CloudStackVolume clonedCloudStackVolume = new CloudStackVolume(); + clonedCloudStackVolume.setFile(clonedFile); + clonedCloudStackVolume.setDatastoreId(cloudstackVolume.getDatastoreId()); + clonedCloudStackVolume.setVolumeInfo(cloudstackVolume.getVolumeInfo()); + return clonedCloudStackVolume; + } catch (FeignException e) { + logger.error("FeignException occurred while cloning file [{}], Status: {}, Exception: {}", + sourcePath, e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to clone file: " + e.getMessage()); + } catch (Exception e) { + logger.error("Exception occurred while cloning file [{}], Exception: {}", sourcePath, e.getMessage()); + throw new CloudRuntimeException("Failed to clone file: " + e.getMessage()); + } + } + + /** + * Grows the cloned qcow2 to the requested size via a host-side {@code qemu-img resize}. + */ @Override - public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) { + public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeInBytes) { + if (cloudstackVolume == null || cloudstackVolume.getVolumeInfo() == null) { + logger.error("resizeCloudStackVolume: Resize failed. Invalid request: {}", cloudstackVolume); + throw new CloudRuntimeException("Failed to resize file, invalid request"); + } + if (sizeInBytes <= 0) { + throw new CloudRuntimeException("Failed to resize file, invalid size " + sizeInBytes); + } + + DataObject volumeInfo = cloudstackVolume.getVolumeInfo(); + Answer answer = resizeVolumeOnKVMHost(volumeInfo, sizeInBytes); + if (answer == null || !answer.getResult()) { + String errMsg = answer != null ? answer.getDetails() : "Failed to resize qcow2 on KVM host"; + logger.error("resizeCloudStackVolume: " + errMsg); + throw new CloudRuntimeException(errMsg); + } + logger.info("resizeCloudStackVolume: Resized volume [{}] to {} bytes", volumeInfo.getUuid(), sizeInBytes); + } + + private Answer resizeVolumeOnKVMHost(DataObject volumeInfo, long sizeInBytes) { + VolumeObject volumeObject = (VolumeObject) volumeInfo; + VolumeVO volume = volumeDao.findById(volumeObject.getId()); + if (volume == null) { + throw new CloudRuntimeException("Volume not found with id: " + volumeObject.getId()); + } + StoragePoolVO storagePool = primaryDataStoreDao.findById(volume.getPoolId()); + if (storagePool == null) { + throw new CloudRuntimeException("Storage Pool not found for id: " + volume.getPoolId()); + } + + ResizeVolumeCommand cmd = new ResizeVolumeCommand(volume.getPath(), new StorageFilerTO(storagePool), + volume.getSize(), sizeInBytes, false, null); + EndPoint ep = epSelector.select(volumeInfo); + if (ep == null) { + String errMsg = "No remote endpoint to send ResizeVolumeCommand, check if host is up"; + logger.error(errMsg); + return new Answer(cmd, false, errMsg); + } + logger.info("resizeVolumeOnKVMHost: Sending command to endpoint: {}", ep.getHostAddr()); + return ep.sendMessage(cmd); } @Override @@ -176,12 +278,15 @@ public void deleteAccessGroup(AccessGroup accessGroup) { String exportPolicyId = details.get(OntapStorageConstants.EXPORT_POLICY_ID); try { - nasFeignClient.deleteExportPolicyById(authHeader,exportPolicyId); + nasFeignClient.deleteExportPolicyById(authHeader, exportPolicyId); logger.info("deleteAccessGroup: Successfully deleted export policy '{}'", exportPolicyName); - } catch (Exception e) { + } catch (FeignException e) { + if (OntapStorageUtils.isOntapObjectNotFoundError(e)) { + logger.warn("deleteAccessGroup: Export policy '{}' not found in ONTAP, treating as no-op", exportPolicyName); + return; + } logger.error("deleteAccessGroup: Failed to delete export policy. Exception: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to delete export policy: " + e.getMessage(), e); - } } catch (Exception e) { logger.error("deleteAccessGroup: Failed to delete export policy. Exception: {}", e.getMessage(), e); @@ -191,7 +296,134 @@ public void deleteAccessGroup(AccessGroup accessGroup) { @Override public AccessGroup updateAccessGroup(AccessGroup accessGroup) { - return null; + if (accessGroup == null) { + throw new CloudRuntimeException("Invalid accessGroup object - accessGroup is null"); + } + // Check if an AccessGroup was constructed without associating it to a storage pool. + if (accessGroup.getStoragePoolId() == null) { + throw new CloudRuntimeException("Invalid accessGroup object - storagePoolId is null"); + } + // At least one host is required regardless of ADD or REMOVE action. + // An empty list means there is nothing to add to or remove from the export policy client list. + if (accessGroup.getHostsToConnect() == null || accessGroup.getHostsToConnect().isEmpty()) { + throw new CloudRuntimeException("Invalid accessGroup object - hostsToConnect is null or empty"); + } + + Map details = storagePoolDetailsDao.listDetailsKeyPairs(accessGroup.getStoragePoolId()); + if (details == null || details.isEmpty()) { + throw new CloudRuntimeException("No storage pool details found for storagePoolId: " + accessGroup.getStoragePoolId()); + } + String exportPolicyId = details.get(OntapStorageConstants.EXPORT_POLICY_ID); + if (exportPolicyId == null || exportPolicyId.isEmpty()) { + throw new CloudRuntimeException("No export policy found for storagePoolId: " + accessGroup.getStoragePoolId()); + } + + + try { + String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + ExportPolicy existingPolicy = nasFeignClient.getExportPolicyById(authHeader, exportPolicyId); + // Check if the export policy was deleted externally on ONTAP or the stored ID is stale. + if (existingPolicy == null) { + throw new CloudRuntimeException("Failed to fetch existing export policy with id: " + exportPolicyId); + } + + List rules = existingPolicy.getRules(); + if (rules == null || rules.isEmpty()) { + throw new CloudRuntimeException("Export policy " + existingPolicy.getName() + + " has no rules — unexpected state, the plugin always creates a rule at pool registration"); + } + + ExportRule targetRule = rules.get(0); + + Set hostMatches = new HashSet<>(); + for (HostVO host : accessGroup.getHostsToConnect()) { + String hostStorageIp = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : null; + String ip = (hostStorageIp != null && !hostStorageIp.isEmpty()) ? hostStorageIp + : (host.getPrivateIpAddress() != null ? host.getPrivateIpAddress().trim() : null); + // Occurs when a CloudStack host has neither a storage IP nor a private IP configured + // (misconfigured or partially registered host). Skip it to avoid inserting a broken + // or empty match entry into the ONTAP export rule. + if (ip == null || ip.isEmpty()) { + logger.warn("updateAccessGroup: Host {} has no storage/private IP, skipping export rule update", host.getName()); + continue; + } + hostMatches.add(ip + "/32"); + } + + // Occurs when every host in hostsToConnect had no valid IP (all were skipped above). + // There is nothing to add or remove, so skip the ONTAP API call and return early. + if (hostMatches.isEmpty()) { + accessGroup.setPolicy(existingPolicy); + return accessGroup; + } + + boolean updated = false; + // Differentiates between removing hosts (e.g., host decommissioned or removed from the cluster) + // and the default ADD path (e.g., new host being connected to the storage pool). + List exportClients = targetRule.getClients(); + // Existing rules can legitimately have no clients yet; treat that as an empty list. + if (exportClients == null) { + exportClients = new ArrayList<>(); + targetRule.setClients(exportClients); + } + + if (AccessGroup.HostRuleAction.REMOVE.equals(accessGroup.getHostRuleAction())) { + updated = exportClients.removeIf(c -> c != null && c.getMatch() != null && hostMatches.contains(c.getMatch())); + // None of the requested host IPs were present in the policy — log for diagnostics + // so operators can investigate whether the policy state is already correct or stale. + if (!updated) { + logger.info("updateAccessGroup: No matching host IPs found in export policy {} for removal", existingPolicy.getName()); + } + } else { + Set existingMatches = new HashSet<>(); + for (ExportRule.ExportClient exportClient : exportClients) { + // Skips null client entries or entries with a null match field that may have been + // inserted externally on ONTAP. Avoids polluting the dedup set with null values + // which would cause subsequent hosts to be incorrectly treated as duplicates. + if (exportClient != null && exportClient.getMatch() != null) { + existingMatches.add(exportClient.getMatch()); + } + } + + for (String match : hostMatches) { + // Set.add() returns false when the element was already present, acting as a dedup check. + // Prevents inserting a duplicate client match entry for a host that is already allowed + // in the export policy — ONTAP may reject or behave unpredictably with duplicate matches. + if (existingMatches.add(match)) { + ExportRule.ExportClient exportClient = new ExportRule.ExportClient(); + exportClient.setMatch(match); + exportClients.add(exportClient); + updated = true; + } + } + } + + // Occurs when the export policy is already in the desired state: + // ADD path — all provided host IPs were already present (all were duplicates). + // REMOVE path — none of the provided host IPs matched any existing entry. + // In both cases, skip the ONTAP PATCH call to avoid an unnecessary round-trip. + if (!updated) { + // Only log the "nothing to add" message for the ADD path; the REMOVE no-op + // is already logged above in its own branch to avoid double-logging. + if (!AccessGroup.HostRuleAction.REMOVE.equals(accessGroup.getHostRuleAction())) { + logger.info("updateAccessGroup: No new host IPs to add to export policy {}", existingPolicy.getName()); + } + accessGroup.setPolicy(existingPolicy); + return accessGroup; + } + + ExportPolicy updateRequest = new ExportPolicy(); + updateRequest.setRules(rules); + nasFeignClient.updateExportPolicy(authHeader, exportPolicyId, updateRequest); + + existingPolicy.setRules(rules); + accessGroup.setPolicy(existingPolicy); + logger.info("updateAccessGroup: Successfully updated export policy {} with new host client rules", existingPolicy.getName()); + return accessGroup; + } catch (Exception e) { + logger.error("updateAccessGroup: Failed to update export policy for pool {}", accessGroup.getStoragePoolId(), e); + throw new CloudRuntimeException("Failed to update export policy for NFS host connection: " + e.getMessage(), e); + } } @Override @@ -307,10 +539,10 @@ private ExportPolicy createExportPolicyRequest(AccessGroup accessGroup,String sv List exportClients = new ArrayList<>(); List hosts = accessGroup.getHostsToConnect(); for (HostVO host : hosts) { - String hostStorageIp = host.getStorageIpAddress(); + String hostStorageIp = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : null; String ip = (hostStorageIp != null && !hostStorageIp.isEmpty()) ? hostStorageIp - : host.getPrivateIpAddress(); + : (host.getPrivateIpAddress() != null ? host.getPrivateIpAddress().trim() : null); String ipToUse = ip + "/32"; ExportRule.ExportClient exportClient = new ExportRule.ExportClient(); exportClient.setMatch(ipToUse); @@ -409,6 +641,28 @@ private Answer deleteVolumeOnKVMHost(DataObject volumeInfo) { } } + /** + * Deletes a file from a FlexVolume, treating an already-absent file as success. + */ + public void deleteFileByPath(String flexVolUuid, String filePath) { + logger.info("deleteFileByPath: Deleting file [{}] from FlexVol [{}]", filePath, flexVolUuid); + try { + nasFeignClient.deleteFile(getAuthHeader(), flexVolUuid, filePath); + logger.debug("deleteFileByPath: Deleted file [{}]", filePath); + } catch (FeignException e) { + if (e.status() == 404) { + logger.warn("deleteFileByPath: File [{}] does not exist (status 404), skipping deletion", filePath); + return; + } + logger.error("FeignException occurred while deleting file [{}], Status: {}, Exception: {}", + filePath, e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to delete file: " + e.getMessage()); + } catch (Exception e) { + logger.error("Exception occurred while deleting file [{}], Exception: {}", filePath, e.getMessage()); + throw new CloudRuntimeException("Failed to delete file: " + e.getMessage()); + } + } + private FileInfo getFile(String volumeUuid, String filePath) { logger.info("Get File: {} for volume: {}", filePath, volumeUuid); 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 2b0e65f9f7ea..7fa993b9794c 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -29,6 +29,7 @@ import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunMap; +import org.apache.cloudstack.storage.feign.model.LunSpace; import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; @@ -124,8 +125,81 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } } + /** + * Clones a LUN inside the FlexVolume using ONTAP's {@code clone.source} form of LUN create. + * + *

The resulting LUN shares blocks with its source and is created in constant time. + * It is a sis-clone, so it stays readable after the source LUN is deleted.

+ */ @Override - public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) {} + public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) { + if (cloudstackVolume == null || cloudstackVolume.getLun() == null) { + logger.error("cloneCloudStackVolume: LUN clone failed. Invalid request: {}", cloudstackVolume); + throw new CloudRuntimeException("Failed to clone Lun, invalid request"); + } + Lun lunRequest = cloudstackVolume.getLun(); + if (lunRequest.getClone() == null || lunRequest.getClone().getSource() == null) { + logger.error("cloneCloudStackVolume: LUN clone failed. No clone source in request for Lun {}", lunRequest.getName()); + throw new CloudRuntimeException("Failed to clone Lun, no clone source provided"); + } + logger.trace("cloneCloudStackVolume: Cloning Lun {} from source {}", lunRequest.getName(), lunRequest.getClone().getSource().getUuid()); + try { + String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + OntapResponse clonedLun = sanFeignClient.createLun(authHeader, true, lunRequest); + if (clonedLun == null || CollectionUtils.isEmpty(clonedLun.getRecords())) { + logger.error("cloneCloudStackVolume: LUN clone returned no records for Lun {}", lunRequest.getName()); + throw new CloudRuntimeException("Failed to clone Lun: " + lunRequest.getName()); + } + Lun lun = clonedLun.getRecords().get(0); + logger.debug("cloneCloudStackVolume: LUN cloned successfully. Lun: {}", lun); + + CloudStackVolume clonedCloudStackVolume = new CloudStackVolume(); + clonedCloudStackVolume.setLun(lun); + return clonedCloudStackVolume; + } catch (FeignException e) { + logger.error("FeignException occurred while cloning LUN: {}, Status: {}, Exception: {}", + lunRequest.getName(), e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to clone Lun: " + e.getMessage()); + } catch (Exception e) { + logger.error("Exception occurred while cloning LUN: {}, Exception: {}", lunRequest.getName(), e.getMessage()); + throw new CloudRuntimeException("Failed to clone Lun: " + e.getMessage()); + } + } + + /** + * Grows an existing LUN to {@code sizeInBytes}. + * + *

Needed after cloning a cached template, because a clone inherits the size of its source + * while the service offering may ask for a larger disk.

+ */ + @Override + public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeInBytes) { + if (cloudstackVolume == null || cloudstackVolume.getLun() == null || cloudstackVolume.getLun().getUuid() == null) { + logger.error("resizeCloudStackVolume: Lun resize failed. Invalid request: {}", cloudstackVolume); + throw new CloudRuntimeException("Failed to resize Lun, invalid request"); + } + if (sizeInBytes <= 0) { + throw new CloudRuntimeException("Failed to resize Lun, invalid size " + sizeInBytes); + } + String lunUuid = cloudstackVolume.getLun().getUuid(); + logger.trace("resizeCloudStackVolume: Resizing Lun {} to {} bytes", lunUuid, sizeInBytes); + try { + String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + LunSpace lunSpace = new LunSpace(); + lunSpace.setSize(sizeInBytes); + Lun patch = new Lun(); + patch.setSpace(lunSpace); + sanFeignClient.updateLun(authHeader, lunUuid, patch); + logger.debug("resizeCloudStackVolume: Lun {} resized to {} bytes", lunUuid, sizeInBytes); + } catch (FeignException e) { + logger.error("FeignException occurred while resizing LUN: {}, Status: {}, Exception: {}", + lunUuid, e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to resize Lun: " + e.getMessage()); + } catch (Exception e) { + logger.error("Exception occurred while resizing LUN: {}, Exception: {}", lunUuid, e.getMessage()); + throw new CloudRuntimeException("Failed to resize Lun: " + e.getMessage()); + } + } @Override public CloudStackVolume getCloudStackVolume(Map values) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index e5224237e526..51205069ee2a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -109,4 +109,15 @@ public class OntapStorageConstants { /** vm_snapshot_details key for ONTAP FlexVolume-level VM snapshots. */ public static final String ONTAP_FLEXVOL_SNAPSHOT = "ontapFlexVolSnapshot"; + + /** Name prefix of the LUN that caches a template on the FlexVol, suffixed with the template id. */ + public static final String TEMPLATE_LUN_PREFIX = "cs_tmpl_"; + + /** + * Key of the {@code volume_details} row that {@code StorageSystemDataMotionStrategy} writes + * immediately before {@code createAsync} when a volume is to be cloned from a template already + * cached on this pool. The value is the CloudStack template id. The literal must stay in sync + * with the string used by the orchestrator. + */ + public static final String CLONE_OF_TEMPLATE = "cloneOfTemplate"; } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java index 8a74e77b3371..18b10f6fe76f 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java @@ -19,25 +19,23 @@ package org.apache.cloudstack.storage.utils; -import com.cloud.exception.InvalidParameterValueException; -import com.cloud.utils.StringUtils; -import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import java.nio.charset.StandardCharsets; +import java.util.Map; + +import feign.FeignException; 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.util.Base64Utils; -import java.nio.charset.StandardCharsets; -import java.util.Map; +import com.cloud.alert.AlertManager; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.StringUtils; +import com.cloud.utils.exception.CloudRuntimeException; public class OntapStorageUtils { @@ -57,48 +55,6 @@ public static String generateAuthHeader (String username, String password) { return BASIC + StringUtils.SPACE + new String(encodedBytes); } - public static CloudStackVolume createCloudStackVolumeRequestByProtocol(StoragePoolVO storagePool, Map details, DataObject volumeObject) { - CloudStackVolume cloudStackVolumeRequest = null; - - String protocol = details.get(OntapStorageConstants.PROTOCOL); - ProtocolType protocolType = ProtocolType.valueOf(protocol); - switch (protocolType) { - case NFS3: - cloudStackVolumeRequest = new CloudStackVolume(); - cloudStackVolumeRequest.setDatastoreId(String.valueOf(storagePool.getId())); - cloudStackVolumeRequest.setVolumeInfo(volumeObject); - break; - case ISCSI: - Svm svm = new Svm(); - svm.setName(details.get(OntapStorageConstants.SVM_NAME)); - cloudStackVolumeRequest = new CloudStackVolume(); - Lun lunRequest = new Lun(); - lunRequest.setSvm(svm); - - LunSpace lunSpace = new LunSpace(); - lunSpace.setSize(volumeObject.getSize()); - lunRequest.setSpace(lunSpace); - //Lun name is full path like in unified "/vol/VolumeName/LunName" - String lunName = volumeObject.getName().replace(OntapStorageConstants.HYPHEN, OntapStorageConstants.UNDERSCORE); - if(!isValidName(lunName)) { - String errMsg = "createAsync: Invalid dataObject name [" + lunName + "]. It must start with a letter and can only contain letters, digits, and underscores, and be up to 200 characters long."; - throw new InvalidParameterValueException(errMsg); - } - String lunFullName = getLunName(storagePool.getName(), lunName); - lunRequest.setName(lunFullName); - - String osType = getOSTypeFromHypervisor(storagePool.getHypervisor().name()); - lunRequest.setOsType(Lun.OsTypeEnum.valueOf(osType)); - - cloudStackVolumeRequest.setLun(lunRequest); - break; - default: - throw new CloudRuntimeException("Unsupported protocol " + protocol); - - } - return cloudStackVolumeRequest; - } - public static boolean isValidName(String name) { // Check for null and length constraint first if (name == null || name.length() > 200) { @@ -119,7 +75,22 @@ public static String getOSTypeFromHypervisor(String hypervisorType) { } } + public static void sendStorageAlert(AlertManager alertMgr, Long zoneId, Long podId, String subject, String body) { + if (alertMgr != null) { + alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC, zoneId != null ? zoneId : 0L, podId, subject, body); + } + } + + /** + * Returns a connected {@link StorageStrategy} for operations on an existing pool (snapshots, + * delete, revert, grant/revoke). Does not require aggregate free space for the full pool size. + */ public static StorageStrategy getStrategyByStoragePoolDetails(Map details) { + return getStrategyByStoragePoolDetails(details, false); + } + + public static StorageStrategy getStrategyByStoragePoolDetails(Map details, + boolean validateAggregatesForVolumeCreation) { if (details == null || details.isEmpty()) { logger.error("getStrategyByStoragePoolDetails: Storage pool details are null or empty"); throw new CloudRuntimeException("Storage pool details are null or empty"); @@ -129,7 +100,7 @@ public static StorageStrategy getStrategyByStoragePoolDetails(Map OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH) { + normalized = normalized.substring(0, OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH); + } + return normalized; + } + + /** + * Builds an ONTAP-safe snapshot name that preserves the CloudStack UI snapshot name + * and appends a uniqueness suffix. + */ + public static String buildOntapSnapshotName(String cloudStackSnapshotName, String uniquenessSuffix) { + String normalizedBase = (cloudStackSnapshotName == null || cloudStackSnapshotName.trim().isEmpty()) + ? "snapshot" + : getOntapSnapshotName(cloudStackSnapshotName); + String suffix = (uniquenessSuffix == null || uniquenessSuffix.isEmpty()) + ? "" + : "_" + uniquenessSuffix.replaceAll("[^a-zA-Z0-9_]", "_"); + int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH; + int maxBaseLength = maxLength - suffix.length(); + if (maxBaseLength <= 0) { + return normalizedBase.substring(0, maxLength); + } + if (normalizedBase.length() > maxBaseLength) { + normalizedBase = normalizedBase.substring(0, maxBaseLength); + } + return normalizedBase + suffix; + } + + /** + * Extracts a resource UUID from an ONTAP job description path. + * + *

Example: {@code POST /api/application/consistency-groups/{cg}/snapshots/{uuid}} + * with {@code pathSegment} {@code "/snapshots/"} returns the snapshot UUID.

+ */ + public static String extractUuidFromOntapJobDescription(String description, String pathSegment) { + if (description == null || pathSegment == null || pathSegment.isEmpty()) { + return null; + } + int idx = description.indexOf(pathSegment); + if (idx < 0) { + return null; + } + String remainder = description.substring(idx + pathSegment.length()).trim(); + if (remainder.isEmpty()) { + return null; + } + int queryIdx = remainder.indexOf('?'); + if (queryIdx >= 0) { + remainder = remainder.substring(0, queryIdx); + } + int slashIdx = remainder.indexOf('/'); + if (slashIdx >= 0) { + remainder = remainder.substring(0, slashIdx); + } + return remainder.isEmpty() ? null : remainder; + } + + /** + * Returns true when the exception indicates the ONTAP Object was already removed. + * Delete workflows treat a missing backend object as idempotent success. + */ + public static boolean isOntapObjectNotFoundError(Throwable error) { + if (error == null) { + return false; + } + if(error instanceof FeignException) { + FeignException feignException = (FeignException) error; + if (feignException.status() == 404) { + return true; + } + } + String message = error.getMessage(); + if (message != null) { + String lower = message.toLowerCase(); + if (lower.contains("404") || lower.contains("not found") || lower.contains("does not exist") + || lower.contains("entry doesn't exist")) { + return true; + } + } else { + logger.warn("Error message is null for exception: {}", error.getClass().getName()); + return false; + } + return false; + } + } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index 3c139e23cb88..88ce7c080039 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -21,15 +21,20 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.host.Host; import com.cloud.host.HostVO; +import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; +import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.command.CommandResult; @@ -38,6 +43,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.service.UnifiedNASStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -56,6 +62,7 @@ import java.util.HashMap; import java.util.Map; +import static com.cloud.agent.api.to.DataObjectType.TEMPLATE; import static com.cloud.agent.api.to.DataObjectType.VOLUME; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -64,10 +71,13 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; @@ -89,12 +99,21 @@ class OntapPrimaryDatastoreDriverTest { @Mock private VolumeDetailsDao volumeDetailsDao; + @Mock + private VMTemplatePoolDao vmTemplatePoolDao; + + @Mock + private VMTemplateStoragePoolVO templatePoolRef; + @Mock private DataStore dataStore; @Mock private VolumeInfo volumeInfo; + @Mock + private TemplateInfo templateInfo; + @Mock private StoragePoolVO storagePool; @@ -107,6 +126,9 @@ class OntapPrimaryDatastoreDriverTest { @Mock private UnifiedSANStrategy sanStrategy; + @Mock + private UnifiedNASStrategy nasStrategy; + @Mock private AsyncCompletionCallback createCallback; @@ -134,6 +156,8 @@ void testGetCapabilities() { // so StorageSystemSnapshotStrategy handles snapshot backup to secondary storage assertEquals(Boolean.TRUE.toString(), capabilities.get("STORAGE_SYSTEM_SNAPSHOT")); assertEquals(Boolean.TRUE.toString(), capabilities.get("CAN_CREATE_VOLUME_FROM_SNAPSHOT")); + assertEquals(Boolean.TRUE.toString(), capabilities.get("CAN_REVERT_VOLUME_TO_SNAPSHOT")); + assertEquals(Boolean.TRUE.toString(), capabilities.get("CAN_CREATE_VOLUME_FROM_VOLUME")); } @Test @@ -165,7 +189,9 @@ void testCreateAsync_VolumeWithISCSI_Success() { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(volumeDao.findById(100L)).thenReturn(volumeVO); @@ -174,18 +200,12 @@ void testCreateAsync_VolumeWithISCSI_Success() { Lun mockLun = new Lun(); mockLun.setName("/vol/vol1/lun1"); mockLun.setUuid("lun-uuid-123"); - // Create request volume (returned by Utility.createCloudStackVolumeRequestByProtocol) - CloudStackVolume requestVolume = new CloudStackVolume(); - requestVolume.setLun(mockLun); - // Create response volume (returned by sanStrategy.createCloudStackVolume) CloudStackVolume responseVolume = new CloudStackVolume(); responseVolume.setLun(mockLun); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) .thenReturn(sanStrategy); - utilityMock.when(() -> OntapStorageUtils.createCloudStackVolumeRequestByProtocol( - any(), any(), any())).thenReturn(requestVolume); when(sanStrategy.createCloudStackVolume(any())).thenReturn(responseVolume); // Execute @@ -201,6 +221,7 @@ void testCreateAsync_VolumeWithISCSI_Success() { verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_UUID), eq("lun-uuid-123"), eq(false)); verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_NAME), eq("/vol/vol1/lun1"), eq(false)); + verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2); verify(volumeDao).update(eq(100L), any(VolumeVO.class)); } } @@ -219,17 +240,16 @@ void testCreateAsync_VolumeWithNFS_Success() { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(volumeDao.findById(100L)).thenReturn(volumeVO); when(volumeVO.getId()).thenReturn(100L); CloudStackVolume mockCloudStackVolume = new CloudStackVolume(); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(sanStrategy); - utilityMock.when(() -> OntapStorageUtils.createCloudStackVolumeRequestByProtocol( - any(), any(), any())).thenReturn(mockCloudStackVolume); when(sanStrategy.createCloudStackVolume(any())).thenReturn(mockCloudStackVolume); @@ -243,6 +263,7 @@ void testCreateAsync_VolumeWithNFS_Success() { CreateCmdResult result = resultCaptor.getValue(); assertNotNull(result); assertTrue(result.isSuccess()); + verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2); verify(volumeDao).update(eq(100L), any(VolumeVO.class)); } } @@ -275,7 +296,7 @@ void testDeleteAsync_ISCSIVolume_Success() { when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_NAME)).thenReturn(lunNameDetail); when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)).thenReturn(lunUuidDetail); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(sanStrategy); @@ -318,6 +339,53 @@ void testDeleteAsync_NFSVolume_Success() { // NFS deletion doesn't fail, handled by hypervisor } + @Test + void testDeleteAsync_Template_DeletesCacheLun() { + when(dataStore.getId()).thenReturn(1L); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); + + driver.deleteAsync(dataStore, templateInfo, commandCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(commandCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); + verify(sanStrategy).deleteCloudStackVolume(requestCaptor.capture()); + assertEquals("template-lun-uuid", requestCaptor.getValue().getLun().getUuid()); + } + } + + @Test + void testDeleteAsync_Template_NoCachedLun_SucceedsWithoutCallingOntap() { + when(dataStore.getId()).thenReturn(1L); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(null); + + driver.deleteAsync(dataStore, templateInfo, commandCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(commandCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + verify(sanStrategy, never()).deleteCloudStackVolume(any()); + } + @Test void testGrantAccess_NullParameters_ThrowsException() { assertThrows(CloudRuntimeException.class, @@ -359,7 +427,7 @@ void testGrantAccess_ClusterScope_Success() { existingIgroup.setName("igroup1"); existingAccessGroup.setIgroup(existingIgroup); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(sanStrategy); utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())) @@ -410,7 +478,7 @@ void testGrantAccess_IgroupNotFound_CreatesNewIgroup() { createdIgroup.setName("igroup1"); createdAccessGroup.setIgroup(createdIgroup); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(sanStrategy); utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())) @@ -435,12 +503,12 @@ void testGrantAccess_IgroupNotFound_CreatesNewIgroup() { @Test void testRevokeAccess_NFSVolume_SkipsRevoke() { // Setup - NFS volumes have no LUN mapping, so revokeAccess is a no-op + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); when(dataStore.getId()).thenReturn(1L); when(volumeInfo.getType()).thenReturn(VOLUME); when(volumeInfo.getId()).thenReturn(100L); when(volumeDao.findById(100L)).thenReturn(volumeVO); - when(volumeVO.getId()).thenReturn(100L); when(volumeVO.getName()).thenReturn("test-volume"); when(storagePoolDao.findById(1L)).thenReturn(storagePool); @@ -449,7 +517,7 @@ void testRevokeAccess_NFSVolume_SkipsRevoke() { when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(host.getName()).thenReturn("host1"); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(sanStrategy); @@ -496,7 +564,7 @@ void testRevokeAccess_ISCSIVolume_Success() { AccessGroup mockAccessGroup = new AccessGroup(); mockAccessGroup.setIgroup(mockIgroup); - try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(sanStrategy); utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())) @@ -538,6 +606,354 @@ void testRevokeAccess_ISCSIVolume_Success() { } } + @Test + void testGetDataObjectSizeIncludingHypervisorSnapshotReserve_NoReserveAdded() { + when(templateInfo.getSize()).thenReturn(5368709120L); + + assertEquals(5368709120L, driver.getDataObjectSizeIncludingHypervisorSnapshotReserve(templateInfo, storagePool)); + } + + @Test + void testGetBytesRequiredForTemplate_AlreadyCached_ReturnsZero() { + when(storagePool.getId()).thenReturn(1L); + when(templateInfo.getId()).thenReturn(50L); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + + assertEquals(0L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); + } + + @Test + void testGetBytesRequiredForTemplate_NotCached_ReturnsVirtualSize() { + when(storagePool.getId()).thenReturn(1L); + when(templateInfo.getId()).thenReturn(50L); + when(templateInfo.getSize()).thenReturn(5368709120L); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(null); + + assertEquals(5368709120L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); + } + + @Test + void testCreateAsync_TemplateWithISCSI_CreatesLunAndRecordsCloneSource() { + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + when(templateInfo.getSize()).thenReturn(5368709120L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getId()).thenReturn(7L); + + Lun templateLun = new Lun(); + templateLun.setName("/vol/vol1/cs_tmpl_50"); + templateLun.setUuid("template-lun-uuid"); + CloudStackVolume created = new CloudStackVolume(); + created.setLun(templateLun); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.createCloudStackVolume(any())).thenReturn(created); + + driver.createAsync(dataStore, templateInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + // local_download_path carries the clone source; install_path is left for grantAccess + verify(templatePoolRef).setLocalDownloadPath("template-lun-uuid"); + verify(templatePoolRef).setTemplateSize(5368709120L); + verify(templatePoolRef, never()).setInstallPath(any()); + verify(vmTemplatePoolDao).update(eq(7L), any(VMTemplateStoragePoolVO.class)); + } + } + + @Test + void testCreateAsync_TemplateWithISCSI_UnknownSize_Fails() { + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + when(templateInfo.getSize()).thenReturn(0L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + + driver.createAsync(dataStore, templateInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + } + + @Test + void testCreateAsync_TemplateWithNFS_IsMetadataOnly() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getUuid()).thenReturn("template-uuid"); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + driver.createAsync(dataStore, templateInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + verify(vmTemplatePoolDao, never()).update(any(Long.class), any(VMTemplateStoragePoolVO.class)); + } + + @Test + void testCreateAsync_VolumeClonedFromTemplate_ClonesWithoutGrowing() { + stubVolumeCloneFromTemplate(5368709120L, 5368709120L); + + Lun clonedLun = new Lun(); + clonedLun.setName("/vol/vol1/test_volume"); + clonedLun.setUuid("cloned-lun-uuid"); + CloudStackVolume cloned = new CloudStackVolume(); + cloned.setLun(clonedLun); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.cloneCloudStackVolume(any())).thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); + verify(sanStrategy).cloneCloudStackVolume(requestCaptor.capture()); + assertEquals("template-lun-uuid", requestCaptor.getValue().getLun().getClone().getSource().getUuid()); + verify(sanStrategy, never()).createCloudStackVolume(any()); + verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); + verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_UUID), eq("cloned-lun-uuid"), eq(false)); + } + } + + @Test + void testCreateAsync_VolumeClonedFromTemplate_GrowsWhenOfferingIsLarger() { + stubVolumeCloneFromTemplate(5368709120L, 21474836480L); + + Lun clonedLun = new Lun(); + clonedLun.setName("/vol/vol1/test_volume"); + clonedLun.setUuid("cloned-lun-uuid"); + CloudStackVolume cloned = new CloudStackVolume(); + cloned.setLun(clonedLun); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.cloneCloudStackVolume(any())).thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + verify(sanStrategy).resizeCloudStackVolume(eq(cloned), eq(21474836480L)); + } + } + + /** + * Sets up a volume create that the orchestrator has marked as a clone of a cached template. + */ + private void stubVolumeCloneFromTemplate(long templateSize, long volumeSize) { + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(volumeInfo.getType()).thenReturn(VOLUME); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getName()).thenReturn("test-volume"); + when(volumeInfo.getSize()).thenReturn(volumeSize); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + lenient().when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.Iscsi); + when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getId()).thenReturn(100L); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.CLONE_OF_TEMPLATE)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.CLONE_OF_TEMPLATE, "50", false)); + + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + lenient().when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); + when(templatePoolRef.getTemplateSize()).thenReturn(templateSize); + } + + @Test + void testGrantAccess_Template_WritesInstallPathAndRefreshesStoreTarget() { + PrimaryDataStore primaryDataStore = mock(PrimaryDataStore.class); + Map dataStoreDetails = new HashMap<>(); + dataStoreDetails.put(PrimaryDataStore.MANAGED_STORE_TARGET, "stale-value"); + + when(primaryDataStore.getId()).thenReturn(1L); + when(primaryDataStore.getDetails()).thenReturn(dataStoreDetails); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER); + when(storagePool.getPath()).thenReturn("iqn.1992-08.com.netapp:sn.123456"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getId()).thenReturn(7L); + + when(host.getName()).thenReturn("host1"); + when(host.getUuid()).thenReturn("host-uuid-1"); + + AccessGroup existingAccessGroup = new AccessGroup(); + Igroup existingIgroup = new Igroup(); + existingIgroup.setName("igroup1"); + existingAccessGroup.setIgroup(existingIgroup); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); + utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); + + when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup); + when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); + + assertTrue(driver.grantAccess(templateInfo, host, primaryDataStore)); + + String expectedPath = "/iqn.1992-08.com.netapp:sn.123456/3"; + verify(templatePoolRef).setInstallPath(expectedPath); + verify(vmTemplatePoolDao).update(eq(7L), any(VMTemplateStoragePoolVO.class)); + + ArgumentCaptor> detailsCaptor = ArgumentCaptor.forClass(Map.class); + verify(primaryDataStore).setDetails(detailsCaptor.capture()); + assertEquals(expectedPath, detailsCaptor.getValue().get(PrimaryDataStore.MANAGED_STORE_TARGET)); + } + } + + @Test + void testGrantAccess_TemplateOnNFS_SkipsMapping() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + + when(dataStore.getId()).thenReturn(1L); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getUuid()).thenReturn("template-uuid"); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + assertTrue(driver.grantAccess(templateInfo, host, dataStore)); + verify(vmTemplatePoolDao, never()).update(any(Long.class), any(VMTemplateStoragePoolVO.class)); + } + + @Test + void testCreateAsync_VolumeClonedFromTemplateNFS_ClonesFile() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + stubVolumeCloneFromTemplate(5368709120L, 5368709120L); + when(volumeInfo.getUuid()).thenReturn("volume-uuid"); + when(templatePoolRef.getInstallPath()).thenReturn("template-uuid"); + + CloudStackVolume cloned = new CloudStackVolume(); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(nasStrategy); + when(nasStrategy.cloneCloudStackVolume(any())).thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); + verify(nasStrategy).cloneCloudStackVolume(requestCaptor.capture()); + assertEquals("template-uuid", requestCaptor.getValue().getFile().getPath()); + assertEquals("volume-uuid", requestCaptor.getValue().getDestinationPath()); + verify(nasStrategy, never()).resizeCloudStackVolume(any(), anyLong()); + } + } + + @Test + void testDeleteAsync_Template_NFS_DeletesCachedFile() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid"); + + when(dataStore.getId()).thenReturn(1L); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getInstallPath()).thenReturn("template-uuid"); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(nasStrategy); + + driver.deleteAsync(dataStore, templateInfo, commandCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(commandCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + verify(nasStrategy).deleteFileByPath("flexvol-uuid", "template-uuid"); + } + } + + @Test + void testRevokeAccess_Template_UnmapsCacheLun() { + when(dataStore.getId()).thenReturn(1L); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + when(host.getStorageUrl()).thenReturn("iqn.1993-08.org.debian:01:host1"); + when(host.getName()).thenReturn("host1"); + when(host.getUuid()).thenReturn("host-uuid-1"); + + Lun templateLun = new Lun(); + templateLun.setName("/vol/vol1/cs_tmpl_50"); + templateLun.setUuid("template-lun-uuid"); + CloudStackVolume cachedTemplate = new CloudStackVolume(); + cachedTemplate.setLun(templateLun); + + Igroup igroup = mock(Igroup.class); + when(igroup.getName()).thenReturn("igroup1"); + when(igroup.getUuid()).thenReturn("igroup-uuid-123"); + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setIgroup(igroup); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); + utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); + + when(sanStrategy.getCloudStackVolume(argThat(map -> + map != null && "/vol/vol1/cs_tmpl_50".equals(map.get("name"))))) + .thenReturn(cachedTemplate); + when(sanStrategy.getAccessGroup(any())).thenReturn(accessGroup); + when(sanStrategy.validateInitiatorInAccessGroup(anyString(), anyString(), any(Igroup.class))).thenReturn(true); + + driver.revokeAccess(templateInfo, host, dataStore); + + verify(sanStrategy).disableLogicalAccess(argThat(map -> + map != null && "template-lun-uuid".equals(map.get("lun.uuid")) + && "igroup-uuid-123".equals(map.get("igroup.uuid")))); + } + } + @Test void testCanHostAccessStoragePool_ReturnsTrue() { assertTrue(driver.canHostAccessStoragePool(host, storagePool)); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java index df9afe2542f9..cf8a3b1cd504 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java @@ -134,8 +134,12 @@ public void deleteCloudStackVolume(CloudStackVolume cloudstackVolume) { } @Override - public void copyCloudStackVolume(CloudStackVolume cloudstackVolume) { + public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) { + return null; + } + @Override + public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeInBytes) { } @Override diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java index c4d5ddf6878c..c945490ac2fc 100755 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java @@ -20,6 +20,7 @@ package org.apache.cloudstack.storage.service; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.storage.ResizeVolumeCommand; import com.cloud.host.HostVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VolumeDao; @@ -28,7 +29,9 @@ import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.command.CreateObjectCommand; +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.client.JobFeignClient; import org.apache.cloudstack.storage.feign.client.NASFeignClient; import org.apache.cloudstack.storage.feign.client.VolumeFeignClient; @@ -37,6 +40,9 @@ import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.ExportPolicy; +import org.apache.cloudstack.storage.feign.model.ExportRule; +import org.apache.cloudstack.storage.feign.model.FileCloneRequest; +import org.apache.cloudstack.storage.feign.model.FileInfo; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.response.JobResponse; @@ -63,6 +69,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -72,10 +79,13 @@ import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import feign.FeignException; + @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) public class UnifiedNASStrategyTest { @@ -110,6 +120,9 @@ public class UnifiedNASStrategyTest { @Mock private StoragePoolDetailsDao storagePoolDetailsDao; + @Mock + private PrimaryDataStoreDao primaryDataStoreDao; + private TestableUnifiedNASStrategy strategy; private OntapStorage ontapStorage; @@ -128,6 +141,7 @@ public void setUp() throws Exception { injectField("volumeDao", volumeDao); injectField("epSelector", epSelector); injectField("storagePoolDetailsDao", storagePoolDetailsDao); + injectField("primaryDataStoreDao", primaryDataStoreDao); } private void injectField(String fieldName, Object mockedField) throws Exception { @@ -513,6 +527,26 @@ public void testDeleteAccessGroup_Failed() { }); } + // Test deleteAccessGroup - Export policy not found should be treated as no-op + @Test + public void testDeleteAccessGroup_NotFound404_NoThrow() { + AccessGroup accessGroup = mock(AccessGroup.class); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.EXPORT_POLICY_NAME, "export-policy-1"); + details.put(OntapStorageConstants.EXPORT_POLICY_ID, "1"); + + when(accessGroup.getStoragePoolId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(details); + + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(404); + doThrow(feignException).when(nasFeignClient).deleteExportPolicyById(anyString(), eq("1")); + + strategy.deleteAccessGroup(accessGroup); + + verify(nasFeignClient).deleteExportPolicyById(anyString(), eq("1")); + } + // Test deleteCloudStackVolume - Success @Test public void testDeleteCloudStackVolume_Success() throws Exception { @@ -582,4 +616,427 @@ public void testDeleteCloudStackVolume_AnswerNull() throws Exception { strategy.deleteCloudStackVolume(cloudStackVolume); }); } + + // ------------------------------------------------------------------------- + // updateAccessGroup tests + // ------------------------------------------------------------------------- + + private Map detailsWithExportPolicyId() { + Map details = new HashMap<>(); + details.put(OntapStorageConstants.EXPORT_POLICY_ID, "policy-42"); + return details; + } + + private ExportPolicy existingPolicyWithClients(String... matchIps) { + ExportRule rule = new ExportRule(); + List clients = new ArrayList<>(); + for (String ip : matchIps) { + ExportRule.ExportClient client = new ExportRule.ExportClient(); + client.setMatch(ip); + clients.add(client); + } + rule.setClients(clients); + ExportPolicy policy = new ExportPolicy(); + policy.setName("test-policy"); + policy.setRules(new ArrayList<>(List.of(rule))); + return policy; + } + + // updateAccessGroup - null accessGroup + @Test + public void testUpdateAccessGroup_NullAccessGroup() { + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(null)); + } + + // updateAccessGroup - null storagePoolId + @Test + public void testUpdateAccessGroup_NullStoragePoolId() { + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setHostsToConnect(List.of(mock(HostVO.class))); + // storagePoolId is null by default + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + + // updateAccessGroup - null hostsToConnect + @Test + public void testUpdateAccessGroup_NullHostsToConnect() { + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + // hostsToConnect is null by default + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + + // updateAccessGroup - empty hostsToConnect + @Test + public void testUpdateAccessGroup_EmptyHostsToConnect() { + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(new ArrayList<>()); + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + + // updateAccessGroup - storagePoolDetailsDao returns null + @Test + public void testUpdateAccessGroup_NoStoragePoolDetails() { + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(mock(HostVO.class))); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(null); + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + + // updateAccessGroup - details missing EXPORT_POLICY_ID key + @Test + public void testUpdateAccessGroup_MissingExportPolicyId() { + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(mock(HostVO.class))); + Map details = new HashMap<>(); + details.put("someOtherKey", "someValue"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(details); + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + + // updateAccessGroup - getExportPolicyById returns null + @Test + public void testUpdateAccessGroup_ExportPolicyNotFound() { + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(mock(HostVO.class))); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(null); + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + + // updateAccessGroup - existing policy has null rules + @Test + public void testUpdateAccessGroup_NullRules() { + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(mock(HostVO.class))); + ExportPolicy policy = new ExportPolicy(); + policy.setName("test-policy"); + policy.setRules(null); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(policy); + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + + // updateAccessGroup - existing policy has empty rules list + @Test + public void testUpdateAccessGroup_EmptyRules() { + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(mock(HostVO.class))); + ExportPolicy policy = new ExportPolicy(); + policy.setName("test-policy"); + policy.setRules(new ArrayList<>()); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(policy); + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + + // updateAccessGroup - all hosts have no IP: returns early without ONTAP patch + @Test + public void testUpdateAccessGroup_AllHostsHaveNoIp_ReturnsEarly() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn(null); + when(host.getPrivateIpAddress()).thenReturn(null); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + + ExportPolicy existingPolicy = existingPolicyWithClients("10.0.0.1/32"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(existingPolicy); + + AccessGroup result = strategy.updateAccessGroup(accessGroup); + + assertNotNull(result); + assertSame(existingPolicy, result.getPolicy()); + verify(nasFeignClient, never()).updateExportPolicy(anyString(), anyString(), any()); + } + + // updateAccessGroup - ADD: new host IP added to policy + @Test + public void testUpdateAccessGroup_Add_NewHost_Success() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn("10.0.0.2"); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + // default action is ADD + + ExportPolicy existingPolicy = existingPolicyWithClients("10.0.0.1/32"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(existingPolicy); + + AccessGroup result = strategy.updateAccessGroup(accessGroup); + + assertNotNull(result); + assertSame(existingPolicy, result.getPolicy()); + // Existing client + new client = 2 + assertEquals(2, existingPolicy.getRules().get(0).getClients().size()); + verify(nasFeignClient).updateExportPolicy(anyString(), eq("policy-42"), any(ExportPolicy.class)); + } + + // updateAccessGroup - ADD: host uses private IP when storage IP is absent + @Test + public void testUpdateAccessGroup_Add_UsesPrivateIpWhenStorageIpAbsent() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn(null); + when(host.getPrivateIpAddress()).thenReturn("192.168.1.50"); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + + ExportPolicy existingPolicy = existingPolicyWithClients(); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(existingPolicy); + + AccessGroup result = strategy.updateAccessGroup(accessGroup); + + assertNotNull(result); + List clients = existingPolicy.getRules().get(0).getClients(); + assertEquals(1, clients.size()); + assertEquals("192.168.1.50/32", clients.get(0).getMatch()); + verify(nasFeignClient).updateExportPolicy(anyString(), eq("policy-42"), any(ExportPolicy.class)); + } + + // updateAccessGroup - ADD: host IP already present in policy (no-op) + @Test + public void testUpdateAccessGroup_Add_DuplicateHost_NoUpdate() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn("10.0.0.1"); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + + ExportPolicy existingPolicy = existingPolicyWithClients("10.0.0.1/32"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(existingPolicy); + + AccessGroup result = strategy.updateAccessGroup(accessGroup); + + assertNotNull(result); + assertSame(existingPolicy, result.getPolicy()); + // Client count must remain 1 (no duplicate inserted) + assertEquals(1, existingPolicy.getRules().get(0).getClients().size()); + verify(nasFeignClient, never()).updateExportPolicy(anyString(), anyString(), any()); + } + + // updateAccessGroup - ADD: existing rule has null clients list + @Test + public void testUpdateAccessGroup_Add_NullClientsInRule() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn("10.0.0.5"); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + + ExportRule rule = new ExportRule(); + rule.setClients(null); // null clients list + ExportPolicy policy = new ExportPolicy(); + policy.setName("test-policy"); + policy.setRules(new ArrayList<>(List.of(rule))); + + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(policy); + + AccessGroup result = strategy.updateAccessGroup(accessGroup); + + assertNotNull(result); + assertEquals(1, rule.getClients().size()); + assertEquals("10.0.0.5/32", rule.getClients().get(0).getMatch()); + verify(nasFeignClient).updateExportPolicy(anyString(), eq("policy-42"), any(ExportPolicy.class)); + } + + // updateAccessGroup - REMOVE: matching host IP removed from policy + @Test + public void testUpdateAccessGroup_Remove_MatchingHost_Success() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn("10.0.0.1"); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + accessGroup.setHostRuleAction(AccessGroup.HostRuleAction.REMOVE); + + ExportPolicy existingPolicy = existingPolicyWithClients("10.0.0.1/32", "10.0.0.2/32"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(existingPolicy); + + AccessGroup result = strategy.updateAccessGroup(accessGroup); + + assertNotNull(result); + assertSame(existingPolicy, result.getPolicy()); + // Only 10.0.0.2/32 should remain + List clients = existingPolicy.getRules().get(0).getClients(); + assertEquals(1, clients.size()); + assertEquals("10.0.0.2/32", clients.get(0).getMatch()); + verify(nasFeignClient).updateExportPolicy(anyString(), eq("policy-42"), any(ExportPolicy.class)); + } + + // updateAccessGroup - REMOVE: IP not in policy (no-op) + @Test + public void testUpdateAccessGroup_Remove_IpNotPresent_NoUpdate() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn("10.0.0.99"); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + accessGroup.setHostRuleAction(AccessGroup.HostRuleAction.REMOVE); + + ExportPolicy existingPolicy = existingPolicyWithClients("10.0.0.1/32"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(existingPolicy); + + AccessGroup result = strategy.updateAccessGroup(accessGroup); + + assertNotNull(result); + assertSame(existingPolicy, result.getPolicy()); + assertEquals(1, existingPolicy.getRules().get(0).getClients().size()); + verify(nasFeignClient, never()).updateExportPolicy(anyString(), anyString(), any()); + } + + // updateAccessGroup - FeignException from ONTAP wrapped in CloudRuntimeException + @Test + public void testUpdateAccessGroup_FeignExceptionWrapped() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn("10.0.0.1"); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))) + .thenThrow(new RuntimeException("ONTAP unreachable")); + + assertThrows(CloudRuntimeException.class, () -> strategy.updateAccessGroup(accessGroup)); + } + // updateAccessGroup - whitespace in storage IP is trimmed before building match + @Test + public void testUpdateAccessGroup_TrimsWhitespaceFromStorageIp() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn(" 10.0.0.2 "); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + + ExportPolicy existingPolicy = existingPolicyWithClients(); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(existingPolicy); + + strategy.updateAccessGroup(accessGroup); + + List clients = existingPolicy.getRules().get(0).getClients(); + assertEquals(1, clients.size()); + assertEquals("10.0.0.2/32", clients.get(0).getMatch()); + } + + // updateAccessGroup - whitespace in private IP is trimmed when storage IP absent + @Test + public void testUpdateAccessGroup_TrimsWhitespaceFromPrivateIp() { + HostVO host = mock(HostVO.class); + when(host.getStorageIpAddress()).thenReturn(null); + when(host.getPrivateIpAddress()).thenReturn(" 192.168.1.10 "); + + AccessGroup accessGroup = new AccessGroup(); + accessGroup.setStoragePoolId(1L); + accessGroup.setHostsToConnect(List.of(host)); + + ExportPolicy existingPolicy = existingPolicyWithClients(); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(detailsWithExportPolicyId()); + when(nasFeignClient.getExportPolicyById(anyString(), eq("policy-42"))).thenReturn(existingPolicy); + + strategy.updateAccessGroup(accessGroup); + + List clients = existingPolicy.getRules().get(0).getClients(); + assertEquals(1, clients.size()); + assertEquals("192.168.1.10/32", clients.get(0).getMatch()); + } + + @Test + public void testCloneCloudStackVolume_Success() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + when(volumeObject.getId()).thenReturn(100L); + when(volumeObject.getUuid()).thenReturn("volume-uuid"); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeDao.update(anyLong(), any(VolumeVO.class))).thenReturn(true); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(details); + + FileInfo source = new FileInfo(); + source.setPath("template-uuid"); + CloudStackVolume request = new CloudStackVolume(); + request.setDatastoreId("1"); + request.setVolumeInfo(volumeObject); + request.setFile(source); + request.setDestinationPath("volume-uuid"); + + when(nasFeignClient.cloneFile(anyString(), any(FileCloneRequest.class))).thenReturn(new JobResponse()); + + CloudStackVolume result = strategy.cloneCloudStackVolume(request); + + assertNotNull(result); + assertEquals("volume-uuid", result.getFile().getPath()); + ArgumentCaptor captor = ArgumentCaptor.forClass(FileCloneRequest.class); + verify(nasFeignClient).cloneFile(anyString(), captor.capture()); + assertEquals("template-uuid", captor.getValue().getSourcePath()); + assertEquals("volume-uuid", captor.getValue().getDestinationPath()); + assertEquals("flexvol1", captor.getValue().getVolume().getName()); + } + + @Test + public void testCloneCloudStackVolume_InvalidRequest_ThrowsException() { + assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolume(null)); + } + + @Test + public void testResizeCloudStackVolume_SendsResizeCommand() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + EndPoint endPoint = mock(EndPoint.class); + + when(volumeObject.getId()).thenReturn(100L); + when(volumeObject.getUuid()).thenReturn("volume-uuid"); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPath()).thenReturn("volume-uuid"); + when(volumeVO.getSize()).thenReturn(5368709120L); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(storagePool); + when(epSelector.select(volumeObject)).thenReturn(endPoint); + when(endPoint.sendMessage(any(ResizeVolumeCommand.class))).thenReturn(new Answer(null, true, "Success")); + + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + strategy.resizeCloudStackVolume(request, 21474836480L); + + verify(endPoint).sendMessage(any(ResizeVolumeCommand.class)); + } + + @Test + public void testDeleteFileByPath_Treats404AsSuccess() { + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(404); + doThrow(feignException).when(nasFeignClient).deleteFile(anyString(), eq("flexvol-uuid"), eq("template-uuid")); + + strategy.deleteFileByPath("flexvol-uuid", "template-uuid"); + + verify(nasFeignClient).deleteFile(anyString(), eq("flexvol-uuid"), eq("template-uuid")); + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index ec9023a6c760..53e8ef1cc429 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -789,29 +789,78 @@ void testValidateInitiatorInAccessGroup_IgroupNotFound_ReturnsFalse() { } @Test - void testCopyCloudStackVolume_NullRequest_DoesNotThrow() { - // copyCloudStackVolume is not yet implemented (no-op), so it should not throw - assertDoesNotThrow(() -> unifiedSANStrategy.copyCloudStackVolume(null)); + void testCloneCloudStackVolume_Success() { + Lun.Source source = new Lun.Source(); + source.setUuid("source-lun-uuid"); + Lun.Clone clone = new Lun.Clone(); + clone.setSource(source); + Lun lun = new Lun(); + lun.setName("/vol/vol1/cloned"); + lun.setClone(clone); + + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + Lun clonedLun = new Lun(); + clonedLun.setName("/vol/vol1/cloned"); + clonedLun.setUuid("cloned-lun-uuid"); + + OntapResponse response = new OntapResponse<>(); + response.setRecords(List.of(clonedLun)); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))) + .thenReturn(response); + + CloudStackVolume result = unifiedSANStrategy.cloneCloudStackVolume(request); + + assertNotNull(result); + assertEquals("cloned-lun-uuid", result.getLun().getUuid()); + verify(sanFeignClient).createLun(eq(authHeader), eq(true), any(Lun.class)); + } + } + + @Test + void testCloneCloudStackVolume_NullRequest_ThrowsException() { + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolume(null)); } @Test - void testCopyCloudStackVolume_NullLun_DoesNotThrow() { - // copyCloudStackVolume is not yet implemented (no-op), so it should not throw + void testCloneCloudStackVolume_MissingSource_ThrowsException() { + Lun lun = new Lun(); + lun.setName("/vol/vol1/cloned"); CloudStackVolume request = new CloudStackVolume(); - request.setLun(null); + request.setLun(lun); - assertDoesNotThrow(() -> unifiedSANStrategy.copyCloudStackVolume(request)); + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolume(request)); } @Test - void testCopyCloudStackVolume_ValidRequest_DoesNotThrow() { - // copyCloudStackVolume is not yet implemented (no-op), so it should not throw + void testResizeCloudStackVolume_ValidRequest_PatchesSize() { Lun lun = new Lun(); - lun.setName("/vol/vol1/lun1"); + lun.setUuid("lun-uuid-123"); CloudStackVolume request = new CloudStackVolume(); request.setLun(lun); - assertDoesNotThrow(() -> unifiedSANStrategy.copyCloudStackVolume(request)); + unifiedSANStrategy.resizeCloudStackVolume(request, 21474836480L); + + ArgumentCaptor lunCaptor = ArgumentCaptor.forClass(Lun.class); + verify(sanFeignClient).updateLun(any(), eq("lun-uuid-123"), lunCaptor.capture()); + assertEquals(21474836480L, lunCaptor.getValue().getSpace().getSize()); + } + + @Test + void testResizeCloudStackVolume_NoUuid_Throws() { + CloudStackVolume request = new CloudStackVolume(); + request.setLun(new Lun()); + + assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.resizeCloudStackVolume(request, 100L)); + verify(sanFeignClient, never()).updateLun(any(), any(), any()); } @Test From bb2ac37e165ea84d6a9762d984e5dae7ff9e07ce Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 24 Aug 2026 11:35:02 +0530 Subject: [PATCH 02/15] CSTACKEX-259: Fileclone pojo has correction --- .../storage/feign/model/FileCloneRequest.java | 33 +++++++++---------- .../storage/service/UnifiedNASStrategy.java | 7 ++-- .../service/UnifiedNASStrategyTest.java | 2 ++ 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java index 7ac5214f986c..a9f2a106e9a8 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java @@ -34,9 +34,6 @@ @JsonInclude(JsonInclude.Include.NON_NULL) public class FileCloneRequest { - @JsonProperty("svm") - private Svm svm; - @JsonProperty("volume") private VolumeRef volume; @@ -52,22 +49,12 @@ public class FileCloneRequest { public FileCloneRequest() { } - public FileCloneRequest(String svmName, String flexVolName, String sourcePath, String destinationPath) { - this.svm = new Svm(); - this.svm.setName(svmName); - this.volume = new VolumeRef(flexVolName); + public FileCloneRequest(String flexVolUuid, String flexVolName, String sourcePath, String destinationPath) { + this.volume = new VolumeRef(flexVolUuid, flexVolName); this.sourcePath = sourcePath; this.destinationPath = destinationPath; } - public Svm getSvm() { - return svm; - } - - public void setSvm(Svm svm) { - this.svm = svm; - } - public VolumeRef getVolume() { return volume; } @@ -104,16 +91,28 @@ public void setOverwriteDestination(Boolean overwriteDestination) { @JsonInclude(JsonInclude.Include.NON_NULL) public static class VolumeRef { + @JsonProperty("uuid") + private String uuid; + @JsonProperty("name") private String name; public VolumeRef() { } - public VolumeRef(String name) { + public VolumeRef(String uuid, String name) { + this.uuid = uuid; this.name = name; } + public String getUuid() { + return uuid; + } + + public void setUuid(String uuid) { + this.uuid = uuid; + } + public String getName() { return name; } @@ -125,7 +124,7 @@ public void setName(String name) { @Override public String toString() { - return "FileCloneRequest{volume=" + (volume != null ? volume.getName() : null) + return "FileCloneRequest{volume=" + (volume != null ? volume.getUuid() : null) + ", sourcePath=" + sourcePath + ", destinationPath=" + destinationPath + "}"; } 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 98656c7d8be2..83b6b58a7fa9 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 @@ -141,14 +141,17 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) } Map details = storagePoolDetailsDao.listDetailsKeyPairs(Long.parseLong(cloudstackVolume.getDatastoreId())); - String svmName = details.get(OntapStorageConstants.SVM_NAME); + String flexVolUuid = details.get(OntapStorageConstants.VOLUME_UUID); String flexVolName = details.get(OntapStorageConstants.VOLUME_NAME); + if (flexVolUuid == null || flexVolUuid.isEmpty()) { + throw new CloudRuntimeException("Failed to clone file, FlexVolume uuid is missing from pool details"); + } String sourcePath = cloudstackVolume.getFile().getPath(); String destinationPath = cloudstackVolume.getDestinationPath(); logger.info("cloneCloudStackVolume: Cloning file [{}] to [{}] in FlexVol [{}]", sourcePath, destinationPath, flexVolName); try { - FileCloneRequest request = new FileCloneRequest(svmName, flexVolName, sourcePath, destinationPath); + FileCloneRequest request = new FileCloneRequest(flexVolUuid, flexVolName, sourcePath, destinationPath); JobResponse jobResponse = nasFeignClient.cloneFile(getAuthHeader(), request); pollJobIfPresent(jobResponse, "clone file [" + sourcePath + "] to [" + destinationPath + "]"); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java index c945490ac2fc..85c9c04a0294 100755 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java @@ -976,6 +976,7 @@ public void testCloneCloudStackVolume_Success() { Map details = new HashMap<>(); details.put(OntapStorageConstants.SVM_NAME, "svm1"); details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + details.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(details); FileInfo source = new FileInfo(); @@ -997,6 +998,7 @@ public void testCloneCloudStackVolume_Success() { assertEquals("template-uuid", captor.getValue().getSourcePath()); assertEquals("volume-uuid", captor.getValue().getDestinationPath()); assertEquals("flexvol1", captor.getValue().getVolume().getName()); + assertEquals("flexvol-uuid-1", captor.getValue().getVolume().getUuid()); } @Test From 8b4fd0c17abd2531fb3cafb499c7491f12de66e2 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 25 Aug 2026 18:54:58 +0530 Subject: [PATCH 03/15] CSTACKEX-259: setting name for the LUN template --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 8 +++++++- .../org/apache/cloudstack/storage/feign/model/Lun.java | 2 ++ .../cloudstack/storage/service/UnifiedSANStrategy.java | 5 ++++- .../storage/driver/OntapPrimaryDatastoreDriverTest.java | 1 + 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index b9eaeb2d5424..7be7c288f70c 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 @@ -1357,7 +1357,12 @@ private CloudStackVolume createTemplateLunRequest(StoragePoolVO storagePool, Map } /** - * Builds the request that clones {@code local_download_path} (LUN uuid) into a new LUN. + * Builds the request that clones the cached template LUN into a new volume LUN. + * + *

Source identity mirrors the NFS file-clone path workflow: {@code clone.source.name} is + * the same deterministic ONTAP path used at template create + * ({@code /vol//cs_tmpl_}). {@code local_download_path} (LUN uuid) is + * still sent as a secondary identity.

* *

Size is omitted: ONTAP rejects a size on a clone create, and the clone inherits the * source size. Growing to the requested volume size is a separate PATCH.

@@ -1381,6 +1386,7 @@ private CloudStackVolume createCloneLunRequest(StoragePoolVO storagePool, Map clonedLun = sanFeignClient.createLun(authHeader, true, lunRequest); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index 88ce7c080039..a79291ebf371 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -735,6 +735,7 @@ void testCreateAsync_VolumeClonedFromTemplate_ClonesWithoutGrowing() { ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); verify(sanStrategy).cloneCloudStackVolume(requestCaptor.capture()); + assertEquals("/vol/vol1/cs_tmpl_50", requestCaptor.getValue().getLun().getClone().getSource().getName()); assertEquals("template-lun-uuid", requestCaptor.getValue().getLun().getClone().getSource().getUuid()); verify(sanStrategy, never()).createCloudStackVolume(any()); verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); From f0710383b2b58304de3dbcb04d259c1275fe08fe Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Fri, 28 Aug 2026 11:47:09 +0530 Subject: [PATCH 04/15] CSTACKEX-259: incorporating review cmments --- .../driver/OntapPrimaryDatastoreDriver.java | 39 +++++++++- .../OntapPrimaryDatastoreDriverTest.java | 76 +++++++++++++++++++ 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 7be7c288f70c..4a8138f226ca 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 @@ -34,6 +34,7 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.SnapshotDetailsDao; import com.cloud.storage.dao.SnapshotDetailsVO; @@ -48,11 +49,13 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; +import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.commons.lang3.StringUtils; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.command.CommandResult; import org.apache.cloudstack.storage.command.CreateObjectAnswer; @@ -399,6 +402,14 @@ private void deleteNfsTemplateCache(Map details, TemplateInfo te return; } String flexVolUuid = details.get(OntapStorageConstants.VOLUME_UUID); + if (flexVolUuid == null || flexVolUuid.isEmpty()) { + // Misconfigured pool detail — fail eviction rather than calling ONTAP with a null + // volume UUID (which would hit /api/storage/volumes/null and still look like success + // upstream if we swallowed the error). + throw new CloudRuntimeException("FlexVolume UUID (volumeUUID) is missing from storage pool details; " + + "cannot delete NFS template cache file [" + filePath + "] for template [" + + templateInfo.getId() + "]"); + } ((UnifiedNASStrategy) storageStrategy).deleteFileByPath(flexVolUuid, filePath); logger.info("deleteTemplateOnPrimary: Deleted template cache file [{}] for template [{}]", filePath, templateInfo.getId()); @@ -938,13 +949,37 @@ public long getBytesRequiredForTemplate(TemplateInfo templateInfo, StoragePool s if (templateInfo == null || storagePool == null) { return 0; } - // Already cached on this pool, so deploying from it costs no additional space. - if (vmTemplatePoolDao.findByPoolTemplate(storagePool.getId(), templateInfo.getId(), null) != null) { + // template_spool_ref is inserted in Allocated/NOT_DOWNLOADED before the cache exists; + // only skip reservation when the template is truly cached on this pool. + VMTemplateStoragePoolVO templatePoolRef = + vmTemplatePoolDao.findByPoolTemplate(storagePool.getId(), templateInfo.getId(), null); + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + if (isTemplateCachedOnPool(templatePoolRef, details)) { return 0; } return getDataObjectSizeIncludingHypervisorSnapshotReserve(templateInfo, storagePool); } + /** + * Returns true when the primary template cache is present and usable for clone/deploy. + * A spool_ref row alone is not enough: CloudStack creates it before the LUN/file exists. + */ + private boolean isTemplateCachedOnPool(VMTemplateStoragePoolVO templatePoolRef, Map details) { + if (templatePoolRef == null) { + return false; + } + if (templatePoolRef.getDownloadState() != VMTemplateStorageResourceAssoc.Status.DOWNLOADED) { + return false; + } + if (templatePoolRef.getState() != ObjectInDataStoreStateMachine.State.Ready) { + return false; + } + if (details != null && isIscsi(details)) { + return StringUtils.isNotBlank(templatePoolRef.getLocalDownloadPath()); + } + return StringUtils.isNotBlank(templatePoolRef.getInstallPath()); + } + @Override public long getUsedBytes(StoragePool storagePool) { return 0; diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index a79291ebf371..d4aba62c7e3e 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -25,6 +25,7 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VolumeVO; import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.dao.VMTemplatePoolDao; @@ -33,6 +34,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; @@ -617,7 +619,11 @@ void testGetDataObjectSizeIncludingHypervisorSnapshotReserve_NoReserveAdded() { void testGetBytesRequiredForTemplate_AlreadyCached_ReturnsZero() { when(storagePool.getId()).thenReturn(1L); when(templateInfo.getId()).thenReturn(50L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); + when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready); + when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); assertEquals(0L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); } @@ -627,11 +633,53 @@ void testGetBytesRequiredForTemplate_NotCached_ReturnsVirtualSize() { when(storagePool.getId()).thenReturn(1L); when(templateInfo.getId()).thenReturn(50L); when(templateInfo.getSize()).thenReturn(5368709120L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(null); assertEquals(5368709120L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); } + @Test + void testGetBytesRequiredForTemplate_SpoolRefNotDownloaded_ReturnsVirtualSize() { + when(storagePool.getId()).thenReturn(1L); + when(templateInfo.getId()).thenReturn(50L); + when(templateInfo.getSize()).thenReturn(5368709120L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.NOT_DOWNLOADED); + when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Allocated); + + assertEquals(5368709120L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); + } + + @Test + void testGetBytesRequiredForTemplate_DownloadedWithoutBackendIdentity_ReturnsVirtualSize() { + when(storagePool.getId()).thenReturn(1L); + when(templateInfo.getId()).thenReturn(50L); + when(templateInfo.getSize()).thenReturn(5368709120L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); + when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready); + when(templatePoolRef.getLocalDownloadPath()).thenReturn(null); + + assertEquals(5368709120L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); + } + + @Test + void testGetBytesRequiredForTemplate_NfsCached_ReturnsZero() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + when(storagePool.getId()).thenReturn(1L); + when(templateInfo.getId()).thenReturn(50L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); + when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready); + when(templatePoolRef.getInstallPath()).thenReturn("/mnt/pool/template-uuid"); + + assertEquals(0L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); + } + @Test void testCreateAsync_TemplateWithISCSI_CreatesLunAndRecordsCloneSource() { when(dataStore.getId()).thenReturn(1L); @@ -909,6 +957,34 @@ void testDeleteAsync_Template_NFS_DeletesCachedFile() { } } + @Test + void testDeleteAsync_Template_NFS_FailsWhenFlexVolUuidMissing() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + storagePoolDetails.remove(OntapStorageConstants.VOLUME_UUID); + + when(dataStore.getId()).thenReturn(1L); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getInstallPath()).thenReturn("template-uuid"); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(nasStrategy); + + driver.deleteAsync(dataStore, templateInfo, commandCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(commandCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("volumeUUID")); + verify(nasStrategy, never()).deleteFileByPath(any(), any()); + } + } + @Test void testRevokeAccess_Template_UnmapsCacheLun() { when(dataStore.getId()).thenReturn(1L); From 988b37cae31933b29d68e7f1e40fec2662b38bf0 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 31 Aug 2026 11:31:19 +0530 Subject: [PATCH 05/15] CSTACKEX-259: corected junit changes introduced as part of merge conflict --- .../OntapPrimaryDatastoreDriverTest.java | 69 +++++++++++++++++-- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index d4aba62c7e3e..f7ad563b7fd5 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -192,7 +192,7 @@ void testCreateAsync_VolumeWithISCSI_Success() { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); when(storagePool.getName()).thenReturn("vol1"); - when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.Iscsi); when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); @@ -223,7 +223,7 @@ void testCreateAsync_VolumeWithISCSI_Success() { verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_UUID), eq("lun-uuid-123"), eq(false)); verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_NAME), eq("/vol/vol1/lun1"), eq(false)); - verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2); + verify(volumeVO).setFormat(Storage.ImageFormat.RAW); verify(volumeDao).update(eq(100L), any(VolumeVO.class)); } } @@ -251,9 +251,9 @@ void testCreateAsync_VolumeWithNFS_Success() { try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) - .thenReturn(sanStrategy); + .thenReturn(nasStrategy); - when(sanStrategy.createCloudStackVolume(any())).thenReturn(mockCloudStackVolume); + when(nasStrategy.createCloudStackVolume(any())).thenReturn(mockCloudStackVolume); // Execute driver.createAsync(dataStore, volumeInfo, createCallback); @@ -270,6 +270,66 @@ void testCreateAsync_VolumeWithNFS_Success() { } } + @Test + void testCreateAsync_UnsupportedHypervisor_FailsWithError() { + // Use NFS so createVolumeRequest does not fail earlier in getOSTypeFromHypervisor; + // the failure under test is image-format resolution for non-KVM hypervisors. + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(volumeInfo.getType()).thenReturn(VOLUME); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getName()).thenReturn("test-volume"); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.VMware); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + + CloudStackVolume mockCloudStackVolume = new CloudStackVolume(); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(nasStrategy); + when(nasStrategy.createCloudStackVolume(any())).thenReturn(mockCloudStackVolume); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Unsupported hypervisor [VMware]")); + } + } + + @Test + void testCreateAsync_KvmUnsupportedProtocol_FailsWithError() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, "FC"); + + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(volumeInfo.getType()).thenReturn(VOLUME); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getName()).thenReturn("test-volume"); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + assertTrue(resultCaptor.getValue().getResult().contains("Unsupported protocol FC")); + } + } + @Test void testDeleteAsync_NullStore_ThrowsException() { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); @@ -647,7 +707,6 @@ void testGetBytesRequiredForTemplate_SpoolRefNotDownloaded_ReturnsVirtualSize() when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.NOT_DOWNLOADED); - when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Allocated); assertEquals(5368709120L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); } From 240b31ab0c3be6edc91d2047b16d378feac263ec Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 31 Aug 2026 14:31:53 +0530 Subject: [PATCH 06/15] CSTACKEX-259: downloaded state check is not required --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 11 ++++------- .../driver/OntapPrimaryDatastoreDriverTest.java | 10 +++------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 4a8138f226ca..dc58d7d9094c 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 @@ -34,7 +34,6 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.SnapshotDetailsDao; import com.cloud.storage.dao.SnapshotDetailsVO; @@ -949,8 +948,8 @@ public long getBytesRequiredForTemplate(TemplateInfo templateInfo, StoragePool s if (templateInfo == null || storagePool == null) { return 0; } - // template_spool_ref is inserted in Allocated/NOT_DOWNLOADED before the cache exists; - // only skip reservation when the template is truly cached on this pool. + // template_spool_ref is inserted in Allocated before the cache exists; + // only skip reservation when the template is Ready and has a backend identity. VMTemplateStoragePoolVO templatePoolRef = vmTemplatePoolDao.findByPoolTemplate(storagePool.getId(), templateInfo.getId(), null); Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); @@ -962,15 +961,13 @@ public long getBytesRequiredForTemplate(TemplateInfo templateInfo, StoragePool s /** * Returns true when the primary template cache is present and usable for clone/deploy. - * A spool_ref row alone is not enough: CloudStack creates it before the LUN/file exists. + * A spool_ref row alone is not enough: CloudStack creates it in Allocated before the LUN/file exists. + * Ready is sufficient; downloadState is set alongside Ready on the managed-cache success path. */ private boolean isTemplateCachedOnPool(VMTemplateStoragePoolVO templatePoolRef, Map details) { if (templatePoolRef == null) { return false; } - if (templatePoolRef.getDownloadState() != VMTemplateStorageResourceAssoc.Status.DOWNLOADED) { - return false; - } if (templatePoolRef.getState() != ObjectInDataStoreStateMachine.State.Ready) { return false; } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index f7ad563b7fd5..b4bc1fe681a6 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -25,7 +25,6 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VolumeVO; import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.dao.VMTemplatePoolDao; @@ -681,7 +680,6 @@ void testGetBytesRequiredForTemplate_AlreadyCached_ReturnsZero() { when(templateInfo.getId()).thenReturn(50L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); - when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready); when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); @@ -700,25 +698,24 @@ void testGetBytesRequiredForTemplate_NotCached_ReturnsVirtualSize() { } @Test - void testGetBytesRequiredForTemplate_SpoolRefNotDownloaded_ReturnsVirtualSize() { + void testGetBytesRequiredForTemplate_SpoolRefNotReady_ReturnsVirtualSize() { when(storagePool.getId()).thenReturn(1L); when(templateInfo.getId()).thenReturn(50L); when(templateInfo.getSize()).thenReturn(5368709120L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); - when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.NOT_DOWNLOADED); + when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Allocated); assertEquals(5368709120L, driver.getBytesRequiredForTemplate(templateInfo, storagePool)); } @Test - void testGetBytesRequiredForTemplate_DownloadedWithoutBackendIdentity_ReturnsVirtualSize() { + void testGetBytesRequiredForTemplate_ReadyWithoutBackendIdentity_ReturnsVirtualSize() { when(storagePool.getId()).thenReturn(1L); when(templateInfo.getId()).thenReturn(50L); when(templateInfo.getSize()).thenReturn(5368709120L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); - when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready); when(templatePoolRef.getLocalDownloadPath()).thenReturn(null); @@ -732,7 +729,6 @@ void testGetBytesRequiredForTemplate_NfsCached_ReturnsZero() { when(templateInfo.getId()).thenReturn(50L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); - when(templatePoolRef.getDownloadState()).thenReturn(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); when(templatePoolRef.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready); when(templatePoolRef.getInstallPath()).thenReturn("/mnt/pool/template-uuid"); From 0e92cdd67820e32c4166121779738dc35ecd80b5 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Thu, 3 Sep 2026 12:08:00 +0530 Subject: [PATCH 07/15] CSTACKEX-259: review comments resolve --- .../driver/OntapPrimaryDatastoreDriver.java | 139 +++++++++++------ .../storage/service/UnifiedSANStrategy.java | 18 +++ .../OntapPrimaryDatastoreDriverTest.java | 144 ++++++++++++++++++ .../service/UnifiedSANStrategyTest.java | 79 ++++++++++ 4 files changed, 336 insertions(+), 44 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 dc58d7d9094c..059c3c65627e 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 @@ -170,7 +170,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet // Create the backend storage object: a clone of the cached template when the // orchestrator asked for one, otherwise a blank LUN (iSCSI) or qcow2 file (NFS). Long cloneOfTemplateId = getTemplateIdForCloning(volInfo.getId()); - CloudStackVolume created = cloneOfTemplateId != null + CloudStackVolume clonedCloudStackVolume = cloneOfTemplateId != null ? cloneCloudStackVolumeFromTemplate(storagePool, volInfo, details, cloneOfTemplateId) : createCloudStackVolume(storagePool, volInfo, details); @@ -180,17 +180,14 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet logger.info("createAsync: Volume format set to [{}] for hypervisor [{}]", volumeVO.getFormat(), storagePool.getHypervisor()); if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { - String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null; - if (lunName == null) { - throw new CloudRuntimeException("Missing LUN name for volume " + volInfo.getId()); - } + // createCloudStackVolume validates the Feign response (LUN name + uuid) before returning + Lun createdLun = clonedCloudStackVolume.getLun(); + String lunName = createdLun.getName(); // Persist LUN details for future operations (delete, grant/revoke access) - volumeDetailsDao.addDetail(volInfo.getId(), OntapStorageConstants.LUN_DOT_UUID, created.getLun().getUuid(), false); + volumeDetailsDao.addDetail(volInfo.getId(), OntapStorageConstants.LUN_DOT_UUID, createdLun.getUuid(), false); volumeDetailsDao.addDetail(volInfo.getId(), OntapStorageConstants.LUN_DOT_NAME, lunName, false); - if (created.getLun().getUuid() != null) { - volumeVO.setFolder(created.getLun().getUuid()); - } + volumeVO.setFolder(createdLun.getUuid()); logger.info("createAsync: Created LUN [{}] for volume [{}]. LUN mapping will occur during grantAccess() to per-host igroup.", lunName, volumeVO.getId()); @@ -263,22 +260,31 @@ private CreateCmdResult createTemplateOnPrimary(StoragePoolVO storagePool, Templ } StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); - CloudStackVolume created = storageStrategy.createCloudStackVolume( - createTemplateLunRequest(storagePool, details, templateInfo.getId(), sizeInBytes)); - - if (created == null || created.getLun() == null || created.getLun().getName() == null) { - throw new CloudRuntimeException("ONTAP returned no LUN for the cache of template [" + templateInfo.getId() + "]"); + CloudStackVolume createdCloudStackVolume = null; + try { + createdCloudStackVolume = storageStrategy.createCloudStackVolume( + createTemplateLunRequest(storagePool, details, templateInfo.getId(), sizeInBytes)); + // createCloudStackVolume validates the Feign response (LUN name + uuid) before returning + Lun lun = createdCloudStackVolume.getLun(); + templatePoolRef.setLocalDownloadPath(lun.getUuid()); + templatePoolRef.setTemplateSize(sizeInBytes); + vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); + + logger.info("createTemplateOnPrimary: Created template cache LUN [{}] (uuid [{}], {} bytes) on pool [{}] for template [{}]", + lun.getName(), lun.getUuid(), sizeInBytes, storagePool.getId(), templateInfo.getId()); + + return new CreateCmdResult(lun.getName(), new Answer(null, true, null)); + } catch (Exception e) { + // Compensating delete: orchestrator only cleans DB metadata on create failure. + bestEffortDeleteTemplateCacheLun(storageStrategy, details.get(OntapStorageConstants.SVM_NAME), + getTemplateLunName(storagePool, templateInfo.getId()), + createdCloudStackVolume != null && createdCloudStackVolume.getLun() != null ? createdCloudStackVolume.getLun().getUuid() : null); + if (e instanceof CloudRuntimeException) { + throw (CloudRuntimeException) e; + } + throw new CloudRuntimeException("Failed to create template cache LUN for template [" + templateInfo.getId() + + "]: " + e.getMessage(), e); } - - Lun lun = created.getLun(); - templatePoolRef.setLocalDownloadPath(lun.getUuid()); - templatePoolRef.setTemplateSize(sizeInBytes); - vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); - - logger.info("createTemplateOnPrimary: Created template cache LUN [{}] (uuid [{}], {} bytes) on pool [{}] for template [{}]", - lun.getName(), lun.getUuid(), sizeInBytes, storagePool.getId(), templateInfo.getId()); - - return new CreateCmdResult(lun.getName(), new Answer(null, true, null)); } /** @@ -298,7 +304,8 @@ private CloudStackVolume cloneCloudStackVolumeFromTemplate(StoragePoolVO storage : createCloneFileRequest(storagePool, volumeInfo, templatePoolRef, templateId); CloudStackVolume cloned = storageStrategy.cloneCloudStackVolume(request); - if (cloned == null || (iscsi && (cloned.getLun() == null || cloned.getLun().getName() == null))) { + // SAN cloneCloudStackVolume validates the Feign response (LUN name + uuid) before returning + if (cloned == null) { throw new CloudRuntimeException("ONTAP returned nothing when cloning template [" + templateId + "] for volume [" + volumeInfo.getId() + "]"); } @@ -365,31 +372,60 @@ private void deleteTemplateOnPrimary(DataStore store, TemplateInfo templateInfo) StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); if (isIscsi(details)) { - deleteIscsiTemplateCache(storagePool, templateInfo, templatePoolRef, storageStrategy); + deleteIscsiTemplateCache(storagePool, templateInfo, templatePoolRef, storageStrategy, details); } else { deleteNfsTemplateCache(details, templateInfo, templatePoolRef, storageStrategy); } } private void deleteIscsiTemplateCache(StoragePoolVO storagePool, TemplateInfo templateInfo, - VMTemplateStoragePoolVO templatePoolRef, StorageStrategy storageStrategy) { + VMTemplateStoragePoolVO templatePoolRef, StorageStrategy storageStrategy, + Map details) { + String lunName = getTemplateLunName(storagePool, templateInfo.getId()); String lunUuid = templatePoolRef.getLocalDownloadPath(); - if (lunUuid == null || lunUuid.isEmpty()) { - logger.warn("deleteTemplateOnPrimary: No cached LUN recorded for template [{}] on pool [{}]; nothing to delete", - templateInfo.getId(), storagePool.getId()); - return; + deleteTemplateCacheLun(storageStrategy, details.get(OntapStorageConstants.SVM_NAME), lunName, lunUuid); + logger.info("deleteTemplateOnPrimary: Deleted template cache LUN [{}] for template [{}] on pool [{}]", + lunName, templateInfo.getId(), storagePool.getId()); + } + + /** + * Deletes a template-cache LUN. When {@code lunUuid} is missing, resolves the LUN by its + * deterministic name so eviction and create-time rollback still work. + */ + private void deleteTemplateCacheLun(StorageStrategy storageStrategy, String svmName, String lunName, String lunUuid) { + String uuid = lunUuid; + if (uuid == null || uuid.isEmpty()) { + CloudStackVolume existing = getCloudStackVolumeByName(storageStrategy, svmName, lunName); + if (existing == null || existing.getLun() == null || existing.getLun().getUuid() == null) { + logger.warn("deleteTemplateCacheLun: LUN [{}] not found on SVM [{}]; nothing to delete", lunName, svmName); + return; + } + uuid = existing.getLun().getUuid(); } Lun lun = new Lun(); - lun.setUuid(lunUuid); - lun.setName(getTemplateLunName(storagePool, templateInfo.getId())); + lun.setUuid(uuid); + lun.setName(lunName); CloudStackVolume deleteRequest = new CloudStackVolume(); deleteRequest.setLun(lun); storageStrategy.deleteCloudStackVolume(deleteRequest); + } - logger.info("deleteTemplateOnPrimary: Deleted template cache LUN [{}] for template [{}] on pool [{}]", - lun.getName(), templateInfo.getId(), storagePool.getId()); + /** + * Best-effort cleanup of a template-cache LUN after a failed create. Never masks the original + * create failure. + */ + private void bestEffortDeleteTemplateCacheLun(StorageStrategy storageStrategy, String svmName, String lunName, + String lunUuid) { + try { + deleteTemplateCacheLun(storageStrategy, svmName, lunName, lunUuid); + logger.info("bestEffortDeleteTemplateCacheLun: Removed leftover template cache LUN [{}] on SVM [{}]", + lunName, svmName); + } catch (Exception cleanupEx) { + logger.warn("bestEffortDeleteTemplateCacheLun: Failed to remove leftover template cache LUN [{}] on SVM [{}]: {}", + lunName, svmName, cleanupEx.getMessage()); + } } private void deleteNfsTemplateCache(Map details, TemplateInfo templateInfo, @@ -682,15 +718,30 @@ private void grantAccessTemplate(TemplateInfo templateInfo, Host host, DataStore ensureAccessGroupForHost(sanStrategy, host, storagePool, svmName, accessGroupName); - String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName); - String iscsiPath = buildIscsiPath(storagePool, lunNumber); - - templatePoolRef.setInstallPath(iscsiPath); - vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); - refreshManagedStoreTarget(dataStore, iscsiPath); - - logger.info("grantAccessTemplate: Mapped template cache LUN [{}] to igroup [{}] as [{}] for template [{}]", - lunName, accessGroupName, iscsiPath, templateInfo.getId()); + boolean lunMapped = false; + try { + String lunNumber = sanStrategy.ensureLunMapped(svmName, lunName, accessGroupName); + lunMapped = true; + String iscsiPath = buildIscsiPath(storagePool, lunNumber); + + templatePoolRef.setInstallPath(iscsiPath); + vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); + refreshManagedStoreTarget(dataStore, iscsiPath); + + logger.info("grantAccessTemplate: Mapped template cache LUN [{}] to igroup [{}] as [{}] for template [{}]", + lunName, accessGroupName, iscsiPath, templateInfo.getId()); + } catch (RuntimeException e) { + // Undo only this call's side effect (map). Do not delete the cache LUN — create already succeeded. + if (lunMapped) { + try { + unmapLunFromHost(sanStrategy, svmName, lunName, accessGroupName, host); + } catch (Exception rollbackEx) { + logger.warn("grantAccessTemplate: Failed to unmap template cache LUN [{}] from igroup [{}] after grant failure: {}", + lunName, accessGroupName, rollbackEx.getMessage()); + } + } + throw e; + } } /** diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 5d360ccc5dc0..9db0df768e4f 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 @@ -79,6 +79,7 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume throw new CloudRuntimeException("Failed to create Lun: " + cloudstackVolume.getLun().getName()); } Lun lun = createdLun.getRecords().get(0); + validateCreatedLun(lun, cloudstackVolume.getLun().getName(), "createCloudStackVolume"); logger.debug("createCloudStackVolume: LUN created successfully. Lun: {}", lun); CloudStackVolume createdCloudStackVolume = new CloudStackVolume(); @@ -88,6 +89,8 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume logger.error("FeignException occurred while creating LUN: {}, Status: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.status(), e.getMessage()); throw new CloudRuntimeException("Failed to create Lun: " + e.getMessage()); + } catch (CloudRuntimeException e) { + throw e; } catch (Exception e) { logger.error("Exception occurred while creating LUN: {}, Exception: {}", cloudstackVolume.getLun().getName(), e.getMessage()); throw new CloudRuntimeException("Failed to create Lun: " + e.getMessage()); @@ -154,6 +157,7 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) throw new CloudRuntimeException("Failed to clone Lun: " + lunRequest.getName()); } Lun lun = clonedLun.getRecords().get(0); + validateCreatedLun(lun, lunRequest.getName(), "cloneCloudStackVolume"); logger.debug("cloneCloudStackVolume: LUN cloned successfully. Lun: {}", lun); CloudStackVolume clonedCloudStackVolume = new CloudStackVolume(); @@ -163,12 +167,26 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) logger.error("FeignException occurred while cloning LUN: {}, Status: {}, Exception: {}", lunRequest.getName(), e.status(), e.getMessage()); throw new CloudRuntimeException("Failed to clone Lun: " + e.getMessage()); + } catch (CloudRuntimeException e) { + throw e; } catch (Exception e) { logger.error("Exception occurred while cloning LUN: {}, Exception: {}", lunRequest.getName(), e.getMessage()); throw new CloudRuntimeException("Failed to clone Lun: " + e.getMessage()); } } + + /** + * Ensures ONTAP returned a usable LUN identity from create/clone. Callers in the datastore + * driver rely on non-null name and uuid, so reject incomplete records at the Feign boundary. + */ + private void validateCreatedLun(Lun lun, String requestName, String operation) { + if (lun == null || lun.getName() == null || lun.getUuid() == null) { + logger.error("{}: ONTAP returned incomplete LUN for {}", operation, requestName); + throw new CloudRuntimeException("ONTAP returned incomplete LUN for: " + requestName); + } + } + /** * Grows an existing LUN to {@code sizeInBytes}. * diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index b4bc1fe681a6..78dee9b0eb95 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -78,6 +78,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; @@ -428,6 +429,46 @@ void testDeleteAsync_Template_DeletesCacheLun() { } } + + @Test + void testDeleteAsync_Template_ResolvesLunByNameWhenUuidMissing() { + when(dataStore.getId()).thenReturn(1L); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getLocalDownloadPath()).thenReturn(null); + + Lun templateLun = new Lun(); + templateLun.setName("/vol/vol1/cs_tmpl_50"); + templateLun.setUuid("resolved-lun-uuid"); + CloudStackVolume existing = new CloudStackVolume(); + existing.setLun(templateLun); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); + when(sanStrategy.getCloudStackVolume(argThat(map -> + map != null && "/vol/vol1/cs_tmpl_50".equals(map.get("name"))))) + .thenReturn(existing); + doNothing().when(sanStrategy).deleteCloudStackVolume(any()); + + driver.deleteAsync(dataStore, templateInfo, commandCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(commandCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + ArgumentCaptor deleteCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); + verify(sanStrategy).deleteCloudStackVolume(deleteCaptor.capture()); + assertEquals("resolved-lun-uuid", deleteCaptor.getValue().getLun().getUuid()); + assertEquals("/vol/vol1/cs_tmpl_50", deleteCaptor.getValue().getLun().getName()); + } + } + @Test void testDeleteAsync_Template_NoCachedLun_SucceedsWithoutCallingOntap() { when(dataStore.getId()).thenReturn(1L); @@ -775,6 +816,49 @@ void testCreateAsync_TemplateWithISCSI_CreatesLunAndRecordsCloneSource() { } } + + @Test + void testCreateAsync_TemplateWithISCSI_RollsBackLunWhenDbUpdateFails() { + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + when(templateInfo.getSize()).thenReturn(5368709120L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getId()).thenReturn(7L); + doThrow(new RuntimeException("db update failed")).when(vmTemplatePoolDao) + .update(eq(7L), any(VMTemplateStoragePoolVO.class)); + + Lun templateLun = new Lun(); + templateLun.setName("/vol/vol1/cs_tmpl_50"); + templateLun.setUuid("template-lun-uuid"); + CloudStackVolume created = new CloudStackVolume(); + created.setLun(templateLun); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.createCloudStackVolume(any())).thenReturn(created); + doNothing().when(sanStrategy).deleteCloudStackVolume(any()); + + driver.createAsync(dataStore, templateInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + + ArgumentCaptor deleteCaptor = ArgumentCaptor.forClass(CloudStackVolume.class); + verify(sanStrategy).deleteCloudStackVolume(deleteCaptor.capture()); + assertEquals("template-lun-uuid", deleteCaptor.getValue().getLun().getUuid()); + assertEquals("/vol/vol1/cs_tmpl_50", deleteCaptor.getValue().getLun().getName()); + } + } + @Test void testCreateAsync_TemplateWithISCSI_UnknownSize_Fails() { when(dataStore.getId()).thenReturn(1L); @@ -941,6 +1025,66 @@ void testGrantAccess_Template_WritesInstallPathAndRefreshesStoreTarget() { } } + + @Test + void testGrantAccess_Template_UnmapsWhenInstallPathUpdateFails() { + PrimaryDataStore primaryDataStore = mock(PrimaryDataStore.class); + Map dataStoreDetails = new HashMap<>(); + dataStoreDetails.put(PrimaryDataStore.MANAGED_STORE_TARGET, "stale-value"); + + when(primaryDataStore.getId()).thenReturn(1L); + when(primaryDataStore.getDetails()).thenReturn(dataStoreDetails); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER); + when(storagePool.getPath()).thenReturn("iqn.1992-08.com.netapp:sn.123456"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getId()).thenReturn(7L); + doThrow(new RuntimeException("install path update failed")).when(vmTemplatePoolDao) + .update(eq(7L), any(VMTemplateStoragePoolVO.class)); + + when(host.getName()).thenReturn("host1"); + when(host.getUuid()).thenReturn("host-uuid-1"); + when(host.getStorageUrl()).thenReturn("iqn.1993-08.org.debian:01:host1"); + + AccessGroup existingAccessGroup = new AccessGroup(); + Igroup existingIgroup = new Igroup(); + existingIgroup.setName("igroup1"); + existingIgroup.setUuid("igroup-uuid-123"); + existingAccessGroup.setIgroup(existingIgroup); + + Lun templateLun = new Lun(); + templateLun.setName("/vol/vol1/cs_tmpl_50"); + templateLun.setUuid("template-lun-uuid"); + CloudStackVolume cachedTemplate = new CloudStackVolume(); + cachedTemplate.setLun(templateLun); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); + utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); + + when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup); + when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); + when(sanStrategy.getCloudStackVolume(argThat(map -> + map != null && "/vol/vol1/cs_tmpl_50".equals(map.get("name"))))) + .thenReturn(cachedTemplate); + when(sanStrategy.validateInitiatorInAccessGroup(anyString(), anyString(), any(Igroup.class))).thenReturn(true); + doNothing().when(sanStrategy).disableLogicalAccess(any()); + + assertThrows(CloudRuntimeException.class, + () -> driver.grantAccess(templateInfo, host, primaryDataStore)); + + verify(sanStrategy).disableLogicalAccess(argThat(map -> + map != null && "template-lun-uuid".equals(map.get("lun.uuid")) + && "igroup-uuid-123".equals(map.get("igroup.uuid")))); + } + } + @Test void testGrantAccess_TemplateOnNFS_SkipsMapping() { storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index 53e8ef1cc429..04408bd727c2 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -1468,6 +1468,85 @@ void testCreateCloudStackVolume_EmptyResponse_ThrowsException() { } } + + @Test + void testCreateCloudStackVolume_IncompleteLunMissingName_ThrowsException() { + Lun lun = new Lun(); + lun.setName("/vol/vol1/lun1"); + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + Lun incomplete = new Lun(); + incomplete.setUuid("lun-uuid-123"); + // name intentionally null + OntapResponse response = new OntapResponse<>(); + response.setRecords(List.of(incomplete)); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))) + .thenReturn(response); + + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.createCloudStackVolume(request)); + assertTrue(ex.getMessage().contains("incomplete LUN")); + } + } + + @Test + void testCreateCloudStackVolume_IncompleteLunMissingUuid_ThrowsException() { + Lun lun = new Lun(); + lun.setName("/vol/vol1/lun1"); + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + Lun incomplete = new Lun(); + incomplete.setName("/vol/vol1/lun1"); + // uuid intentionally null + OntapResponse response = new OntapResponse<>(); + response.setRecords(List.of(incomplete)); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))) + .thenReturn(response); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.createCloudStackVolume(request)); + } + } + + @Test + void testCloneCloudStackVolume_IncompleteLunMissingUuid_ThrowsException() { + Lun.Source source = new Lun.Source(); + source.setUuid("source-lun-uuid"); + Lun.Clone clone = new Lun.Clone(); + clone.setSource(source); + Lun lun = new Lun(); + lun.setName("/vol/vol1/cloned"); + lun.setClone(clone); + + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + Lun incomplete = new Lun(); + incomplete.setName("/vol/vol1/cloned"); + OntapResponse response = new OntapResponse<>(); + response.setRecords(List.of(incomplete)); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))) + .thenReturn(response); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolume(request)); + } + } + @Test void testCreateCloudStackVolume_NullResponse_ThrowsException() { Lun lun = new Lun(); From 5b242f62674ffeff4a17dd02c93f829add171c88 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Fri, 4 Sep 2026 11:04:39 +0530 Subject: [PATCH 08/15] CSTACKEX-259: resolving unit test failure --- .../storage/driver/OntapPrimaryDatastoreDriverTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index 78dee9b0eb95..6a296763ada6 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -1029,11 +1029,8 @@ void testGrantAccess_Template_WritesInstallPathAndRefreshesStoreTarget() { @Test void testGrantAccess_Template_UnmapsWhenInstallPathUpdateFails() { PrimaryDataStore primaryDataStore = mock(PrimaryDataStore.class); - Map dataStoreDetails = new HashMap<>(); - dataStoreDetails.put(PrimaryDataStore.MANAGED_STORE_TARGET, "stale-value"); when(primaryDataStore.getId()).thenReturn(1L); - when(primaryDataStore.getDetails()).thenReturn(dataStoreDetails); when(templateInfo.getType()).thenReturn(TEMPLATE); when(templateInfo.getId()).thenReturn(50L); @@ -1053,9 +1050,9 @@ void testGrantAccess_Template_UnmapsWhenInstallPathUpdateFails() { when(host.getStorageUrl()).thenReturn("iqn.1993-08.org.debian:01:host1"); AccessGroup existingAccessGroup = new AccessGroup(); - Igroup existingIgroup = new Igroup(); - existingIgroup.setName("igroup1"); - existingIgroup.setUuid("igroup-uuid-123"); + Igroup existingIgroup = mock(Igroup.class); + when(existingIgroup.getName()).thenReturn("igroup1"); + when(existingIgroup.getUuid()).thenReturn("igroup-uuid-123"); existingAccessGroup.setIgroup(existingIgroup); Lun templateLun = new Lun(); From 5f04b5822c0e7e0f817a2b7c4fb1abdabfef093f Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Thu, 10 Sep 2026 15:09:32 +0530 Subject: [PATCH 09/15] CSTACKEX-259: review comment incorporation --- .../driver/OntapPrimaryDatastoreDriver.java | 19 +++---------------- .../OntapPrimaryDatastoreDriverTest.java | 3 ++- 2 files changed, 5 insertions(+), 17 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 059c3c65627e..7b436e9c6d54 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 @@ -230,19 +230,6 @@ private CloudStackVolume createCloudStackVolume(StoragePoolVO storagePool, Volum /** * Creates the backend object that caches a template on this pool's FlexVolume. * - *

This is the first half of the cache-and-clone flow driven by - * {@code VolumeServiceImpl.createManagedStorageVolumeFromTemplateAsync}. Only the empty - * container is created here; the framework then sends a {@code CopyCommand} to a KVM host - * which writes the image content into it.

- * - *

For iSCSI the ONTAP identity of the cache is recorded on {@code template_spool_ref}: - * {@code local_download_path} holds the LUN uuid, which is the clone source later on. - * {@code install_path} is deliberately left for {@code grantAccess} to fill, because it must - * be {@code //} and the LUN number does not exist until the LUN is - * mapped to an igroup.

- * - *

For NFS nothing is pre-created on the array: the KVM agent writes the qcow2 into the - * mounted FlexVolume and reports the path, which the framework stores as {@code install_path}.

*/ private CreateCmdResult createTemplateOnPrimary(StoragePoolVO storagePool, TemplateInfo templateInfo, Map details) { if (!isIscsi(details)) { @@ -384,7 +371,7 @@ private void deleteIscsiTemplateCache(StoragePoolVO storagePool, TemplateInfo te String lunName = getTemplateLunName(storagePool, templateInfo.getId()); String lunUuid = templatePoolRef.getLocalDownloadPath(); deleteTemplateCacheLun(storageStrategy, details.get(OntapStorageConstants.SVM_NAME), lunName, lunUuid); - logger.info("deleteTemplateOnPrimary: Deleted template cache LUN [{}] for template [{}] on pool [{}]", + logger.info("deleteIscsiTemplateCache: Deleted template cache LUN [{}] for template [{}] on pool [{}]", lunName, templateInfo.getId(), storagePool.getId()); } @@ -432,7 +419,7 @@ private void deleteNfsTemplateCache(Map details, TemplateInfo te VMTemplateStoragePoolVO templatePoolRef, StorageStrategy storageStrategy) { String filePath = templatePoolRef.getInstallPath(); if (filePath == null || filePath.isEmpty()) { - logger.warn("deleteTemplateOnPrimary: No install_path recorded for template [{}]; nothing to delete", + logger.warn("deleteNfsTemplateCache: No install_path recorded for template [{}]; nothing to delete", templateInfo.getId()); return; } @@ -446,7 +433,7 @@ private void deleteNfsTemplateCache(Map details, TemplateInfo te + templateInfo.getId() + "]"); } ((UnifiedNASStrategy) storageStrategy).deleteFileByPath(flexVolUuid, filePath); - logger.info("deleteTemplateOnPrimary: Deleted template cache file [{}] for template [{}]", + logger.info("deleteNfsTemplateCache: Deleted template cache file [{}] for template [{}]", filePath, templateInfo.getId()); } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index 6a296763ada6..b6b0639e29b7 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -964,7 +964,7 @@ private void stubVolumeCloneFromTemplate(long templateSize, long volumeSize) { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); lenient().when(storagePool.getName()).thenReturn("vol1"); - when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.Iscsi); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.OntapiSCSI); when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); @@ -1103,6 +1103,7 @@ void testGrantAccess_TemplateOnNFS_SkipsMapping() { void testCreateAsync_VolumeClonedFromTemplateNFS_ClonesFile() { storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); stubVolumeCloneFromTemplate(5368709120L, 5368709120L); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); when(volumeInfo.getUuid()).thenReturn("volume-uuid"); when(templatePoolRef.getInstallPath()).thenReturn("template-uuid"); From ccbbb0897239160d9fefa348e83b6f798a5a4909 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Thu, 10 Sep 2026 18:25:17 +0530 Subject: [PATCH 10/15] CSTACKEX-259: refactor the protocol specific impl for the template cache create --- .../driver/OntapPrimaryDatastoreDriver.java | 89 +++++++------------ .../storage/service/StorageStrategy.java | 24 +++++ .../storage/service/UnifiedNASStrategy.java | 13 +++ .../storage/service/UnifiedSANStrategy.java | 85 ++++++++++++++++++ .../OntapPrimaryDatastoreDriverTest.java | 34 ++++--- .../storage/service/StorageStrategyTest.java | 7 ++ .../service/UnifiedNASStrategyTest.java | 17 ++++ .../service/UnifiedSANStrategyTest.java | 54 +++++++++++ 8 files changed, 254 insertions(+), 69 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 7b436e9c6d54..9c0fb78a914f 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 @@ -230,50 +230,50 @@ private CloudStackVolume createCloudStackVolume(StoragePoolVO storagePool, Volum /** * Creates the backend object that caches a template on this pool's FlexVolume. * + *

Protocol-specific work is delegated to {@link StorageStrategy#createTemplateCache}. + * This method maps the result to {@link CreateCmdResult} and records SAN identity on + * {@code template_spool_ref} ({@code local_download_path} = LUN uuid).

*/ private CreateCmdResult createTemplateOnPrimary(StoragePoolVO storagePool, TemplateInfo templateInfo, Map details) { - if (!isIscsi(details)) { - logger.info("createTemplateOnPrimary: NFS pool [{}], template [{}] will be written directly to the mounted FlexVolume", - storagePool.getId(), templateInfo.getId()); - return new CreateCmdResult(templateInfo.getUuid(), new Answer(null, true, null)); - } - - VMTemplateStoragePoolVO templatePoolRef = findTemplatePoolRef(storagePool.getId(), templateInfo.getId()); - - long sizeInBytes = getDataObjectSizeIncludingHypervisorSnapshotReserve(templateInfo, storagePool); - if (sizeInBytes <= 0) { - throw new CloudRuntimeException("Unknown virtual size for template [" + templateInfo.getId() - + "]; cannot size the template LUN on pool [" + storagePool.getId() + "]"); - } - StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); - CloudStackVolume createdCloudStackVolume = null; + long sizeInBytes = getDataObjectSizeIncludingHypervisorSnapshotReserve(templateInfo, storagePool); + CloudStackVolume created = null; try { - createdCloudStackVolume = storageStrategy.createCloudStackVolume( - createTemplateLunRequest(storagePool, details, templateInfo.getId(), sizeInBytes)); - // createCloudStackVolume validates the Feign response (LUN name + uuid) before returning - Lun lun = createdCloudStackVolume.getLun(); - templatePoolRef.setLocalDownloadPath(lun.getUuid()); - templatePoolRef.setTemplateSize(sizeInBytes); - vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); - - logger.info("createTemplateOnPrimary: Created template cache LUN [{}] (uuid [{}], {} bytes) on pool [{}] for template [{}]", - lun.getName(), lun.getUuid(), sizeInBytes, storagePool.getId(), templateInfo.getId()); - - return new CreateCmdResult(lun.getName(), new Answer(null, true, null)); + created = storageStrategy.createTemplateCache(storagePool, templateInfo, details, sizeInBytes); + String path = recordTemplateCacheOnSpoolRef(storagePool, templateInfo, created, sizeInBytes); + return new CreateCmdResult(path, new Answer(null, true, null)); } catch (Exception e) { - // Compensating delete: orchestrator only cleans DB metadata on create failure. - bestEffortDeleteTemplateCacheLun(storageStrategy, details.get(OntapStorageConstants.SVM_NAME), - getTemplateLunName(storagePool, templateInfo.getId()), - createdCloudStackVolume != null && createdCloudStackVolume.getLun() != null ? createdCloudStackVolume.getLun().getUuid() : null); + // Compensating delete for SAN: strategy cleans create-time LUN failures; this covers + // post-create failures (e.g. template_spool_ref update) after a LUN was returned. + if (created != null && created.getLun() != null) { + bestEffortDeleteTemplateCacheLun(storageStrategy, details.get(OntapStorageConstants.SVM_NAME), + created.getLun().getName(), created.getLun().getUuid()); + } if (e instanceof CloudRuntimeException) { throw (CloudRuntimeException) e; } - throw new CloudRuntimeException("Failed to create template cache LUN for template [" + templateInfo.getId() + throw new CloudRuntimeException("Failed to create template cache for template [" + templateInfo.getId() + "]: " + e.getMessage(), e); } } + /** + * Persists SAN cache identity on {@code template_spool_ref} when a LUN was created. + * NFS returns the template uuid as the create path; {@code install_path} is filled later. + */ + private String recordTemplateCacheOnSpoolRef(StoragePoolVO storagePool, TemplateInfo templateInfo, + CloudStackVolume created, long sizeInBytes) { + if (created == null || created.getLun() == null) { + return templateInfo.getUuid(); + } + Lun lun = created.getLun(); + VMTemplateStoragePoolVO templatePoolRef = findTemplatePoolRef(storagePool.getId(), templateInfo.getId()); + templatePoolRef.setLocalDownloadPath(lun.getUuid()); + templatePoolRef.setTemplateSize(sizeInBytes); + vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); + return lun.getName(); + } + /** * Clones the cached template into a new volume on the same FlexVolume. * @@ -1401,31 +1401,6 @@ private String getTemplateLunName(StoragePoolVO storagePool, long templateId) { return OntapStorageUtils.getLunName(storagePool.getName(), OntapStorageConstants.TEMPLATE_LUN_PREFIX + templateId); } - /** - * Builds the request that creates an empty LUN to cache a template. - * - *

The LUN is sized to the template's virtual disk size. That is the size KVM writes - * after {@code qemu-img convert} to RAW, so it must not be the compressed QCOW2 physical size.

- */ - private CloudStackVolume createTemplateLunRequest(StoragePoolVO storagePool, Map details, - long templateId, long sizeInBytes) { - Svm svm = new Svm(); - svm.setName(details.get(OntapStorageConstants.SVM_NAME)); - - Lun lunRequest = new Lun(); - lunRequest.setSvm(svm); - lunRequest.setName(getTemplateLunName(storagePool, templateId)); - lunRequest.setOsType(Lun.OsTypeEnum.valueOf( - OntapStorageUtils.getOSTypeFromHypervisor(storagePool.getHypervisor().name()))); - LunSpace lunSpace = new LunSpace(); - lunSpace.setSize(sizeInBytes); - lunRequest.setSpace(lunSpace); - - CloudStackVolume request = new CloudStackVolume(); - request.setLun(lunRequest); - return request; - } - /** * Builds the request that clones the cached template LUN into a new volume LUN. * 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 3d20036a493f..0b48d2e374cd 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 @@ -40,6 +40,7 @@ import org.apache.cloudstack.storage.feign.model.Volume; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; @@ -54,6 +55,8 @@ import java.util.Map; import java.util.Objects; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; + /** * Storage Strategy represents the communication path for all the ONTAP storage options * @@ -495,6 +498,27 @@ public String getNetworkInterface() { */ abstract public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume); + /** + * Creates the protocol-specific backend object that caches a template on this pool. + * + *

iSCSI creates an empty LUN ({@code /vol/<flexVol>/cs_tmpl_<id>}) sized to + * {@code sizeInBytes}. NFS is a no-op on the array: the KVM agent later writes the qcow2 + * into the mounted FlexVolume.

+ * + *

Returns a {@link CloudStackVolume} so the driver can map it to {@code CreateCmdResult} + * and update {@code template_spool_ref}. SAN populates {@code lun}; NAS returns an empty + * volume (no LUN / file yet).

+ * + * @param storagePool CloudStack primary storage pool (one FlexVolume) + * @param templateInfo template being cached + * @param details pool details (SVM, protocol, etc.) + * @param sizeInBytes virtual size for the cache object (required for SAN; ignored for NAS) + * @return created cache identity, or an empty {@link CloudStackVolume} when nothing is + * pre-created on the array + */ + abstract public CloudStackVolume createTemplateCache(StoragePoolVO storagePool, TemplateInfo templateInfo, + Map details, long sizeInBytes); + /** * Method encapsulates the behavior based on the opted protocol in subclasses. * it is going to mimic 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 83b6b58a7fa9..a29eed7bccb0 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 @@ -30,6 +30,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.storage.command.CreateObjectCommand; import org.apache.cloudstack.storage.command.DeleteCommand; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -101,6 +102,18 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume } } + /** + * NFS template cache: nothing is pre-created on the array. The KVM agent writes the qcow2 + * into the mounted FlexVolume; the framework records {@code install_path} afterward. + */ + @Override + public CloudStackVolume createTemplateCache(StoragePoolVO storagePool, TemplateInfo templateInfo, + Map details, long sizeInBytes) { + logger.info("createTemplateCache: NFS pool [{}], template [{}] will be written directly to the mounted FlexVolume", + storagePool.getId(), templateInfo.getId()); + return null; + } + @Override CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) { return null; diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 9db0df768e4f..b9e32b081e4d 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -22,7 +22,9 @@ import com.cloud.host.HostVO; import com.cloud.utils.exception.CloudRuntimeException; import feign.FeignException; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Initiator; import org.apache.cloudstack.storage.feign.model.Svm; @@ -97,6 +99,89 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume } } + /** + * Creates an empty LUN that caches a template on this pool's FlexVolume. + * + *

Sized to the template virtual disk size (what KVM writes after {@code qemu-img convert} + * to RAW). On create failure, best-effort deletes any leftover LUN before rethrowing.

+ */ + @Override + public CloudStackVolume createTemplateCache(StoragePoolVO storagePool, TemplateInfo templateInfo, + Map details, long sizeInBytes) { + if (sizeInBytes <= 0) { + throw new CloudRuntimeException("Unknown virtual size for template [" + templateInfo.getId() + + "]; cannot size the template LUN on pool [" + storagePool.getId() + "]"); + } + + CloudStackVolume request = buildTemplateLunRequest(storagePool, details, templateInfo.getId(), sizeInBytes); + String lunName = request.getLun().getName(); + CloudStackVolume created = null; + try { + created = createCloudStackVolume(request); + Lun lun = created.getLun(); + logger.info("createTemplateCache: Created template cache LUN [{}] (uuid [{}], {} bytes) on pool [{}] for template [{}]", + lun.getName(), lun.getUuid(), sizeInBytes, storagePool.getId(), templateInfo.getId()); + return created; + } catch (Exception e) { + bestEffortDeleteTemplateCacheLun(details.get(OntapStorageConstants.SVM_NAME), lunName, + created != null && created.getLun() != null ? created.getLun().getUuid() : null); + if (e instanceof CloudRuntimeException) { + throw (CloudRuntimeException) e; + } + throw new CloudRuntimeException("Failed to create template cache LUN for template [" + templateInfo.getId() + + "]: " + e.getMessage(), e); + } + } + + private CloudStackVolume buildTemplateLunRequest(StoragePoolVO storagePool, Map details, + long templateId, long sizeInBytes) { + Svm svm = new Svm(); + svm.setName(details.get(OntapStorageConstants.SVM_NAME)); + + Lun lunRequest = new Lun(); + lunRequest.setSvm(svm); + lunRequest.setName(OntapStorageUtils.getLunName(storagePool.getName(), + OntapStorageConstants.TEMPLATE_LUN_PREFIX + templateId)); + lunRequest.setOsType(Lun.OsTypeEnum.valueOf( + OntapStorageUtils.getOSTypeFromHypervisor(storagePool.getHypervisor().name()))); + LunSpace lunSpace = new LunSpace(); + lunSpace.setSize(sizeInBytes); + lunRequest.setSpace(lunSpace); + + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lunRequest); + return request; + } + + private void bestEffortDeleteTemplateCacheLun(String svmName, String lunName, String lunUuid) { + try { + String uuid = lunUuid; + if (uuid == null || uuid.isEmpty()) { + Map lookup = Map.of( + OntapStorageConstants.NAME, lunName, + OntapStorageConstants.SVM_DOT_NAME, svmName); + CloudStackVolume existing = getCloudStackVolume(lookup); + if (existing == null || existing.getLun() == null || existing.getLun().getUuid() == null) { + logger.warn("bestEffortDeleteTemplateCacheLun: LUN [{}] not found on SVM [{}]; nothing to delete", + lunName, svmName); + return; + } + uuid = existing.getLun().getUuid(); + } + Lun lun = new Lun(); + lun.setUuid(uuid); + lun.setName(lunName); + CloudStackVolume deleteRequest = new CloudStackVolume(); + deleteRequest.setLun(lun); + deleteCloudStackVolume(deleteRequest); + logger.info("bestEffortDeleteTemplateCacheLun: Removed leftover template cache LUN [{}] on SVM [{}]", + lunName, svmName); + } catch (Exception cleanupEx) { + logger.warn("bestEffortDeleteTemplateCacheLun: Failed to remove leftover template cache LUN [{}] on SVM [{}]: {}", + lunName, svmName, cleanupEx.getMessage()); + } + } + @Override CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) { return null; diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index b6b0639e29b7..8399a08ee26c 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -800,7 +800,7 @@ void testCreateAsync_TemplateWithISCSI_CreatesLunAndRecordsCloneSource() { try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); - when(sanStrategy.createCloudStackVolume(any())).thenReturn(created); + when(sanStrategy.createTemplateCache(any(), any(), any(), anyLong())).thenReturn(created); driver.createAsync(dataStore, templateInfo, createCallback); @@ -843,7 +843,7 @@ void testCreateAsync_TemplateWithISCSI_RollsBackLunWhenDbUpdateFails() { try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); - when(sanStrategy.createCloudStackVolume(any())).thenReturn(created); + when(sanStrategy.createTemplateCache(any(), any(), any(), anyLong())).thenReturn(created); doNothing().when(sanStrategy).deleteCloudStackVolume(any()); driver.createAsync(dataStore, templateInfo, createCallback); @@ -870,13 +870,18 @@ void testCreateAsync_TemplateWithISCSI_UnknownSize_Fails() { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); - driver.createAsync(dataStore, templateInfo, createCallback); + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.createTemplateCache(any(), any(), any(), anyLong())) + .thenThrow(new CloudRuntimeException("Unknown virtual size for template [50]")); + + driver.createAsync(dataStore, templateInfo, createCallback); - ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); - verify(createCallback).complete(resultCaptor.capture()); - assertFalse(resultCaptor.getValue().isSuccess()); + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + } } @Test @@ -892,12 +897,17 @@ void testCreateAsync_TemplateWithNFS_IsMetadataOnly() { when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - driver.createAsync(dataStore, templateInfo, createCallback); + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(nasStrategy); + when(nasStrategy.createTemplateCache(any(), any(), any(), anyLong())).thenReturn(new CloudStackVolume()); - ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); - verify(createCallback).complete(resultCaptor.capture()); - assertTrue(resultCaptor.getValue().isSuccess()); - verify(vmTemplatePoolDao, never()).update(any(Long.class), any(VMTemplateStoragePoolVO.class)); + driver.createAsync(dataStore, templateInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + verify(vmTemplatePoolDao, never()).update(any(Long.class), any(VMTemplateStoragePoolVO.class)); + } } @Test diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java index cf8a3b1cd504..5ea1d8decd61 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java @@ -124,6 +124,13 @@ public CloudStackVolume createCloudStackVolume(CloudStackVolume cloudstackVolume return null; } + @Override + public CloudStackVolume createTemplateCache(org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool, + org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo templateInfo, + Map details, long sizeInBytes) { + return null; + } + @Override CloudStackVolume updateCloudStackVolume(CloudStackVolume cloudstackVolume) { return null; diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java index 85c9c04a0294..b6d9a5317701 100755 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java @@ -69,6 +69,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; @@ -210,6 +211,22 @@ public void testCreateCloudStackVolume_Success() throws Exception { verify(endPoint).sendMessage(any(CreateObjectCommand.class)); } + @Test + public void testCreateTemplateCache_IsNoOp() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getId()).thenReturn(1L); + org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo templateInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo.class); + when(templateInfo.getId()).thenReturn(50L); + + CloudStackVolume result = strategy.createTemplateCache(storagePool, templateInfo, Map.of(), 0L); + + assertNotNull(result); + assertNull(result.getLun()); + assertNull(result.getFile()); + } + // Test createCloudStackVolume - Volume Not Found @Test public void testCreateCloudStackVolume_VolumeNotFound() { diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index 04408bd727c2..fdc1db737549 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -157,6 +157,60 @@ void testCreateCloudStackVolume_Success() { } } + @Test + void testCreateTemplateCache_Success() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getHypervisor()).thenReturn(com.cloud.hypervisor.Hypervisor.HypervisorType.KVM); + + org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo templateInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo.class); + when(templateInfo.getId()).thenReturn(50L); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + + Lun createdLun = new Lun(); + createdLun.setName("/vol/vol1/cs_tmpl_50"); + createdLun.setUuid("template-lun-uuid"); + OntapResponse response = new OntapResponse<>(); + response.setRecords(List.of(createdLun)); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, org.mockito.Mockito.CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))).thenReturn(response); + + CloudStackVolume result = unifiedSANStrategy.createTemplateCache( + storagePool, templateInfo, details, 5368709120L); + + assertNotNull(result); + assertEquals("template-lun-uuid", result.getLun().getUuid()); + assertEquals("/vol/vol1/cs_tmpl_50", result.getLun().getName()); + + ArgumentCaptor lunCaptor = ArgumentCaptor.forClass(Lun.class); + verify(sanFeignClient).createLun(eq(authHeader), eq(true), lunCaptor.capture()); + assertEquals("/vol/vol1/cs_tmpl_50", lunCaptor.getValue().getName()); + assertEquals(5368709120L, lunCaptor.getValue().getSpace().getSize()); + } + } + + @Test + void testCreateTemplateCache_UnknownSize_ThrowsException() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getId()).thenReturn(1L); + org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo templateInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo.class); + when(templateInfo.getId()).thenReturn(50L); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.createTemplateCache(storagePool, templateInfo, Map.of(), 0L)); + verify(sanFeignClient, never()).createLun(any(), anyBoolean(), any()); + } + @Test void testCreateCloudStackVolume_NullRequest_ThrowsException() { assertThrows(CloudRuntimeException.class, From 09107d8ac81cb01d74ba19d743b995cf33783205 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Thu, 10 Sep 2026 19:33:19 +0530 Subject: [PATCH 11/15] CSTACKEX-259: correcting the coverage for junits --- .../OntapPrimaryDatastoreDriverTest.java | 179 ++++++++++++++-- .../service/UnifiedNASStrategyTest.java | 123 ++++++++++- .../service/UnifiedSANStrategyTest.java | 198 ++++++++++++++++++ 3 files changed, 484 insertions(+), 16 deletions(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index 8399a08ee26c..d30614631201 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -786,8 +786,6 @@ void testCreateAsync_TemplateWithISCSI_CreatesLunAndRecordsCloneSource() { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); - when(storagePool.getName()).thenReturn("vol1"); - when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); when(templatePoolRef.getId()).thenReturn(7L); @@ -827,8 +825,6 @@ void testCreateAsync_TemplateWithISCSI_RollsBackLunWhenDbUpdateFails() { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); - when(storagePool.getName()).thenReturn("vol1"); - when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); when(templatePoolRef.getId()).thenReturn(7L); @@ -864,11 +860,9 @@ void testCreateAsync_TemplateWithISCSI_UnknownSize_Fails() { when(dataStore.getId()).thenReturn(1L); when(dataStore.getName()).thenReturn("ontap-pool"); when(templateInfo.getType()).thenReturn(TEMPLATE); - when(templateInfo.getId()).thenReturn(50L); when(templateInfo.getSize()).thenReturn(0L); when(storagePoolDao.findById(1L)).thenReturn(storagePool); - when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { @@ -894,12 +888,11 @@ void testCreateAsync_TemplateWithNFS_IsMetadataOnly() { when(templateInfo.getUuid()).thenReturn("template-uuid"); when(storagePoolDao.findById(1L)).thenReturn(storagePool); - when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(nasStrategy); - when(nasStrategy.createTemplateCache(any(), any(), any(), anyLong())).thenReturn(new CloudStackVolume()); + when(nasStrategy.createTemplateCache(any(), any(), any(), anyLong())).thenReturn(null); driver.createAsync(dataStore, templateInfo, createCallback); @@ -969,23 +962,23 @@ private void stubVolumeCloneFromTemplate(long templateSize, long volumeSize) { when(volumeInfo.getType()).thenReturn(VOLUME); when(volumeInfo.getId()).thenReturn(100L); when(volumeInfo.getName()).thenReturn("test-volume"); - when(volumeInfo.getSize()).thenReturn(volumeSize); + lenient().when(volumeInfo.getSize()).thenReturn(volumeSize); when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); lenient().when(storagePool.getName()).thenReturn("vol1"); - when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.OntapiSCSI); - when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); + lenient().when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.OntapiSCSI); + lenient().when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(volumeDao.findById(100L)).thenReturn(volumeVO); - when(volumeVO.getId()).thenReturn(100L); + lenient().when(volumeVO.getId()).thenReturn(100L); when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.CLONE_OF_TEMPLATE)) .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.CLONE_OF_TEMPLATE, "50", false)); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); lenient().when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); - when(templatePoolRef.getTemplateSize()).thenReturn(templateSize); + lenient().when(templatePoolRef.getTemplateSize()).thenReturn(templateSize); } @Test @@ -1267,4 +1260,164 @@ void testCanProvideStorageStats_ReturnsFalse() { void testCanProvideVolumeStats_ReturnsFalse() { assertFalse(driver.canProvideVolumeStats()); } + + @Test + void testGetBytesRequiredForTemplate_NullArgs_ReturnsZero() { + assertEquals(0L, driver.getBytesRequiredForTemplate(null, storagePool)); + assertEquals(0L, driver.getBytesRequiredForTemplate(templateInfo, null)); + } + + @Test + void testCreateAsync_Template_StrategyThrows_Fails() { + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + when(templateInfo.getSize()).thenReturn(5368709120L); + when(templateInfo.getName()).thenReturn("tmpl"); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.createTemplateCache(any(), any(), any(), anyLong())) + .thenThrow(new RuntimeException("unexpected")); + + driver.createAsync(dataStore, templateInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromTemplate_NullCloneResult_Fails() { + stubVolumeCloneFromTemplate(5368709120L, 5368709120L); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.cloneCloudStackVolume(any())).thenReturn(null); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromTemplateNFS_MissingInstallPath_Fails() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + stubVolumeCloneFromTemplate(5368709120L, 5368709120L); + when(templatePoolRef.getInstallPath()).thenReturn(null); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(nasStrategy); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + verify(nasStrategy, never()).cloneCloudStackVolume(any()); + } + } + + @Test + void testDeleteAsync_Template_NoSpoolRef_Succeeds() { + when(dataStore.getId()).thenReturn(1L); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(null); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + + driver.deleteAsync(dataStore, templateInfo, commandCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(commandCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + verify(sanStrategy, never()).deleteCloudStackVolume(any()); + } + } + + @Test + void testGrantAccess_Template_CreatesIgroupWhenMissing() { + PrimaryDataStore primaryDataStore = mock(PrimaryDataStore.class); + Map dataStoreDetails = new HashMap<>(); + dataStoreDetails.put(PrimaryDataStore.MANAGED_STORE_TARGET, "stale-value"); + + when(primaryDataStore.getId()).thenReturn(1L); + when(primaryDataStore.getDetails()).thenReturn(dataStoreDetails); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER); + when(storagePool.getPath()).thenReturn("iqn.1992-08.com.netapp:sn.123456"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getId()).thenReturn(7L); + + HostVO hostVo = mock(HostVO.class); + when(hostVo.getName()).thenReturn("host1"); + when(hostVo.getUuid()).thenReturn("host-uuid-1"); + + AccessGroup createdAccessGroup = new AccessGroup(); + Igroup createdIgroup = new Igroup(); + createdIgroup.setName("igroup1"); + createdAccessGroup.setIgroup(createdIgroup); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); + utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); + + when(sanStrategy.getAccessGroup(any())).thenReturn(null); + when(sanStrategy.createAccessGroup(any())).thenReturn(createdAccessGroup); + when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); + + assertTrue(driver.grantAccess(templateInfo, hostVo, primaryDataStore)); + + verify(sanStrategy).createAccessGroup(any()); + verify(templatePoolRef).setInstallPath("/iqn.1992-08.com.netapp:sn.123456/3"); + } + } + + @Test + void testCreateAsync_VolumeClonedFromTemplate_MissingSpoolRef_Fails() { + when(dataStore.getId()).thenReturn(1L); + when(dataStore.getName()).thenReturn("ontap-pool"); + when(volumeInfo.getType()).thenReturn(VOLUME); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getName()).thenReturn("test-volume"); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.CLONE_OF_TEMPLATE)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.CLONE_OF_TEMPLATE, "50", false)); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(null); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + verify(sanStrategy, never()).cloneCloudStackVolume(any()); + } + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java index b6d9a5317701..826ed90636cf 100755 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java @@ -222,9 +222,8 @@ public void testCreateTemplateCache_IsNoOp() { CloudStackVolume result = strategy.createTemplateCache(storagePool, templateInfo, Map.of(), 0L); - assertNotNull(result); - assertNull(result.getLun()); - assertNull(result.getFile()); + // NFS seeds via host CopyCommand; strategy intentionally returns null (no LUN/file yet). + assertNull(result); } // Test createCloudStackVolume - Volume Not Found @@ -1058,4 +1057,122 @@ public void testDeleteFileByPath_Treats404AsSuccess() { verify(nasFeignClient).deleteFile(anyString(), eq("flexvol-uuid"), eq("template-uuid")); } + + @Test + public void testDeleteFileByPath_Non404Feign_Throws() { + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(500); + when(feignException.getMessage()).thenReturn("server error"); + doThrow(feignException).when(nasFeignClient).deleteFile(anyString(), eq("flexvol-uuid"), eq("template-uuid")); + + assertThrows(CloudRuntimeException.class, + () -> strategy.deleteFileByPath("flexvol-uuid", "template-uuid")); + } + + @Test + public void testCloneCloudStackVolume_MissingFlexVolUuid_Throws() { + FileInfo source = new FileInfo(); + source.setPath("template-uuid"); + CloudStackVolume request = new CloudStackVolume(); + request.setDatastoreId("1"); + request.setFile(source); + request.setDestinationPath("volume-uuid"); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(details); + + assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolume(request)); + verify(nasFeignClient, never()).cloneFile(anyString(), any(FileCloneRequest.class)); + } + + @Test + public void testCloneCloudStackVolume_MissingDatastoreId_Throws() { + FileInfo source = new FileInfo(); + source.setPath("template-uuid"); + CloudStackVolume request = new CloudStackVolume(); + request.setFile(source); + request.setDestinationPath("volume-uuid"); + + assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolume(request)); + } + + @Test + public void testCloneCloudStackVolume_FeignException_Throws() { + VolumeObject volumeObject = mock(VolumeObject.class); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + details.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(details); + + FileInfo source = new FileInfo(); + source.setPath("template-uuid"); + CloudStackVolume request = new CloudStackVolume(); + request.setDatastoreId("1"); + request.setVolumeInfo(volumeObject); + request.setFile(source); + request.setDestinationPath("volume-uuid"); + + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(500); + when(feignException.getMessage()).thenReturn("clone failed"); + when(nasFeignClient.cloneFile(anyString(), any(FileCloneRequest.class))).thenThrow(feignException); + + assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolume(request)); + } + + @Test + public void testResizeCloudStackVolume_InvalidRequest_Throws() { + assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(null, 100L)); + CloudStackVolume empty = new CloudStackVolume(); + assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(empty, 100L)); + + VolumeObject volumeObject = mock(VolumeObject.class); + CloudStackVolume withVol = new CloudStackVolume(); + withVol.setVolumeInfo(volumeObject); + assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(withVol, 0L)); + } + + @Test + public void testResizeCloudStackVolume_KvmHostFails_Throws() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + EndPoint endPoint = mock(EndPoint.class); + + when(volumeObject.getId()).thenReturn(100L); + when(volumeObject.getUuid()).thenReturn("volume-uuid"); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPath()).thenReturn("volume-uuid"); + when(volumeVO.getSize()).thenReturn(5368709120L); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(storagePool); + when(epSelector.select(volumeObject)).thenReturn(endPoint); + when(endPoint.sendMessage(any(ResizeVolumeCommand.class))).thenReturn(new Answer(null, false, "resize failed")); + + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(request, 21474836480L)); + } + + @Test + public void testResizeCloudStackVolume_NoEndpoint_Throws() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + + when(volumeObject.getId()).thenReturn(100L); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeVO.getPath()).thenReturn("volume-uuid"); + when(volumeVO.getSize()).thenReturn(5368709120L); + when(volumeVO.getPoolId()).thenReturn(1L); + when(primaryDataStoreDao.findById(1L)).thenReturn(storagePool); + when(epSelector.select(volumeObject)).thenReturn(null); + + CloudStackVolume request = new CloudStackVolume(); + request.setVolumeInfo(volumeObject); + + assertThrows(CloudRuntimeException.class, () -> strategy.resizeCloudStackVolume(request, 21474836480L)); + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index fdc1db737549..700b63d15575 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -211,6 +211,118 @@ void testCreateTemplateCache_UnknownSize_ThrowsException() { verify(sanFeignClient, never()).createLun(any(), anyBoolean(), any()); } + @Test + void testCreateTemplateCache_CreateFails_BestEffortDeletesLeftoverLun() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getHypervisor()).thenReturn(com.cloud.hypervisor.Hypervisor.HypervisorType.KVM); + + org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo templateInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo.class); + when(templateInfo.getId()).thenReturn(50L); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + + FeignException createEx = mock(FeignException.class); + when(createEx.status()).thenReturn(500); + when(createEx.getMessage()).thenReturn("create failed"); + + Lun leftover = new Lun(); + leftover.setName("/vol/vol1/cs_tmpl_50"); + leftover.setUuid("leftover-uuid"); + OntapResponse leftoverResponse = new OntapResponse<>(); + leftoverResponse.setRecords(List.of(leftover)); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, org.mockito.Mockito.CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))).thenThrow(createEx); + when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(leftoverResponse); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.createTemplateCache(storagePool, templateInfo, details, 5368709120L)); + + verify(sanFeignClient).deleteLun(eq(authHeader), eq("leftover-uuid"), anyMap()); + } + } + + @Test + void testCreateTemplateCache_CreateFails_NoLeftoverLun_SkipsDelete() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getHypervisor()).thenReturn(com.cloud.hypervisor.Hypervisor.HypervisorType.KVM); + + org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo templateInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo.class); + when(templateInfo.getId()).thenReturn(50L); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + + FeignException createEx = mock(FeignException.class); + when(createEx.status()).thenReturn(500); + when(createEx.getMessage()).thenReturn("create failed"); + + OntapResponse empty = new OntapResponse<>(); + empty.setRecords(List.of()); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, org.mockito.Mockito.CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))).thenThrow(createEx); + when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(empty); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.createTemplateCache(storagePool, templateInfo, details, 5368709120L)); + + verify(sanFeignClient, never()).deleteLun(any(), any(), anyMap()); + } + } + + @Test + void testCreateTemplateCache_CleanupFailureDoesNotMaskCreateError() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getHypervisor()).thenReturn(com.cloud.hypervisor.Hypervisor.HypervisorType.KVM); + + org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo templateInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo.class); + when(templateInfo.getId()).thenReturn(50L); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + + FeignException createEx = mock(FeignException.class); + when(createEx.status()).thenReturn(500); + when(createEx.getMessage()).thenReturn("create failed"); + + Lun leftover = new Lun(); + leftover.setName("/vol/vol1/cs_tmpl_50"); + leftover.setUuid("leftover-uuid"); + OntapResponse leftoverResponse = new OntapResponse<>(); + leftoverResponse.setRecords(List.of(leftover)); + + FeignException deleteEx = mock(FeignException.class); + when(deleteEx.status()).thenReturn(500); + when(deleteEx.getMessage()).thenReturn("delete failed"); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, org.mockito.Mockito.CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))).thenThrow(createEx); + when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(leftoverResponse); + doThrow(deleteEx).when(sanFeignClient).deleteLun(eq(authHeader), eq("leftover-uuid"), anyMap()); + + CloudRuntimeException thrown = assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.createTemplateCache(storagePool, templateInfo, details, 5368709120L)); + assertTrue(thrown.getMessage().contains("create failed") || thrown.getMessage().contains("Failed to create")); + } + } + @Test void testCreateCloudStackVolume_NullRequest_ThrowsException() { assertThrows(CloudRuntimeException.class, @@ -917,6 +1029,92 @@ void testResizeCloudStackVolume_NoUuid_Throws() { verify(sanFeignClient, never()).updateLun(any(), any(), any()); } + @Test + void testResizeCloudStackVolume_InvalidSize_Throws() { + Lun lun = new Lun(); + lun.setUuid("lun-uuid-123"); + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.resizeCloudStackVolume(request, 0L)); + assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.resizeCloudStackVolume(null, 100L)); + verify(sanFeignClient, never()).updateLun(any(), any(), any()); + } + + @Test + void testResizeCloudStackVolume_FeignException_Throws() { + Lun lun = new Lun(); + lun.setUuid("lun-uuid-123"); + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(500); + when(feignException.getMessage()).thenReturn("resize failed"); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + doThrow(feignException).when(sanFeignClient).updateLun(eq(authHeader), eq("lun-uuid-123"), any(Lun.class)); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.resizeCloudStackVolume(request, 21474836480L)); + } + } + + @Test + void testCloneCloudStackVolume_EmptyRecords_ThrowsException() { + Lun.Source source = new Lun.Source(); + source.setUuid("source-lun-uuid"); + Lun.Clone clone = new Lun.Clone(); + clone.setSource(source); + Lun lun = new Lun(); + lun.setName("/vol/vol1/cloned"); + lun.setClone(clone); + + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + OntapResponse response = new OntapResponse<>(); + response.setRecords(List.of()); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))).thenReturn(response); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolume(request)); + } + } + + @Test + void testCloneCloudStackVolume_FeignException_ThrowsException() { + Lun.Source source = new Lun.Source(); + source.setUuid("source-lun-uuid"); + Lun.Clone clone = new Lun.Clone(); + clone.setSource(source); + Lun lun = new Lun(); + lun.setName("/vol/vol1/cloned"); + lun.setClone(clone); + + CloudStackVolume request = new CloudStackVolume(); + request.setLun(lun); + + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(500); + when(feignException.getMessage()).thenReturn("clone failed"); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + when(sanFeignClient.createLun(eq(authHeader), eq(true), any(Lun.class))).thenThrow(feignException); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolume(request)); + } + } + @Test void testSetOntapStorage() { OntapStorage newStorage = mock(OntapStorage.class); From 120e86f12121206d9924ac0e543ebbea5bfcab99 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 21 Sep 2026 18:58:11 +0530 Subject: [PATCH 12/15] resolving conflicts related gaps --- .../driver/OntapPrimaryDatastoreDriver.java | 20 ++- .../storage/service/StorageStrategy.java | 153 +++++++++++++++++- .../storage/service/model/AccessGroup.java | 19 ++- .../storage/utils/OntapStorageConstants.java | 12 ++ 4 files changed, 190 insertions(+), 14 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 9c0fb78a914f..9b77fe0ec3cd 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 @@ -26,6 +26,7 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.host.Host; import com.cloud.host.HostVO; +import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.Storage; import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; @@ -176,7 +177,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet volumeVO.setPoolType(storagePool.getPoolType()); volumeVO.setPoolId(storagePool.getId()); - volumeVO.setFormat(getImageFormatByHypervisor(storagePool.getHypervisor())); + volumeVO.setFormat(getImageFormat(storagePool)); logger.info("createAsync: Volume format set to [{}] for hypervisor [{}]", volumeVO.getFormat(), storagePool.getHypervisor()); if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { @@ -1481,11 +1482,20 @@ private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId) } - private Storage.ImageFormat getImageFormatByHypervisor(HypervisorType hypervisorType) { - if (HypervisorType.KVM.equals(hypervisorType)) { - return Storage.ImageFormat.QCOW2; + private Storage.ImageFormat getImageFormat(StoragePoolVO storagePool) { + HypervisorType hypervisorType = storagePool.getHypervisor(); + if (!HypervisorType.KVM.equals(hypervisorType)) { + throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution"); + } + Storage.StoragePoolType spType = storagePool.getPoolType(); + switch (spType) { + case Iscsi: + return Storage.ImageFormat.RAW; + case NetworkFilesystem: + return Storage.ImageFormat.QCOW2; + default: + throw new CloudRuntimeException("Unsupported pool type [" + spType + "] for ONTAP image format resolution"); } - throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution"); } /** * Persists snapshot metadata in snapshot_details table. 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 0b48d2e374cd..3c6c52a148a0 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 @@ -83,6 +83,9 @@ public abstract class StorageStrategy { */ private List aggregates; + /** ONTAP SVM UUID resolved during the last successful {@link #connect(boolean)} call. */ + private String resolvedSvmUuid; + private static final Logger logger = LogManager.getLogger(StorageStrategy.class); public StorageStrategy(OntapStorage ontapStorage) { @@ -102,9 +105,26 @@ public StorageStrategy(OntapStorage ontapStorage) { } // Connect method to validate ONTAP cluster, credentials, protocol, and SVM + /** + * Validates ONTAP cluster reachability, credentials, SVM state, protocol, and aggregate capacity + * for new FlexVol creation (primary pool provisioning). + */ public boolean connect() { + return connect(true); + } + + /** + * Validates ONTAP cluster reachability and SVM/protocol settings. + * + *

Aggregate free-space checks apply only when {@code validateAggregatesForVolumeCreation} is + * {@code true} (pool provisioning). Snapshot, delete, revert, and grant/revoke paths must use + * {@code false} — they operate on an existing FlexVol and must not compare aggregate space to + * the full pool capacity stored in pool details.

+ */ + public boolean connect(boolean validateAggregatesForVolumeCreation) { logger.info("Attempting to connect to ONTAP cluster at " + storage.getStorageIP() + " and validate SVM " + - storage.getSvmName() + ", protocol " + storage.getProtocol()); + storage.getSvmName() + ", protocol " + storage.getProtocol() + + (validateAggregatesForVolumeCreation ? " (with aggregate validation)" : " (operations only)")); String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); String svmName = storage.getSvmName(); try { @@ -134,6 +154,14 @@ public boolean connect() { logger.error("ISCSI protocol is not enabled on SVM " + svmName); throw new CloudRuntimeException("ISCSI protocol is not enabled on SVM " + svmName); } + this.resolvedSvmUuid = svm.getUuid(); + + if (!validateAggregatesForVolumeCreation) { + logger.debug("Skipping aggregate capacity validation — not required for existing-volume operations"); + logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided"); + return true; + } + List aggrs = svm.getAggregates(); if (aggrs == null || aggrs.isEmpty()) { logger.error("No aggregates are assigned to SVM " + svmName); @@ -182,6 +210,13 @@ public boolean connect() { return true; } + /** + * ONTAP SVM UUID resolved during the last successful {@link #connect(boolean)} call. + */ + public String getResolvedSvmUuid() { + return resolvedSvmUuid; + } + // Common methods like create/delete etc., should be here /** @@ -697,7 +732,13 @@ public String getAuthHeader() { * @return true if the job completed successfully */ public Boolean jobPollForSuccess(String jobUUID, int maxRetries, int sleepTimeInMilliSecs) { - //Create URI for GET Job API + return jobPollUntilSuccess(jobUUID, maxRetries, sleepTimeInMilliSecs) != null; + } + + /** + * Polls an ONTAP async job until it succeeds and returns the completed job record. + */ + public Job jobPollUntilSuccess(String jobUUID, int maxRetries, int sleepTimeInMilliSecs) { int jobRetryCount = 0; Job jobResp = null; try { @@ -722,14 +763,112 @@ public Boolean jobPollForSuccess(String jobUUID, int maxRetries, int sleepTimeIn jobRetryCount++; Thread.sleep(sleepTimeInMilliSecs); } - if (jobResp == null || !jobResp.getState().equals(OntapStorageConstants.JOB_SUCCESS)) { - return false; - } + return jobResp; } catch (FeignException.FeignClientException e) { throw new CloudRuntimeException("Failed to fetch job status: " + e.getMessage()); } catch (InterruptedException e) { - throw new RuntimeException(e); + Thread.currentThread().interrupt(); + throw new CloudRuntimeException("Interrupted while polling ONTAP job " + jobUUID, e); + } + } + + /** + * Polls an ONTAP async job when the API response includes a job reference. + * + *

When no job is returned (common for CLI passthrough SFSR on synchronous completion), + * the operation is treated as successful after HTTP 2xx.

+ */ + public void pollJobIfPresent(JobResponse response, String operationName) { + pollJobIfPresent(response, operationName, + OntapStorageConstants.ONTAP_CG_JOB_MAX_RETRIES, + OntapStorageConstants.ONTAP_CG_JOB_POLL_INTERVAL_MS); + } + + /** + * Polls an ONTAP async job when present, using caller-supplied retry settings. + */ + public void pollJobIfPresent(JobResponse response, String operationName, + int maxRetries, int pollIntervalMs) { + if (response == null || response.getJob() == null || response.getJob().getUuid() == null) { + logger.debug("pollJobIfPresent: No async job returned for operation [{}], continuing without polling", + operationName); + return; + } + jobPollForSuccess(response.getJob().getUuid(), maxRetries, pollIntervalMs); + } + + /** + * Polls an ONTAP async job when present and returns the completed job. + */ + public Job pollJobIfPresentAndGetCompletedJob(JobResponse response, String operationName) { + return pollJobIfPresentAndGetCompletedJob(response, operationName, + OntapStorageConstants.ONTAP_CG_JOB_MAX_RETRIES, + OntapStorageConstants.ONTAP_CG_JOB_POLL_INTERVAL_MS); + } + + public Job pollJobIfPresentAndGetCompletedJob(JobResponse response, String operationName, + int maxRetries, int pollIntervalMs) { + if (response == null || response.getJob() == null || response.getJob().getUuid() == null) { + logger.debug("pollJobIfPresentAndGetCompletedJob: No async job for operation [{}]", operationName); + return null; + } + return jobPollUntilSuccess(response.getJob().getUuid(), maxRetries, pollIntervalMs); + } + + /** + * Completes CLI-based SFSR ({@code restore-file}) orchestration: poll job when returned, + * otherwise accept synchronous success. + */ + public void executeCliSfsrRestore(JobResponse response, String operationName) { + pollJobIfPresent(response, operationName, + OntapStorageConstants.ONTAP_SFSR_JOB_MAX_RETRIES, + OntapStorageConstants.ONTAP_SFSR_JOB_POLL_INTERVAL_MS); + } + + /** + * Deletes a FlexVolume snapshot on ONTAP for a CloudStack volume snapshot. + * + * @param flexVolUuid ONTAP FlexVolume UUID + * @param snapshotUuid ONTAP FlexVolume snapshot UUID + * @param snapshotName ONTAP FlexVolume snapshot name (for logging) + */ + public void deleteFlexVolSnapshotForCloudStackVolume(String flexVolUuid, String snapshotUuid, String snapshotName) { + if (flexVolUuid == null || flexVolUuid.isEmpty() || snapshotUuid == null || snapshotUuid.isEmpty()) { + throw new CloudRuntimeException("FlexVolume UUID and snapshot UUID are required to delete an ONTAP snapshot"); + } + + logger.info("deleteFlexVolSnapshotForCloudStackVolume: issuing ONTAP REST delete for snapshot [{}] " + + "(uuid={}) on FlexVol [{}]", snapshotName, snapshotUuid, flexVolUuid); + + try { + JobResponse jobResponse = snapshotFeignClient.deleteSnapshot(getAuthHeader(), flexVolUuid, snapshotUuid); + + if (jobResponse == null || jobResponse.getJob() == null) { + logger.debug("deleteFlexVolSnapshotForCloudStackVolume: no async job returned for snapshot [{}] " + + "(uuid={}); treating HTTP success as completion", snapshotName, snapshotUuid); + } else { + logger.debug("deleteFlexVolSnapshotForCloudStackVolume: polling ONTAP delete job [{}] for snapshot [{}]", + jobResponse.getJob().getUuid(), snapshotName); + } + + pollJobIfPresent(jobResponse, "delete FlexVol snapshot [" + snapshotName + "] uuid [" + snapshotUuid + "]", + OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_MAX_RETRIES, + OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_POLL_INTERVAL_MS); + + logger.info("deleteFlexVolSnapshotForCloudStackVolume: ONTAP FlexVol snapshot [{}] (uuid={}) removed from [{}]", + snapshotName, snapshotUuid, flexVolUuid); + } catch (Exception e) { + if (OntapStorageUtils.isOntapObjectNotFoundError(e)) { + logger.warn("deleteFlexVolSnapshotForCloudStackVolume: ONTAP snapshot [{}] (uuid={}) on FlexVol [{}] " + + "already absent; treating delete as success: {}", snapshotName, snapshotUuid, flexVolUuid, + e.getMessage()); + return; + } + if (e instanceof CloudRuntimeException) { + throw (CloudRuntimeException) e; + } + throw new CloudRuntimeException("Failed to delete ONTAP FlexVol snapshot [" + snapshotName + "]: " + + e.getMessage(), e); } - return true; } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java index 9815724fc1aa..8b1aa24fe5d9 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/AccessGroup.java @@ -19,21 +19,28 @@ package org.apache.cloudstack.storage.service.model; -import com.cloud.host.HostVO; +import java.util.List; + import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.storage.feign.model.ExportPolicy; import org.apache.cloudstack.storage.feign.model.Igroup; -import java.util.List; +import com.cloud.host.HostVO; public class AccessGroup { + public enum HostRuleAction { + ADD, + REMOVE + } + private Igroup igroup; private ExportPolicy exportPolicy; private List hostsToConnect; private Long storagePoolId; private Scope scope; + private HostRuleAction hostRuleAction = HostRuleAction.ADD; public Igroup getIgroup() { @@ -74,4 +81,12 @@ public Scope getScope() { public void setScope(Scope scope) { this.scope = scope; } + + public HostRuleAction getHostRuleAction() { + return hostRuleAction; + } + + public void setHostRuleAction(HostRuleAction hostRuleAction) { + this.hostRuleAction = hostRuleAction; + } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 51205069ee2a..ea5fcdfc427a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -57,6 +57,18 @@ public class OntapStorageConstants { public static final String JOB_FAILURE = "failure"; public static final String JOB_SUCCESS = "success"; + /** Consistency-group / generic async job polling defaults. */ + public static final int ONTAP_CG_JOB_MAX_RETRIES = 60; + public static final int ONTAP_CG_JOB_POLL_INTERVAL_MS = 2000; + + /** Single-file SnapRestore (SFSR) CLI job polling defaults. */ + public static final int ONTAP_SFSR_JOB_MAX_RETRIES = 60; + public static final int ONTAP_SFSR_JOB_POLL_INTERVAL_MS = 2000; + + /** FlexVol snapshot delete job polling defaults. */ + public static final int ONTAP_SNAPSHOT_DELETE_JOB_MAX_RETRIES = 30; + public static final int ONTAP_SNAPSHOT_DELETE_JOB_POLL_INTERVAL_MS = 2000; + public static final String TRUE = "true"; public static final String FALSE = "false"; From cb342ae0a8ac5b4c68d1f019114b21972c5bc06f Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 21 Sep 2026 19:25:22 +0530 Subject: [PATCH 13/15] junit resolution --- .../storage/driver/OntapPrimaryDatastoreDriverTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index d30614631201..d0e6b26df2ad 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -967,7 +967,11 @@ private void stubVolumeCloneFromTemplate(long templateSize, long volumeSize) { when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePool.getId()).thenReturn(1L); lenient().when(storagePool.getName()).thenReturn("vol1"); - lenient().when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.OntapiSCSI); + // Upstream has no OntapiSCSI; match lifecycle mapping (ISCSI->Iscsi, NFS->NetworkFilesystem). + boolean nfs = ProtocolType.NFS3.name().equalsIgnoreCase( + storagePoolDetails.get(OntapStorageConstants.PROTOCOL)); + lenient().when(storagePool.getPoolType()).thenReturn( + nfs ? Storage.StoragePoolType.NetworkFilesystem : Storage.StoragePoolType.Iscsi); lenient().when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); From cad486cbd4cfc4629de4c01dd603eb773f1c6be6 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 22 Sep 2026 16:18:49 +0530 Subject: [PATCH 14/15] ensuring that template exists then only proceed for the VM create by using template --- .../driver/OntapPrimaryDatastoreDriver.java | 91 +++++++++++++++++++ .../OntapPrimaryDatastoreDriverTest.java | 76 ++++++++++++++++ 2 files changed, 167 insertions(+) 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 9b77fe0ec3cd..b4cb1c8b8ba8 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 @@ -35,6 +35,7 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.SnapshotDetailsDao; import com.cloud.storage.dao.SnapshotDetailsVO; @@ -287,6 +288,12 @@ private CloudStackVolume cloneCloudStackVolumeFromTemplate(StoragePoolVO storage StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); boolean iscsi = isIscsi(details); + if (iscsi) { + // Ready + DOWNLOADED spool_ref skips copy; if the cache LUN is gone, invalidate so the + // next deploy recopies into a recreated LUN instead of cloning an empty/missing source. + ensureTemplateCachePresentForClone(storagePool, details, templatePoolRef, templateId); + } + CloudStackVolume request = iscsi ? createCloneLunRequest(storagePool, details, volumeInfo, templatePoolRef, templateId) : createCloneFileRequest(storagePool, volumeInfo, templatePoolRef, templateId); @@ -704,6 +711,10 @@ private void grantAccessTemplate(TemplateInfo templateInfo, Host host, DataStore UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); String accessGroupName = OntapStorageUtils.getIgroupName(svmName, host.getUuid()); + // Framework may skip createAsync when template_spool_ref is already Ready (e.g. after a + // failed copy left Ready + NOT_DOWNLOADED). Recreate the LUN if it is missing on ONTAP. + ensureTemplateCacheLunExists(sanStrategy, storagePool, templateInfo, templatePoolRef, details, svmName, lunName); + ensureAccessGroupForHost(sanStrategy, host, storagePool, svmName, accessGroupName); boolean lunMapped = false; @@ -732,6 +743,86 @@ private void grantAccessTemplate(TemplateInfo templateInfo, Host host, DataStore } } + /** + * Ensures the template-cache LUN exists on ONTAP before mapping it to a host igroup. + * + *

{@code VolumeServiceImpl} skips {@code createAsync} when {@code template_spool_ref} is + * already {@code Ready}. A prior failed copy can leave that Ready row while the LUN was + * deleted or never persisted, which makes {@code ensureLunMapped} fail with ONTAP error + * 5374876 (LUN not found). Recreate the LUN in that case and refresh spool-ref identity. + * Download state is forced back to {@code NOT_DOWNLOADED} so a stale {@code DOWNLOADED} + * marker cannot skip the copy into the new empty LUN.

+ */ + private void ensureTemplateCacheLunExists(UnifiedSANStrategy sanStrategy, StoragePoolVO storagePool, + TemplateInfo templateInfo, VMTemplateStoragePoolVO templatePoolRef, + Map details, String svmName, String lunName) { + CloudStackVolume existing = getCloudStackVolumeByName(sanStrategy, svmName, lunName); + if (existing != null && existing.getLun() != null) { + if (templatePoolRef.getLocalDownloadPath() == null || templatePoolRef.getLocalDownloadPath().isEmpty()) { + templatePoolRef.setLocalDownloadPath(existing.getLun().getUuid()); + vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); + } + return; + } + + logger.warn("ensureTemplateCacheLunExists: Template cache LUN [{}] missing on SVM [{}] for template [{}] " + + "on pool [{}]; recreating before grantAccess", + lunName, svmName, templateInfo.getId(), storagePool.getId()); + + long sizeInBytes = getDataObjectSizeIncludingHypervisorSnapshotReserve(templateInfo, storagePool); + CloudStackVolume created = sanStrategy.createTemplateCache(storagePool, templateInfo, details, sizeInBytes); + if (created == null || created.getLun() == null) { + throw new CloudRuntimeException("Failed to recreate missing template cache LUN [" + lunName + + "] for template [" + templateInfo.getId() + "]"); + } + markTemplateCacheNeedsRecopy(templatePoolRef, created.getLun().getUuid(), sizeInBytes); + } + + /** + * Before cloning from a cached template, verify the iSCSI cache LUN still exists. + * If the spool_ref is stale (Ready/DOWNLOADED but LUN gone), mark it for recopy and fail + * this attempt so the next deploy enters {@code copyTemplateToManagedTemplateVolume}, + * which recreates the LUN via {@link #ensureTemplateCacheLunExists}. + */ + private void ensureTemplateCachePresentForClone(StoragePoolVO storagePool, Map details, + VMTemplateStoragePoolVO templatePoolRef, long templateId) { + String svmName = details.get(OntapStorageConstants.SVM_NAME); + String lunName = getTemplateLunName(storagePool, templateId); + UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); + + CloudStackVolume existing = getCloudStackVolumeByName(sanStrategy, svmName, lunName); + if (existing != null && existing.getLun() != null) { + return; + } + + logger.warn("ensureTemplateCachePresentForClone: Cache LUN [{}] missing for template [{}] on pool [{}]; " + + "resetting template_spool_ref to NOT_DOWNLOADED so the next deploy recopies", + lunName, templateId, storagePool.getId()); + markTemplateCacheNeedsRecopy(templatePoolRef, null, templatePoolRef.getTemplateSize()); + throw new CloudRuntimeException("Template cache LUN [" + lunName + "] for template [" + templateId + + "] is missing on ONTAP; template_spool_ref was reset to NOT_DOWNLOADED — retry the deploy"); + } + + /** + * Updates {@code template_spool_ref} after recreating a missing cache LUN (or when invalidating + * a stale DOWNLOADED row). Keeps the row {@code Ready} so {@code getTemplate} still finds it, + * but forces {@code NOT_DOWNLOADED} so {@code VolumeServiceImpl} will copy the image again. + */ + private void markTemplateCacheNeedsRecopy(VMTemplateStoragePoolVO templatePoolRef, String lunUuid, long sizeInBytes) { + if (lunUuid != null && !lunUuid.isEmpty()) { + templatePoolRef.setLocalDownloadPath(lunUuid); + } else { + templatePoolRef.setLocalDownloadPath(null); + } + if (sizeInBytes > 0) { + templatePoolRef.setTemplateSize(sizeInBytes); + } + templatePoolRef.setDownloadPercent(0); + templatePoolRef.setDownloadState(VMTemplateStorageResourceAssoc.Status.NOT_DOWNLOADED); + templatePoolRef.setInstallPath(null); + vmTemplatePoolDao.update(templatePoolRef.getId(), templatePoolRef); + } + /** * Ensures an igroup containing this host's initiator exists on the SVM. * diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index d0e6b26df2ad..713f1cc04e01 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -1017,6 +1017,14 @@ void testGrantAccess_Template_WritesInstallPathAndRefreshesStoreTarget() { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); + Lun templateLun = new Lun(); + templateLun.setName("/vol/vol1/cs_tmpl_50"); + templateLun.setUuid("template-lun-uuid"); + CloudStackVolume cachedTemplate = new CloudStackVolume(); + cachedTemplate.setLun(templateLun); + when(sanStrategy.getCloudStackVolume(any())).thenReturn(cachedTemplate); + when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); + when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup); when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); @@ -1025,6 +1033,7 @@ void testGrantAccess_Template_WritesInstallPathAndRefreshesStoreTarget() { String expectedPath = "/iqn.1992-08.com.netapp:sn.123456/3"; verify(templatePoolRef).setInstallPath(expectedPath); verify(vmTemplatePoolDao).update(eq(7L), any(VMTemplateStoragePoolVO.class)); + verify(sanStrategy, never()).createTemplateCache(any(), any(), any(), anyLong()); ArgumentCaptor> detailsCaptor = ArgumentCaptor.forClass(Map.class); verify(primaryDataStore).setDetails(detailsCaptor.capture()); @@ -1077,6 +1086,7 @@ void testGrantAccess_Template_UnmapsWhenInstallPathUpdateFails() { when(sanStrategy.getCloudStackVolume(argThat(map -> map != null && "/vol/vol1/cs_tmpl_50".equals(map.get("name"))))) .thenReturn(cachedTemplate); + when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); when(sanStrategy.validateInitiatorInAccessGroup(anyString(), anyString(), any(Igroup.class))).thenReturn(true); doNothing().when(sanStrategy).disableLogicalAccess(any()); @@ -1086,6 +1096,63 @@ void testGrantAccess_Template_UnmapsWhenInstallPathUpdateFails() { verify(sanStrategy).disableLogicalAccess(argThat(map -> map != null && "template-lun-uuid".equals(map.get("lun.uuid")) && "igroup-uuid-123".equals(map.get("igroup.uuid")))); + verify(sanStrategy, never()).createTemplateCache(any(), any(), any(), anyLong()); + } + } + + @Test + void testGrantAccess_Template_RecreatesMissingCacheLunBeforeMap() { + PrimaryDataStore primaryDataStore = mock(PrimaryDataStore.class); + Map dataStoreDetails = new HashMap<>(); + dataStoreDetails.put(PrimaryDataStore.MANAGED_STORE_TARGET, "stale-value"); + + when(primaryDataStore.getId()).thenReturn(1L); + when(primaryDataStore.getDetails()).thenReturn(dataStoreDetails); + when(templateInfo.getType()).thenReturn(TEMPLATE); + when(templateInfo.getId()).thenReturn(50L); + when(templateInfo.getSize()).thenReturn(5L * 1024 * 1024 * 1024); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getName()).thenReturn("vol1"); + when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER); + when(storagePool.getPath()).thenReturn("iqn.1992-08.com.netapp:sn.123456"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); + when(templatePoolRef.getId()).thenReturn(7L); + when(templatePoolRef.getLocalDownloadPath()).thenReturn(null); + + when(host.getName()).thenReturn("host1"); + when(host.getUuid()).thenReturn("host-uuid-1"); + + AccessGroup existingAccessGroup = new AccessGroup(); + Igroup existingIgroup = new Igroup(); + existingIgroup.setName("igroup1"); + existingAccessGroup.setIgroup(existingIgroup); + + Lun createdLun = new Lun(); + createdLun.setName("/vol/vol1/cs_tmpl_50"); + createdLun.setUuid("recreated-lun-uuid"); + CloudStackVolume created = new CloudStackVolume(); + created.setLun(createdLun); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); + utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); + + when(sanStrategy.getCloudStackVolume(any())).thenReturn(null); + when(sanStrategy.createTemplateCache(eq(storagePool), eq(templateInfo), eq(storagePoolDetails), anyLong())) + .thenReturn(created); + when(sanStrategy.getAccessGroup(any())).thenReturn(existingAccessGroup); + when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); + + assertTrue(driver.grantAccess(templateInfo, host, primaryDataStore)); + + verify(sanStrategy).createTemplateCache(eq(storagePool), eq(templateInfo), eq(storagePoolDetails), anyLong()); + verify(templatePoolRef).setLocalDownloadPath("recreated-lun-uuid"); + verify(templatePoolRef).setDownloadState(com.cloud.storage.VMTemplateStorageResourceAssoc.Status.NOT_DOWNLOADED); + verify(templatePoolRef).setInstallPath("/iqn.1992-08.com.netapp:sn.123456/3"); + verify(sanStrategy).ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1")); } } @@ -1386,6 +1453,14 @@ void testGrantAccess_Template_CreatesIgroupWhenMissing() { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); utilityMock.when(() -> OntapStorageUtils.getIgroupName(anyString(), anyString())).thenReturn("igroup1"); + Lun templateLun = new Lun(); + templateLun.setName("/vol/vol1/cs_tmpl_50"); + templateLun.setUuid("template-lun-uuid"); + CloudStackVolume cachedTemplate = new CloudStackVolume(); + cachedTemplate.setLun(templateLun); + when(sanStrategy.getCloudStackVolume(any())).thenReturn(cachedTemplate); + when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); + when(sanStrategy.getAccessGroup(any())).thenReturn(null); when(sanStrategy.createAccessGroup(any())).thenReturn(createdAccessGroup); when(sanStrategy.ensureLunMapped(eq("svm1"), eq("/vol/vol1/cs_tmpl_50"), eq("igroup1"))).thenReturn("3"); @@ -1394,6 +1469,7 @@ void testGrantAccess_Template_CreatesIgroupWhenMissing() { verify(sanStrategy).createAccessGroup(any()); verify(templatePoolRef).setInstallPath("/iqn.1992-08.com.netapp:sn.123456/3"); + verify(sanStrategy, never()).createTemplateCache(any(), any(), any(), anyLong()); } } From 876f40a6598e704ebfe68319d1ffd2078958db96 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 22 Sep 2026 16:44:35 +0530 Subject: [PATCH 15/15] ndle stale entries in template tables --- .../driver/OntapPrimaryDatastoreDriver.java | 8 ++++---- .../driver/OntapPrimaryDatastoreDriverTest.java | 15 ++++++++++++++- 2 files changed, 18 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 b4cb1c8b8ba8..01f49276ef47 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 @@ -291,7 +291,7 @@ private CloudStackVolume cloneCloudStackVolumeFromTemplate(StoragePoolVO storage if (iscsi) { // Ready + DOWNLOADED spool_ref skips copy; if the cache LUN is gone, invalidate so the // next deploy recopies into a recreated LUN instead of cloning an empty/missing source. - ensureTemplateCachePresentForClone(storagePool, details, templatePoolRef, templateId); + ensureTemplateCachePresentForClone(storageStrategy, storagePool, details, templatePoolRef, templateId); } CloudStackVolume request = iscsi @@ -784,13 +784,13 @@ private void ensureTemplateCacheLunExists(UnifiedSANStrategy sanStrategy, Storag * this attempt so the next deploy enters {@code copyTemplateToManagedTemplateVolume}, * which recreates the LUN via {@link #ensureTemplateCacheLunExists}. */ - private void ensureTemplateCachePresentForClone(StoragePoolVO storagePool, Map details, + private void ensureTemplateCachePresentForClone(StorageStrategy storageStrategy, StoragePoolVO storagePool, + Map details, VMTemplateStoragePoolVO templatePoolRef, long templateId) { String svmName = details.get(OntapStorageConstants.SVM_NAME); String lunName = getTemplateLunName(storagePool, templateId); - UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); - CloudStackVolume existing = getCloudStackVolumeByName(sanStrategy, svmName, lunName); + CloudStackVolume existing = getCloudStackVolumeByName(storageStrategy, svmName, lunName); if (existing != null && existing.getLun() != null) { return; } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index 713f1cc04e01..f21eb172373e 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -915,6 +915,7 @@ void testCreateAsync_VolumeClonedFromTemplate_ClonesWithoutGrowing() { try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + stubExistingIscsiTemplateCacheLun(); when(sanStrategy.cloneCloudStackVolume(any())).thenReturn(cloned); driver.createAsync(dataStore, volumeInfo, createCallback); @@ -945,6 +946,7 @@ void testCreateAsync_VolumeClonedFromTemplate_GrowsWhenOfferingIsLarger() { try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + stubExistingIscsiTemplateCacheLun(); when(sanStrategy.cloneCloudStackVolume(any())).thenReturn(cloned); driver.createAsync(dataStore, volumeInfo, createCallback); @@ -983,6 +985,17 @@ private void stubVolumeCloneFromTemplate(long templateSize, long volumeSize) { when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); lenient().when(templatePoolRef.getLocalDownloadPath()).thenReturn("template-lun-uuid"); lenient().when(templatePoolRef.getTemplateSize()).thenReturn(templateSize); + lenient().when(templatePoolRef.getId()).thenReturn(7L); + } + + /** Stub ONTAP lookup used by {@code ensureTemplateCachePresentForClone} (iSCSI only). */ + private void stubExistingIscsiTemplateCacheLun() { + Lun templateLun = new Lun(); + templateLun.setName("/vol/vol1/cs_tmpl_50"); + templateLun.setUuid("template-lun-uuid"); + CloudStackVolume cachedTemplate = new CloudStackVolume(); + cachedTemplate.setLun(templateLun); + when(sanStrategy.getCloudStackVolume(any())).thenReturn(cachedTemplate); } @Test @@ -1120,7 +1133,6 @@ void testGrantAccess_Template_RecreatesMissingCacheLunBeforeMap() { when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(vmTemplatePoolDao.findByPoolTemplate(1L, 50L, null)).thenReturn(templatePoolRef); when(templatePoolRef.getId()).thenReturn(7L); - when(templatePoolRef.getLocalDownloadPath()).thenReturn(null); when(host.getName()).thenReturn("host1"); when(host.getUuid()).thenReturn("host-uuid-1"); @@ -1369,6 +1381,7 @@ void testCreateAsync_VolumeClonedFromTemplate_NullCloneResult_Fails() { try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + stubExistingIscsiTemplateCacheLun(); when(sanStrategy.cloneCloudStackVolume(any())).thenReturn(null); driver.createAsync(dataStore, volumeInfo, createCallback);