From 901fc4378206bcc04bbbc7c77ab0d409b8233924 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Fri, 18 Sep 2026 11:47:38 +0530 Subject: [PATCH 1/4] CSTACKEX-306: support for create CS colume from CS snapshot on the local primary storage pool --- .../driver/OntapPrimaryDatastoreDriver.java | 216 ++++++++++--- .../storage/feign/model/FileCloneRequest.java | 54 +++- .../storage/service/StorageStrategy.java | 27 ++ .../storage/service/UnifiedNASStrategy.java | 71 ++++ .../storage/service/UnifiedSANStrategy.java | 97 ++++++ .../service/model/CloudStackVolume.java | 14 + .../storage/utils/OntapStorageConstants.java | 21 ++ .../storage/utils/OntapStorageUtils.java | 43 +++ .../OntapPrimaryDatastoreDriverTest.java | 302 ++++++++++++++++++ .../storage/service/StorageStrategyTest.java | 11 + .../service/UnifiedNASStrategyTest.java | 124 +++++++ .../service/UnifiedSANStrategyTest.java | 213 ++++++++++++ .../storage/utils/OntapStorageUtilsTest.java | 25 ++ 13 files changed, 1174 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 ed942b438e16..f1e6618913e3 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,12 +170,39 @@ 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: 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 clonedCloudStackVolume = cloneOfTemplateId != null - ? cloneCloudStackVolumeFromTemplate(storagePool, volInfo, details, cloneOfTemplateId) - : createCloudStackVolume(storagePool, volInfo, details); + /* + * Create-volume combinations on ONTAP primary (v1): + * + * 1) cloneOfSnapshot — StorageSystemDataMotionStrategy sets volume_details.cloneOfSnapshot + * when createVolume(snapshotid) targets a managed backend snapshot. + * - Same primary pool / FlexVol only (PRIMARY_POOL_ID must match dataStore). + * - DATA or ROOT snapshot → new attachable data volume (ROOT is never bootable + * via this path; bootable recovery is createTemplate(snapshotid) → deploy). + * - Backend: iSCSI → POST /api/storage/luns (clone.source in .snapshot/); + * NFS → POST /api/storage/file/clone with snapshot.name. + * - IOPS: MIN_IOPS/MAX_IOPS may be on snapshot_details; apply is TODO below. + * + * 2) cloneOfTemplate — deploy / create from cached template on this pool. + * + * 3) else — blank LUN (iSCSI) or qcow2 (NFS). + * + * Mutually exclusive from the motion/orchestrator layer; snapshot is checked first + * (SolidFire-style) so a restore never accidentally falls through to blank create. + */ + Long cloneOfSnapshotId = getSnapshotIdForCloning(volInfo.getId()); + CloudStackVolume clonedCloudStackVolume; + if (cloneOfSnapshotId != null) { + clonedCloudStackVolume = cloneCloudStackVolumeFromSnapshot( + storagePool, volInfo, details, cloneOfSnapshotId); + // TODO(CSTACKEX-306): apply persisted MIN_IOPS / MAX_IOPS from snapshot_details + // onto this CloudStack volume (and ONTAP QoS if applicable) after successful clone. + } else { + Long cloneOfTemplateId = getTemplateIdForCloning(volInfo.getId()); + clonedCloudStackVolume = cloneOfTemplateId != null + ? cloneCloudStackVolumeFromTemplate( + storagePool, volInfo, details, cloneOfTemplateId) + : createCloudStackVolume(storagePool, volInfo, details); + } volumeVO.setPoolType(storagePool.getPoolType()); volumeVO.setPoolId(storagePool.getId()); @@ -327,6 +354,102 @@ private Long getTemplateIdForCloning(long volumeId) { return Long.valueOf(detail.getValue()); } + /** + * Returns the CloudStack snapshot id to clone from when {@code volume_details.cloneOfSnapshot} + * is set, or null when this create is not a restore-from-snapshot. + * + *

Set by {@code StorageSystemDataMotionStrategy.handleCreateManagedVolumeFromManagedSnapshot} + * for the duration of {@code createAsync} only (same pattern as {@link #getTemplateIdForCloning}).

+ */ + private Long getSnapshotIdForCloning(long volumeId) { + VolumeDetailVO detail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.CLONE_OF_SNAPSHOT); + if (detail == null || detail.getValue() == null || detail.getValue().isEmpty()) { + return null; + } + return Long.valueOf(detail.getValue()); + } + + /** + * Creates a new volume on this pool by cloning a file/LUN from a CloudStack volume snapshot + * that already lives on the same FlexVolume. + * + *

Combinations (product + plugin v1):

+ * + * + *

Optional grow when the disk offering is larger than the snapshot size (same pattern as + * clone-from-template).

+ */ + private CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, VolumeInfo volumeInfo, + Map details, long csSnapshotId) { + String snapshotName = requireSnapshotDetail(csSnapshotId, OntapStorageConstants.ONTAP_SNAP_NAME); + String volumePath = requireSnapshotDetail(csSnapshotId, OntapStorageConstants.VOLUME_PATH); + String primaryPoolId = requireSnapshotDetail(csSnapshotId, OntapStorageConstants.PRIMARY_POOL_ID); + String snapProtocol = requireSnapshotDetail(csSnapshotId, OntapStorageConstants.PROTOCOL); + + // Same-pool / same-protocol gate (v1). Fail before any ONTAP call. + if (!String.valueOf(storagePool.getId()).equals(primaryPoolId)) { + throw new CloudRuntimeException("Create volume from snapshot [" + csSnapshotId + + "] requires the snapshot's primary pool [" + primaryPoolId + + "]; requested pool is [" + storagePool.getId() + "] (cross-pool restore is not supported in v1)"); + } + String poolProtocol = details.get(OntapStorageConstants.PROTOCOL); + if (poolProtocol == null || !poolProtocol.equalsIgnoreCase(snapProtocol)) { + throw new CloudRuntimeException("Create volume from snapshot [" + csSnapshotId + + "] protocol mismatch: snapshot=[" + snapProtocol + "], pool=[" + poolProtocol + "]"); + } + + StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); + + logger.info("cloneCloudStackVolumeFromSnapshot: Cloning from CS snapshot [{}] (ONTAP snap [{}], path [{}]) " + + "for volume [{}] on pool [{}] protocol [{}]", + csSnapshotId, snapshotName, volumePath, volumeInfo.getId(), storagePool.getId(), poolProtocol); + + CloudStackVolume cloned = storageStrategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, volumePath, snapshotName); + if (cloned == null) { + throw new CloudRuntimeException("ONTAP returned nothing when cloning snapshot [" + csSnapshotId + + "] for volume [" + volumeInfo.getId() + "]"); + } + + long requestedSize = getDataObjectSizeIncludingHypervisorSnapshotReserve(volumeInfo, storagePool); + long snapshotSize = resolveSnapshotSizeBytes(csSnapshotId); + if (snapshotSize > 0 && requestedSize > snapshotSize) { + logger.info("cloneCloudStackVolumeFromSnapshot: Growing clone of snapshot [{}] from {} to {} bytes for volume [{}]", + csSnapshotId, snapshotSize, requestedSize, volumeInfo.getId()); + storageStrategy.resizeCloudStackVolume(cloned, requestedSize); + } + + return cloned; + } + + private String requireSnapshotDetail(long csSnapshotId, String key) { + String value = getSnapshotDetail(csSnapshotId, key); + if (value == null || value.isEmpty()) { + throw new CloudRuntimeException("Missing snapshot_details [" + key + "] for snapshot [" + csSnapshotId + + "]; cannot create volume from snapshot"); + } + return value; + } + + private long resolveSnapshotSizeBytes(long csSnapshotId) { + SnapshotVO snapshotVO = snapshotDao.findById(csSnapshotId); + if (snapshotVO == null || snapshotVO.getSize() <= 0) { + return 0L; + } + return snapshotVO.getSize(); + } + private VMTemplateStoragePoolVO findTemplatePoolRef(long poolId, long templateId) { VMTemplateStoragePoolVO templatePoolRef = vmTemplatePoolDao.findByPoolTemplate(poolId, templateId, null); if (templatePoolRef == null) { @@ -1122,9 +1245,11 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallbackUsed by create-volume-from-snapshot (same FlexVol). Omitted for live template-cache clones.

+ */ + @JsonProperty("snapshot") + private SnapshotRef snapshot; + public FileCloneRequest() { } @@ -55,6 +64,14 @@ public FileCloneRequest(String flexVolUuid, String flexVolName, String sourcePat this.destinationPath = destinationPath; } + public FileCloneRequest(String flexVolUuid, String flexVolName, String sourcePath, String destinationPath, + String snapshotName) { + this(flexVolUuid, flexVolName, sourcePath, destinationPath); + if (snapshotName != null && !snapshotName.isEmpty()) { + this.snapshot = new SnapshotRef(snapshotName); + } + } + public VolumeRef getVolume() { return volume; } @@ -87,6 +104,14 @@ public void setOverwriteDestination(Boolean overwriteDestination) { this.overwriteDestination = overwriteDestination; } + public SnapshotRef getSnapshot() { + return snapshot; + } + + public void setSnapshot(SnapshotRef snapshot) { + this.snapshot = snapshot; + } + @JsonIgnoreProperties(ignoreUnknown = true) @JsonInclude(JsonInclude.Include.NON_NULL) public static class VolumeRef { @@ -122,10 +147,37 @@ public void setName(String name) { } } + /** + * Snapshot identity for {@code POST /api/storage/file/clone} when cloning from a FlexVol snapshot. + */ + @JsonIgnoreProperties(ignoreUnknown = true) + @JsonInclude(JsonInclude.Include.NON_NULL) + public static class SnapshotRef { + + @JsonProperty("name") + private String name; + + public SnapshotRef() { + } + + public SnapshotRef(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.getUuid() : null) + ", sourcePath=" + sourcePath - + ", destinationPath=" + destinationPath + "}"; + + ", destinationPath=" + destinationPath + + ", snapshot=" + (snapshot != null ? snapshot.getName() : null) + "}"; } } 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 e482301967f1..a6b0a56279da 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 @@ -65,6 +65,7 @@ import feign.FeignException; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; /** * Storage Strategy represents the communication path for all the ONTAP storage options @@ -829,6 +830,32 @@ abstract public CloudStackVolume createTemplateCache(StoragePoolVO storagePool, */ abstract public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume); + /** + * Creates a new file/LUN in the same FlexVolume by cloning from a FlexVolume snapshot. + * + *

Product scope (v1): same primary pool / FlexVol only. Cross-pool restore is + * descoped — operators may later {@code migrateVolume} if another pool is required.

+ * + *

ONTAP backends (protocol-specific; each subclass builds its own request):

+ *
    + *
  • NAS (NFS3) — {@code POST /api/storage/file/clone} with {@code snapshot.name}
    • + *
  • SAN (iSCSI) — {@code POST /api/storage/luns} with {@code clone.source.name} = + * {@code /vol/<fv>/.snapshot/<snap>/<lun>}
    • + *
+ * + * @param storagePool target CloudStack primary pool (same FlexVol as the snapshot) + * @param details pool details (SVM, FlexVol name/uuid, protocol, …) + * @param volumeInfo destination CloudStack volume being created + * @param sourceVolumePath snapshotted object path from {@code snapshot_details.VOLUME_PATH} + * @param snapshotName ONTAP FlexVol snapshot name from {@code snapshot_details} + * @return created CloudStackVolume with protocol-specific identity (LUN uuid or file path) + */ + abstract public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, + Map details, + VolumeInfo volumeInfo, + String sourceVolumePath, + String snapshotName); + /** * Grows an existing backend object to {@code sizeInBytes}. * 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 4a9f45f7301e..e8cc2093397f 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 @@ -31,6 +31,7 @@ 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.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.command.CreateObjectCommand; import org.apache.cloudstack.storage.command.DeleteCommand; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -188,6 +189,76 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) } } + /** + * Creates a new qcow2 (or other file) in the FlexVol by cloning from a FlexVolume snapshot + * via {@code POST /api/storage/file/clone} with {@code snapshot.name}. + * + *

Builds the NFS request and executes it (mirrors {@link #createTemplateCache}). + * SAN uses the LUN REST clone path instead.

+ * + *

Combinations covered here:

+ *
    + *
  • DATA or ROOT snapshot → new data volume (ROOT restore is never bootable as a volume; + * CloudStack still uses this path for {@code createVolume(snapshotid)}; bootable ROOT + * recovery remains {@code createTemplate} → deploy)
    • + *
  • Same pool / FlexVol only (v1)
    • + *
  • IOPS from snapshot_details are not applied here — see driver TODO
    • + *
+ */ + @Override + public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, Map details, + VolumeInfo volumeInfo, String sourceVolumePath, + String snapshotName) { + if (storagePool == null || details == null || volumeInfo == null) { + throw new CloudRuntimeException("Failed to clone file from snapshot, invalid request"); + } + if (sourceVolumePath == null || sourceVolumePath.isEmpty()) { + throw new CloudRuntimeException("Failed to clone file from snapshot, source path is required"); + } + if (snapshotName == null || snapshotName.isEmpty()) { + throw new CloudRuntimeException("Failed to clone file from snapshot, snapshot name is required"); + } + + 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 from snapshot, FlexVolume uuid is missing from pool details"); + } + + String sourcePath = OntapStorageUtils.toFlexVolRelativePath(sourceVolumePath, flexVolName); + String destinationPath = OntapStorageUtils.toFlexVolRelativePath(volumeInfo.getUuid(), flexVolName); + + logger.info("cloneCloudStackVolumeFromSnapshot [NFS]: Cloning file [{}] -> [{}] from snapshot [{}] on FlexVol [{}]", + sourcePath, destinationPath, snapshotName, flexVolName); + try { + FileCloneRequest request = new FileCloneRequest(flexVolUuid, flexVolName, sourcePath, destinationPath, snapshotName); + JobResponse jobResponse = nasFeignClient.cloneFile(getAuthHeader(), request); + pollJobIfPresent(jobResponse, "clone file from snapshot [" + snapshotName + "] [" + sourcePath + + "] to [" + destinationPath + "]"); + + updateCloudStackVolumeMetadata(String.valueOf(storagePool.getId()), volumeInfo); + + FileInfo clonedFile = new FileInfo(); + clonedFile.setPath(destinationPath); + + CloudStackVolume clonedCloudStackVolume = new CloudStackVolume(); + clonedCloudStackVolume.setFile(clonedFile); + clonedCloudStackVolume.setDatastoreId(String.valueOf(storagePool.getId())); + clonedCloudStackVolume.setVolumeInfo(volumeInfo); + clonedCloudStackVolume.setSnapshotName(snapshotName); + return clonedCloudStackVolume; + } catch (FeignException e) { + logger.error("FeignException while cloning file from snapshot [{}], Status: {}, Exception: {}", + snapshotName, e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to clone file from snapshot: " + e.getMessage()); + } catch (CloudRuntimeException e) { + throw e; + } catch (Exception e) { + logger.error("Exception while cloning file from snapshot [{}]: {}", snapshotName, e.getMessage()); + throw new CloudRuntimeException("Failed to clone file from snapshot: " + e.getMessage()); + } + } + /** * Grows the cloned qcow2 to the requested size via a host-side {@code qemu-img resize}. */ 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 b9e32b081e4d..180190b55a9a 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 @@ -23,6 +23,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import feign.FeignException; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.model.Igroup; @@ -260,6 +261,102 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) } } + /** + * Creates a new LUN by cloning from a FlexVolume snapshot via the LUN REST API + * ({@code POST /api/storage/luns} with {@code clone.source.name} pointing into + * {@code /vol/<fv>/.snapshot/<snap>/...}). + * + *

Builds the protocol-specific request and executes it (mirrors {@link #createTemplateCache}). + * NFS uses file-clone in {@link UnifiedNASStrategy} instead.

+ * + *

Combinations covered here:

+ *
    + *
  • DATA or ROOT snapshot → new data volume on the same OntapiSCSI pool
    • + *
  • Same pool / FlexVol only (v1); cross-pool is not implemented
    • + *
  • In-place revert remains {@link #revertSnapshotForCloudStackVolume}; this method always + * creates a new LUN
    • + *
  • IOPS from snapshot_details are not applied here — see driver TODO
    • + *
+ */ + @Override + public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, Map details, + VolumeInfo volumeInfo, String sourceVolumePath, + String snapshotName) { + if (storagePool == null || details == null || volumeInfo == null) { + throw new CloudRuntimeException("Failed to clone Lun from snapshot, invalid request"); + } + if (sourceVolumePath == null || sourceVolumePath.isEmpty()) { + throw new CloudRuntimeException("Failed to clone Lun from snapshot, source LUN path is required"); + } + if (snapshotName == null || snapshotName.isEmpty()) { + throw new CloudRuntimeException("Failed to clone Lun from snapshot, snapshot name is required"); + } + + Lun lunRequest = buildCloneLunFromSnapshotRequest(storagePool, details, volumeInfo, sourceVolumePath, snapshotName); + logger.info("cloneCloudStackVolumeFromSnapshot [iSCSI]: Cloning LUN [{}] from snapshot source [{}]", + lunRequest.getName(), lunRequest.getClone().getSource().getName()); + 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("cloneCloudStackVolumeFromSnapshot: LUN clone returned no records for Lun {}", + lunRequest.getName()); + throw new CloudRuntimeException("Failed to clone Lun from snapshot: " + lunRequest.getName()); + } + Lun lun = clonedLun.getRecords().get(0); + validateCreatedLun(lun, lunRequest.getName(), "cloneCloudStackVolumeFromSnapshot"); + logger.debug("cloneCloudStackVolumeFromSnapshot: LUN cloned successfully. Lun: {}", lun); + + CloudStackVolume clonedCloudStackVolume = new CloudStackVolume(); + clonedCloudStackVolume.setLun(lun); + return clonedCloudStackVolume; + } catch (FeignException e) { + logger.error("FeignException while cloning LUN from snapshot, Status: {}, Exception: {}", + e.status(), e.getMessage()); + throw new CloudRuntimeException("Failed to clone Lun from snapshot: " + e.getMessage()); + } catch (CloudRuntimeException e) { + throw e; + } catch (Exception e) { + logger.error("Exception while cloning LUN from snapshot: {}", e.getMessage()); + throw new CloudRuntimeException("Failed to clone Lun from snapshot: " + e.getMessage()); + } + } + + /** + * Builds {@code POST /api/storage/luns} clone request with snapshot-qualified source name + * {@code /vol/<flexVol>/.snapshot/<snap>/<lun>} (name required; uuid cannot + * identify a snapshot-resident LUN). + */ + private Lun buildCloneLunFromSnapshotRequest(StoragePoolVO storagePool, Map details, + VolumeInfo volumeInfo, String sourceVolumePath, + String snapshotName) { + String lunName = volumeInfo.getName().replace(OntapStorageConstants.HYPHEN, OntapStorageConstants.UNDERSCORE); + if (!OntapStorageUtils.isValidName(lunName)) { + throw new CloudRuntimeException("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."); + } + + String flexVolName = details.get(OntapStorageConstants.VOLUME_NAME); + if (flexVolName == null || flexVolName.isEmpty()) { + flexVolName = storagePool.getName(); + } + String snapshotSourceName = OntapStorageUtils.toLunCloneSourcePathInSnapshot( + sourceVolumePath, flexVolName, snapshotName); + + Svm svm = new Svm(); + svm.setName(details.get(OntapStorageConstants.SVM_NAME)); + + Lun.Source source = new Lun.Source(); + source.setName(snapshotSourceName); + 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); + return lunRequest; + } /** * Ensures ONTAP returned a usable LUN identity from create/clone. Callers in the datastore diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java index ab38e3045f51..5147e9ff7ac2 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java @@ -51,6 +51,12 @@ public class CloudStackVolume { */ private String destinationPath; + /** + * ONTAP FlexVolume snapshot name when cloning a new file/LUN from a snapshot + * (create-volume-from-snapshot). Null for live clones (e.g. template cache). + */ + private String snapshotName; + private DataObject volumeInfo; // This is needed as we need DataObject to be passed to agent to create volume public FileInfo getFile() { @@ -101,4 +107,12 @@ public void setDestinationPath(String destinationPath) { this.destinationPath = destinationPath; } + public String getSnapshotName() { + return snapshotName; + } + + public void setSnapshotName(String snapshotName) { + this.snapshotName = snapshotName; + } + } 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 4ac49c95dfa1..27e3dd5d3660 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 @@ -94,6 +94,12 @@ public class OntapStorageConstants { public static final String VOLUME_PATH_PREFIX = "/vol/"; + /** + * Path segment inserted after the FlexVol name when identifying a LUN inside a FlexVol snapshot + * for {@code POST /api/storage/luns} clone ({@code /vol/<fv>/.snapshot/<snap>/<lun>}). + */ + public static final String SNAPSHOT_PATH_SEGMENT = "/.snapshot/"; + public static final String ONTAP_NAME_REGEX = "^[a-zA-Z][a-zA-Z0-9_]*$"; public static final String KVM = "KVM"; @@ -115,6 +121,13 @@ public class OntapStorageConstants { public static final String VOLUME_PATH = "volume_path"; public static final String PRIMARY_POOL_ID = "primary_pool_id"; public static final String ONTAP_SNAP_SIZE = "ontap_snap_size"; + /** + * Optional {@code snapshot_details} keys: min/max IOPS from the source volume at take-snapshot + * time. Persisted only when the volume has configured values; applied to volumes created from + * the snapshot in a later change (see TODO on create-from-snapshot). + */ + public static final String MIN_IOPS = "min_iops"; + public static final String MAX_IOPS = "max_iops"; public static final String FILE_PATH = "file_path"; public static final int MAX_SNAPSHOT_NAME_LENGTH = 255; public static final String ONTAP_TEMP_CG_PREFIX = "cs-temp-cg-"; @@ -148,6 +161,14 @@ public class OntapStorageConstants { */ public static final String CLONE_OF_TEMPLATE = "cloneOfTemplate"; + /** + * Key of the {@code volume_details} row that {@code StorageSystemDataMotionStrategy} writes + * immediately before {@code createAsync} when a volume is to be created from a CloudStack + * snapshot already present on this pool. The value is the CloudStack snapshot id. The literal + * must stay in sync with the string used by the orchestrator. + */ + public static final String CLONE_OF_SNAPSHOT = "cloneOfSnapshot"; + // ASUP (AutoSupport) / EMS telemetry public static final String ADVANCED_CONFIG_KEY_CATEGORY = "Advanced"; public static final String ASUP_CATEGORY = "provisioning"; 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 e2b419ea46f0..5155f9f868f6 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 @@ -138,6 +138,49 @@ public static String getLunName(String volName, String lunName) { return OntapStorageConstants.VOLUME_PATH_PREFIX + volName + OntapStorageConstants.SLASH + lunName; } + /** + * Converts a path stored in CloudStack (absolute LUN {@code /vol/<flexVol>/...} or already + * relative NFS file path) into the FlexVol-relative path expected by + * {@code POST /api/storage/file/clone}. + */ + public static String toFlexVolRelativePath(String path, String flexVolName) { + if (path == null || path.isEmpty()) { + return path; + } + if (flexVolName != null && !flexVolName.isEmpty()) { + String prefix = OntapStorageConstants.VOLUME_PATH_PREFIX + flexVolName + OntapStorageConstants.SLASH; + if (path.startsWith(prefix)) { + return path.substring(prefix.length()); + } + } + // Already relative (typical NFS uuid path) or unexpected absolute form — strip a leading slash. + return path.startsWith(OntapStorageConstants.SLASH) ? path.substring(1) : path; + } + + /** + * Builds the ONTAP LUN clone source name that points at a LUN inside a FlexVol snapshot. + * + *

Format required by {@code POST /api/storage/luns} when cloning from a snapshot: + * {@code /vol/<flexVol>/.snapshot/<snapshotName>/<relativeLunPath>}.

+ * + *

{@code clone.source.uuid} cannot identify a snapshot-resident LUN; name must be used.

+ */ + public static String toLunCloneSourcePathInSnapshot(String lunPath, String flexVolName, String snapshotName) { + if (flexVolName == null || flexVolName.isEmpty()) { + throw new InvalidParameterValueException("FlexVolume name is required to build a snapshot LUN path"); + } + if (snapshotName == null || snapshotName.isEmpty()) { + throw new InvalidParameterValueException("Snapshot name is required to build a snapshot LUN path"); + } + String relativeLunPath = toFlexVolRelativePath(lunPath, flexVolName); + if (relativeLunPath == null || relativeLunPath.isEmpty()) { + throw new InvalidParameterValueException("LUN path is required to build a snapshot LUN path"); + } + return OntapStorageConstants.VOLUME_PATH_PREFIX + flexVolName + + OntapStorageConstants.SNAPSHOT_PATH_SEGMENT + snapshotName + + OntapStorageConstants.SLASH + relativeLunPath; + } + /** * Builds an ONTAP-safe name token from user-provided snapshot text. */ 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 db1806c8473e..714c9c760115 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 @@ -24,9 +24,13 @@ import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; +import com.cloud.storage.SnapshotVO; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.VolumeDetailVO; +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; @@ -43,6 +47,7 @@ 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.FileInfo; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.service.UnifiedNASStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; @@ -101,6 +106,12 @@ class OntapPrimaryDatastoreDriverTest { @Mock private VolumeDetailsDao volumeDetailsDao; + @Mock + private SnapshotDetailsDao snapshotDetailsDao; + + @Mock + private SnapshotDao snapshotDao; + @Mock private VMTemplatePoolDao vmTemplatePoolDao; @@ -948,6 +959,293 @@ void testCreateAsync_VolumeClonedFromTemplate_GrowsWhenOfferingIsLarger() { } } + @Test + void testCreateAsync_VolumeClonedFromSnapshot_IscsiSuccessWithoutGrow() { + stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.ISCSI.name()); + + Lun clonedLun = new Lun(); + clonedLun.setName("/vol/vol1/test_volume"); + clonedLun.setUuid("snap-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.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + verify(sanStrategy).cloneCloudStackVolumeFromSnapshot( + eq(storagePool), any(), eq(volumeInfo), eq("/vol/vol1/source_lun"), eq("snap_cs200")); + verify(sanStrategy, never()).cloneCloudStackVolume(any()); + verify(sanStrategy, never()).createCloudStackVolume(any()); + verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); + verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_UUID), eq("snap-cloned-lun-uuid"), eq(false)); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_GrowsWhenOfferingIsLarger() { + stubVolumeCloneFromSnapshot(5368709120L, 21474836480L, ProtocolType.ISCSI.name()); + + Lun clonedLun = new Lun(); + clonedLun.setName("/vol/vol1/test_volume"); + clonedLun.setUuid("snap-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.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + verify(sanStrategy).resizeCloudStackVolume(eq(cloned), eq(21474836480L)); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_NfsSuccess() { + stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.NFS3.name()); + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(volumeInfo.getUuid()).thenReturn("new-volume-uuid"); + + CloudStackVolume cloned = new CloudStackVolume(); + FileInfo file = new FileInfo(); + file.setPath("new-volume-uuid"); + cloned.setFile(file); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(nasStrategy); + when(nasStrategy.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + + verify(nasStrategy).cloneCloudStackVolumeFromSnapshot( + eq(storagePool), any(), eq(volumeInfo), eq("source-file-uuid"), eq("snap_cs200")); + verify(nasStrategy, never()).createCloudStackVolume(any()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_PoolMismatch_Fails() { + stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.ISCSI.name()); + when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.PRIMARY_POOL_ID)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.PRIMARY_POOL_ID, "999", false)); + + 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()).cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_MissingDetail_Fails() { + stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.ISCSI.name()); + when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.ONTAP_SNAP_NAME)).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()).cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_ProtocolMismatch_Fails() { + stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.ISCSI.name()); + // Pool is iSCSI (default stub details) but snapshot was taken on NFS. + when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.PROTOCOL)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name(), false)); + + 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()).cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_NullStrategyResult_Fails() { + stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.ISCSI.name()); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .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_VolumeClonedFromSnapshot_StrategyThrows_Fails() { + stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.ISCSI.name()); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenThrow(new CloudRuntimeException("ONTAP clone failed")); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_PrefersSnapshotOverTemplate() { + // Corner: snapshot id is resolved first; template is only consulted when snapshot is absent. + stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.ISCSI.name()); + + Lun clonedLun = new Lun(); + clonedLun.setName("/vol/vol1/test_volume"); + clonedLun.setUuid("snap-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.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + verify(sanStrategy).cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString()); + verify(volumeDetailsDao, never()).findDetail(100L, OntapStorageConstants.CLONE_OF_TEMPLATE); + verify(sanStrategy, never()).cloneCloudStackVolume(any()); + verify(sanStrategy, never()).createCloudStackVolume(any()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_NfsGrowsWhenOfferingIsLarger() { + stubVolumeCloneFromSnapshot(5368709120L, 21474836480L, ProtocolType.NFS3.name()); + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(volumeInfo.getUuid()).thenReturn("new-volume-uuid"); + + CloudStackVolume cloned = new CloudStackVolume(); + FileInfo file = new FileInfo(); + file.setPath("new-volume-uuid"); + cloned.setFile(file); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(nasStrategy); + when(nasStrategy.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + verify(nasStrategy).resizeCloudStackVolume(eq(cloned), eq(21474836480L)); + verify(sanStrategy, never()).cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString()); + } + } + + @Test + void testCreateAsync_VolumeClonedFromSnapshot_SkipsGrowWhenSnapshotSizeUnknown() { + stubVolumeCloneFromSnapshot(0L, 21474836480L, ProtocolType.ISCSI.name()); + + Lun clonedLun = new Lun(); + clonedLun.setName("/vol/vol1/test_volume"); + clonedLun.setUuid("snap-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.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenReturn(cloned); + + driver.createAsync(dataStore, volumeInfo, createCallback); + + verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class); + verify(createCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + } + } + + /** + * Sets up a volume create that the orchestrator has marked as a clone of a CloudStack snapshot. + */ + private void stubVolumeCloneFromSnapshot(long snapshotSize, long volumeSize, String protocol) { + 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"); + lenient().when(volumeInfo.getSize()).thenReturn(volumeSize); + + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + // getId is only used after snapshot_details validation succeeds; lenient for early-fail tests. + lenient().when(storagePool.getId()).thenReturn(1L); + lenient().when(storagePool.getName()).thenReturn("vol1"); + 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); + lenient().when(volumeVO.getId()).thenReturn(100L); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.CLONE_OF_SNAPSHOT)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.CLONE_OF_SNAPSHOT, "200", false)); + + String volumePath = ProtocolType.NFS3.name().equalsIgnoreCase(protocol) + ? "source-file-uuid" + : "/vol/vol1/source_lun"; + if (!ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { + storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + } + // Lenient so early-fail tests can override/null individual keys without STRICT_STUBS noise. + lenient().when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.ONTAP_SNAP_NAME)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.ONTAP_SNAP_NAME, "snap_cs200", false)); + lenient().when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.VOLUME_PATH)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.VOLUME_PATH, volumePath, false)); + lenient().when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.PRIMARY_POOL_ID)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.PRIMARY_POOL_ID, "1", false)); + lenient().when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.PROTOCOL)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.PROTOCOL, protocol, false)); + + SnapshotVO snapshotVO = mock(SnapshotVO.class); + lenient().when(snapshotDao.findById(200L)).thenReturn(snapshotVO); + lenient().when(snapshotVO.getSize()).thenReturn(snapshotSize); + } + /** * Sets up a volume create that the orchestrator has marked as a clone of a cached template. */ @@ -968,6 +1266,9 @@ private void stubVolumeCloneFromTemplate(long templateSize, long volumeSize) { when(volumeDao.findById(100L)).thenReturn(volumeVO); lenient().when(volumeVO.getId()).thenReturn(100L); + // createAsync checks cloneOfSnapshot before cloneOfTemplate; under STRICT_STUBS an + // unstubbed alternate key on the same method is treated as an argument mismatch. + lenient().when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.CLONE_OF_SNAPSHOT)).thenReturn(null); when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.CLONE_OF_TEMPLATE)) .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.CLONE_OF_TEMPLATE, "50", false)); @@ -1400,6 +1701,7 @@ void testCreateAsync_VolumeClonedFromTemplate_MissingSpoolRef_Fails() { when(storagePool.getId()).thenReturn(1L); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); when(volumeDao.findById(100L)).thenReturn(volumeVO); + lenient().when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.CLONE_OF_SNAPSHOT)).thenReturn(null); 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); 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 2c516544cd49..ca8c52809391 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 @@ -26,6 +26,8 @@ import java.util.List; import java.util.Map; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.feign.client.AggregateFeignClient; import org.apache.cloudstack.storage.feign.client.ClusterFeignClient; import org.apache.cloudstack.storage.feign.client.JobFeignClient; @@ -167,6 +169,15 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) return null; } + @Override + public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, + Map details, + VolumeInfo volumeInfo, + String sourceVolumePath, + String snapshotName) { + return null; + } + @Override public void resizeCloudStackVolume(CloudStackVolume cloudstackVolume, long sizeInBytes) { } 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 dd90363af045..a9f2b40aec7e 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 @@ -1017,6 +1017,130 @@ public void testCloneCloudStackVolume_Success() { assertEquals("flexvol-uuid-1", captor.getValue().getVolume().getUuid()); } + @Test + public void testCloneCloudStackVolumeFromSnapshot_Success() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + when(volumeObject.getId()).thenReturn(100L); + when(volumeObject.getUuid()).thenReturn("new-volume-uuid"); + when(storagePool.getId()).thenReturn(1L); + 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"); + details.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); + + when(nasFeignClient.cloneFile(anyString(), any(FileCloneRequest.class))).thenReturn(new JobResponse()); + + CloudStackVolume result = strategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeObject, "source-file-uuid", "snap_cs200"); + + assertNotNull(result); + assertEquals("new-volume-uuid", result.getFile().getPath()); + assertEquals("snap_cs200", result.getSnapshotName()); + ArgumentCaptor captor = ArgumentCaptor.forClass(FileCloneRequest.class); + verify(nasFeignClient).cloneFile(anyString(), captor.capture()); + assertEquals("source-file-uuid", captor.getValue().getSourcePath()); + assertEquals("new-volume-uuid", captor.getValue().getDestinationPath()); + assertNotNull(captor.getValue().getSnapshot()); + assertEquals("snap_cs200", captor.getValue().getSnapshot().getName()); + } + + @Test + public void testCloneCloudStackVolumeFromSnapshot_MissingSnapshotName_Throws() { + StoragePoolVO storagePool = mock(StoragePoolVO.class); + VolumeObject volumeObject = mock(VolumeObject.class); + when(volumeObject.getUuid()).thenReturn("new-volume-uuid"); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); + details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + + assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeObject, "source-file-uuid", null)); + verify(nasFeignClient, never()).cloneFile(anyString(), any(FileCloneRequest.class)); + } + + @Test + public void testCloneCloudStackVolumeFromSnapshot_MissingSourcePath_Throws() { + StoragePoolVO storagePool = mock(StoragePoolVO.class); + VolumeObject volumeObject = mock(VolumeObject.class); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); + details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + + assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeObject, null, "snap_cs200")); + verify(nasFeignClient, never()).cloneFile(anyString(), any(FileCloneRequest.class)); + } + + @Test + public void testCloneCloudStackVolumeFromSnapshot_MissingFlexVolUuid_Throws() { + StoragePoolVO storagePool = mock(StoragePoolVO.class); + VolumeObject volumeObject = mock(VolumeObject.class); + when(volumeObject.getUuid()).thenReturn("new-volume-uuid"); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + + assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeObject, "source-file-uuid", "snap_cs200")); + verify(nasFeignClient, never()).cloneFile(anyString(), any(FileCloneRequest.class)); + } + + @Test + public void testCloneCloudStackVolumeFromSnapshot_NullArgs_Throws() { + assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolumeFromSnapshot( + null, new HashMap<>(), mock(VolumeObject.class), "src", "snap")); + verify(nasFeignClient, never()).cloneFile(anyString(), any(FileCloneRequest.class)); + } + + @Test + public void testCloneCloudStackVolumeFromSnapshot_AbsoluteSourcePath_StrippedToRelative() { + VolumeObject volumeObject = mock(VolumeObject.class); + VolumeVO volumeVO = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + when(volumeObject.getId()).thenReturn(100L); + when(volumeObject.getUuid()).thenReturn("new-volume-uuid"); + when(storagePool.getId()).thenReturn(1L); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(volumeDao.update(anyLong(), any(VolumeVO.class))).thenReturn(true); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + details.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); + + when(nasFeignClient.cloneFile(anyString(), any(FileCloneRequest.class))).thenReturn(new JobResponse()); + + strategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeObject, "/vol/flexvol1/source-file-uuid", "snap_cs200"); + + ArgumentCaptor captor = ArgumentCaptor.forClass(FileCloneRequest.class); + verify(nasFeignClient).cloneFile(anyString(), captor.capture()); + assertEquals("source-file-uuid", captor.getValue().getSourcePath()); + assertNotNull(captor.getValue().getSnapshot()); + } + + @Test + public void testCloneCloudStackVolumeFromSnapshot_FeignException_Throws() { + VolumeObject volumeObject = mock(VolumeObject.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + when(volumeObject.getUuid()).thenReturn("new-volume-uuid"); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + details.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); + + 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.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeObject, "source-file-uuid", "snap_cs200")); + } + @Test public void testCloneCloudStackVolume_InvalidRequest_ThrowsException() { assertThrows(CloudRuntimeException.class, () -> strategy.cloneCloudStackVolume(null)); 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 700b63d15575..701cf3e16a17 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 @@ -75,6 +75,9 @@ class UnifiedSANStrategyTest { @Mock private SANFeignClient sanFeignClient; + @Mock + private org.apache.cloudstack.storage.feign.client.NASFeignClient nasFeignClient; + @Mock private OntapStorage ontapStorage; @@ -105,6 +108,10 @@ void setUp() { sanFeignClientField.setAccessible(true); sanFeignClientField.set(unifiedSANStrategy, sanFeignClient); + java.lang.reflect.Field nasFeignClientField = StorageStrategy.class.getDeclaredField("nasFeignClient"); + nasFeignClientField.setAccessible(true); + nasFeignClientField.set(unifiedSANStrategy, nasFeignClient); + // Also inject the storage field from parent class to ensure proper mocking java.lang.reflect.Field storageField = StorageStrategy.class.getDeclaredField("storage"); storageField.setAccessible(true); @@ -1006,6 +1013,212 @@ void testCloneCloudStackVolume_MissingSource_ThrowsException() { () -> unifiedSANStrategy.cloneCloudStackVolume(request)); } + @Test + void testCloneCloudStackVolumeFromSnapshot_Success() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getName()).thenReturn("vol1"); + + org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo volumeInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo.class); + when(volumeInfo.getName()).thenReturn("new_lun"); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + details.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + + Lun createdLun = new Lun(); + createdLun.setName("/vol/vol1/new_lun"); + createdLun.setUuid("new-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.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, "/vol/vol1/source_lun", "snap_cs200"); + + assertNotNull(result); + assertEquals("new-lun-uuid", result.getLun().getUuid()); + ArgumentCaptor lunCaptor = ArgumentCaptor.forClass(Lun.class); + verify(sanFeignClient).createLun(eq(authHeader), eq(true), lunCaptor.capture()); + assertEquals("/vol/vol1/.snapshot/snap_cs200/source_lun", + lunCaptor.getValue().getClone().getSource().getName()); + assertEquals("/vol/vol1/new_lun", lunCaptor.getValue().getName()); + verify(nasFeignClient, never()).cloneFile(any(), any()); + } + } + + @Test + void testCloneCloudStackVolumeFromSnapshot_MissingSnapshotName_Throws() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo volumeInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo.class); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + details.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, "/vol/vol1/source_lun", null)); + verify(sanFeignClient, never()).createLun(any(), anyBoolean(), any()); + } + + @Test + void testCloneCloudStackVolumeFromSnapshot_MissingSourcePath_Throws() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo volumeInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo.class); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + details.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, null, "snap_cs200")); + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, "", "snap_cs200")); + verify(sanFeignClient, never()).createLun(any(), anyBoolean(), any()); + } + + @Test + void testCloneCloudStackVolumeFromSnapshot_NullArgs_Throws() { + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolumeFromSnapshot( + null, new HashMap<>(), + mock(org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo.class), + "/vol/vol1/source_lun", "snap")); + verify(sanFeignClient, never()).createLun(any(), anyBoolean(), any()); + } + + @Test + void testCloneCloudStackVolumeFromSnapshot_RelativeSourcePath_BuildsSnapshotQualifiedName() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getName()).thenReturn("vol1"); + + org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo volumeInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo.class); + when(volumeInfo.getName()).thenReturn("dest-lun"); + + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + details.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + + Lun createdLun = new Lun(); + createdLun.setName("/vol/vol1/dest_lun"); + createdLun.setUuid("new-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); + + unifiedSANStrategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, "source_lun", "snap_cs200"); + + ArgumentCaptor lunCaptor = ArgumentCaptor.forClass(Lun.class); + verify(sanFeignClient).createLun(eq(authHeader), eq(true), lunCaptor.capture()); + assertEquals("/vol/vol1/.snapshot/snap_cs200/source_lun", + lunCaptor.getValue().getClone().getSource().getName()); + assertEquals("/vol/vol1/dest_lun", lunCaptor.getValue().getName()); + } + } + + @Test + void testCloneCloudStackVolumeFromSnapshot_EmptyRecords_Throws() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getName()).thenReturn("vol1"); + org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo volumeInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo.class); + when(volumeInfo.getName()).thenReturn("new_lun"); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + details.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + + 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))).thenReturn(empty); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, "/vol/vol1/source_lun", "snap_cs200")); + } + } + + @Test + void testCloneCloudStackVolumeFromSnapshot_IncompleteLun_Throws() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getName()).thenReturn("vol1"); + org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo volumeInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo.class); + when(volumeInfo.getName()).thenReturn("new_lun"); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + details.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + + Lun incomplete = new Lun(); + incomplete.setName("/vol/vol1/new_lun"); + OntapResponse response = new OntapResponse<>(); + response.setRecords(List.of(incomplete)); + + 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); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, "/vol/vol1/source_lun", "snap_cs200")); + } + } + + @Test + void testCloneCloudStackVolumeFromSnapshot_FeignException_Throws() { + org.apache.cloudstack.storage.datastore.db.StoragePoolVO storagePool = + mock(org.apache.cloudstack.storage.datastore.db.StoragePoolVO.class); + when(storagePool.getName()).thenReturn("vol1"); + org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo volumeInfo = + mock(org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo.class); + when(volumeInfo.getName()).thenReturn("new_lun"); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.SVM_NAME, "svm1"); + details.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(500); + when(feignException.getMessage()).thenReturn("clone 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(feignException); + + assertThrows(CloudRuntimeException.class, + () -> unifiedSANStrategy.cloneCloudStackVolumeFromSnapshot( + storagePool, details, volumeInfo, "/vol/vol1/source_lun", "snap_cs200")); + } + } + @Test void testResizeCloudStackVolume_ValidRequest_PatchesSize() { Lun lun = new Lun(); diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java index ebe7da25ed12..1fa61966af88 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java @@ -93,4 +93,29 @@ public void isOntapSnapshotNotFoundError_rejectsUnrelatedErrors() { assertFalse(OntapStorageUtils.isOntapObjectNotFoundError( new CloudRuntimeException("Job failed with error: permission denied"))); } + + @Test + public void toFlexVolRelativePath_stripsVolPrefix() { + assertEquals("lun1", OntapStorageUtils.toFlexVolRelativePath("/vol/vol1/lun1", "vol1")); + assertEquals("file-uuid", OntapStorageUtils.toFlexVolRelativePath("file-uuid", "vol1")); + assertEquals("file-uuid", OntapStorageUtils.toFlexVolRelativePath("/file-uuid", "vol1")); + } + + @Test + public void toLunCloneSourcePathInSnapshot_buildsSnapshotQualifiedPath() { + assertEquals("/vol/vol1/.snapshot/snap_cs200/source_lun", + OntapStorageUtils.toLunCloneSourcePathInSnapshot("/vol/vol1/source_lun", "vol1", "snap_cs200")); + assertEquals("/vol/vol1/.snapshot/snap_cs200/source_lun", + OntapStorageUtils.toLunCloneSourcePathInSnapshot("source_lun", "vol1", "snap_cs200")); + } + + @Test + public void toLunCloneSourcePathInSnapshot_rejectsBlankInputs() { + org.junit.jupiter.api.Assertions.assertThrows(com.cloud.exception.InvalidParameterValueException.class, + () -> OntapStorageUtils.toLunCloneSourcePathInSnapshot("/vol/vol1/lun", "vol1", null)); + org.junit.jupiter.api.Assertions.assertThrows(com.cloud.exception.InvalidParameterValueException.class, + () -> OntapStorageUtils.toLunCloneSourcePathInSnapshot("/vol/vol1/lun", "", "snap")); + org.junit.jupiter.api.Assertions.assertThrows(com.cloud.exception.InvalidParameterValueException.class, + () -> OntapStorageUtils.toLunCloneSourcePathInSnapshot("", "vol1", "snap")); + } } From 473790c262be0265fc6b6fc183f5835c23a96aa7 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Thu, 24 Sep 2026 10:32:49 +0530 Subject: [PATCH 2/4] CSTACKEX-306: setting the exception msg more align to the situation --- .../cloudstack/storage/volume/VolumeServiceImpl.java | 9 ++++++--- .../cloudstack/storage/service/UnifiedSANStrategy.java | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 613bcd6d1b91..0e2ed66eb4de 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -1007,7 +1007,6 @@ private TemplateInfo createManagedTemplateVolume(TemplateInfo srcTemplateInfo, P } else { errMesg = callback.result.getResult(); } - templateOnPrimary.processEvent(Event.OperationFailed); throw new CloudRuntimeException(String.format("Unable to create template %s on primary storage %s: %s", templateOnPrimary.getImage(), destPrimaryDataStore, errMesg)); } @@ -1015,8 +1014,12 @@ private TemplateInfo createManagedTemplateVolume(TemplateInfo srcTemplateInfo, P } catch (Throwable e) { logger.debug("Failed to create template volume on storage", e); - templateOnPrimary.processEvent(Event.OperationFailed); - throw new CloudRuntimeException(e.getMessage()); + try { + templateOnPrimary.processEvent(Event.OperationFailed); + } catch (Exception stateEx) { + logger.warn("Unable to mark template {} as failed on primary storage {}: {}", templateOnPrimary.getImage(), destPrimaryDataStore, stateEx.getMessage()); + } + throw new CloudRuntimeException(e.getMessage(), e); } finally { _tmpltPoolDao.releaseFromLockTable(templatePoolRefId); } 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 180190b55a9a..70279c05427b 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 @@ -111,7 +111,9 @@ public CloudStackVolume createTemplateCache(StoragePoolVO storagePool, TemplateI 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() + "]"); + + "]; cannot size the template LUN on pool [" + storagePool.getId() + "]. The template size in vm_template" + + " is unset; verify the template was registered/seeded with its virtual size (virtualsize in" + + " template.properties on secondary storage)."); } CloudStackVolume request = buildTemplateLunRequest(storagePool, details, templateInfo.getId(), sizeInBytes); From db96c19206a91dc110acbdd2f9ec0e385d65e6de Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Thu, 24 Sep 2026 12:04:38 +0530 Subject: [PATCH 3/4] CSTACKEX-306: create template from the C voluem snapshot --- .../driver/OntapPrimaryDatastoreDriver.java | 197 ++++++++++-- .../storage/utils/OntapStorageConstants.java | 12 + .../OntapPrimaryDatastoreDriverTest.java | 285 ++++++++++++++++++ 3 files changed, 466 insertions(+), 28 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 f1e6618913e3..5d7934f673bc 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java @@ -27,6 +27,7 @@ import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage; import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; @@ -58,6 +59,7 @@ 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.CopyCmdAnswer; import org.apache.cloudstack.storage.command.CreateObjectAnswer; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; @@ -116,6 +118,11 @@ public Map getCapabilities() { // 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()); + // createTemplate(snapshotid) reads the snapshot from this pool instead of backing it up to secondary first. + mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_TEMPLATE_FROM_SNAPSHOT.toString(), Boolean.TRUE.toString()); + // Must be present and false: StorageSystemDataMotionStrategy then clones the snapshot into a + // temporary volume (see copyAsync) and copies that volume to secondary storage. + mapCapabilities.put(OntapStorageConstants.CAN_DIRECT_ATTACH_SNAPSHOT, Boolean.FALSE.toString()); return mapCapabilities; } @@ -204,30 +211,7 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet : createCloudStackVolume(storagePool, volInfo, details); } - volumeVO.setPoolType(storagePool.getPoolType()); - volumeVO.setPoolId(storagePool.getId()); - volumeVO.setFormat(getImageFormat(storagePool)); - logger.info("createAsync: Volume format set to [{}] for pool type [{}]", volumeVO.getFormat(), storagePool.getPoolType()); - - if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { - // 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, createdLun.getUuid(), false); - volumeDetailsDao.addDetail(volInfo.getId(), OntapStorageConstants.LUN_DOT_NAME, lunName, false); - volumeVO.setFolder(createdLun.getUuid()); - - logger.info("createAsync: Created LUN [{}] for volume [{}]. LUN mapping will occur during grantAccess() to per-host igroup.", - lunName, volumeVO.getId()); - createCmdResult = new CreateCmdResult(lunName, new Answer(null, true, null)); - } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { - createCmdResult = new CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null)); - logger.info("createAsync: Managed NFS volume [{}] with path [{}] associated with pool {}", - volumeVO.getId(), volInfo.getUuid(), storagePool.getId()); - } - volumeDao.update(volumeVO.getId(), volumeVO); + createCmdResult = recordCreatedVolume(storagePool, volInfo, volumeVO, details, clonedCloudStackVolume); } } else if (dataObject.getType() == DataObjectType.TEMPLATE) { createCmdResult = createTemplateOnPrimary(storagePool, (TemplateInfo) dataObject, details); @@ -249,6 +233,41 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet } } + /** + * Records pool association, image format and protocol-specific identity (LUN name/uuid for iSCSI) + * of a volume that was just created or cloned on ONTAP, and returns the create result whose path + * the framework stores on the volume. + */ + private CreateCmdResult recordCreatedVolume(StoragePoolVO storagePool, VolumeInfo volInfo, VolumeVO volumeVO, + Map details, CloudStackVolume createdCloudStackVolume) { + CreateCmdResult createCmdResult = null; + volumeVO.setPoolType(storagePool.getPoolType()); + volumeVO.setPoolId(storagePool.getId()); + volumeVO.setFormat(getImageFormat(storagePool)); + logger.info("createAsync: Volume format set to [{}] for pool type [{}]", volumeVO.getFormat(), storagePool.getPoolType()); + + if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { + // createCloudStackVolume validates the Feign response (LUN name + uuid) before returning + Lun createdLun = createdCloudStackVolume.getLun(); + String lunName = createdLun.getName(); + + // Persist LUN details for future operations (delete, grant/revoke access) + volumeDetailsDao.addDetail(volInfo.getId(), OntapStorageConstants.LUN_DOT_UUID, createdLun.getUuid(), false); + volumeDetailsDao.addDetail(volInfo.getId(), OntapStorageConstants.LUN_DOT_NAME, lunName, false); + volumeVO.setFolder(createdLun.getUuid()); + + logger.info("createAsync: Created LUN [{}] for volume [{}]. LUN mapping will occur during grantAccess() to per-host igroup.", + lunName, volumeVO.getId()); + createCmdResult = new CreateCmdResult(lunName, new Answer(null, true, null)); + } else if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { + createCmdResult = new CreateCmdResult(volInfo.getUuid(), new Answer(null, true, null)); + logger.info("createAsync: Managed NFS volume [{}] with path [{}] associated with pool {}", + volumeVO.getId(), volInfo.getUuid(), storagePool.getId()); + } + volumeDao.update(volumeVO.getId(), volumeVO); + return createCmdResult; + } + /** * Creates a volume on the ONTAP backend. */ @@ -596,6 +615,7 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac CloudStackVolume cloudStackVolumeRequest = createDeleteCloudStackVolumeRequest(storagePool, details, volumeInfo); storageStrategy.deleteCloudStackVolume(cloudStackVolumeRequest); logger.info("deleteAsync: Volume deleted: " + volumeInfo.getId()); + removeTemporarySnapshotCopyRecord(volumeInfo.getId()); commandResult.setResult(null); commandResult.setSuccess(true); } else if (data.getType() == DataObjectType.TEMPLATE) { @@ -614,7 +634,10 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac commandResult.setSuccess(false); commandResult.setResult(e.getMessage()); } finally { - callback.complete(commandResult); + // StorageSystemDataMotionStrategy deletes its temporary snapshot copy with a null callback. + if (callback != null) { + callback.complete(commandResult); + } } } @@ -705,17 +728,118 @@ private long resolveSnapshotPoolId(String poolIdStr, long snapshotId) { @Override public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback callback) { - throw new UnsupportedOperationException("Copy operation is not supported for ONTAP primary storage."); + copyAsync(srcData, destData, null, callback); } + /** + * Clones a CloudStack volume snapshot into the temporary volume that + * {@code StorageSystemDataMotionStrategy} creates while copying the snapshot to secondary storage + * for {@code createTemplate(snapshotid)}. The strategy then maps that volume to a KVM host, copies + * it into the template and deletes it again through {@link #deleteAsync}. + * + *

The strategy invokes this synchronously with a null callback, so failures are thrown.

+ */ @Override public void copyAsync(DataObject srcData, DataObject destData, Host destHost, AsyncCompletionCallback callback) { - throw new UnsupportedOperationException("Copy operation is not supported for ONTAP primary storage."); + if (!canCopy(srcData, destData)) { + throw new UnsupportedOperationException("Copy operation is not supported for ONTAP primary storage."); + } + + CopyCommandResult result; + try { + VolumeInfo volInfo = (VolumeInfo) destData; + StoragePoolVO storagePool = storagePoolDao.findById(destData.getDataStore().getId()); + if (storagePool == null) { + throw new CloudRuntimeException("Storage Pool not found for id: " + destData.getDataStore().getId()); + } + Map details = storagePoolDetailsDao.listDetailsKeyPairs(storagePool.getId()); + validateProtocol(details, destData.getDataStore()); + VolumeVO volumeVO = volumeDao.findById(volInfo.getId()); + + renameTemporarySnapshotCopy(volInfo, volumeVO, srcData.getId()); + logger.info("copyAsync: Cloning CS snapshot [{}] into temporary volume [{}] on pool [{}] for template creation", + srcData.getId(), volInfo.getId(), storagePool.getId()); + CloudStackVolume cloned = cloneCloudStackVolumeFromSnapshot(storagePool, volInfo, details, srcData.getId()); + if (ProtocolType.NFS3.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) { + volumeVO.setPath(volInfo.getUuid()); + } + recordCreatedVolume(storagePool, volInfo, volumeVO, details, cloned); + result = new CopyCommandResult(null, new CopyCmdAnswer(volInfo.getTO())); + } catch (Exception e) { + logger.error("copyAsync: Failed to clone snapshot [{}] into volume [{}]: {}", srcData.getId(), destData.getId(), e.getMessage()); + result = new CopyCommandResult(null, new CopyCmdAnswer(e.getMessage())); + result.setResult(e.getMessage()); + } + + if (callback != null) { + callback.complete(result); + } else if (!result.isSuccess()) { + throw new CloudRuntimeException("Failed to clone snapshot [" + srcData.getId() + "] into volume [" + + destData.getId() + "]: " + result.getResult()); + } } + /** + * Only accepts the temporary snapshot-to-volume copy used for template creation. + * + *

{@code DataMotionServiceImpl} consults {@code canCopy} before choosing a data motion strategy, so + * accepting every snapshot-to-volume copy here would bypass + * {@code StorageSystemDataMotionStrategy} for createVolume(snapshotid). Template caching + * (template to template, template to volume) must keep returning false as well.

+ */ @Override public boolean canCopy(DataObject srcData, DataObject destData) { - return false; + if (srcData == null || destData == null || srcData.getDataStore() == null || destData.getDataStore() == null) { + return false; + } + if (srcData.getType() != DataObjectType.SNAPSHOT || destData.getType() != DataObjectType.VOLUME) { + return false; + } + if (srcData.getDataStore().getRole() != DataStoreRole.Primary + || srcData.getDataStore().getId() != destData.getDataStore().getId()) { + return false; + } + return isTemporarySnapshotCopyVolume(volumeDao.findById(destData.getId())); + } + + /** + * The temporary volume created by {@code StorageSystemDataMotionStrategy} for a snapshot copy is + * persisted in Allocated state without a disk offering; user volumes always carry a disk offering. + */ + private boolean isTemporarySnapshotCopyVolume(VolumeVO volumeVO) { + if (volumeVO == null || volumeVO.getState() != Volume.State.Allocated) { + return false; + } + Long diskOfferingId = volumeVO.getDiskOfferingId(); + return diskOfferingId == null || diskOfferingId == 0L; + } + + /** + * The framework names the temporary volume {@code _.TMP}, which is not a valid + * ONTAP LUN name, so it is renamed before cloning. The in-memory object is updated as well because the + * SAN clone derives the LUN name from {@link VolumeInfo#getName()}. + */ + private void renameTemporarySnapshotCopy(VolumeInfo volInfo, VolumeVO volumeVO, long csSnapshotId) { + String name = OntapStorageConstants.TEMP_SNAPSHOT_COPY_NAME_PREFIX + csSnapshotId + OntapStorageConstants.UNDERSCORE + volInfo.getId(); + volumeVO.setName(name); + if (volInfo.getVolume() instanceof VolumeVO) { + ((VolumeVO) volInfo.getVolume()).setName(name); + } + } + + /** + * {@code StorageSystemDataMotionStrategy} deletes the temporary snapshot copy on the array but leaves its + * volume row behind on success, so it is removed here once the backend object is gone. + */ + private void removeTemporarySnapshotCopyRecord(long volumeId) { + VolumeVO volumeVO = volumeDao.findById(volumeId); + if (!isTemporarySnapshotCopyVolume(volumeVO) + || volumeVO.getName() == null || !volumeVO.getName().startsWith(OntapStorageConstants.TEMP_SNAPSHOT_COPY_NAME_PREFIX)) { + return; + } + volumeDetailsDao.removeDetails(volumeId); + volumeDao.remove(volumeId); + logger.info("deleteAsync: Removed temporary snapshot copy volume record [{}]", volumeId); } @Override @@ -775,6 +899,7 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore volumeVO.setPoolType(storagePool.getPoolType()); volumeVO.setPoolId(storagePool.getId()); volumeDao.update(volumeVO.getId(), volumeVO); + syncTemporarySnapshotCopyPath(dataObject, volumeVO); } else if (dataObject.getType() == DataObjectType.TEMPLATE) { grantAccessTemplate((TemplateInfo) dataObject, host, dataStore, storagePool); } else { @@ -788,6 +913,22 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore } } + /** + * For the temporary snapshot copy, {@code StorageSystemDataMotionStrategy} builds the CopyCommand from + * the same in-memory volume object it passed to grantAccess, so the iSCSI path resolved by the LUN + * mapping must be reflected on that object too. + */ + private void syncTemporarySnapshotCopyPath(DataObject dataObject, VolumeVO volumeVO) { + if (!isTemporarySnapshotCopyVolume(volumeVO) || !(dataObject instanceof VolumeInfo)) { + return; + } + Volume inMemoryVolume = ((VolumeInfo) dataObject).getVolume(); + if (inMemoryVolume instanceof VolumeVO) { + ((VolumeVO) inMemoryVolume).setPath(volumeVO.getPath()); + ((VolumeVO) inMemoryVolume).set_iScsiName(volumeVO.get_iScsiName()); + } + } + private void grantAccessIscsi(Host host, VolumeVO volumeVO, Map details, String svmName, StoragePoolVO storagePool) { String cloudStackVolumeName = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_NAME).getValue(); UnifiedSANStrategy sanStrategy = (UnifiedSANStrategy) OntapStorageUtils.getStrategyByStoragePoolDetails(details); 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 27e3dd5d3660..421b8ff028e6 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 @@ -169,6 +169,18 @@ public class OntapStorageConstants { */ public static final String CLONE_OF_SNAPSHOT = "cloneOfSnapshot"; + /** + * Driver capability read by {@code StorageSystemDataMotionStrategy} when copying a snapshot to + * secondary storage. The literal must stay in sync with the string used by the orchestrator. + */ + public static final String CAN_DIRECT_ATTACH_SNAPSHOT = "CAN_DIRECT_ATTACH_SNAPSHOT"; + + /** + * Name prefix of the temporary volume a snapshot is cloned into for createTemplate(snapshotid), + * suffixed with the CloudStack snapshot id and volume id. + */ + public static final String TEMP_SNAPSHOT_COPY_NAME_PREFIX = "cs_tmp_snap_"; + // ASUP (AutoSupport) / EMS telemetry public static final String ADVANCED_CONFIG_KEY_CATEGORY = "Advanced"; public static final String ASUP_CATEGORY = "provisioning"; 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 714c9c760115..6c13a546dc07 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 @@ -22,10 +22,12 @@ import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor; +import com.cloud.storage.DataStoreRole; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.dao.SnapshotDao; @@ -35,10 +37,12 @@ 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.CopyCommandResult; 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.SnapshotInfo; 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; @@ -68,6 +72,7 @@ import java.util.HashMap; import java.util.Map; +import static com.cloud.agent.api.to.DataObjectType.SNAPSHOT; 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; @@ -77,6 +82,7 @@ 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.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; @@ -171,6 +177,8 @@ void testGetCapabilities() { 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")); + assertEquals(Boolean.TRUE.toString(), capabilities.get("CAN_CREATE_TEMPLATE_FROM_SNAPSHOT")); + assertEquals(Boolean.FALSE.toString(), capabilities.get("CAN_DIRECT_ATTACH_SNAPSHOT")); } @Test @@ -381,6 +389,8 @@ void testDeleteAsync_ISCSIVolume_Success() { assertNotNull(result); assertTrue(result.isSuccess()); verify(sanStrategy).deleteCloudStackVolume(any(CloudStackVolume.class)); + verify(volumeDao, never()).remove(anyLong()); + verify(volumeDetailsDao, never()).removeDetails(anyLong()); } } @@ -553,6 +563,7 @@ void testGrantAccess_ClusterScope_Success() { verify(sanStrategy).getAccessGroup(any()); verify(sanStrategy).ensureLunMapped(anyString(), anyString(), anyString()); verify(sanStrategy, never()).validateInitiatorInAccessGroup(anyString(), anyString(), any(Igroup.class)); + verify(volumeInfo, never()).getVolume(); } } @@ -1717,4 +1728,278 @@ void testCreateAsync_VolumeClonedFromTemplate_MissingSpoolRef_Fails() { verify(sanStrategy, never()).cloneCloudStackVolume(any()); } } + + private static VolumeVO temporarySnapshotCopyVolume() { + return new VolumeVO(Volume.Type.DATADISK, "ROOT-5_20260924.TMP", 1L, 1L, 2L, 0L, + Storage.ProvisioningType.THIN, 5368709120L, 0L, 0L, ""); + } + + private SnapshotInfo stubSnapshotToVolumeCopy(VolumeVO destVolume) { + SnapshotInfo snapshotInfo = mock(SnapshotInfo.class); + lenient().when(snapshotInfo.getType()).thenReturn(SNAPSHOT); + lenient().when(snapshotInfo.getDataStore()).thenReturn(dataStore); + lenient().when(snapshotInfo.getId()).thenReturn(200L); + lenient().when(volumeInfo.getType()).thenReturn(VOLUME); + lenient().when(volumeInfo.getDataStore()).thenReturn(dataStore); + lenient().when(volumeInfo.getId()).thenReturn(100L); + lenient().when(dataStore.getId()).thenReturn(1L); + lenient().when(dataStore.getRole()).thenReturn(DataStoreRole.Primary); + lenient().when(volumeDao.findById(100L)).thenReturn(destVolume); + return snapshotInfo; + } + + private void stubSnapshotCloneSource(String protocol) { + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + lenient().when(storagePool.getId()).thenReturn(1L); + lenient().when(storagePool.getName()).thenReturn("vol1"); + lenient().when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM); + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, protocol); + storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "vol1"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + lenient().when(volumeInfo.getSize()).thenReturn(5368709120L); + + String volumePath = ProtocolType.NFS3.name().equals(protocol) ? "source-file-uuid" : "/vol/vol1/source_lun"; + lenient().when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.ONTAP_SNAP_NAME)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.ONTAP_SNAP_NAME, "snap_cs200", false)); + lenient().when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.VOLUME_PATH)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.VOLUME_PATH, volumePath, false)); + lenient().when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.PRIMARY_POOL_ID)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.PRIMARY_POOL_ID, "1", false)); + lenient().when(snapshotDetailsDao.findDetail(200L, OntapStorageConstants.PROTOCOL)) + .thenReturn(new SnapshotDetailsVO(200L, OntapStorageConstants.PROTOCOL, protocol, false)); + SnapshotVO snapshotVO = mock(SnapshotVO.class); + lenient().when(snapshotDao.findById(200L)).thenReturn(snapshotVO); + lenient().when(snapshotVO.getSize()).thenReturn(5368709120L); + } + + @Test + void testCanCopy_TemporarySnapshotCopyOnSamePool_ReturnsTrue() { + SnapshotInfo snapshotInfo = stubSnapshotToVolumeCopy(temporarySnapshotCopyVolume()); + + assertTrue(driver.canCopy(snapshotInfo, volumeInfo)); + } + + @Test + void testCanCopy_CreateVolumeFromSnapshot_ReturnsFalse() { + VolumeVO userVolume = temporarySnapshotCopyVolume(); + userVolume.setDiskOfferingId(5L); + userVolume.setState(Volume.State.Creating); + SnapshotInfo snapshotInfo = stubSnapshotToVolumeCopy(userVolume); + + assertFalse(driver.canCopy(snapshotInfo, volumeInfo)); + } + + @Test + void testCanCopy_AllocatedVolumeWithDiskOffering_ReturnsFalse() { + VolumeVO userVolume = temporarySnapshotCopyVolume(); + userVolume.setDiskOfferingId(5L); + SnapshotInfo snapshotInfo = stubSnapshotToVolumeCopy(userVolume); + + assertFalse(driver.canCopy(snapshotInfo, volumeInfo)); + } + + @Test + void testCanCopy_DifferentPool_ReturnsFalse() { + SnapshotInfo snapshotInfo = stubSnapshotToVolumeCopy(temporarySnapshotCopyVolume()); + DataStore otherStore = mock(DataStore.class); + when(otherStore.getId()).thenReturn(2L); + when(volumeInfo.getDataStore()).thenReturn(otherStore); + + assertFalse(driver.canCopy(snapshotInfo, volumeInfo)); + verify(volumeDao, never()).findById(anyLong()); + } + + @Test + void testCanCopy_TemplateCacheCopies_ReturnFalse() { + TemplateInfo templateOnPrimary = mock(TemplateInfo.class); + lenient().when(templateInfo.getType()).thenReturn(TEMPLATE); + lenient().when(templateInfo.getDataStore()).thenReturn(dataStore); + lenient().when(templateOnPrimary.getType()).thenReturn(TEMPLATE); + lenient().when(templateOnPrimary.getDataStore()).thenReturn(dataStore); + lenient().when(volumeInfo.getType()).thenReturn(VOLUME); + lenient().when(volumeInfo.getDataStore()).thenReturn(dataStore); + + assertFalse(driver.canCopy(templateInfo, templateOnPrimary)); + assertFalse(driver.canCopy(templateOnPrimary, volumeInfo)); + verify(volumeDao, never()).findById(anyLong()); + } + + @Test + void testCopyAsync_UnsupportedPair_Throws() { + lenient().when(templateInfo.getType()).thenReturn(TEMPLATE); + lenient().when(templateInfo.getDataStore()).thenReturn(dataStore); + lenient().when(volumeInfo.getType()).thenReturn(VOLUME); + lenient().when(volumeInfo.getDataStore()).thenReturn(dataStore); + + assertThrows(UnsupportedOperationException.class, () -> driver.copyAsync(templateInfo, volumeInfo, null, null)); + } + + @Test + void testCopyAsync_IscsiTemporarySnapshotCopy_ClonesLunAndRecordsIdentity() { + VolumeVO dbVolume = temporarySnapshotCopyVolume(); + VolumeVO inMemoryVolume = temporarySnapshotCopyVolume(); + SnapshotInfo snapshotInfo = stubSnapshotToVolumeCopy(dbVolume); + when(volumeInfo.getVolume()).thenReturn(inMemoryVolume); + stubSnapshotCloneSource(ProtocolType.ISCSI.name()); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.OntapiSCSI); + + Lun clonedLun = new Lun(); + clonedLun.setName("/vol/vol1/cs_tmp_snap_200_100"); + clonedLun.setUuid("tmp-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.cloneCloudStackVolumeFromSnapshot(eq(storagePool), any(), eq(volumeInfo), + eq("/vol/vol1/source_lun"), eq("snap_cs200"))).thenReturn(cloned); + + driver.copyAsync(snapshotInfo, volumeInfo, null, null); + + assertEquals("cs_tmp_snap_200_100", dbVolume.getName()); + assertEquals("cs_tmp_snap_200_100", inMemoryVolume.getName()); + assertEquals(Storage.ImageFormat.RAW, dbVolume.getFormat()); + assertEquals("tmp-lun-uuid", dbVolume.getFolder()); + verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_UUID), eq("tmp-lun-uuid"), eq(false)); + verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_NAME), eq("/vol/vol1/cs_tmp_snap_200_100"), eq(false)); + verify(volumeDao).update(anyLong(), eq(dbVolume)); + verify(sanStrategy, never()).resizeCloudStackVolume(any(), anyLong()); + } + } + + @Test + void testCopyAsync_NfsTemporarySnapshotCopy_SetsFilePath() { + VolumeVO dbVolume = temporarySnapshotCopyVolume(); + SnapshotInfo snapshotInfo = stubSnapshotToVolumeCopy(dbVolume); + when(volumeInfo.getVolume()).thenReturn(temporarySnapshotCopyVolume()); + when(volumeInfo.getUuid()).thenReturn("tmp-volume-uuid"); + stubSnapshotCloneSource(ProtocolType.NFS3.name()); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + + CloudStackVolume cloned = new CloudStackVolume(); + FileInfo file = new FileInfo(); + file.setPath("tmp-volume-uuid"); + cloned.setFile(file); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(nasStrategy); + when(nasStrategy.cloneCloudStackVolumeFromSnapshot(eq(storagePool), any(), eq(volumeInfo), + eq("source-file-uuid"), eq("snap_cs200"))).thenReturn(cloned); + + driver.copyAsync(snapshotInfo, volumeInfo, null, null); + + assertEquals("tmp-volume-uuid", dbVolume.getPath()); + assertEquals(Storage.ImageFormat.QCOW2, dbVolume.getFormat()); + verify(volumeDao).update(anyLong(), eq(dbVolume)); + } + } + + @Test + void testCopyAsync_CloneFailsWithoutCallback_Throws() { + SnapshotInfo snapshotInfo = stubSnapshotToVolumeCopy(temporarySnapshotCopyVolume()); + when(volumeInfo.getVolume()).thenReturn(temporarySnapshotCopyVolume()); + stubSnapshotCloneSource(ProtocolType.ISCSI.name()); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenThrow(new CloudRuntimeException("clone failed")); + + CloudRuntimeException ex = assertThrows(CloudRuntimeException.class, + () -> driver.copyAsync(snapshotInfo, volumeInfo, null, null)); + assertTrue(ex.getMessage().contains("clone failed")); + verify(volumeDetailsDao, never()).addDetail(anyLong(), anyString(), anyString(), anyBoolean()); + } + } + + @Test + void testCopyAsync_CloneFailsWithCallback_CompletesWithFailure() { + SnapshotInfo snapshotInfo = stubSnapshotToVolumeCopy(temporarySnapshotCopyVolume()); + when(volumeInfo.getVolume()).thenReturn(temporarySnapshotCopyVolume()); + stubSnapshotCloneSource(ProtocolType.ISCSI.name()); + @SuppressWarnings("unchecked") + AsyncCompletionCallback copyCallback = mock(AsyncCompletionCallback.class); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy); + when(sanStrategy.cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString())) + .thenThrow(new CloudRuntimeException("clone failed")); + + driver.copyAsync(snapshotInfo, volumeInfo, null, copyCallback); + + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CopyCommandResult.class); + verify(copyCallback).complete(resultCaptor.capture()); + assertFalse(resultCaptor.getValue().isSuccess()); + } + } + + @Test + void testDeleteAsync_TemporarySnapshotCopy_NullCallbackRemovesRecord() { + VolumeVO tempVolume = temporarySnapshotCopyVolume(); + tempVolume.setName("cs_tmp_snap_200_100"); + when(dataStore.getId()).thenReturn(1L); + when(volumeInfo.getType()).thenReturn(VOLUME); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeDao.findById(100L)).thenReturn(tempVolume); + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_NAME)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/cs_tmp_snap_200_100", false)); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_UUID)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_UUID, "tmp-lun-uuid", false)); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class, CALLS_REAL_METHODS)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)).thenReturn(sanStrategy); + + driver.deleteAsync(dataStore, volumeInfo, null); + + verify(sanStrategy).deleteCloudStackVolume(any(CloudStackVolume.class)); + verify(volumeDetailsDao).removeDetails(100L); + verify(volumeDao).remove(100L); + } + } + + @Test + void testGrantAccess_TemporarySnapshotCopy_SyncsIscsiPathOnInMemoryVolume() { + String iscsiPath = "/iqn.1992-08.com.netapp:sn.123456/0"; + VolumeVO dbVolume = mock(VolumeVO.class); + when(dbVolume.getId()).thenReturn(100L); + when(dbVolume.getState()).thenReturn(Volume.State.Allocated); + when(dbVolume.getDiskOfferingId()).thenReturn(0L); + when(dbVolume.getPath()).thenReturn(iscsiPath); + when(dbVolume.get_iScsiName()).thenReturn(iscsiPath); + VolumeVO inMemoryVolume = temporarySnapshotCopyVolume(); + + when(dataStore.getId()).thenReturn(1L); + when(volumeInfo.getType()).thenReturn(VOLUME); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeInfo.getVolume()).thenReturn(inMemoryVolume); + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePool.getId()).thenReturn(1L); + when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER); + when(storagePool.getPath()).thenReturn("iqn.1992-08.com.netapp:sn.123456"); + when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.OntapiSCSI); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(volumeDao.findById(100L)).thenReturn(dbVolume); + when(host.getUuid()).thenReturn("host-uuid-1"); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.LUN_DOT_NAME)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.LUN_DOT_NAME, "/vol/vol1/cs_tmp_snap_200_100", false)); + + 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(anyString(), anyString(), anyString())).thenReturn("0"); + + assertTrue(driver.grantAccess(volumeInfo, host, dataStore)); + + assertEquals(iscsiPath, inMemoryVolume.getPath()); + assertEquals(iscsiPath, inMemoryVolume.get_iScsiName()); + } + } } From 90aaf4e68b990dc5f067f320d7b4c2eb3a5e0118 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Thu, 24 Sep 2026 15:56:15 +0530 Subject: [PATCH 4/4] CSTACKEX-306: incorporate review comments --- .../driver/OntapPrimaryDatastoreDriver.java | 91 ++++++++----------- .../storage/service/StorageStrategy.java | 8 +- .../storage/service/UnifiedNASStrategy.java | 16 ++-- .../storage/service/UnifiedSANStrategy.java | 34 +++---- .../OntapPrimaryDatastoreDriverTest.java | 6 +- 5 files changed, 73 insertions(+), 82 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 5d7934f673bc..df39804b75cc 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 @@ -89,6 +89,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -194,21 +195,21 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet * 3) else — blank LUN (iSCSI) or qcow2 (NFS). * * Mutually exclusive from the motion/orchestrator layer; snapshot is checked first - * (SolidFire-style) so a restore never accidentally falls through to blank create. + * (SolidFire-style) so a create-from-snapshot never accidentally falls through to blank create. */ Long cloneOfSnapshotId = getSnapshotIdForCloning(volInfo.getId()); + Long cloneOfTemplateId = getTemplateIdForCloning(volInfo.getId()); CloudStackVolume clonedCloudStackVolume; if (cloneOfSnapshotId != null) { clonedCloudStackVolume = cloneCloudStackVolumeFromSnapshot( storagePool, volInfo, details, cloneOfSnapshotId); // TODO(CSTACKEX-306): apply persisted MIN_IOPS / MAX_IOPS from snapshot_details // onto this CloudStack volume (and ONTAP QoS if applicable) after successful clone. + } else if (cloneOfTemplateId != null) { + clonedCloudStackVolume = cloneCloudStackVolumeFromTemplate( + storagePool, volInfo, details, cloneOfTemplateId); } else { - Long cloneOfTemplateId = getTemplateIdForCloning(volInfo.getId()); - clonedCloudStackVolume = cloneOfTemplateId != null - ? cloneCloudStackVolumeFromTemplate( - storagePool, volInfo, details, cloneOfTemplateId) - : createCloudStackVolume(storagePool, volInfo, details); + clonedCloudStackVolume = createCloudStackVolume(storagePool, volInfo, details); } createCmdResult = recordCreatedVolume(storagePool, volInfo, volumeVO, details, clonedCloudStackVolume); @@ -375,7 +376,7 @@ private Long getTemplateIdForCloning(long volumeId) { /** * Returns the CloudStack snapshot id to clone from when {@code volume_details.cloneOfSnapshot} - * is set, or null when this create is not a restore-from-snapshot. + * is set, or null when this create is not a create-volume-from-snapshot. * *

Set by {@code StorageSystemDataMotionStrategy.handleCreateManagedVolumeFromManagedSnapshot} * for the duration of {@code createAsync} only (same pattern as {@link #getTemplateIdForCloning}).

@@ -395,7 +396,7 @@ private Long getSnapshotIdForCloning(long volumeId) { *

Combinations (product + plugin v1):

*
    *
  • Same pool only — {@code snapshot_details.PRIMARY_POOL_ID} must equal this - * {@code storagePool}. Cross-pool restore is descoped; use migrate later if needed.
    • + * {@code storagePool}. Creating the volume on a different pool is descoped; use migrate later if needed. *
  • DATA snapshot → attachable data disk (disk offering usually inherited).
    • *
  • ROOT snapshot → still a data disk here (not bootable). Bootable path remains * {@code createTemplate(snapshotid)} then deploy.
    • @@ -420,7 +421,7 @@ private CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storage if (!String.valueOf(storagePool.getId()).equals(primaryPoolId)) { throw new CloudRuntimeException("Create volume from snapshot [" + csSnapshotId + "] requires the snapshot's primary pool [" + primaryPoolId - + "]; requested pool is [" + storagePool.getId() + "] (cross-pool restore is not supported in v1)"); + + "]; requested pool is [" + storagePool.getId() + "] (creating a volume from a snapshot on a different pool is not supported)"); } String poolProtocol = details.get(OntapStorageConstants.PROTOCOL); if (poolProtocol == null || !poolProtocol.equalsIgnoreCase(snapProtocol)) { @@ -1388,9 +1389,20 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback snapshotDetails = new LinkedHashMap<>(); + snapshotDetails.put(OntapStorageConstants.SRC_CS_VOLUME_ID, String.valueOf(volumeInfo.getId())); + snapshotDetails.put(OntapStorageConstants.BASE_ONTAP_FV_ID, flexVolUuid); + snapshotDetails.put(OntapStorageConstants.ONTAP_SNAP_ID, ontapSnapshotUuid); + snapshotDetails.put(OntapStorageConstants.ONTAP_SNAP_NAME, snapshotName); + snapshotDetails.put(OntapStorageConstants.VOLUME_PATH, volumePath); + snapshotDetails.put(OntapStorageConstants.PRIMARY_POOL_ID, String.valueOf(volumeVO.getPoolId())); + snapshotDetails.put(OntapStorageConstants.PROTOCOL, protocol); + // iSCSI only — needed for LUN restore / identity; null (skipped) for NFS. + snapshotDetails.put(OntapStorageConstants.LUN_DOT_UUID, lunUuid); + // TODO(CSTACKEX-306): on create-volume-from-snapshot, read these and apply to the new volume. + putPositiveIopsDetail(snapshotDetails, OntapStorageConstants.MIN_IOPS, volumeVO.getMinIops()); + putPositiveIopsDetail(snapshotDetails, OntapStorageConstants.MAX_IOPS, volumeVO.getMaxIops()); + updateSnapshotDetails(snapshot.getId(), snapshotDetails); CreateObjectAnswer createObjectAnswer = new CreateObjectAnswer(snapshotObjectTo); result = new CreateCmdResult(null, createObjectAnswer); @@ -1789,57 +1801,34 @@ private Storage.ImageFormat getImageFormat(StoragePoolVO storagePool) { *

    All rows go through {@link #persistSnapshotDetail} so DAO writes stay consistent; * optional fields (LUN uuid, IOPS) are skipped when unset.

    * - * @param csSnapshotId CloudStack snapshot ID - * @param csVolumeId Source CloudStack volume ID - * @param flexVolUuid ONTAP FlexVolume UUID - * @param ontapSnapshotUuid ONTAP FlexVolume snapshot UUID - * @param snapshotName ONTAP snapshot name - * @param volumePath Path of the volume file/LUN within the FlexVolume (for restore) - * @param storagePoolId Primary storage pool ID - * @param protocol Storage protocol (NFS3 or ISCSI) - * @param lunUuid LUN UUID (only for iSCSI, null for NFS) - * @param minIops Source volume min IOPS if configured; null/<=0 skipped - * @param maxIops Source volume max IOPS if configured; null/<=0 skipped + * @param csSnapshotId CloudStack snapshot ID + * @param details {@code snapshot_details} name to value, keyed by {@link OntapStorageConstants}; + * entries with null or blank values are skipped */ - private void updateSnapshotDetails(long csSnapshotId, long csVolumeId, String flexVolUuid, - String ontapSnapshotUuid, String snapshotName, - String volumePath, long storagePoolId, String protocol, - String lunUuid, Long minIops, Long maxIops) { - persistSnapshotDetail(csSnapshotId, OntapStorageConstants.SRC_CS_VOLUME_ID, String.valueOf(csVolumeId)); - persistSnapshotDetail(csSnapshotId, OntapStorageConstants.BASE_ONTAP_FV_ID, flexVolUuid); - persistSnapshotDetail(csSnapshotId, OntapStorageConstants.ONTAP_SNAP_ID, ontapSnapshotUuid); - persistSnapshotDetail(csSnapshotId, OntapStorageConstants.ONTAP_SNAP_NAME, snapshotName); - persistSnapshotDetail(csSnapshotId, OntapStorageConstants.VOLUME_PATH, volumePath); - persistSnapshotDetail(csSnapshotId, OntapStorageConstants.PRIMARY_POOL_ID, String.valueOf(storagePoolId)); - persistSnapshotDetail(csSnapshotId, OntapStorageConstants.PROTOCOL, protocol); - // iSCSI only — needed for LUN restore / identity; omitted for NFS. - persistSnapshotDetail(csSnapshotId, OntapStorageConstants.LUN_DOT_UUID, lunUuid); - // Optional: only when the source volume has IOPS configured (no-op otherwise). - // TODO(CSTACKEX-306): on create-volume-from-snapshot, read these and apply to the new volume. - persistSnapshotIopsDetail(csSnapshotId, OntapStorageConstants.MIN_IOPS, minIops); - persistSnapshotIopsDetail(csSnapshotId, OntapStorageConstants.MAX_IOPS, maxIops); + private void updateSnapshotDetails(long csSnapshotId, Map details) { + for (Map.Entry detail : details.entrySet()) { + persistSnapshotDetail(csSnapshotId, detail.getKey(), detail.getValue()); + } } /** - * Persists one {@code snapshot_details} row. No-op when {@code value} is null or blank so - * optional keys (e.g. LUN uuid on NFS) share the same DAO path as required keys. + * Adds an IOPS entry to {@code details} only when {@code iops} is non-null and positive. */ - private void persistSnapshotDetail(long csSnapshotId, String detailName, String value) { - if (value == null || value.isEmpty()) { - return; + private static void putPositiveIopsDetail(Map details, String detailName, Long iops) { + if (iops != null && iops > 0) { + details.put(detailName, String.valueOf(iops)); } - snapshotDetailsDao.persist(new SnapshotDetailsVO(csSnapshotId, detailName, value, false)); } /** - * Persists a snapshot IOPS detail when {@code iops} is non-null and positive; otherwise no-op. + * Persists one {@code snapshot_details} row. No-op when {@code value} is null or blank so + * optional keys (e.g. LUN uuid on NFS) share the same DAO path as required keys. */ - private void persistSnapshotIopsDetail(long csSnapshotId, String detailName, Long iops) { - if (iops == null || iops <= 0) { + private void persistSnapshotDetail(long csSnapshotId, String detailName, String value) { + if (value == null || value.isEmpty()) { return; } - persistSnapshotDetail(csSnapshotId, detailName, String.valueOf(iops)); - logger.debug("updateSnapshotDetails: persisted {}={} for snapshot [{}]", detailName, iops, csSnapshotId); + snapshotDetailsDao.persist(new SnapshotDetailsVO(csSnapshotId, detailName, value, false)); } } 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 a6b0a56279da..4e8ce0f75b68 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 @@ -833,8 +833,8 @@ abstract public CloudStackVolume createTemplateCache(StoragePoolVO storagePool, /** * Creates a new file/LUN in the same FlexVolume by cloning from a FlexVolume snapshot. * - *

    Product scope (v1): same primary pool / FlexVol only. Cross-pool restore is - * descoped — operators may later {@code migrateVolume} if another pool is required.

    + *

    Product scope (v1): same primary pool / FlexVol only. Creating the volume on a + * different pool is descoped — operators may later {@code migrateVolume} if another pool is required.

    * *

    ONTAP backends (protocol-specific; each subclass builds its own request):

    *
      @@ -844,14 +844,14 @@ abstract public CloudStackVolume createTemplateCache(StoragePoolVO storagePool, *
    * * @param storagePool target CloudStack primary pool (same FlexVol as the snapshot) - * @param details pool details (SVM, FlexVol name/uuid, protocol, …) + * @param poolDetails pool details (SVM, FlexVol name/uuid, protocol, …) * @param volumeInfo destination CloudStack volume being created * @param sourceVolumePath snapshotted object path from {@code snapshot_details.VOLUME_PATH} * @param snapshotName ONTAP FlexVol snapshot name from {@code snapshot_details} * @return created CloudStackVolume with protocol-specific identity (LUN uuid or file path) */ abstract public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, - Map details, + Map poolDetails, VolumeInfo volumeInfo, String sourceVolumePath, String snapshotName); 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 e8cc2093397f..c7b184dde9ec 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 @@ -158,7 +158,7 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) 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"); + throw new CloudRuntimeException("Failed to clone file, FlexVolume uuid is missing from pool poolDetails"); } String sourcePath = cloudstackVolume.getFile().getPath(); String destinationPath = cloudstackVolume.getDestinationPath(); @@ -206,10 +206,10 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) *
*/ @Override - public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, Map details, + public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, Map poolDetails, VolumeInfo volumeInfo, String sourceVolumePath, String snapshotName) { - if (storagePool == null || details == null || volumeInfo == null) { + if (storagePool == null || poolDetails == null || volumeInfo == null) { throw new CloudRuntimeException("Failed to clone file from snapshot, invalid request"); } if (sourceVolumePath == null || sourceVolumePath.isEmpty()) { @@ -219,10 +219,10 @@ public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storageP throw new CloudRuntimeException("Failed to clone file from snapshot, snapshot name is required"); } - String flexVolUuid = details.get(OntapStorageConstants.VOLUME_UUID); - String flexVolName = details.get(OntapStorageConstants.VOLUME_NAME); + String flexVolUuid = poolDetails.get(OntapStorageConstants.VOLUME_UUID); + String flexVolName = poolDetails.get(OntapStorageConstants.VOLUME_NAME); if (flexVolUuid == null || flexVolUuid.isEmpty()) { - throw new CloudRuntimeException("Failed to clone file from snapshot, FlexVolume uuid is missing from pool details"); + throw new CloudRuntimeException("Failed to clone file from snapshot, FlexVolume uuid is missing from pool poolDetails"); } String sourcePath = OntapStorageUtils.toFlexVolRelativePath(sourceVolumePath, flexVolName); @@ -337,7 +337,7 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { logger.info("createAccessGroup: ExportPolicy created: {}, now attaching this policy to storage pool volume", createdPolicy.getName()); // attach export policy to volume of storage pool assignExportPolicyToVolume(volumeUUID,createdPolicy.getName()); - // save the export policy details in storage pool details + // save the export policy poolDetails in storage pool poolDetails storagePoolDetailsDao.addDetail(accessGroup.getStoragePoolId(), OntapStorageConstants.EXPORT_POLICY_ID, String.valueOf(createdPolicy.getId()), true); storagePoolDetailsDao.addDetail(accessGroup.getStoragePoolId(), OntapStorageConstants.EXPORT_POLICY_NAME, createdPolicy.getName(), true); logger.info("Successfully assigned exportPolicy {} to volume {}", policyRequest.getName(), volumeName); @@ -398,7 +398,7 @@ public AccessGroup updateAccessGroup(AccessGroup accessGroup) { Map details = storagePoolDetailsDao.listDetailsKeyPairs(accessGroup.getStoragePoolId()); if (details == null || details.isEmpty()) { - throw new CloudRuntimeException("No storage pool details found for storagePoolId: " + accessGroup.getStoragePoolId()); + throw new CloudRuntimeException("No storage pool poolDetails found for storagePoolId: " + accessGroup.getStoragePoolId()); } String exportPolicyId = details.get(OntapStorageConstants.EXPORT_POLICY_ID); if (exportPolicyId == null || exportPolicyId.isEmpty()) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 70279c05427b..bae3cebc8784 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 @@ -281,10 +281,10 @@ public CloudStackVolume cloneCloudStackVolume(CloudStackVolume cloudstackVolume) * */ @Override - public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, Map details, + public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storagePool, Map poolDetails, VolumeInfo volumeInfo, String sourceVolumePath, String snapshotName) { - if (storagePool == null || details == null || volumeInfo == null) { + if (storagePool == null || poolDetails == null || volumeInfo == null) { throw new CloudRuntimeException("Failed to clone Lun from snapshot, invalid request"); } if (sourceVolumePath == null || sourceVolumePath.isEmpty()) { @@ -294,7 +294,7 @@ public CloudStackVolume cloneCloudStackVolumeFromSnapshot(StoragePoolVO storageP throw new CloudRuntimeException("Failed to clone Lun from snapshot, snapshot name is required"); } - Lun lunRequest = buildCloneLunFromSnapshotRequest(storagePool, details, volumeInfo, sourceVolumePath, snapshotName); + Lun lunRequest = buildCloneLunFromSnapshotRequest(storagePool, poolDetails, volumeInfo, sourceVolumePath, snapshotName); logger.info("cloneCloudStackVolumeFromSnapshot [iSCSI]: Cloning LUN [{}] from snapshot source [{}]", lunRequest.getName(), lunRequest.getClone().getSource().getName()); try { @@ -439,10 +439,10 @@ public CloudStackVolume getCloudStackVolume(Map values) { return null; } logger.error("FeignException occurred while fetching Lun, Status: {}, Exception: {}", e.status(), e.getMessage()); - throw new CloudRuntimeException("Failed to fetch Lun details: " + e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Lun poolDetails: " + e.getMessage()); } catch (Exception e) { logger.error("Exception occurred while fetching Lun, Exception: {}", e.getMessage()); - throw new CloudRuntimeException("Failed to fetch Lun details: " + e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Lun poolDetails: " + e.getMessage()); } } @@ -453,9 +453,9 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { logger.error("createAccessGroup: Igroup creation failed. Invalid request: {}", accessGroup); throw new CloudRuntimeException("Failed to create Igroup, invalid request"); } - // Get StoragePool details + // Get StoragePool poolDetails if (accessGroup.getStoragePoolId() == null) { - throw new CloudRuntimeException("Failed to create Igroup, invalid datastore details in the request"); + throw new CloudRuntimeException("Failed to create Igroup, invalid datastore poolDetails in the request"); } if (accessGroup.getHostsToConnect() == null || accessGroup.getHostsToConnect().isEmpty()) { throw new CloudRuntimeException("Failed to create Igroup, no hosts to connect provided in the request"); @@ -464,7 +464,7 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) { String igroupName = null; try { Map dataStoreDetails = storagePoolDetailsDao.listDetailsKeyPairs(accessGroup.getStoragePoolId()); - logger.trace("createAccessGroup: Successfully fetched datastore details."); + logger.trace("createAccessGroup: Successfully fetched datastore poolDetails."); // Generate Igroup request Igroup igroupRequest = new Igroup(); @@ -540,9 +540,9 @@ public void deleteAccessGroup(AccessGroup accessGroup) { logger.error("deleteAccessGroup: Igroup deletion failed. Invalid request: {}", accessGroup); throw new CloudRuntimeException("Failed to delete Igroup, invalid request"); } - // Get StoragePool details + // Get StoragePool poolDetails if (accessGroup.getStoragePoolId() == null) { - throw new CloudRuntimeException("Failed to delete Igroup, invalid datastore details in the request"); + throw new CloudRuntimeException("Failed to delete Igroup, invalid datastore poolDetails in the request"); } try { String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); @@ -649,10 +649,10 @@ public AccessGroup getAccessGroup(Map values) { return null; } logger.error("FeignException occurred while fetching Igroup, Status: {}, Exception: {}", e.status(), e.getMessage()); - throw new CloudRuntimeException("Failed to fetch Igroup details: " + e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Igroup poolDetails: " + e.getMessage()); } catch (Exception e) { logger.error("Exception occurred while fetching Igroup, Exception: {}", e.getMessage()); - throw new CloudRuntimeException("Failed to fetch Igroup details: " + e.getMessage()); + throw new CloudRuntimeException("Failed to fetch Igroup poolDetails: " + e.getMessage()); } } @@ -698,7 +698,7 @@ public String enableLogicalAccess(Map values) { throw feignEx; } } - // Get the LunMap details + // Get the LunMap poolDetails OntapResponse lunMapResponse = null; try { lunMapResponse = sanFeignClient.getLunMapResponse(authHeader, @@ -713,12 +713,12 @@ public String enableLogicalAccess(Map values) { lunNumber = lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString(); } else { - logger.error("enableLogicalAccess: Failed to fetch LunMap details for Lun: {} and igroup: {}. LunMap response is null or empty.", lunName, igroupName); - throw new CloudRuntimeException("Failed to fetch LunMap details for Lun: " + lunName + " and igroup: " + igroupName); + logger.error("enableLogicalAccess: Failed to fetch LunMap poolDetails for Lun: {} and igroup: {}. LunMap response is null or empty.", lunName, igroupName); + throw new CloudRuntimeException("Failed to fetch LunMap poolDetails for Lun: " + lunName + " and igroup: " + igroupName); } } catch (Exception e) { - logger.error("enableLogicalAccess: Failed to fetch LunMap details for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e); - throw new CloudRuntimeException("Failed to fetch LunMap details for Lun: " + lunName + " and igroup: " + igroupName); + logger.error("enableLogicalAccess: Failed to fetch LunMap poolDetails for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e); + throw new CloudRuntimeException("Failed to fetch LunMap poolDetails for Lun: " + lunName + " and igroup: " + igroupName); } logger.trace("enableLogicalAccess: LunMap created successfully, LunMap: {}", lunMapResponse.getRecords().get(0)); } catch (Exception e) { 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 6c13a546dc07..eec20c936518 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 @@ -1141,8 +1141,10 @@ void testCreateAsync_VolumeClonedFromSnapshot_StrategyThrows_Fails() { @Test void testCreateAsync_VolumeClonedFromSnapshot_PrefersSnapshotOverTemplate() { - // Corner: snapshot id is resolved first; template is only consulted when snapshot is absent. + // Corner: when both details are present, the snapshot clone wins over the template clone. stubVolumeCloneFromSnapshot(5368709120L, 5368709120L, ProtocolType.ISCSI.name()); + when(volumeDetailsDao.findDetail(100L, OntapStorageConstants.CLONE_OF_TEMPLATE)) + .thenReturn(new VolumeDetailVO(100L, OntapStorageConstants.CLONE_OF_TEMPLATE, "50", false)); Lun clonedLun = new Lun(); clonedLun.setName("/vol/vol1/test_volume"); @@ -1158,7 +1160,7 @@ void testCreateAsync_VolumeClonedFromSnapshot_PrefersSnapshotOverTemplate() { driver.createAsync(dataStore, volumeInfo, createCallback); verify(sanStrategy).cloneCloudStackVolumeFromSnapshot(any(), any(), any(), anyString(), anyString()); - verify(volumeDetailsDao, never()).findDetail(100L, OntapStorageConstants.CLONE_OF_TEMPLATE); + verify(vmTemplatePoolDao, never()).findByPoolTemplate(anyLong(), anyLong(), any()); verify(sanStrategy, never()).cloneCloudStackVolume(any()); verify(sanStrategy, never()).createCloudStackVolume(any()); }