From 47f5aae0090a9f9ae84c7f8ddef158e6aea925ba Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Fri, 12 Jun 2026 15:22:58 +0530 Subject: [PATCH 01/24] CSTACKEX-198: Snapshot workflows backed by clone feature --- .../driver/OntapPrimaryDatastoreDriver.java | 152 +++++++++--------- .../storage/feign/client/NASFeignClient.java | 6 + .../storage/feign/client/SANFeignClient.java | 25 +-- .../feign/client/SnapshotFeignClient.java | 76 --------- .../model/CliSnapshotRestoreRequest.java | 121 -------------- ...toreRequest.java => FileCloneRequest.java} | 69 ++++++-- .../cloudstack/storage/feign/model/Lun.java | 48 ++++++ .../feign/model/LunRestoreRequest.java | 132 --------------- .../storage/service/StorageStrategy.java | 4 + .../storage/service/UnifiedNASStrategy.java | 35 ++-- .../storage/service/UnifiedSANStrategy.java | 44 ++--- .../storage/utils/OntapStorageConstants.java | 4 +- .../storage/utils/OntapStorageUtils.java | 21 +++ .../vmsnapshot/OntapVMSnapshotStrategy.java | 131 ++++++++------- .../OntapPrimaryDatastoreDriverTest.java | 108 +++++++++++++ .../service/UnifiedNASStrategyTest.java | 31 ++++ .../service/UnifiedSANStrategyTest.java | 38 +++++ .../OntapVMSnapshotStrategyTest.java | 4 +- 18 files changed, 502 insertions(+), 547 deletions(-) delete mode 100644 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/CliSnapshotRestoreRequest.java rename plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/{SnapshotFileRestoreRequest.java => FileCloneRequest.java} (51%) delete mode 100644 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunRestoreRequest.java diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 04996b74d2a5..f7a6119ee50d 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 @@ -55,8 +55,9 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; -import org.apache.cloudstack.storage.feign.model.FlexVolSnapshot; +import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.Svm; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.SANStrategy; @@ -67,7 +68,6 @@ import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.utils.OntapStorageUtils; -import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; @@ -639,7 +639,7 @@ public long getUsedIops(StoragePool storagePool) { */ @Override public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback callback) { - logger.info("OntapPrimaryDatastoreDriver.takeSnapshot: Creating FlexVolume snapshot for snapshot [{}]", snapshot.getId()); + logger.info("OntapPrimaryDatastoreDriver.takeSnapshot: Creating clone-backed snapshot for snapshot [{}]", snapshot.getId()); CreateCmdResult result; try { @@ -665,64 +665,84 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback queryParams = new HashMap<>(); - queryParams.put("name", snapshotName); - queryParams.put("fields", "uuid,name"); - - OntapResponse response = snapshotClient.getSnapshots(authHeader, flexVolUuid, queryParams); - if (response != null && response.getRecords() != null && !response.getRecords().isEmpty()) { - return response.getRecords().get(0).getUuid(); + private String resolveLunUuidByName(StorageStrategy storageStrategy, String authHeader, String svmName, String lunName) { + OntapResponse lunResponse = storageStrategy.getSanFeignClient().getLunResponse(authHeader, + Map.of(OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.NAME, lunName)); + if (lunResponse == null || lunResponse.getRecords() == null || lunResponse.getRecords().isEmpty()) { + throw new CloudRuntimeException("Failed to resolve LUN UUID for clone " + lunName); } - return null; + return lunResponse.getRecords().get(0).getUuid(); } /** @@ -814,16 +821,20 @@ public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snaps // Retrieve snapshot details stored during takeSnapshot String flexVolUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.BASE_ONTAP_FV_ID); - String ontapSnapshotUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_ID); + String ontapCloneId = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_CLONE_ID); String snapshotName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_NAME); + String ontapCloneName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_CLONE_NAME); + if (ontapCloneName == null) { + ontapCloneName = snapshotName; + } String volumePath = getSnapshotDetail(snapshotId, OntapStorageConstants.VOLUME_PATH); String poolIdStr = getSnapshotDetail(snapshotId, OntapStorageConstants.PRIMARY_POOL_ID); String protocol = getSnapshotDetail(snapshotId, OntapStorageConstants.PROTOCOL); - if (flexVolUuid == null || snapshotName == null || volumePath == null || poolIdStr == null) { + if (flexVolUuid == null || snapshotName == null || ontapCloneName == null || volumePath == null || poolIdStr == null) { throw new CloudRuntimeException("Missing required snapshot details for snapshot " + snapshotId + " (flexVolUuid=" + flexVolUuid + ", snapshotName=" + snapshotName + - ", volumePath=" + volumePath + ", poolId=" + poolIdStr + ")"); + ", cloneName=" + ontapCloneName + ", volumePath=" + volumePath + ", poolId=" + poolIdStr + ")"); } long poolId = Long.parseLong(poolIdStr); @@ -840,12 +851,12 @@ public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snaps // Prepare protocol-specific parameters (lunUuid is only needed for backward compatibility) String lunUuid = null; if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { - lunUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.LUN_DOT_UUID); + lunUuid = ontapCloneId; } // Delegate to strategy class for protocol-specific restore JobResponse jobResponse = storageStrategy.revertSnapshotForCloudStackVolume( - snapshotName, flexVolUuid, ontapSnapshotUuid, volumePath, lunUuid, flexVolName); + ontapCloneName, flexVolUuid, ontapCloneId, volumePath, lunUuid, flexVolName); if (jobResponse == null || jobResponse.getJob() == null) { throw new CloudRuntimeException("Failed to initiate restore from snapshot [" + @@ -974,21 +985,6 @@ private CloudStackVolume createDeleteCloudStackVolumeRequest(StoragePool storage // Snapshot Helper Methods // ────────────────────────────────────────────────────────────────────────── - /** - * Builds a snapshot name with proper length constraints. - * Format: {@code -} - */ - private String buildSnapshotName(String volumeName, String snapshotUuid) { - String name = volumeName + "-" + snapshotUuid; - int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH; - int trimRequired = name.length() - maxLength; - - if (trimRequired > 0) { - name = StringUtils.left(volumeName, volumeName.length() - trimRequired) + "-" + snapshotUuid; - } - return name; - } - /** * Persists snapshot metadata in snapshot_details table. * @@ -1003,7 +999,7 @@ private String buildSnapshotName(String volumeName, String snapshotUuid) { * @param lunUuid LUN UUID (only for iSCSI, null for NFS) */ private void updateSnapshotDetails(long csSnapshotId, long csVolumeId, String flexVolUuid, - String ontapSnapshotUuid, String snapshotName, + String ontapCloneId, String snapshotName, String ontapCloneName, String volumePath, long storagePoolId, String protocol, String lunUuid) { SnapshotDetailsVO snapshotDetail = new SnapshotDetailsVO(csSnapshotId, @@ -1015,13 +1011,21 @@ private void updateSnapshotDetails(long csSnapshotId, long csVolumeId, String fl snapshotDetailsDao.persist(snapshotDetail); snapshotDetail = new SnapshotDetailsVO(csSnapshotId, - OntapStorageConstants.ONTAP_SNAP_ID, ontapSnapshotUuid, false); + OntapStorageConstants.ONTAP_SNAP_ID, ontapCloneId, false); snapshotDetailsDao.persist(snapshotDetail); snapshotDetail = new SnapshotDetailsVO(csSnapshotId, OntapStorageConstants.ONTAP_SNAP_NAME, snapshotName, false); snapshotDetailsDao.persist(snapshotDetail); + snapshotDetail = new SnapshotDetailsVO(csSnapshotId, + OntapStorageConstants.ONTAP_CLONE_ID, ontapCloneId, false); + snapshotDetailsDao.persist(snapshotDetail); + + snapshotDetail = new SnapshotDetailsVO(csSnapshotId, + OntapStorageConstants.ONTAP_CLONE_NAME, ontapCloneName, false); + snapshotDetailsDao.persist(snapshotDetail); + snapshotDetail = new SnapshotDetailsVO(csSnapshotId, OntapStorageConstants.VOLUME_PATH, volumePath, false); snapshotDetailsDao.persist(snapshotDetail); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java index 8cf21b94b2f1..14d3011d81f8 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java @@ -21,7 +21,9 @@ import feign.QueryMap; import org.apache.cloudstack.storage.feign.model.ExportPolicy; +import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.FileInfo; +import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import feign.Headers; import feign.Param; @@ -58,6 +60,10 @@ void createFile(@Param("authHeader") String authHeader, @Param("path") String filePath, FileInfo file); + @RequestLine("POST /api/storage/file/clone") + @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) + JobResponse cloneFile(@Param("authHeader") String authHeader, FileCloneRequest request); + // Export Policy Operations @RequestLine("POST /api/protocols/nfs/export-policies") @Headers({"Authorization: {authHeader}"}) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index 7281dc2ecbeb..e9857a431acd 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java @@ -23,7 +23,6 @@ import org.apache.cloudstack.storage.feign.model.IscsiService; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunMap; -import org.apache.cloudstack.storage.feign.model.LunRestoreRequest; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import feign.Headers; @@ -42,6 +41,10 @@ public interface SANFeignClient { @Headers({"Authorization: {authHeader}"}) OntapResponse createLun(@Param("authHeader") String authHeader, @Param("returnRecords") boolean returnRecords, Lun lun); + @RequestLine("POST /api/storage/luns") + @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) + JobResponse cloneLun(@Param("authHeader") String authHeader, Lun lun); + @RequestLine("GET /api/storage/luns") @Headers({"Authorization: {authHeader}"}) OntapResponse getLunResponse(@Param("authHeader") String authHeader, @QueryMap Map queryMap); @@ -90,24 +93,4 @@ public interface SANFeignClient { void deleteLunMap(@Param("authHeader") String authHeader, @Param("lunUuid") String lunUUID, @Param("igroupUuid") String igroupUUID); - - // LUN Restore API - /** - * Restores a LUN from a FlexVolume snapshot. - * - *

ONTAP REST: {@code POST /api/storage/luns/{lun.uuid}/restore}

- * - *

This API restores the LUN data from a specified snapshot to a destination path. - * The LUN must exist and the snapshot must contain the LUN data.

- * - * @param authHeader Basic auth header - * @param lunUuid UUID of the LUN to restore - * @param request Request body with snapshot name and destination path - * @return JobResponse containing the async job reference - */ - @RequestLine("POST /api/storage/luns/{lunUuid}/restore") - @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) - JobResponse restoreLun(@Param("authHeader") String authHeader, - @Param("lunUuid") String lunUuid, - LunRestoreRequest request); } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java index 2f0e050d6f55..2f5a000caf8a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java @@ -22,9 +22,7 @@ import feign.Param; import feign.QueryMap; import feign.RequestLine; -import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.feign.model.FlexVolSnapshot; -import org.apache.cloudstack.storage.feign.model.SnapshotFileRestoreRequest; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; @@ -107,78 +105,4 @@ JobResponse deleteSnapshot(@Param("authHeader") String authHeader, @Param("volumeUuid") String volumeUuid, @Param("snapshotUuid") String snapshotUuid); - /** - * Restores a volume to a specific snapshot. - * - *

ONTAP REST: {@code PATCH /api/storage/volumes/{volume_uuid}/snapshots/{uuid}} - * with body {@code {"restore": true}} triggers a snapshot restore operation.

- * - *

Note: This is a destructive operation — all data written after the - * snapshot was taken will be lost.

- * - * @param authHeader Basic auth header - * @param volumeUuid UUID of the ONTAP FlexVolume - * @param snapshotUuid UUID of the snapshot to restore to - * @param body Request body, typically {@code {"restore": true}} - * @return JobResponse containing the async job reference - */ - @RequestLine("PATCH /api/storage/volumes/{volumeUuid}/snapshots/{snapshotUuid}?restore_to_snapshot=true") - @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) - JobResponse restoreSnapshot(@Param("authHeader") String authHeader, - @Param("volumeUuid") String volumeUuid, - @Param("snapshotUuid") String snapshotUuid); - - /** - * Restores a single file or LUN from a FlexVolume snapshot. - * - *

ONTAP REST: - * {@code POST /api/storage/volumes/{volume_uuid}/snapshots/{snapshot_uuid}/files/{file_path}/restore}

- * - *

This restores only the specified file/LUN from the snapshot to the - * given {@code destination_path}, without reverting the entire FlexVolume. - * Ideal when multiple VMs share the same FlexVolume.

- * - * @param authHeader Basic auth header - * @param volumeUuid UUID of the ONTAP FlexVolume - * @param snapshotUuid UUID of the snapshot containing the file - * @param filePath path of the file within the snapshot (URL-encoded if needed) - * @param request request body with {@code destination_path} - * @return JobResponse containing the async job reference - */ - @RequestLine("POST /api/storage/volumes/{volumeUuid}/snapshots/{snapshotUuid}/files/{filePath}/restore") - @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) - JobResponse restoreFileFromSnapshot(@Param("authHeader") String authHeader, - @Param("volumeUuid") String volumeUuid, - @Param("snapshotUuid") String snapshotUuid, - @Param("filePath") String filePath, - SnapshotFileRestoreRequest request); - - /** - * Restores a single file or LUN from a FlexVolume snapshot using the CLI native API. - * - *

ONTAP REST (CLI passthrough): - * {@code POST /api/private/cli/volume/snapshot/restore-file}

- * - *

This CLI-based API is more reliable and works for both NFS files and iSCSI LUNs. - * The request body contains all required parameters: vserver, volume, snapshot, and path.

- * - *

Example payload: - * 

-     * {
-     *   "vserver": "vs0",
-     *   "volume": "rajiv_ONTAP_SP1",
-     *   "snapshot": "DATA-3-428726fe-7440-4b41-8d47-3f654e5d9814",
-     *   "path": "/d266bb2c-d479-47ad-81c3-a070e8bb58c0"
-     * }
-     *
- *

- * - * @param authHeader Basic auth header - * @param request CLI snapshot restore request containing vserver, volume, snapshot, and path - * @return JobResponse containing the async job reference (if applicable) - */ - @RequestLine("POST /api/private/cli/volume/snapshot/restore-file") - @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) - JobResponse restoreFileFromSnapshotCli(@Param("authHeader") String authHeader, - CliSnapshotRestoreRequest request); } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/CliSnapshotRestoreRequest.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/CliSnapshotRestoreRequest.java deleted file mode 100644 index be242523f534..000000000000 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/CliSnapshotRestoreRequest.java +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.cloudstack.storage.feign.model; - -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonProperty; - -/** - * Request body for the ONTAP CLI-based Snapshot File Restore API. - * - *

ONTAP REST endpoint (CLI passthrough): - * {@code POST /api/private/cli/volume/snapshot/restore-file}

- * - *

This API restores a single file or LUN from a FlexVolume snapshot to a - * specified destination path using the CLI native implementation. - * It works for both NFS files and iSCSI LUNs.

- * - *

Example payload: - * 

- * {
- *   "vserver": "vs0",
- *   "volume": "rajiv_ONTAP_SP1",
- *   "snapshot": "DATA-3-428726fe-7440-4b41-8d47-3f654e5d9814",
- *   "path": "/d266bb2c-d479-47ad-81c3-a070e8bb58c0"
- * }
- *
- *

- */ -@JsonIgnoreProperties(ignoreUnknown = true) -@JsonInclude(JsonInclude.Include.NON_NULL) -public class CliSnapshotRestoreRequest { - - @JsonProperty("vserver") - private String vserver; - - @JsonProperty("volume") - private String volume; - - @JsonProperty("snapshot") - private String snapshot; - - @JsonProperty("path") - private String path; - - public CliSnapshotRestoreRequest() { - } - - /** - * Creates a CLI snapshot restore request. - * - * @param vserver The SVM (vserver) name - * @param volume The FlexVolume name - * @param snapshot The snapshot name - * @param path The file/LUN path to restore (e.g., "/uuid.qcow2" or "/lun_name") - */ - public CliSnapshotRestoreRequest(String vserver, String volume, String snapshot, String path) { - this.vserver = vserver; - this.volume = volume; - this.snapshot = snapshot; - this.path = path; - } - - public String getVserver() { - return vserver; - } - - public void setVserver(String vserver) { - this.vserver = vserver; - } - - public String getVolume() { - return volume; - } - - public void setVolume(String volume) { - this.volume = volume; - } - - public String getSnapshot() { - return snapshot; - } - - public void setSnapshot(String snapshot) { - this.snapshot = snapshot; - } - - public String getPath() { - return path; - } - - public void setPath(String path) { - this.path = path; - } - - @Override - public String toString() { - return "CliSnapshotRestoreRequest{" + - "vserver='" + vserver + '\'' + - ", volume='" + volume + '\'' + - ", snapshot='" + snapshot + '\'' + - ", path='" + path + '\'' + - '}'; - } -} diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/SnapshotFileRestoreRequest.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java similarity index 51% rename from plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/SnapshotFileRestoreRequest.java rename to plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java index 1f02e0c07470..20b8d8d64faa 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/SnapshotFileRestoreRequest.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/FileCloneRequest.java @@ -18,31 +18,37 @@ */ package org.apache.cloudstack.storage.feign.model; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; -/** - * Request body for the ONTAP Snapshot File Restore API. - * - *

ONTAP REST endpoint: - * {@code POST /api/storage/volumes/{volume.uuid}/snapshots/{snapshot.uuid}/files/{file.path}/restore}

- * - *

This API restores a single file or LUN from a FlexVolume snapshot to a - * specified destination path, without reverting the entire FlexVolume.

- */ -@JsonIgnoreProperties(ignoreUnknown = true) @JsonInclude(JsonInclude.Include.NON_NULL) -public class SnapshotFileRestoreRequest { +public class FileCloneRequest { + @JsonProperty("volume") + private VolumeRef volume; + + @JsonProperty("source_path") + private String sourcePath; @JsonProperty("destination_path") private String destinationPath; - public SnapshotFileRestoreRequest() { + @JsonProperty("is_override") + private Boolean isOverride; + + public VolumeRef getVolume() { + return volume; } - public SnapshotFileRestoreRequest(String destinationPath) { - this.destinationPath = destinationPath; + public void setVolume(VolumeRef volume) { + this.volume = volume; + } + + public String getSourcePath() { + return sourcePath; + } + + public void setSourcePath(String sourcePath) { + this.sourcePath = sourcePath; } public String getDestinationPath() { @@ -52,4 +58,37 @@ public String getDestinationPath() { public void setDestinationPath(String destinationPath) { this.destinationPath = destinationPath; } + + public Boolean getIsOverride() { + return isOverride; + } + + public void setIsOverride(Boolean isOverride) { + this.isOverride = isOverride; + } + + @JsonInclude(JsonInclude.Include.NON_NULL) + public static class VolumeRef { + @JsonProperty("name") + private String name; + + @JsonProperty("uuid") + private String uuid; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getUuid() { + return uuid; + } + + public void setUuid(String uuid) { + this.uuid = uuid; + } + } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java index 364790958c8a..8f6c09acabb0 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/Lun.java @@ -86,6 +86,12 @@ public static PropertyClassEnum fromValue(String value) { @JsonProperty("clone") private Clone clone = null; + @JsonProperty("location") + private Location location = null; + + @JsonProperty("is_override") + private Boolean isOverride = null; + /** * The operating system type of the LUN.<br/> Required in POST when creating a LUN that is not a clone of another. Disallowed in POST when creating a LUN clone. */ @@ -260,6 +266,22 @@ public void setClone(Clone clone) { this.clone = clone; } + public Location getLocation() { + return location; + } + + public void setLocation(Location location) { + this.location = location; + } + + public Boolean getIsOverride() { + return isOverride; + } + + public void setIsOverride(Boolean isOverride) { + this.isOverride = isOverride; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -338,4 +360,30 @@ public void setUuid(String uuid) { this.uuid = uuid; } } + + public static class Location { + @JsonProperty("volume") + private LocationVolume volume = null; + + public LocationVolume getVolume() { + return volume; + } + + public void setVolume(LocationVolume volume) { + this.volume = volume; + } + } + + public static class LocationVolume { + @JsonProperty("name") + private String name = null; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunRestoreRequest.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunRestoreRequest.java deleted file mode 100644 index c645e4a5a16f..000000000000 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/LunRestoreRequest.java +++ /dev/null @@ -1,132 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.cloudstack.storage.feign.model; - -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonProperty; - -/** - * Request body for the ONTAP LUN Restore API. - * - *

ONTAP REST endpoint: - * {@code POST /api/storage/luns/{lun.uuid}/restore}

- * - *

This API restores a LUN from a FlexVolume snapshot to a specified - * destination path. Unlike file restore, this is LUN-specific.

- * - *

Example payload: - * 

- * {
- *   "snapshot": {
- *     "name": "snapshot_name"
- *   },
- *   "destination": {
- *     "path": "/vol/volume_name/lun_name"
- *   }
- * }
- *
- *

- */ -@JsonIgnoreProperties(ignoreUnknown = true) -@JsonInclude(JsonInclude.Include.NON_NULL) -public class LunRestoreRequest { - - @JsonProperty("snapshot") - private SnapshotRef snapshot; - - @JsonProperty("destination") - private Destination destination; - - public LunRestoreRequest() { - } - - public LunRestoreRequest(String snapshotName, String destinationPath) { - this.snapshot = new SnapshotRef(snapshotName); - this.destination = new Destination(destinationPath); - } - - public SnapshotRef getSnapshot() { - return snapshot; - } - - public void setSnapshot(SnapshotRef snapshot) { - this.snapshot = snapshot; - } - - public Destination getDestination() { - return destination; - } - - public void setDestination(Destination destination) { - this.destination = destination; - } - - /** - * Nested class for snapshot reference. - */ - @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; - } - } - - /** - * Nested class for destination path. - */ - @JsonIgnoreProperties(ignoreUnknown = true) - @JsonInclude(JsonInclude.Include.NON_NULL) - public static class Destination { - - @JsonProperty("path") - private String path; - - public Destination() { - } - - public Destination(String path) { - this.path = path; - } - - public String getPath() { - return path; - } - - public void setPath(String path) { - this.path = path; - } - } -} 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 bd808a26d6f8..9ade06eed3b7 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 @@ -630,6 +630,10 @@ public NASFeignClient getNasFeignClient() { return nasFeignClient; } + public SANFeignClient getSanFeignClient() { + return sanFeignClient; + } + /** * Generates the Basic-auth header for ONTAP REST calls. */ 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 477e92630387..c26870d9f024 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 @@ -35,6 +35,7 @@ import org.apache.cloudstack.storage.feign.model.ExportPolicy; import org.apache.cloudstack.storage.feign.model.ExportRule; import org.apache.cloudstack.storage.feign.model.FileInfo; +import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.Nas; import org.apache.cloudstack.storage.feign.model.OntapStorage; @@ -42,7 +43,6 @@ import org.apache.cloudstack.storage.feign.model.Volume; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; -import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.volume.VolumeObject; @@ -454,32 +454,31 @@ private FileInfo getFile(String volumeUuid, String filePath) { public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName) { - logger.info("revertSnapshotForCloudStackVolume [NFS]: Restoring file [{}] from snapshot [{}] on FlexVol [{}]", + logger.info("revertSnapshotForCloudStackVolume [NFS]: Reverting file [{}] using clone [{}] on FlexVol [{}]", volumePath, snapshotName, flexVolName); if (snapshotName == null || snapshotName.isEmpty()) { - throw new CloudRuntimeException("Snapshot name is required for NFS snapshot revert"); + throw new CloudRuntimeException("Clone name is required for NFS snapshot revert"); } if (volumePath == null || volumePath.isEmpty()) { throw new CloudRuntimeException("File path is required for NFS snapshot revert"); } - if (flexVolName == null || flexVolName.isEmpty()) { - throw new CloudRuntimeException("FlexVolume name is required for NFS snapshot revert"); + if (flexVolUuid == null || flexVolUuid.isEmpty()) { + throw new CloudRuntimeException("FlexVolume UUID is required for NFS snapshot revert"); } String authHeader = getAuthHeader(); - String svmName = storage.getSvmName(); - - // Prepare the file path for ONTAP CLI API (ensure it starts with "/") - String ontapFilePath = volumePath.startsWith("/") ? volumePath : "/" + volumePath; - - // Create CLI snapshot restore request - CliSnapshotRestoreRequest restoreRequest = new CliSnapshotRestoreRequest( - svmName, flexVolName, snapshotName, ontapFilePath); - - logger.info("revertSnapshotForCloudStackVolume: Calling CLI file restore API with vserver={}, volume={}, snapshot={}, path={}", - svmName, flexVolName, snapshotName, ontapFilePath); - - return getSnapshotFeignClient().restoreFileFromSnapshotCli(authHeader, restoreRequest); + FileCloneRequest fileCloneRequest = new FileCloneRequest(); + FileCloneRequest.VolumeRef volumeRef = new FileCloneRequest.VolumeRef(); + volumeRef.setUuid(flexVolUuid); + volumeRef.setName(flexVolName); + fileCloneRequest.setVolume(volumeRef); + fileCloneRequest.setSourcePath(snapshotName); + fileCloneRequest.setDestinationPath(volumePath); + fileCloneRequest.setIsOverride(Boolean.TRUE); + + logger.debug("revertSnapshotForCloudStackVolume [NFS]: file clone source={} destination={} isOverride=true", + snapshotName, volumePath); + return getNasFeignClient().cloneFile(authHeader, fileCloneRequest); } } 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 5f1ac265fc50..b80913c4759e 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -29,7 +29,6 @@ import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunMap; -import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; @@ -563,32 +562,39 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName) { - logger.trace("revertSnapshotForCloudStackVolume [iSCSI]: Restoring LUN [{}] from snapshot [{}] on FlexVol [{}]", + logger.trace("revertSnapshotForCloudStackVolume [iSCSI]: Reverting LUN [{}] from clone [{}] on FlexVol [{}]", volumePath, snapshotName, flexVolName); - if (snapshotName == null || snapshotName.isEmpty()) { - throw new CloudRuntimeException("Snapshot name is required for iSCSI snapshot revert"); + if (volumePath == null || volumePath.isEmpty()) { + throw new CloudRuntimeException("Destination LUN name is required for iSCSI snapshot revert"); } if (flexVolName == null || flexVolName.isEmpty()) { throw new CloudRuntimeException("FlexVolume name is required for iSCSI snapshot revert"); } - if (volumePath == null || volumePath.isEmpty()) { - throw new CloudRuntimeException("LUN path is required for iSCSI snapshot revert"); + if (lunUuid == null || lunUuid.isEmpty()) { + throw new CloudRuntimeException("Source clone LUN UUID is required for iSCSI snapshot revert"); } String authHeader = getAuthHeader(); - String svmName = storage.getSvmName(); - - // Prepare the LUN path for ONTAP CLI API (ensure it starts with "/") - String ontapLunPath = volumePath.startsWith("/") ? volumePath : "/" + volumePath; - - // Create CLI snapshot restore request - CliSnapshotRestoreRequest restoreRequest = new CliSnapshotRestoreRequest( - svmName, flexVolName, snapshotName, ontapLunPath); - - logger.trace("revertSnapshotForCloudStackVolume: Calling CLI file restore API with vserver={}, volume={}, snapshot={}, path={}", - svmName, flexVolName, snapshotName, ontapLunPath); - - return getSnapshotFeignClient().restoreFileFromSnapshotCli(authHeader, restoreRequest); + Lun revertCloneRequest = new Lun(); + revertCloneRequest.setName(volumePath); + Svm svm = new Svm(); + svm.setName(storage.getSvmName()); + revertCloneRequest.setSvm(svm); + Lun.Location location = new Lun.Location(); + Lun.LocationVolume locationVolume = new Lun.LocationVolume(); + locationVolume.setName(flexVolName); + location.setVolume(locationVolume); + revertCloneRequest.setLocation(location); + Lun.Clone clone = new Lun.Clone(); + Lun.Source source = new Lun.Source(); + source.setUuid(lunUuid); + clone.setSource(source); + revertCloneRequest.setClone(clone); + revertCloneRequest.setIsOverride(Boolean.TRUE); + + logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: lun clone sourceUuid={} destinationLun={} isOverride=true", + lunUuid, volumePath); + return sanFeignClient.cloneLun(authHeader, revertCloneRequest); } } 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 d0ea1783aa1d..f6b5f25a0e4f 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 @@ -100,11 +100,13 @@ public class OntapStorageConstants { public static final String BASE_ONTAP_FV_ID = "base_ontap_fv_id"; public static final String ONTAP_SNAP_ID = "ontap_snap_id"; public static final String ONTAP_SNAP_NAME = "ontap_snap_name"; + public static final String ONTAP_CLONE_ID = "ontap_clone_id"; + public static final String ONTAP_CLONE_NAME = "ontap_clone_name"; 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"; public static final String FILE_PATH = "file_path"; - public static final int MAX_SNAPSHOT_NAME_LENGTH = 64; + public static final int MAX_SNAPSHOT_NAME_LENGTH = 256; /** vm_snapshot_details key for ONTAP FlexVolume-level VM snapshots. */ public static final String ONTAP_FLEXVOL_SNAPSHOT = "ontapFlexVolSnapshot"; 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 596372edcf16..8ff931507588 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 @@ -154,4 +154,25 @@ public static String getLunName(String volName, String lunName) { return OntapStorageConstants.VOLUME_PATH_PREFIX + volName + OntapStorageConstants.SLASH + lunName; } + /** + * Uses CloudStack UI snapshot name as the preferred ONTAP clone name. + * If needed, normalizes just enough to satisfy ONTAP naming limits. + */ + public static String getOntapCloneName(String snapshotName) { + if (snapshotName == null || snapshotName.trim().isEmpty()) { + throw new InvalidParameterValueException("Snapshot name cannot be null or empty"); + } + String candidate = snapshotName.trim().replaceAll("[^a-zA-Z0-9_]", "_"); + if (!Character.isLetter(candidate.charAt(0))) { + candidate = "s_" + candidate; + } + if (candidate.length() > OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH) { + candidate = candidate.substring(0, OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH); + } + if (!isValidName(candidate)) { + throw new InvalidParameterValueException("Invalid ONTAP clone name derived from snapshot name: " + snapshotName); + } + return candidate; + } + } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index a71df4c2e349..353f2a24aa8f 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -33,8 +33,6 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; -import org.apache.cloudstack.storage.feign.model.CliSnapshotRestoreRequest; -import org.apache.cloudstack.storage.feign.model.FlexVolSnapshot; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.StorageStrategy; @@ -375,7 +373,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { userVm.getInstanceName(), quiesceVm, vmIsRunning); } - // ── Step 2: Create FlexVolume-level snapshots ── + // ── Step 2: Create clone-backed VM snapshot entries ── try { String snapshotNameBase = buildSnapshotName(vmSnapshot); @@ -386,43 +384,66 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { // Build storage strategy from pool details to get the feign client StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(groupInfo.poolDetails); - SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient(); String authHeader = storageStrategy.getAuthHeader(); - - // Use the same snapshot name for all FlexVolumes in this VM snapshot - // (each FlexVolume gets its own independent snapshot with this name) - FlexVolSnapshot snapshotRequest = new FlexVolSnapshot(snapshotNameBase, - "CloudStack VM snapshot " + vmSnapshot.getName() + " for VM " + userVm.getInstanceName()); - - logger.info("takeVMSnapshot: Creating ONTAP FlexVolume snapshot [{}] on FlexVol UUID [{}] covering {} volume(s)", - snapshotNameBase, flexVolUuid, groupInfo.volumeIds.size()); - - JobResponse jobResponse = snapshotClient.createSnapshot(authHeader, flexVolUuid, snapshotRequest); - if (jobResponse == null || jobResponse.getJob() == null) { - throw new CloudRuntimeException("Failed to initiate FlexVolume snapshot on FlexVol UUID [" + flexVolUuid + "]"); - } - - // Poll for job completion - Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); - if (!jobSucceeded) { - throw new CloudRuntimeException("FlexVolume snapshot job failed on FlexVol UUID [" + flexVolUuid + "]"); - } - - // Retrieve the created snapshot UUID by name - String snapshotUuid = resolveSnapshotUuid(snapshotClient, authHeader, flexVolUuid, snapshotNameBase); - String protocol = groupInfo.poolDetails.get(OntapStorageConstants.PROTOCOL); - // Create one detail per CloudStack volume in this FlexVol group (for single-file restore during revert) + // Create one clone per CloudStack volume and persist detail for protocol-specific revert. for (Long volumeId : groupInfo.volumeIds) { String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails); + String cloneName = snapshotNameBase; + String cloneUuid = cloneName; + JobResponse jobResponse; + if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { + org.apache.cloudstack.storage.feign.model.FileCloneRequest cloneRequest = new org.apache.cloudstack.storage.feign.model.FileCloneRequest(); + org.apache.cloudstack.storage.feign.model.FileCloneRequest.VolumeRef volumeRef = new org.apache.cloudstack.storage.feign.model.FileCloneRequest.VolumeRef(); + volumeRef.setUuid(flexVolUuid); + volumeRef.setName(groupInfo.poolDetails.get(OntapStorageConstants.VOLUME_NAME)); + cloneRequest.setVolume(volumeRef); + cloneRequest.setSourcePath(volumePath); + cloneRequest.setDestinationPath(cloneName); + cloneRequest.setIsOverride(Boolean.FALSE); + jobResponse = storageStrategy.getNasFeignClient().cloneFile(authHeader, cloneRequest); + } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { + VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.LUN_DOT_UUID); + String sourceLunUuid = lunDetail != null ? lunDetail.getValue() : null; + if (sourceLunUuid == null || sourceLunUuid.isEmpty()) { + throw new CloudRuntimeException("Source LUN UUID missing for volume " + volumeId); + } + org.apache.cloudstack.storage.feign.model.Lun cloneRequest = new org.apache.cloudstack.storage.feign.model.Lun(); + cloneRequest.setName(cloneName); + org.apache.cloudstack.storage.feign.model.Svm svm = new org.apache.cloudstack.storage.feign.model.Svm(); + svm.setName(groupInfo.poolDetails.get(OntapStorageConstants.SVM_NAME)); + cloneRequest.setSvm(svm); + org.apache.cloudstack.storage.feign.model.Lun.Location location = new org.apache.cloudstack.storage.feign.model.Lun.Location(); + org.apache.cloudstack.storage.feign.model.Lun.LocationVolume locationVolume = new org.apache.cloudstack.storage.feign.model.Lun.LocationVolume(); + locationVolume.setName(groupInfo.poolDetails.get(OntapStorageConstants.VOLUME_NAME)); + location.setVolume(locationVolume); + cloneRequest.setLocation(location); + org.apache.cloudstack.storage.feign.model.Lun.Clone clone = new org.apache.cloudstack.storage.feign.model.Lun.Clone(); + org.apache.cloudstack.storage.feign.model.Lun.Source source = new org.apache.cloudstack.storage.feign.model.Lun.Source(); + source.setUuid(sourceLunUuid); + clone.setSource(source); + cloneRequest.setClone(clone); + cloneRequest.setIsOverride(Boolean.FALSE); + jobResponse = storageStrategy.getSanFeignClient().cloneLun(authHeader, cloneRequest); + cloneUuid = resolveLunUuid(storageStrategy, authHeader, groupInfo.poolDetails.get(OntapStorageConstants.SVM_NAME), cloneName); + } else { + throw new CloudRuntimeException("Unsupported protocol for VM snapshot clone: " + protocol); + } + if (jobResponse == null || jobResponse.getJob() == null) { + throw new CloudRuntimeException("Failed to submit clone-backed VM snapshot for volume " + volumeId); + } + Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); + if (!jobSucceeded) { + throw new CloudRuntimeException("Clone-backed VM snapshot job failed for volume " + volumeId); + } FlexVolSnapshotDetail detail = new FlexVolSnapshotDetail( - flexVolUuid, snapshotUuid, snapshotNameBase, volumePath, groupInfo.poolId, protocol); + flexVolUuid, cloneUuid, cloneName, volumePath, groupInfo.poolId, protocol); createdSnapshots.add(detail); } - logger.info("takeVMSnapshot: ONTAP FlexVolume snapshot [{}] (uuid={}) on FlexVol [{}] completed in {} ms. Covers volumes: {}", - snapshotNameBase, snapshotUuid, flexVolUuid, + logger.info("takeVMSnapshot: Clone-backed VM snapshot [{}] on FlexVol [{}] completed in {} ms. Covers volumes: {}", + snapshotNameBase, flexVolUuid, TimeUnit.MILLISECONDS.convert(System.nanoTime() - startSnapshot, TimeUnit.NANOSECONDS), groupInfo.volumeIds); } @@ -672,25 +693,14 @@ Map groupVolumesByFlexVol(List volumeT * Format: {@code vmsnap__} */ String buildSnapshotName(VMSnapshot vmSnapshot) { - String name = "vmsnap_" + vmSnapshot.getId() + "_" + System.currentTimeMillis(); - // ONTAP snapshot names: max 256 chars, must start with letter, only alphanumeric and underscores - if (name.length() > OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH) { - name = name.substring(0, OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH); - } - return name; + return OntapStorageUtils.getOntapCloneName(vmSnapshot.getName()); } - /** - * Resolves the UUID of a newly created FlexVolume snapshot by name. - */ - String resolveSnapshotUuid(SnapshotFeignClient client, String authHeader, - String flexVolUuid, String snapshotName) { - Map queryParams = new HashMap<>(); - queryParams.put("name", snapshotName); - OntapResponse response = client.getSnapshots(authHeader, flexVolUuid, queryParams); + String resolveLunUuid(StorageStrategy strategy, String authHeader, String svmName, String lunName) { + OntapResponse response = strategy.getSanFeignClient() + .getLunResponse(authHeader, Map.of(OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.NAME, lunName)); if (response == null || response.getRecords() == null || response.getRecords().isEmpty()) { - throw new CloudRuntimeException("Could not find FlexVolume snapshot [" + snapshotName + - "] on FlexVol [" + flexVolUuid + "] after creation"); + throw new CloudRuntimeException("Could not resolve LUN UUID for clone " + lunName); } return response.getRecords().get(0).getUuid(); } @@ -819,43 +829,28 @@ void revertFlexVolSnapshots(List flexVolDetails) { Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient(); - String authHeader = storageStrategy.getAuthHeader(); - - // Get SVM name and FlexVolume name from pool details - String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); String flexVolName = poolDetails.get(OntapStorageConstants.VOLUME_NAME); - - if (svmName == null || svmName.isEmpty()) { - throw new CloudRuntimeException("SVM name not found in pool details for pool [" + detail.poolId + "]"); - } if (flexVolName == null || flexVolName.isEmpty()) { throw new CloudRuntimeException("FlexVolume name not found in pool details for pool [" + detail.poolId + "]"); } - // The path must start with "/" for the ONTAP CLI API - String ontapFilePath = detail.volumePath.startsWith("/") ? detail.volumePath : "/" + detail.volumePath; - logger.info("revertFlexVolSnapshots: Restoring volume [{}] from FlexVol snapshot [{}] on FlexVol [{}] (protocol={})", - ontapFilePath, detail.snapshotName, flexVolName, detail.protocol); - - // Use CLI-based restore API: POST /api/private/cli/volume/snapshot/restore-file - CliSnapshotRestoreRequest restoreRequest = new CliSnapshotRestoreRequest( - svmName, flexVolName, detail.snapshotName, ontapFilePath); - - JobResponse jobResponse = snapshotClient.restoreFileFromSnapshotCli(authHeader, restoreRequest); + detail.volumePath, detail.snapshotName, flexVolName, detail.protocol); + String lunUuid = ProtocolType.ISCSI.name().equalsIgnoreCase(detail.protocol) ? detail.snapshotUuid : null; + JobResponse jobResponse = storageStrategy.revertSnapshotForCloudStackVolume( + detail.snapshotName, detail.flexVolUuid, detail.snapshotUuid, detail.volumePath, lunUuid, flexVolName); if (jobResponse != null && jobResponse.getJob() != null) { Boolean success = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 60, 2000); if (!success) { throw new CloudRuntimeException("Snapshot file restore failed for volume path [" + - ontapFilePath + "] from snapshot [" + detail.snapshotName + + detail.volumePath + "] from snapshot [" + detail.snapshotName + "] on FlexVol [" + flexVolName + "]"); } } logger.info("revertFlexVolSnapshots: Successfully restored volume [{}] from snapshot [{}] on FlexVol [{}]", - ontapFilePath, detail.snapshotName, flexVolName); + detail.volumePath, detail.snapshotName, flexVolName); } } 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 b535217fd235..17c0896a2e9d 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java @@ -25,23 +25,31 @@ import com.cloud.storage.Storage; import com.cloud.storage.VolumeVO; import com.cloud.storage.VolumeDetailVO; +import com.cloud.storage.dao.SnapshotDetailsDao; +import com.cloud.storage.dao.SnapshotDetailsVO; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.command.CommandResult; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.feign.client.NASFeignClient; import org.apache.cloudstack.storage.feign.model.Igroup; +import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.Lun; +import org.apache.cloudstack.storage.feign.model.response.JobResponse; +import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; import org.apache.cloudstack.storage.service.model.ProtocolType; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; import org.junit.jupiter.api.BeforeEach; @@ -71,6 +79,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -89,6 +98,9 @@ class OntapPrimaryDatastoreDriverTest { @Mock private VolumeDetailsDao volumeDetailsDao; + @Mock + private SnapshotDetailsDao snapshotDetailsDao; + @Mock private DataStore dataStore; @@ -107,6 +119,15 @@ class OntapPrimaryDatastoreDriverTest { @Mock private UnifiedSANStrategy sanStrategy; + @Mock + private StorageStrategy storageStrategy; + + @Mock + private NASFeignClient nasFeignClient; + + @Mock + private SnapshotInfo snapshotInfo; + @Mock private AsyncCompletionCallback createCallback; @@ -564,4 +585,91 @@ void testCanProvideStorageStats_ReturnsFalse() { void testCanProvideVolumeStats_ReturnsFalse() { assertFalse(driver.canProvideVolumeStats()); } + + @Test + void testTakeSnapshot_NfsCloneSuccess() { + storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name()); + storagePoolDetails.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); + storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + storagePoolDetails.put(OntapStorageConstants.SVM_NAME, "svm1"); + storagePoolDetails.put(OntapStorageConstants.USERNAME, "admin"); + storagePoolDetails.put(OntapStorageConstants.PASSWORD, "pass"); + storagePoolDetails.put(OntapStorageConstants.STORAGE_IP, "10.0.0.1"); + storagePoolDetails.put(OntapStorageConstants.SIZE, "1024"); + + when(snapshotInfo.getId()).thenReturn(500L); + when(snapshotInfo.getName()).thenReturn("UI Snapshot Name"); + when(snapshotInfo.getBaseVolume()).thenReturn(volumeInfo); + SnapshotObjectTO snapshotObjectTO = mock(SnapshotObjectTO.class); + when(snapshotInfo.getTO()).thenReturn(snapshotObjectTO); + when(volumeInfo.getId()).thenReturn(100L); + when(volumeVO.getId()).thenReturn(100L); + when(volumeVO.getPoolId()).thenReturn(1L); + when(volumeVO.getPath()).thenReturn("vol-100.qcow2"); + when(volumeDao.findById(100L)).thenReturn(volumeVO); + when(storagePoolDao.findById(1L)).thenReturn(storagePool); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(storageStrategy.getAuthHeader()).thenReturn("Basic auth"); + when(storageStrategy.getNasFeignClient()).thenReturn(nasFeignClient); + JobResponse jobResponse = new JobResponse(); + Job job = new Job(); + job.setUuid("job-uuid-1"); + jobResponse.setJob(job); + when(nasFeignClient.cloneFile(anyString(), any())).thenReturn(jobResponse); + when(storageStrategy.jobPollForSuccess("job-uuid-1", 30, 2000)).thenReturn(true); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(storageStrategy); + utilityMock.when(() -> OntapStorageUtils.getOntapCloneName("UI Snapshot Name")) + .thenReturn("UI_Snapshot_Name"); + + driver.takeSnapshot(snapshotInfo, createCallback); + + verify(nasFeignClient).cloneFile(anyString(), any()); + verify(snapshotDetailsDao, atLeastOnce()).persist(any(SnapshotDetailsVO.class)); + verify(createCallback).complete(any(CreateCmdResult.class)); + } + } + + @Test + void testRevertSnapshot_UsesCloneMetadata() { + when(snapshotInfo.getId()).thenReturn(500L); + when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.BASE_ONTAP_FV_ID)) + .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.BASE_ONTAP_FV_ID, "flexvol-uuid-1", false)); + when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.ONTAP_CLONE_ID)) + .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.ONTAP_CLONE_ID, "clone-lun-uuid-1", false)); + when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.ONTAP_SNAP_NAME)) + .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.ONTAP_SNAP_NAME, "UI Snapshot Name", false)); + when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.ONTAP_CLONE_NAME)) + .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.ONTAP_CLONE_NAME, "UI_Snapshot_Name", false)); + when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.VOLUME_PATH)) + .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.VOLUME_PATH, "dest-lun-1", false)); + when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.PRIMARY_POOL_ID)) + .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.PRIMARY_POOL_ID, "1", false)); + when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.PROTOCOL)) + .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.PROTOCOL, ProtocolType.ISCSI.name(), false)); + + storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + JobResponse jobResponse = new JobResponse(); + Job job = new Job(); + job.setUuid("job-uuid-2"); + jobResponse.setJob(job); + when(storageStrategy.revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString(), anyString())) + .thenReturn(jobResponse); + when(storageStrategy.jobPollForSuccess("job-uuid-2", 60, 2000)).thenReturn(true); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(storageStrategy); + + driver.revertSnapshot(snapshotInfo, snapshotInfo, commandCallback); + + verify(storageStrategy).revertSnapshotForCloudStackVolume( + eq("UI_Snapshot_Name"), eq("flexvol-uuid-1"), eq("clone-lun-uuid-1"), + eq("dest-lun-1"), eq("clone-lun-uuid-1"), eq("flexvol1")); + verify(commandCallback).complete(any(CommandResult.class)); + } + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java index c4d5ddf6878c..0e99ff68e942 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 @@ -37,6 +37,7 @@ import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.ExportPolicy; +import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.response.JobResponse; @@ -75,6 +76,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.argThat; @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) @@ -582,4 +584,33 @@ public void testDeleteCloudStackVolume_AnswerNull() throws Exception { strategy.deleteCloudStackVolume(cloudStackVolume); }); } + + @Test + public void testRevertSnapshotForCloudStackVolume_UsesFileCloneWithOverride() { + JobResponse jobResponse = new JobResponse(); + Job job = new Job(); + job.setUuid("job-uuid-1"); + jobResponse.setJob(job); + when(nasFeignClient.cloneFile(anyString(), any(FileCloneRequest.class))).thenReturn(jobResponse); + + JobResponse result = strategy.revertSnapshotForCloudStackVolume( + "clone-snap-1", "flexvol-uuid-1", "snap-uuid-1", "vm-disk.qcow2", null, "flexvol1"); + + assertNotNull(result); + verify(nasFeignClient).cloneFile(anyString(), argThat(req -> + req != null + && req.getIsOverride() != null + && req.getIsOverride() + && "clone-snap-1".equals(req.getSourcePath()) + && "vm-disk.qcow2".equals(req.getDestinationPath()) + && req.getVolume() != null + && "flexvol-uuid-1".equals(req.getVolume().getUuid()) + && "flexvol1".equals(req.getVolume().getName()))); + } + + @Test + public void testRevertSnapshotForCloudStackVolume_MissingFlexVolUuid_Throws() { + assertThrows(CloudRuntimeException.class, () -> strategy.revertSnapshotForCloudStackVolume( + "clone-snap-1", null, "snap-uuid-1", "vm-disk.qcow2", null, "flexvol1")); + } } 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 1c0c84ef91dd..78fe34cd3cb1 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 @@ -30,6 +30,7 @@ import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunMap; import org.apache.cloudstack.storage.feign.model.OntapStorage; +import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -59,6 +60,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; @@ -1802,4 +1804,40 @@ void testEnsureLunMapped_ExistingMapping_ReturnsExistingNumber() { verify(sanFeignClient, never()).createLunMap(any(), anyBoolean(), any(LunMap.class)); } } + + @Test + void testRevertSnapshotForCloudStackVolume_UsesLunCloneWithOverride() { + JobResponse jobResponse = new JobResponse(); + org.apache.cloudstack.storage.feign.model.Job job = new org.apache.cloudstack.storage.feign.model.Job(); + job.setUuid("job-uuid-1"); + jobResponse.setJob(job); + when(sanFeignClient.cloneLun(eq(authHeader), any(Lun.class))).thenReturn(jobResponse); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) + .thenReturn(authHeader); + + JobResponse result = unifiedSANStrategy.revertSnapshotForCloudStackVolume( + "clone-snap-1", "flexvol-uuid-1", "clone-lun-uuid-1", "dest-lun-1", "clone-lun-uuid-1", "flexvol1"); + + assertNotNull(result); + verify(sanFeignClient).cloneLun(eq(authHeader), argThat(lun -> + lun != null + && Boolean.TRUE.equals(lun.getIsOverride()) + && "dest-lun-1".equals(lun.getName()) + && lun.getClone() != null + && lun.getClone().getSource() != null + && "clone-lun-uuid-1".equals(lun.getClone().getSource().getUuid()) + && lun.getLocation() != null + && lun.getLocation().getVolume() != null + && "flexvol1".equals(lun.getLocation().getVolume().getName()) + )); + } + } + + @Test + void testRevertSnapshotForCloudStackVolume_MissingLunUuid_Throws() { + assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.revertSnapshotForCloudStackVolume( + "clone-snap-1", "flexvol-uuid-1", "clone-lun-uuid-1", "dest-lun-1", null, "flexvol1")); + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java index b069ab7246a0..cd095cccefcd 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java @@ -592,11 +592,11 @@ void testFlexVolSnapshotDetail_Parse5Parts_ThrowsException() { @Test void testBuildSnapshotName_Format() { VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); - when(vmSnapshot.getId()).thenReturn(SNAPSHOT_ID); + when(vmSnapshot.getName()).thenReturn("My VM Snapshot #1"); String name = strategy.buildSnapshotName(vmSnapshot); - assertEquals(true, name.startsWith("vmsnap_200_")); + assertEquals(true, name.startsWith("My_VM_Snapshot")); assertEquals(true, name.length() <= OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH); } From 1f449a4ba362c5d752adfc96fdc088f1adaad338 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Fri, 12 Jun 2026 19:07:41 +0530 Subject: [PATCH 02/24] CSTACKEX-198: Lun path needs to be in a pattern --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 8 ++++++-- .../storage/vmsnapshot/OntapVMSnapshotStrategy.java | 7 +++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index f7a6119ee50d..f537b945e0b2 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 @@ -694,8 +694,10 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback Date: Fri, 12 Jun 2026 19:48:04 +0530 Subject: [PATCH 03/24] CSTACKEX-198: update tostring method --- .../driver/OntapPrimaryDatastoreDriver.java | 60 +++++++++++++++---- .../cloudstack/storage/feign/model/Lun.java | 40 +++++++++++++ .../vmsnapshot/OntapVMSnapshotStrategy.java | 1 + 3 files changed, 91 insertions(+), 10 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index f537b945e0b2..1a3f4d4bb4ee 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 @@ -641,6 +641,13 @@ public long getUsedIops(StoragePool storagePool) { public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback callback) { logger.info("OntapPrimaryDatastoreDriver.takeSnapshot: Creating clone-backed snapshot for snapshot [{}]", snapshot.getId()); CreateCmdResult result; + StorageStrategy storageStrategy = null; + String authHeader = null; + String protocol = null; + String flexVolUuid = null; + String cloneName = null; + String cloneLunPath = null; + String svmName = null; try { VolumeInfo volumeInfo = snapshot.getBaseVolume(); @@ -657,19 +664,20 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(volumeVO.getPoolId()); - String protocol = poolDetails.get(OntapStorageConstants.PROTOCOL); - String flexVolUuid = poolDetails.get(OntapStorageConstants.VOLUME_UUID); + protocol = poolDetails.get(OntapStorageConstants.PROTOCOL); + flexVolUuid = poolDetails.get(OntapStorageConstants.VOLUME_UUID); + svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); if (flexVolUuid == null || flexVolUuid.isEmpty()) { throw new CloudRuntimeException("FlexVolume UUID not found in pool details for pool " + volumeVO.getPoolId()); } - StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - String authHeader = storageStrategy.getAuthHeader(); + storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); + authHeader = storageStrategy.getAuthHeader(); SnapshotObjectTO snapshotObjectTo = (SnapshotObjectTO) snapshot.getTO(); String cloudStackSnapshotName = snapshot.getName(); - String cloneName = OntapStorageUtils.getOntapCloneName(cloudStackSnapshotName); + cloneName = OntapStorageUtils.getOntapCloneName(cloudStackSnapshotName); String volumePath = resolveVolumePathOnOntap(volumeVO, protocol, poolDetails); String cloneId = null; String lunUuid = null; @@ -694,7 +702,7 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback Date: Mon, 15 Jun 2026 08:42:31 +0530 Subject: [PATCH 04/24] CSTACKEX-198: adding validations for LUN path --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 3 +++ .../storage/vmsnapshot/OntapVMSnapshotStrategy.java | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 1a3f4d4bb4ee..b4821e700831 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 @@ -704,6 +704,9 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback flexVolGroups = groupVolumesByFlexVol(volumeTOs); - logger.info("takeVMSnapshot: VM [{}] has {} volumes across {} unique FlexVolume(s)", - userVm.getInstanceName(), volumeTOs.size(), flexVolGroups.size()); // ── Step 1: Freeze the VM (only if quiescing is requested AND VM is running) ── if (shouldFreezeThaw) { @@ -411,6 +409,9 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { } String cloneLunPath = OntapStorageUtils.getLunName( groupInfo.poolDetails.get(OntapStorageConstants.VOLUME_NAME), cloneName); + if (!cloneLunPath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { + throw new CloudRuntimeException("Invalid iSCSI clone LUN path generated: " + cloneLunPath); + } org.apache.cloudstack.storage.feign.model.Lun cloneRequest = new org.apache.cloudstack.storage.feign.model.Lun(); cloneRequest.setName(cloneLunPath); org.apache.cloudstack.storage.feign.model.Svm svm = new org.apache.cloudstack.storage.feign.model.Svm(); From b89e9bf0b571bb95d054470f263398d91c46a038 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 09:40:55 +0530 Subject: [PATCH 05/24] CSTACKEX-198: add validtions for mandatory params --- .../driver/OntapPrimaryDatastoreDriver.java | 20 +++++++++++++++-- .../vmsnapshot/OntapVMSnapshotStrategy.java | 22 ++++++++++++++++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index b4821e700831..3fc358a0bdfb 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 @@ -702,23 +702,39 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback Date: Mon, 15 Jun 2026 10:15:20 +0530 Subject: [PATCH 06/24] CSTACKEX-198: exceptionhandling for the failed ones --- .../storage/vmsnapshot/OntapVMSnapshotStrategy.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index 2631951544b3..ce215b9a955b 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -443,7 +443,6 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { source.setUuid(sourceLunUuid); clone.setSource(source); cloneRequest.setClone(clone); - cloneRequest.setIsOverride(Boolean.FALSE); logger.info("CloneRequest: {}", cloneRequest); jobResponse = storageStrategy.getSanFeignClient().cloneLun(authHeader, cloneRequest); cloneUuid = resolveLunUuid(storageStrategy, authHeader, @@ -518,7 +517,11 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { } catch (AgentUnavailableException e) { logger.error("takeVMSnapshot: ONTAP VM Snapshot [{}] failed, agent unavailable: {}", vmSnapshot.getName(), e.getMessage()); throw new CloudRuntimeException("Creating Instance Snapshot: " + vmSnapshot.getName() + " failed: " + e.getMessage()); - } finally { + } catch (Exception e) { + logger.error("takeVMSnapshot: ONTAP VM Snapshot [{}] failed, with exception: {}", vmSnapshot.getName(), e.getMessage()); + throw new CloudRuntimeException("Creating Instance Snapshot: " + vmSnapshot.getName() + " failed: " + e.getMessage()); + } + finally { if (!result) { // Rollback all FlexVolume snapshots created so far (deduplicate by FlexVol+Snapshot) Map rolledBack = new HashMap<>(); From 236c966ab1ad60fcc9043eb9c05bf5f22275dd15 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 11:37:18 +0530 Subject: [PATCH 07/24] CSTACKEX-198: reusing createLun feign instead adding new for cloneLun --- .../driver/OntapPrimaryDatastoreDriver.java | 38 +++++++++---------- .../vmsnapshot/OntapVMSnapshotStrategy.java | 29 ++++++++------ 2 files changed, 36 insertions(+), 31 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 3fc358a0bdfb..836230ee43fd 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 @@ -681,7 +681,7 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback createCloneResponse = storageStrategy.getSanFeignClient().createLun(authHeader, true, cloneRequest); + if (createCloneResponse == null || createCloneResponse.getRecords() == null || createCloneResponse.getRecords().isEmpty()) { + throw new CloudRuntimeException("Failed to create iSCSI clone LUN for volume " + volumeVO.getId()); + } + cloneId = createCloneResponse.getRecords().get(0).getUuid(); + if (cloneId == null || cloneId.isEmpty()) { + cloneId = resolveLunUuidByName(storageStrategy, authHeader, svmNameForClone, cloneLunPath); + } } else { throw new CloudRuntimeException("Unsupported protocol for snapshot clone: " + protocol); } - if (jobResponse == null || jobResponse.getJob() == null) { - throw new CloudRuntimeException("Failed to initiate clone-backed snapshot for volume " + volumeVO.getId()); - } - - // Poll for job completion - Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); - if (!jobSucceeded) { - throw new CloudRuntimeException("Clone create job failed for snapshot " + cloudStackSnapshotName); - } - - if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { - cloneLunPath = OntapStorageUtils.getLunName( - poolDetails.get(OntapStorageConstants.VOLUME_NAME), cloneName); - cloneId = resolveLunUuidByName(storageStrategy, authHeader, - poolDetails.get(OntapStorageConstants.SVM_NAME), cloneLunPath); + if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { + if (nfsJobResponse == null || nfsJobResponse.getJob() == null) { + throw new CloudRuntimeException("Failed to initiate clone-backed snapshot for volume " + volumeVO.getId()); + } + // Poll for async NFS clone completion + Boolean jobSucceeded = storageStrategy.jobPollForSuccess(nfsJobResponse.getJob().getUuid(), 30, 2000); + if (!jobSucceeded) { + throw new CloudRuntimeException("Clone create job failed for snapshot " + cloudStackSnapshotName); + } } snapshotObjectTo.setPath(OntapStorageConstants.ONTAP_CLONE_NAME + "=" + cloneName); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index ce215b9a955b..32e16798dd82 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -33,6 +33,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; +import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.StorageStrategy; @@ -390,7 +391,6 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails); String cloneName = snapshotNameBase; String cloneUuid = cloneName; - JobResponse jobResponse; if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { org.apache.cloudstack.storage.feign.model.FileCloneRequest cloneRequest = new org.apache.cloudstack.storage.feign.model.FileCloneRequest(); org.apache.cloudstack.storage.feign.model.FileCloneRequest.VolumeRef volumeRef = new org.apache.cloudstack.storage.feign.model.FileCloneRequest.VolumeRef(); @@ -400,7 +400,14 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { cloneRequest.setSourcePath(volumePath); cloneRequest.setDestinationPath(cloneName); cloneRequest.setIsOverride(Boolean.FALSE); - jobResponse = storageStrategy.getNasFeignClient().cloneFile(authHeader, cloneRequest); + JobResponse fileJobResponse = storageStrategy.getNasFeignClient().cloneFile(authHeader, cloneRequest); + if (fileJobResponse == null || fileJobResponse.getJob() == null) { + throw new CloudRuntimeException("Failed to submit clone-backed VM snapshot for volume " + volumeId); + } + Boolean jobSucceeded = storageStrategy.jobPollForSuccess(fileJobResponse.getJob().getUuid(), 30, 2000); + if (!jobSucceeded) { + throw new CloudRuntimeException("Clone-backed VM snapshot job failed for volume " + volumeId); + } } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.LUN_DOT_UUID); String sourceLunUuid = lunDetail != null ? lunDetail.getValue() : null; @@ -444,19 +451,17 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { clone.setSource(source); cloneRequest.setClone(clone); logger.info("CloneRequest: {}", cloneRequest); - jobResponse = storageStrategy.getSanFeignClient().cloneLun(authHeader, cloneRequest); - cloneUuid = resolveLunUuid(storageStrategy, authHeader, - svmName, cloneLunPath); + OntapResponse createCloneResponse = storageStrategy.getSanFeignClient().createLun(authHeader, true, cloneRequest); + if (createCloneResponse == null || createCloneResponse.getRecords() == null || createCloneResponse.getRecords().isEmpty()) { + throw new CloudRuntimeException("Failed to create iSCSI clone LUN for volume " + volumeId); + } + cloneUuid = createCloneResponse.getRecords().get(0).getUuid(); + if (cloneUuid == null || cloneUuid.isEmpty()) { + cloneUuid = resolveLunUuid(storageStrategy, authHeader, svmName, cloneLunPath); + } } else { throw new CloudRuntimeException("Unsupported protocol for VM snapshot clone: " + protocol); } - if (jobResponse == null || jobResponse.getJob() == null) { - throw new CloudRuntimeException("Failed to submit clone-backed VM snapshot for volume " + volumeId); - } - Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); - if (!jobSucceeded) { - throw new CloudRuntimeException("Clone-backed VM snapshot job failed for volume " + volumeId); - } FlexVolSnapshotDetail detail = new FlexVolSnapshotDetail( flexVolUuid, cloneUuid, cloneName, volumePath, groupInfo.poolId, protocol); createdSnapshots.add(detail); From 5f9143cb4f4c17612634cb33dbd99df38ce0236f Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 12:15:35 +0530 Subject: [PATCH 08/24] CSTACKEX-198: keeping ONTAP clone object name unique with pattern as <"Snapshot_base_name_from_CS"+"snapshot_id"+"CS-Volume_id"> --- .../storage/vmsnapshot/OntapVMSnapshotStrategy.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index 32e16798dd82..eb4c285013ed 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -389,7 +389,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { // Create one clone per CloudStack volume and persist detail for protocol-specific revert. for (Long volumeId : groupInfo.volumeIds) { String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails); - String cloneName = snapshotNameBase; + String cloneName = buildPerVolumeCloneName(snapshotNameBase, vmSnapshot.getId(), volumeId); String cloneUuid = cloneName; if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { org.apache.cloudstack.storage.feign.model.FileCloneRequest cloneRequest = new org.apache.cloudstack.storage.feign.model.FileCloneRequest(); @@ -725,6 +725,14 @@ String buildSnapshotName(VMSnapshot vmSnapshot) { return OntapStorageUtils.getOntapCloneName(vmSnapshot.getName()); } + /** + * Builds a deterministic per-volume clone name for VM snapshot workflows. + * Keeps VM snapshot name as base while preventing collisions across ROOT/DATA volumes. + */ + String buildPerVolumeCloneName(String snapshotNameBase, Long vmSnapshotId, Long volumeId) { + return OntapStorageUtils.getOntapCloneName(snapshotNameBase + "_s" + vmSnapshotId + "_v" + volumeId); + } + String resolveLunUuid(StorageStrategy strategy, String authHeader, String svmName, String lunName) { OntapResponse response = strategy.getSanFeignClient() .getLunResponse(authHeader, Map.of(OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.NAME, lunName)); From 360cfa4118ed6137f064a5ce634f132eae42c28b Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 13:15:09 +0530 Subject: [PATCH 09/24] CSTACKEX-198: updated delete and get code with clone backed approach --- .../driver/OntapPrimaryDatastoreDriver.java | 83 +++++++------- .../feign/client/SnapshotFeignClient.java | 108 ------------------ .../storage/service/StorageStrategy.java | 12 -- .../vmsnapshot/OntapVMSnapshotStrategy.java | 66 ++++++++--- 4 files changed, 90 insertions(+), 179 deletions(-) delete mode 100644 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 836230ee43fd..ae3d0b46c7b5 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 @@ -54,7 +54,6 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.Svm; @@ -237,7 +236,7 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac commandResult.setResult(null); commandResult.setSuccess(true); } else if (data.getType() == DataObjectType.SNAPSHOT) { - // Delete the ONTAP FlexVolume snapshot that was created by takeSnapshot + // Delete the clone object (file/LUN) that was created by takeSnapshot deleteOntapSnapshot((SnapshotInfo) data, commandResult); } else { throw new CloudRuntimeException("Unsupported data object type: " + data.getType()); @@ -252,30 +251,23 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac } /** - * Deletes an ONTAP FlexVolume snapshot. - * - *

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

- * - * @param snapshotInfo The CloudStack snapshot to delete - * @param commandResult Result object to populate with success/failure + * Deletes a clone-backed ONTAP snapshot object (NFS file clone or iSCSI LUN clone). */ private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult commandResult) { long snapshotId = snapshotInfo.getId(); - logger.info("deleteOntapSnapshot: Deleting ONTAP FlexVolume snapshot for CloudStack snapshot [{}]", snapshotId); + logger.info("deleteOntapSnapshot: Deleting clone-backed ONTAP snapshot object for CloudStack snapshot [{}]", snapshotId); try { - // Retrieve snapshot details stored during takeSnapshot String flexVolUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.BASE_ONTAP_FV_ID); - String ontapSnapshotUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_ID); - String snapshotName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_NAME); + String cloneUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_CLONE_ID); + String cloneName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_CLONE_NAME); String poolIdStr = getSnapshotDetail(snapshotId, OntapStorageConstants.PRIMARY_POOL_ID); + String protocol = getSnapshotDetail(snapshotId, OntapStorageConstants.PROTOCOL); - if (flexVolUuid == null || ontapSnapshotUuid == null) { - logger.warn("deleteOntapSnapshot: Missing ONTAP snapshot details for snapshot [{}]. " + - "flexVolUuid={}, ontapSnapshotUuid={}. Snapshot may have been created by a different method or already deleted.", - snapshotId, flexVolUuid, ontapSnapshotUuid); - // Consider this a success since there's nothing to delete on ONTAP + if (poolIdStr == null || protocol == null || cloneName == null) { + logger.warn("deleteOntapSnapshot: Missing clone metadata for snapshot [{}]. " + + "poolId={}, protocol={}, cloneName={}. Treating as success.", + snapshotId, poolIdStr, protocol, cloneName); commandResult.setSuccess(true); commandResult.setResult(null); return; @@ -285,26 +277,31 @@ private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult comman Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(poolId); StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient(); String authHeader = storageStrategy.getAuthHeader(); + String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); - logger.info("deleteOntapSnapshot: Deleting ONTAP snapshot [{}] (uuid={}) from FlexVol [{}]", - snapshotName, ontapSnapshotUuid, flexVolUuid); - - // Call ONTAP REST API to delete the snapshot - JobResponse jobResponse = snapshotClient.deleteSnapshot(authHeader, flexVolUuid, ontapSnapshotUuid); - - if (jobResponse != null && jobResponse.getJob() != null) { - // Poll for job completion - Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); - if (!jobSucceeded) { - throw new CloudRuntimeException("Delete job failed for snapshot [" + - snapshotName + "] on FlexVol [" + flexVolUuid + "]"); + if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { + if (flexVolUuid == null || flexVolUuid.isEmpty()) { + logger.warn("deleteOntapSnapshot: Missing FlexVol UUID for NFS clone delete on snapshot [{}]. Treating as success.", snapshotId); + commandResult.setSuccess(true); + commandResult.setResult(null); + return; + } + logger.info("deleteOntapSnapshot: Deleting NFS clone file [{}] on FlexVol [{}]", cloneName, flexVolUuid); + storageStrategy.getNasFeignClient().deleteFile(authHeader, flexVolUuid, cloneName); + } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { + if (cloneUuid == null || cloneUuid.isEmpty()) { + String cloneLunPath = OntapStorageUtils.getLunName(poolDetails.get(OntapStorageConstants.VOLUME_NAME), cloneName); + cloneUuid = resolveLunUuidByName(storageStrategy, authHeader, svmName, cloneLunPath); } + logger.info("deleteOntapSnapshot: Deleting iSCSI clone LUN [{}] (uuid={})", cloneName, cloneUuid); + storageStrategy.getSanFeignClient().deleteLun(authHeader, cloneUuid, Map.of("allow_delete_while_mapped", "true")); + } else { + throw new CloudRuntimeException("Unsupported protocol for snapshot delete: " + protocol); } - logger.info("deleteOntapSnapshot: Successfully deleted ONTAP snapshot [{}] (uuid={}) for CloudStack snapshot [{}]", - snapshotName, ontapSnapshotUuid, snapshotId); + logger.info("deleteOntapSnapshot: Successfully deleted clone object [{}] for CloudStack snapshot [{}]", + cloneName, snapshotId); commandResult.setSuccess(true); commandResult.setResult(null); @@ -314,7 +311,7 @@ private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult comman String errorMsg = e.getMessage(); if (errorMsg != null && (errorMsg.contains("404") || errorMsg.contains("not found") || errorMsg.contains("does not exist"))) { - logger.warn("deleteOntapSnapshot: ONTAP snapshot for CloudStack snapshot [{}] not found, " + + logger.warn("deleteOntapSnapshot: Snapshot clone object for CloudStack snapshot [{}] not found, " + "may have been already deleted. Treating as success.", snapshotId); commandResult.setSuccess(true); commandResult.setResult(null); @@ -885,19 +882,19 @@ public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snaps // Retrieve snapshot details stored during takeSnapshot String flexVolUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.BASE_ONTAP_FV_ID); String ontapCloneId = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_CLONE_ID); - String snapshotName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_NAME); String ontapCloneName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_CLONE_NAME); if (ontapCloneName == null) { - ontapCloneName = snapshotName; + // Backward compatibility for snapshots created before clone-name metadata was persisted. + ontapCloneName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_NAME); } String volumePath = getSnapshotDetail(snapshotId, OntapStorageConstants.VOLUME_PATH); String poolIdStr = getSnapshotDetail(snapshotId, OntapStorageConstants.PRIMARY_POOL_ID); String protocol = getSnapshotDetail(snapshotId, OntapStorageConstants.PROTOCOL); - if (flexVolUuid == null || snapshotName == null || ontapCloneName == null || volumePath == null || poolIdStr == null) { + if (flexVolUuid == null || ontapCloneName == null || volumePath == null || poolIdStr == null) { throw new CloudRuntimeException("Missing required snapshot details for snapshot " + snapshotId + - " (flexVolUuid=" + flexVolUuid + ", snapshotName=" + snapshotName + - ", cloneName=" + ontapCloneName + ", volumePath=" + volumePath + ", poolId=" + poolIdStr + ")"); + " (flexVolUuid=" + flexVolUuid + ", cloneName=" + ontapCloneName + + ", volumePath=" + volumePath + ", poolId=" + poolIdStr + ")"); } long poolId = Long.parseLong(poolIdStr); @@ -923,19 +920,19 @@ public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snaps if (jobResponse == null || jobResponse.getJob() == null) { throw new CloudRuntimeException("Failed to initiate restore from snapshot [" + - snapshotName + "]"); + ontapCloneName + "]"); } // Poll for job completion (use longer timeout for large LUNs/files) Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 60, 2000); if (!jobSucceeded) { throw new CloudRuntimeException("Restore job failed for snapshot [" + - snapshotName + "]"); + ontapCloneName + "]"); } - logger.info("revertSnapshot: Successfully restored {} [{}] from snapshot [{}]", + logger.info("revertSnapshot: Successfully restored {} [{}] from clone [{}]", ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) ? "LUN" : "file", - volumePath, snapshotName); + volumePath, ontapCloneName); result.setResult(null); // Success diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java deleted file mode 100644 index 2f5a000caf8a..000000000000 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.cloudstack.storage.feign.client; - -import feign.Headers; -import feign.Param; -import feign.QueryMap; -import feign.RequestLine; -import org.apache.cloudstack.storage.feign.model.FlexVolSnapshot; -import org.apache.cloudstack.storage.feign.model.response.JobResponse; -import org.apache.cloudstack.storage.feign.model.response.OntapResponse; - -import java.util.Map; - -/** - * Feign client for ONTAP FlexVolume snapshot operations. - * - *

Maps to the ONTAP REST API endpoint: - * {@code /api/storage/volumes/{volume_uuid}/snapshots}

- * - *

FlexVolume snapshots are point-in-time, space-efficient copies of an entire - * FlexVolume. Unlike file-level clones, a single FlexVolume snapshot atomically - * captures all files/LUNs within the volume, making it ideal for VM-level - * snapshots when multiple CloudStack disks reside on the same FlexVolume.

- */ -public interface SnapshotFeignClient { - - /** - * Creates a new snapshot for the specified FlexVolume. - * - *

ONTAP REST: {@code POST /api/storage/volumes/{volume_uuid}/snapshots}

- * - * @param authHeader Basic auth header - * @param volumeUuid UUID of the ONTAP FlexVolume - * @param snapshot Snapshot request body (at minimum, the {@code name} field) - * @return JobResponse containing the async job reference - */ - @RequestLine("POST /api/storage/volumes/{volumeUuid}/snapshots") - @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) - JobResponse createSnapshot(@Param("authHeader") String authHeader, - @Param("volumeUuid") String volumeUuid, - FlexVolSnapshot snapshot); - - /** - * Lists snapshots for the specified FlexVolume. - * - *

ONTAP REST: {@code GET /api/storage/volumes/{volume_uuid}/snapshots}

- * - * @param authHeader Basic auth header - * @param volumeUuid UUID of the ONTAP FlexVolume - * @param queryParams Optional query parameters (e.g., {@code name}, {@code fields}) - * @return Paginated response of FlexVolSnapshot records - */ - @RequestLine("GET /api/storage/volumes/{volumeUuid}/snapshots") - @Headers({"Authorization: {authHeader}"}) - OntapResponse getSnapshots(@Param("authHeader") String authHeader, - @Param("volumeUuid") String volumeUuid, - @QueryMap Map queryParams); - - /** - * Retrieves a specific snapshot by UUID. - * - *

ONTAP REST: {@code GET /api/storage/volumes/{volume_uuid}/snapshots/{uuid}}

- * - * @param authHeader Basic auth header - * @param volumeUuid UUID of the ONTAP FlexVolume - * @param snapshotUuid UUID of the snapshot - * @return The FlexVolSnapshot object - */ - @RequestLine("GET /api/storage/volumes/{volumeUuid}/snapshots/{snapshotUuid}") - @Headers({"Authorization: {authHeader}"}) - FlexVolSnapshot getSnapshotByUuid(@Param("authHeader") String authHeader, - @Param("volumeUuid") String volumeUuid, - @Param("snapshotUuid") String snapshotUuid); - - /** - * Deletes a specific snapshot. - * - *

ONTAP REST: {@code DELETE /api/storage/volumes/{volume_uuid}/snapshots/{uuid}}

- * - * @param authHeader Basic auth header - * @param volumeUuid UUID of the ONTAP FlexVolume - * @param snapshotUuid UUID of the snapshot to delete - * @return JobResponse containing the async job reference - */ - @RequestLine("DELETE /api/storage/volumes/{volumeUuid}/snapshots/{snapshotUuid}") - @Headers({"Authorization: {authHeader}"}) - JobResponse deleteSnapshot(@Param("authHeader") String authHeader, - @Param("volumeUuid") String volumeUuid, - @Param("snapshotUuid") String snapshotUuid); - -} 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 9ade06eed3b7..91fdb94c2b25 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 @@ -27,7 +27,6 @@ import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.NASFeignClient; import org.apache.cloudstack.storage.feign.client.SANFeignClient; -import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; import org.apache.cloudstack.storage.feign.client.SvmFeignClient; import org.apache.cloudstack.storage.feign.client.VolumeFeignClient; import org.apache.cloudstack.storage.feign.model.Aggregate; @@ -70,7 +69,6 @@ public abstract class StorageStrategy { protected NetworkFeignClient networkFeignClient; protected SANFeignClient sanFeignClient; protected NASFeignClient nasFeignClient; - protected SnapshotFeignClient snapshotFeignClient; protected OntapStorage storage; @@ -94,7 +92,6 @@ public StorageStrategy(OntapStorage ontapStorage) { this.networkFeignClient = feignClientFactory.createClient(NetworkFeignClient.class, baseURL); this.sanFeignClient = feignClientFactory.createClient(SANFeignClient.class, baseURL); this.nasFeignClient = feignClientFactory.createClient(NASFeignClient.class, baseURL); - this.snapshotFeignClient = feignClientFactory.createClient(SnapshotFeignClient.class, baseURL); } // Connect method to validate ONTAP cluster, credentials, protocol, and SVM @@ -613,15 +610,6 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam */ abstract public String getLogicalAccess(Map values); - // ── FlexVolume Snapshot accessors ──────────────────────────────────────── - - /** - * Returns the {@link SnapshotFeignClient} for ONTAP FlexVolume snapshot operations. - */ - public SnapshotFeignClient getSnapshotFeignClient() { - return snapshotFeignClient; - } - /** * Returns the {@link NASFeignClient} for ONTAP NAS file operations * (including file clone for single-file SnapRestore). diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index eb4c285013ed..c84dfe47d636 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -32,7 +32,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; @@ -783,15 +782,22 @@ void rollbackFlexVolSnapshot(FlexVolSnapshotDetail detail) { try { Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - SnapshotFeignClient client = storageStrategy.getSnapshotFeignClient(); String authHeader = storageStrategy.getAuthHeader(); - logger.info("rollbackFlexVolSnapshot: Rolling back FlexVol snapshot [{}] (uuid={}) on FlexVol [{}]", - detail.snapshotName, detail.snapshotUuid, detail.flexVolUuid); - - JobResponse jobResponse = client.deleteSnapshot(authHeader, detail.flexVolUuid, detail.snapshotUuid); - if (jobResponse != null && jobResponse.getJob() != null) { - storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 10, 2000); + if (ProtocolType.NFS3.name().equalsIgnoreCase(detail.protocol)) { + logger.info("rollbackFlexVolSnapshot: Deleting NFS clone file [{}] on FlexVol [{}]", + detail.snapshotName, detail.flexVolUuid); + storageStrategy.getNasFeignClient().deleteFile(authHeader, detail.flexVolUuid, detail.snapshotName); + } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(detail.protocol)) { + logger.info("rollbackFlexVolSnapshot: Deleting iSCSI clone LUN [{}] (uuid={})", + detail.snapshotName, detail.snapshotUuid); + String cloneUuid = detail.snapshotUuid; + if (cloneUuid == null || cloneUuid.isEmpty()) { + String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); + String cloneLunPath = OntapStorageUtils.getLunName(poolDetails.get(OntapStorageConstants.VOLUME_NAME), detail.snapshotName); + cloneUuid = resolveLunUuid(storageStrategy, authHeader, svmName, cloneLunPath); + } + storageStrategy.getSanFeignClient().deleteLun(authHeader, cloneUuid, Map.of("allow_delete_while_mapped", "true")); } } catch (Exception e) { logger.error("rollbackFlexVolSnapshot: Rollback of FlexVol snapshot failed: {}", e.getMessage(), e); @@ -817,19 +823,35 @@ void deleteFlexVolSnapshots(List flexVolDetails) { if (!deletedSnapshots.containsKey(dedupeKey)) { Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - SnapshotFeignClient client = storageStrategy.getSnapshotFeignClient(); String authHeader = storageStrategy.getAuthHeader(); - logger.info("deleteFlexVolSnapshots: Deleting ONTAP FlexVol snapshot [{}] (uuid={}) on FlexVol [{}]", - detail.snapshotName, detail.snapshotUuid, detail.flexVolUuid); - - JobResponse jobResponse = client.deleteSnapshot(authHeader, detail.flexVolUuid, detail.snapshotUuid); - if (jobResponse != null && jobResponse.getJob() != null) { - storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); + try { + if (ProtocolType.NFS3.name().equalsIgnoreCase(detail.protocol)) { + logger.info("deleteFlexVolSnapshots: Deleting NFS clone file [{}] on FlexVol [{}]", + detail.snapshotName, detail.flexVolUuid); + storageStrategy.getNasFeignClient().deleteFile(authHeader, detail.flexVolUuid, detail.snapshotName); + } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(detail.protocol)) { + logger.info("deleteFlexVolSnapshots: Deleting iSCSI clone LUN [{}] (uuid={})", + detail.snapshotName, detail.snapshotUuid); + String cloneUuid = detail.snapshotUuid; + if (cloneUuid == null || cloneUuid.isEmpty()) { + String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); + String cloneLunPath = OntapStorageUtils.getLunName(poolDetails.get(OntapStorageConstants.VOLUME_NAME), detail.snapshotName); + cloneUuid = resolveLunUuid(storageStrategy, authHeader, svmName, cloneLunPath); + } + storageStrategy.getSanFeignClient().deleteLun(authHeader, cloneUuid, Map.of("allow_delete_while_mapped", "true")); + } + } catch (Exception e) { + if (isSnapshotAlreadyMissing(e)) { + logger.warn("deleteFlexVolSnapshots: Clone [{}] on FlexVol [{}] is already missing. " + + "Treating as success.", detail.snapshotName, detail.flexVolUuid); + } else { + throw e; + } } deletedSnapshots.put(dedupeKey, Boolean.TRUE); - logger.info("deleteFlexVolSnapshots: Deleted ONTAP FlexVol snapshot [{}] on FlexVol [{}]", detail.snapshotName, detail.flexVolUuid); + logger.info("deleteFlexVolSnapshots: Deleted clone [{}] on FlexVol [{}]", detail.snapshotName, detail.flexVolUuid); } // Always remove the DB detail row @@ -837,6 +859,18 @@ void deleteFlexVolSnapshots(List flexVolDetails) { } } + private boolean isSnapshotAlreadyMissing(Exception e) { + String message = e.getMessage(); + if (message == null) { + return false; + } + String lower = message.toLowerCase(); + return lower.contains("entry doesn't exist") + || lower.contains("entry does not exist") + || lower.contains("not found") + || lower.contains("404"); + } + /** * Reverts all volumes of a VM snapshot using ONTAP CLI-based Snapshot File Restore. * From baf883aa5c1e1784a6817512bde55a099ef1f8e7 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 14:02:07 +0530 Subject: [PATCH 10/24] CSTACKEX-198: fixed junit which got impacted with the previous changes --- .../OntapPrimaryDatastoreDriverTest.java | 127 +++++++++++++++++- 1 file changed, 125 insertions(+), 2 deletions(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index 17c0896a2e9d..89f1892bb1df 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 @@ -40,10 +40,12 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.feign.client.NASFeignClient; +import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.Igroup; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.response.JobResponse; +import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; @@ -62,8 +64,10 @@ import org.mockito.junit.jupiter.MockitoExtension; import java.util.HashMap; +import java.util.List; import java.util.Map; +import static com.cloud.agent.api.to.DataObjectType.SNAPSHOT; import static com.cloud.agent.api.to.DataObjectType.VOLUME; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -125,6 +129,9 @@ class OntapPrimaryDatastoreDriverTest { @Mock private NASFeignClient nasFeignClient; + @Mock + private SANFeignClient sanFeignClient; + @Mock private SnapshotInfo snapshotInfo; @@ -639,8 +646,6 @@ void testRevertSnapshot_UsesCloneMetadata() { .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.BASE_ONTAP_FV_ID, "flexvol-uuid-1", false)); when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.ONTAP_CLONE_ID)) .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.ONTAP_CLONE_ID, "clone-lun-uuid-1", false)); - when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.ONTAP_SNAP_NAME)) - .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.ONTAP_SNAP_NAME, "UI Snapshot Name", false)); when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.ONTAP_CLONE_NAME)) .thenReturn(new SnapshotDetailsVO(500L, OntapStorageConstants.ONTAP_CLONE_NAME, "UI_Snapshot_Name", false)); when(snapshotDetailsDao.findDetail(500L, OntapStorageConstants.VOLUME_PATH)) @@ -672,4 +677,122 @@ void testRevertSnapshot_UsesCloneMetadata() { verify(commandCallback).complete(any(CommandResult.class)); } } + + @Test + void testRevertSnapshot_FallbacksToLegacySnapshotNameWhenCloneNameMissing() { + when(snapshotInfo.getId()).thenReturn(501L); + when(snapshotDetailsDao.findDetail(501L, OntapStorageConstants.BASE_ONTAP_FV_ID)) + .thenReturn(new SnapshotDetailsVO(501L, OntapStorageConstants.BASE_ONTAP_FV_ID, "flexvol-uuid-1", false)); + when(snapshotDetailsDao.findDetail(501L, OntapStorageConstants.ONTAP_CLONE_ID)) + .thenReturn(new SnapshotDetailsVO(501L, OntapStorageConstants.ONTAP_CLONE_ID, "clone-lun-uuid-2", false)); + when(snapshotDetailsDao.findDetail(501L, OntapStorageConstants.ONTAP_CLONE_NAME)).thenReturn(null); + when(snapshotDetailsDao.findDetail(501L, OntapStorageConstants.ONTAP_SNAP_NAME)) + .thenReturn(new SnapshotDetailsVO(501L, OntapStorageConstants.ONTAP_SNAP_NAME, "Legacy_UI_Snapshot", false)); + when(snapshotDetailsDao.findDetail(501L, OntapStorageConstants.VOLUME_PATH)) + .thenReturn(new SnapshotDetailsVO(501L, OntapStorageConstants.VOLUME_PATH, "dest-lun-1", false)); + when(snapshotDetailsDao.findDetail(501L, OntapStorageConstants.PRIMARY_POOL_ID)) + .thenReturn(new SnapshotDetailsVO(501L, OntapStorageConstants.PRIMARY_POOL_ID, "1", false)); + when(snapshotDetailsDao.findDetail(501L, OntapStorageConstants.PROTOCOL)) + .thenReturn(new SnapshotDetailsVO(501L, OntapStorageConstants.PROTOCOL, ProtocolType.ISCSI.name(), false)); + + storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + + JobResponse jobResponse = new JobResponse(); + Job job = new Job(); + job.setUuid("job-uuid-legacy"); + jobResponse.setJob(job); + when(storageStrategy.revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString(), anyString())) + .thenReturn(jobResponse); + when(storageStrategy.jobPollForSuccess("job-uuid-legacy", 60, 2000)).thenReturn(true); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(storageStrategy); + + driver.revertSnapshot(snapshotInfo, snapshotInfo, commandCallback); + + verify(storageStrategy).revertSnapshotForCloudStackVolume( + eq("Legacy_UI_Snapshot"), eq("flexvol-uuid-1"), eq("clone-lun-uuid-2"), + eq("dest-lun-1"), eq("clone-lun-uuid-2"), eq("flexvol1")); + verify(commandCallback).complete(any(CommandResult.class)); + } + } + + @Test + void testDeleteAsync_SnapshotNfsClone_UsesDeleteFile() { + when(snapshotInfo.getType()).thenReturn(SNAPSHOT); + when(snapshotInfo.getId()).thenReturn(700L); + + when(snapshotDetailsDao.findDetail(700L, OntapStorageConstants.BASE_ONTAP_FV_ID)) + .thenReturn(new SnapshotDetailsVO(700L, OntapStorageConstants.BASE_ONTAP_FV_ID, "flexvol-uuid-nfs", false)); + when(snapshotDetailsDao.findDetail(700L, OntapStorageConstants.ONTAP_CLONE_ID)) + .thenReturn(new SnapshotDetailsVO(700L, OntapStorageConstants.ONTAP_CLONE_ID, "clone-id-nfs", false)); + when(snapshotDetailsDao.findDetail(700L, OntapStorageConstants.ONTAP_CLONE_NAME)) + .thenReturn(new SnapshotDetailsVO(700L, OntapStorageConstants.ONTAP_CLONE_NAME, "clone-file-nfs.qcow2", false)); + when(snapshotDetailsDao.findDetail(700L, OntapStorageConstants.PRIMARY_POOL_ID)) + .thenReturn(new SnapshotDetailsVO(700L, OntapStorageConstants.PRIMARY_POOL_ID, "1", false)); + when(snapshotDetailsDao.findDetail(700L, OntapStorageConstants.PROTOCOL)) + .thenReturn(new SnapshotDetailsVO(700L, OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name(), false)); + + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(storageStrategy.getAuthHeader()).thenReturn("Basic auth"); + when(storageStrategy.getNasFeignClient()).thenReturn(nasFeignClient); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(storageStrategy); + + driver.deleteAsync(dataStore, snapshotInfo, commandCallback); + + verify(nasFeignClient).deleteFile("Basic auth", "flexvol-uuid-nfs", "clone-file-nfs.qcow2"); + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(commandCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + } + } + + @Test + void testDeleteAsync_SnapshotIscsiClone_ResolvesUuidAndUsesDeleteLun() { + storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); + storagePoolDetails.put(OntapStorageConstants.SVM_NAME, "svm1"); + + when(snapshotInfo.getType()).thenReturn(SNAPSHOT); + when(snapshotInfo.getId()).thenReturn(701L); + + when(snapshotDetailsDao.findDetail(701L, OntapStorageConstants.BASE_ONTAP_FV_ID)) + .thenReturn(new SnapshotDetailsVO(701L, OntapStorageConstants.BASE_ONTAP_FV_ID, "flexvol-uuid-iscsi", false)); + when(snapshotDetailsDao.findDetail(701L, OntapStorageConstants.ONTAP_CLONE_ID)).thenReturn(null); + when(snapshotDetailsDao.findDetail(701L, OntapStorageConstants.ONTAP_CLONE_NAME)) + .thenReturn(new SnapshotDetailsVO(701L, OntapStorageConstants.ONTAP_CLONE_NAME, "clone-lun-name", false)); + when(snapshotDetailsDao.findDetail(701L, OntapStorageConstants.PRIMARY_POOL_ID)) + .thenReturn(new SnapshotDetailsVO(701L, OntapStorageConstants.PRIMARY_POOL_ID, "1", false)); + when(snapshotDetailsDao.findDetail(701L, OntapStorageConstants.PROTOCOL)) + .thenReturn(new SnapshotDetailsVO(701L, OntapStorageConstants.PROTOCOL, ProtocolType.ISCSI.name(), false)); + + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); + when(storageStrategy.getAuthHeader()).thenReturn("Basic auth"); + when(storageStrategy.getSanFeignClient()).thenReturn(sanFeignClient); + + OntapResponse lunResponse = new OntapResponse<>(); + Lun lun = new Lun(); + lun.setUuid("resolved-clone-uuid"); + lunResponse.setRecords(List.of(lun)); + when(sanFeignClient.getLunResponse(eq("Basic auth"), any())).thenReturn(lunResponse); + + try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { + utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) + .thenReturn(storageStrategy); + utilityMock.when(() -> OntapStorageUtils.getLunName("flexvol1", "clone-lun-name")) + .thenReturn("/vol/flexvol1/clone-lun-name"); + + driver.deleteAsync(dataStore, snapshotInfo, commandCallback); + + verify(sanFeignClient).deleteLun(eq("Basic auth"), eq("resolved-clone-uuid"), + argThat(map -> "true".equals(map.get("allow_delete_while_mapped")))); + ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); + verify(commandCallback).complete(resultCaptor.capture()); + assertTrue(resultCaptor.getValue().isSuccess()); + } + } } From a58dcd07ef8a16b0b04f303616fc00d9f738d9aa Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 14:26:22 +0530 Subject: [PATCH 11/24] CSTACKEX-198: revert workflows should also have lun path in ONTAP pattern --- .../storage/service/UnifiedSANStrategy.java | 25 ++++++++++++++++--- .../service/UnifiedSANStrategyTest.java | 7 +++++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index b80913c4759e..4de757a6c123 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 @@ -565,6 +565,9 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String logger.trace("revertSnapshotForCloudStackVolume [iSCSI]: Reverting LUN [{}] from clone [{}] on FlexVol [{}]", volumePath, snapshotName, flexVolName); + if (snapshotName == null || snapshotName.isEmpty()) { + throw new CloudRuntimeException("Source clone LUN name is required for iSCSI snapshot revert"); + } if (volumePath == null || volumePath.isEmpty()) { throw new CloudRuntimeException("Destination LUN name is required for iSCSI snapshot revert"); } @@ -574,10 +577,25 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String if (lunUuid == null || lunUuid.isEmpty()) { throw new CloudRuntimeException("Source clone LUN UUID is required for iSCSI snapshot revert"); } + if (storage.getSvmName() == null || storage.getSvmName().isEmpty()) { + throw new CloudRuntimeException("SVM name is required for iSCSI snapshot revert"); + } + + String sourceLunPath = snapshotName.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX) + ? snapshotName : OntapStorageUtils.getLunName(flexVolName, snapshotName); + String destinationLunPath = volumePath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX) + ? volumePath : OntapStorageUtils.getLunName(flexVolName, volumePath); + + if (!sourceLunPath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { + throw new CloudRuntimeException("Invalid source LUN path for iSCSI snapshot revert: " + sourceLunPath); + } + if (!destinationLunPath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { + throw new CloudRuntimeException("Invalid destination LUN path for iSCSI snapshot revert: " + destinationLunPath); + } String authHeader = getAuthHeader(); Lun revertCloneRequest = new Lun(); - revertCloneRequest.setName(volumePath); + revertCloneRequest.setName(destinationLunPath); Svm svm = new Svm(); svm.setName(storage.getSvmName()); revertCloneRequest.setSvm(svm); @@ -588,13 +606,14 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String revertCloneRequest.setLocation(location); Lun.Clone clone = new Lun.Clone(); Lun.Source source = new Lun.Source(); + source.setName(sourceLunPath); source.setUuid(lunUuid); clone.setSource(source); revertCloneRequest.setClone(clone); revertCloneRequest.setIsOverride(Boolean.TRUE); - logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: lun clone sourceUuid={} destinationLun={} isOverride=true", - lunUuid, volumePath); + logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: lun clone sourcePath={} sourceUuid={} destinationLun={} isOverride=true", + sourceLunPath, lunUuid, destinationLunPath); return sanFeignClient.cloneLun(authHeader, revertCloneRequest); } } 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 78fe34cd3cb1..11f9a59b670d 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 @@ -1816,6 +1816,10 @@ void testRevertSnapshotForCloudStackVolume_UsesLunCloneWithOverride() { try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) .thenReturn(authHeader); + utilityMock.when(() -> OntapStorageUtils.getLunName("flexvol1", "clone-snap-1")) + .thenReturn("/vol/flexvol1/clone-snap-1"); + utilityMock.when(() -> OntapStorageUtils.getLunName("flexvol1", "dest-lun-1")) + .thenReturn("/vol/flexvol1/dest-lun-1"); JobResponse result = unifiedSANStrategy.revertSnapshotForCloudStackVolume( "clone-snap-1", "flexvol-uuid-1", "clone-lun-uuid-1", "dest-lun-1", "clone-lun-uuid-1", "flexvol1"); @@ -1824,9 +1828,10 @@ void testRevertSnapshotForCloudStackVolume_UsesLunCloneWithOverride() { verify(sanFeignClient).cloneLun(eq(authHeader), argThat(lun -> lun != null && Boolean.TRUE.equals(lun.getIsOverride()) - && "dest-lun-1".equals(lun.getName()) + && "/vol/flexvol1/dest-lun-1".equals(lun.getName()) && lun.getClone() != null && lun.getClone().getSource() != null + && "/vol/flexvol1/clone-snap-1".equals(lun.getClone().getSource().getName()) && "clone-lun-uuid-1".equals(lun.getClone().getSource().getUuid()) && lun.getLocation() != null && lun.getLocation().getVolume() != null From 1802b57209355eacd7c5d4b01f41626d38a994ba Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 15:29:20 +0530 Subject: [PATCH 12/24] CSTACKEX-198: use patch of LUN instead post, for revert workflow --- .../storage/feign/client/SANFeignClient.java | 4 ++-- .../storage/service/UnifiedSANStrategy.java | 17 ++++++++++++++--- .../storage/service/UnifiedSANStrategyTest.java | 9 +++++++-- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index e9857a431acd..f08a17685cf7 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java @@ -53,9 +53,9 @@ public interface SANFeignClient { @Headers({"Authorization: {authHeader}"}) Lun getLunByUUID(@Param("authHeader") String authHeader, @Param("uuid") String uuid); - @RequestLine("PATCH /{uuid}") + @RequestLine("PATCH /api/storage/luns/{uuid}") @Headers({"Authorization: {authHeader}"}) - void updateLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid, Lun lun); + JobResponse updateLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid, Lun lun); @RequestLine("DELETE /api/storage/luns/{uuid}") @Headers({"Authorization: {authHeader}"}) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 4de757a6c123..bdf3982f3eda 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 @@ -594,6 +594,7 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String } String authHeader = getAuthHeader(); + String destinationLunUuid = resolveLunUuidByName(authHeader, storage.getSvmName(), destinationLunPath); Lun revertCloneRequest = new Lun(); revertCloneRequest.setName(destinationLunPath); Svm svm = new Svm(); @@ -612,8 +613,18 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String revertCloneRequest.setClone(clone); revertCloneRequest.setIsOverride(Boolean.TRUE); - logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: lun clone sourcePath={} sourceUuid={} destinationLun={} isOverride=true", - sourceLunPath, lunUuid, destinationLunPath); - return sanFeignClient.cloneLun(authHeader, revertCloneRequest); + logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: patch lun destinationUuid={} sourcePath={} sourceUuid={} destinationLun={} isOverride=true", + destinationLunUuid, sourceLunPath, lunUuid, destinationLunPath); + return sanFeignClient.updateLun(authHeader, destinationLunUuid, revertCloneRequest); + } + + private String resolveLunUuidByName(String authHeader, String svmName, String lunName) { + OntapResponse response = sanFeignClient.getLunResponse(authHeader, + Map.of(OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.NAME, lunName)); + if (response == null || response.getRecords() == null || response.getRecords().isEmpty() + || response.getRecords().get(0).getUuid() == null || response.getRecords().get(0).getUuid().isEmpty()) { + throw new CloudRuntimeException("Failed to resolve destination LUN UUID for path: " + lunName); + } + return response.getRecords().get(0).getUuid(); } } 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 11f9a59b670d..7bcd1fe27f8e 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 @@ -1811,7 +1811,12 @@ void testRevertSnapshotForCloudStackVolume_UsesLunCloneWithOverride() { org.apache.cloudstack.storage.feign.model.Job job = new org.apache.cloudstack.storage.feign.model.Job(); job.setUuid("job-uuid-1"); jobResponse.setJob(job); - when(sanFeignClient.cloneLun(eq(authHeader), any(Lun.class))).thenReturn(jobResponse); + OntapResponse destinationLunResponse = new OntapResponse<>(); + Lun destinationLun = new Lun(); + destinationLun.setUuid("dest-lun-uuid-1"); + destinationLunResponse.setRecords(List.of(destinationLun)); + when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(destinationLunResponse); + when(sanFeignClient.updateLun(eq(authHeader), eq("dest-lun-uuid-1"), any(Lun.class))).thenReturn(jobResponse); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) @@ -1825,7 +1830,7 @@ void testRevertSnapshotForCloudStackVolume_UsesLunCloneWithOverride() { "clone-snap-1", "flexvol-uuid-1", "clone-lun-uuid-1", "dest-lun-1", "clone-lun-uuid-1", "flexvol1"); assertNotNull(result); - verify(sanFeignClient).cloneLun(eq(authHeader), argThat(lun -> + verify(sanFeignClient).updateLun(eq(authHeader), eq("dest-lun-uuid-1"), argThat(lun -> lun != null && Boolean.TRUE.equals(lun.getIsOverride()) && "/vol/flexvol1/dest-lun-1".equals(lun.getName()) From 7e7cfb9646da5ed77e6057ba722d4da0191e3ff0 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 16:01:05 +0530 Subject: [PATCH 13/24] CSTACKEX-198: update revert workflow for nfs --- .../storage/feign/client/NASFeignClient.java | 11 ++-- .../storage/service/UnifiedNASStrategy.java | 19 +++---- .../vmsnapshot/OntapVMSnapshotStrategy.java | 57 +++++++------------ .../service/UnifiedNASStrategyTest.java | 18 +++--- .../OntapVMSnapshotStrategyTest.java | 10 +--- 5 files changed, 42 insertions(+), 73 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java index 14d3011d81f8..099b63bdb037 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java @@ -46,12 +46,13 @@ void deleteFile(@Param("authHeader") String authHeader, @Param("volumeUuid") String volumeUUID, @Param("path") String filePath); - @RequestLine("PATCH /api/storage/volumes/{volumeUuid}/files/{path}") + @RequestLine("PATCH /api/storage/volumes/{volumeUuid}/files/{path}?return_records={returnRecords}") @Headers({"Authorization: {authHeader}"}) - void updateFile(@Param("authHeader") String authHeader, - @Param("volumeUuid") String volumeUUID, - @Param("path") String filePath, - FileInfo fileInfo); + JobResponse updateFile(@Param("authHeader") String authHeader, + @Param("volumeUuid") String volumeUUID, + @Param("path") String filePath, + @Param("returnRecords") boolean returnRecords, + FileInfo fileInfo); @RequestLine("POST /api/storage/volumes/{volumeUuid}/files/{path}") @Headers({"Authorization: {authHeader}"}) 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 c26870d9f024..4bca8382f1c4 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 @@ -35,7 +35,6 @@ import org.apache.cloudstack.storage.feign.model.ExportPolicy; import org.apache.cloudstack.storage.feign.model.ExportRule; import org.apache.cloudstack.storage.feign.model.FileInfo; -import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.Nas; import org.apache.cloudstack.storage.feign.model.OntapStorage; @@ -468,17 +467,13 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String } String authHeader = getAuthHeader(); - FileCloneRequest fileCloneRequest = new FileCloneRequest(); - FileCloneRequest.VolumeRef volumeRef = new FileCloneRequest.VolumeRef(); - volumeRef.setUuid(flexVolUuid); - volumeRef.setName(flexVolName); - fileCloneRequest.setVolume(volumeRef); - fileCloneRequest.setSourcePath(snapshotName); - fileCloneRequest.setDestinationPath(volumePath); - fileCloneRequest.setIsOverride(Boolean.TRUE); - - logger.debug("revertSnapshotForCloudStackVolume [NFS]: file clone source={} destination={} isOverride=true", + FileInfo filePatchRequest = new FileInfo(); + filePatchRequest.setPath(volumePath); + filePatchRequest.setTarget(snapshotName); + filePatchRequest.setOverwriteEnabled(Boolean.TRUE); + + logger.debug("revertSnapshotForCloudStackVolume [NFS]: patch file source={} destination={} overwrite=true", snapshotName, volumePath); - return getNasFeignClient().cloneFile(authHeader, fileCloneRequest); + return getNasFeignClient().updateFile(authHeader, flexVolUuid, volumePath, true, filePatchRequest); } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index c84dfe47d636..e3584b715604 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -146,11 +146,6 @@ public StrategyPriority canHandle(VMSnapshot vmSnapshot) { } return StrategyPriority.CANT_HANDLE; } - // Also check legacy STORAGE_SNAPSHOT details for backward compatibility - List legacyDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), STORAGE_SNAPSHOT); - if (CollectionUtils.isNotEmpty(legacyDetails) && allVolumesOnOntapManagedStorage(vmSnapshot.getVmId())) { - return StrategyPriority.HIGHEST; - } return StrategyPriority.CANT_HANDLE; } @@ -600,12 +595,6 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { deleteFlexVolSnapshots(flexVolDetails); } - // Also handle legacy STORAGE_SNAPSHOT details (backward compatibility) - List legacyDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), STORAGE_SNAPSHOT); - if (CollectionUtils.isNotEmpty(legacyDetails)) { - deleteDiskSnapshot(vmSnapshot); - } - processAnswer(vmSnapshotVO, userVm, new DeleteVMSnapshotAnswer(deleteSnapshotCommand, volumeTOs), null); long fullChainSize = 0; for (VolumeObjectTO volumeTo : volumeTOs) { @@ -649,16 +638,12 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { RevertToVMSnapshotCommand revertToSnapshotCommand = new RevertToVMSnapshotCommand(vmInstanceName, userVm.getUuid(), vmSnapshotTO, volumeTOs, guestOS.getDisplayName()); - // Check for FlexVolume snapshots (new approach) - List flexVolDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT); - if (CollectionUtils.isNotEmpty(flexVolDetails)) { - revertFlexVolSnapshots(flexVolDetails); - } - - // Also handle legacy STORAGE_SNAPSHOT details (backward compatibility) - List legacyDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), STORAGE_SNAPSHOT); - if (CollectionUtils.isNotEmpty(legacyDetails)) { - revertDiskSnapshot(vmSnapshot); + // Revert clone-backed snapshot artifacts per volume: + // - NFS: cloneFile(source=clone, destination=live file, isOverride=true) + // - iSCSI: patch LUN (clone.source=clone LUN, destination=live LUN) + List cloneDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT); + if (CollectionUtils.isNotEmpty(cloneDetails)) { + revertCloneBackedSnapshots(cloneDetails); } RevertToVMSnapshotAnswer answer = new RevertToVMSnapshotAnswer(revertToSnapshotCommand, true, ""); @@ -872,28 +857,24 @@ private boolean isSnapshotAlreadyMissing(Exception e) { } /** - * Reverts all volumes of a VM snapshot using ONTAP CLI-based Snapshot File Restore. + * Reverts all volumes of a VM snapshot using clone-backed restore operations. * - *

Instead of restoring the entire FlexVolume to a snapshot (which would affect - * other VMs/files on the same FlexVol), this method restores only the individual - * files or LUNs belonging to this VM using the dedicated ONTAP CLI snapshot file - * restore API:

+ *

Each persisted detail row represents one volume and points to the clone artifact + * created during VM snapshot creation. Revert copies from the clone artifact back to + * the original volume object.

* - *

{@code POST /api/private/cli/volume/snapshot/restore-file}

- * - *

For each persisted detail row (one per CloudStack volume):

*
    - *
  • NFS: restores {@code } from the snapshot to the live volume
    • - *
  • iSCSI: restores {@code } from the snapshot to the live volume
    • + *
  • NFS: clone file from snapshot clone file path to original file path, with overwrite
    • + *
  • iSCSI: patch destination LUN with clone source ({@code clone.source.name/uuid})
    • *
*/ - void revertFlexVolSnapshots(List flexVolDetails) { - for (VMSnapshotDetailsVO detailVO : flexVolDetails) { + void revertCloneBackedSnapshots(List cloneDetails) { + for (VMSnapshotDetailsVO detailVO : cloneDetails) { FlexVolSnapshotDetail detail = FlexVolSnapshotDetail.parse(detailVO.getValue()); if (detail.volumePath == null || detail.volumePath.isEmpty()) { // Legacy detail row without volumePath – cannot do single-file restore - logger.warn("revertFlexVolSnapshots: FlexVol snapshot detail for FlexVol [{}] has no volumePath (legacy format). " + + logger.warn("revertCloneBackedSnapshots: Snapshot detail for FlexVol [{}] has no volumePath (legacy format). " + "Skipping single-file restore for this entry.", detail.flexVolUuid); continue; } @@ -905,7 +886,7 @@ void revertFlexVolSnapshots(List flexVolDetails) { throw new CloudRuntimeException("FlexVolume name not found in pool details for pool [" + detail.poolId + "]"); } - logger.info("revertFlexVolSnapshots: Restoring volume [{}] from FlexVol snapshot [{}] on FlexVol [{}] (protocol={})", + logger.info("revertCloneBackedSnapshots: Reverting volume [{}] using clone source [{}] on FlexVol [{}] (protocol={})", detail.volumePath, detail.snapshotName, flexVolName, detail.protocol); String lunUuid = ProtocolType.ISCSI.name().equalsIgnoreCase(detail.protocol) ? detail.snapshotUuid : null; JobResponse jobResponse = storageStrategy.revertSnapshotForCloudStackVolume( @@ -914,13 +895,13 @@ void revertFlexVolSnapshots(List flexVolDetails) { if (jobResponse != null && jobResponse.getJob() != null) { Boolean success = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 60, 2000); if (!success) { - throw new CloudRuntimeException("Snapshot file restore failed for volume path [" + - detail.volumePath + "] from snapshot [" + detail.snapshotName + + throw new CloudRuntimeException("Clone-backed revert failed for volume path [" + + detail.volumePath + "] from clone [" + detail.snapshotName + "] on FlexVol [" + flexVolName + "]"); } } - logger.info("revertFlexVolSnapshots: Successfully restored volume [{}] from snapshot [{}] on FlexVol [{}]", + logger.info("revertCloneBackedSnapshots: Successfully reverted volume [{}] from clone [{}] on FlexVol [{}]", detail.volumePath, detail.snapshotName, flexVolName); } } 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 0e99ff68e942..8ec218f6a87b 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 @@ -37,7 +37,7 @@ import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.ExportPolicy; -import org.apache.cloudstack.storage.feign.model.FileCloneRequest; +import org.apache.cloudstack.storage.feign.model.FileInfo; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.response.JobResponse; @@ -586,26 +586,22 @@ public void testDeleteCloudStackVolume_AnswerNull() throws Exception { } @Test - public void testRevertSnapshotForCloudStackVolume_UsesFileCloneWithOverride() { + public void testRevertSnapshotForCloudStackVolume_UsesFilePatchWithOverwrite() { JobResponse jobResponse = new JobResponse(); Job job = new Job(); job.setUuid("job-uuid-1"); jobResponse.setJob(job); - when(nasFeignClient.cloneFile(anyString(), any(FileCloneRequest.class))).thenReturn(jobResponse); + when(nasFeignClient.updateFile(anyString(), anyString(), anyString(), eq(true), any(FileInfo.class))).thenReturn(jobResponse); JobResponse result = strategy.revertSnapshotForCloudStackVolume( "clone-snap-1", "flexvol-uuid-1", "snap-uuid-1", "vm-disk.qcow2", null, "flexvol1"); assertNotNull(result); - verify(nasFeignClient).cloneFile(anyString(), argThat(req -> + verify(nasFeignClient).updateFile(anyString(), eq("flexvol-uuid-1"), eq("vm-disk.qcow2"), eq(true), argThat(req -> req != null - && req.getIsOverride() != null - && req.getIsOverride() - && "clone-snap-1".equals(req.getSourcePath()) - && "vm-disk.qcow2".equals(req.getDestinationPath()) - && req.getVolume() != null - && "flexvol-uuid-1".equals(req.getVolume().getUuid()) - && "flexvol1".equals(req.getVolume().getName()))); + && Boolean.TRUE.equals(req.isOverwriteEnabled()) + && "clone-snap-1".equals(req.getTarget()) + && "vm-disk.qcow2".equals(req.getPath()))); } @Test diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java index cd095cccefcd..60796ac0ff51 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java @@ -337,20 +337,16 @@ void testCanHandle_NonAllocated_HasFlexVolSnapshotDetails_AllOnOntap_ReturnsHigh } @Test - void testCanHandle_NonAllocated_HasLegacyStorageSnapshotDetails_AllOnOntap_ReturnsHighest() { + void testCanHandle_NonAllocated_HasLegacyStorageSnapshotDetails_AllOnOntap_ReturnsCantHandle() { setupAllVolumesOnOntap(); VMSnapshotVO vmSnapshot = createMockVmSnapshot(VMSnapshot.State.Ready, VMSnapshot.Type.Disk); - // No FlexVol details + // Only clone-backed ONTAP details are supported now. when(vmSnapshotDetailsDao.findDetails(SNAPSHOT_ID, OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT)).thenReturn(Collections.emptyList()); - // Has legacy details - List details = new ArrayList<>(); - details.add(new VMSnapshotDetailsVO(SNAPSHOT_ID, "kvmStorageSnapshot", "123", true)); - when(vmSnapshotDetailsDao.findDetails(SNAPSHOT_ID, "kvmStorageSnapshot")).thenReturn(details); StrategyPriority result = strategy.canHandle(vmSnapshot); - assertEquals(StrategyPriority.HIGHEST, result); + assertEquals(StrategyPriority.CANT_HANDLE, result); } @Test From f56b30a72deae9982fdbb207a2212c03fab4706f Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 16:41:10 +0530 Subject: [PATCH 14/24] CSTACKEX-198: I am seeing error due to is_override hence removing it for now --- .../apache/cloudstack/storage/service/UnifiedSANStrategy.java | 3 +-- .../cloudstack/storage/service/UnifiedSANStrategyTest.java | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index bdf3982f3eda..03f2e05786b0 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 @@ -611,9 +611,8 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String source.setUuid(lunUuid); clone.setSource(source); revertCloneRequest.setClone(clone); - revertCloneRequest.setIsOverride(Boolean.TRUE); - logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: patch lun destinationUuid={} sourcePath={} sourceUuid={} destinationLun={} isOverride=true", + logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: patch lun destinationUuid={} sourcePath={} sourceUuid={} destinationLun={}", destinationLunUuid, sourceLunPath, lunUuid, destinationLunPath); return sanFeignClient.updateLun(authHeader, destinationLunUuid, revertCloneRequest); } 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 7bcd1fe27f8e..48d1e185be64 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 @@ -1806,7 +1806,7 @@ void testEnsureLunMapped_ExistingMapping_ReturnsExistingNumber() { } @Test - void testRevertSnapshotForCloudStackVolume_UsesLunCloneWithOverride() { + void testRevertSnapshotForCloudStackVolume_UsesLunPatchWithCloneSource() { JobResponse jobResponse = new JobResponse(); org.apache.cloudstack.storage.feign.model.Job job = new org.apache.cloudstack.storage.feign.model.Job(); job.setUuid("job-uuid-1"); @@ -1832,7 +1832,7 @@ void testRevertSnapshotForCloudStackVolume_UsesLunCloneWithOverride() { assertNotNull(result); verify(sanFeignClient).updateLun(eq(authHeader), eq("dest-lun-uuid-1"), argThat(lun -> lun != null - && Boolean.TRUE.equals(lun.getIsOverride()) + && lun.getIsOverride() == null && "/vol/flexvol1/dest-lun-1".equals(lun.getName()) && lun.getClone() != null && lun.getClone().getSource() != null From ea401ce4766ecdc1926d6b964eba3a8857b9b2f0 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 17:52:23 +0530 Subject: [PATCH 15/24] CSTACKEX-198: removing svm name attributes while calling patch of lun --- .../storage/service/UnifiedSANStrategy.java | 11 ++--------- .../storage/service/UnifiedSANStrategyTest.java | 7 +++---- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 03f2e05786b0..63d6bd23bbe3 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 @@ -596,15 +596,8 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String String authHeader = getAuthHeader(); String destinationLunUuid = resolveLunUuidByName(authHeader, storage.getSvmName(), destinationLunPath); Lun revertCloneRequest = new Lun(); - revertCloneRequest.setName(destinationLunPath); - Svm svm = new Svm(); - svm.setName(storage.getSvmName()); - revertCloneRequest.setSvm(svm); - Lun.Location location = new Lun.Location(); - Lun.LocationVolume locationVolume = new Lun.LocationVolume(); - locationVolume.setName(flexVolName); - location.setVolume(locationVolume); - revertCloneRequest.setLocation(location); + // PATCH /storage/luns/{uuid} rejects immutable destination attributes like svm.name. + // For restore, only provide clone source details and target the destination via UUID in URI. Lun.Clone clone = new Lun.Clone(); Lun.Source source = new Lun.Source(); source.setName(sourceLunPath); 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 48d1e185be64..08b06b35ee52 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 @@ -1833,14 +1833,13 @@ void testRevertSnapshotForCloudStackVolume_UsesLunPatchWithCloneSource() { verify(sanFeignClient).updateLun(eq(authHeader), eq("dest-lun-uuid-1"), argThat(lun -> lun != null && lun.getIsOverride() == null - && "/vol/flexvol1/dest-lun-1".equals(lun.getName()) + && lun.getName() == null && lun.getClone() != null && lun.getClone().getSource() != null && "/vol/flexvol1/clone-snap-1".equals(lun.getClone().getSource().getName()) && "clone-lun-uuid-1".equals(lun.getClone().getSource().getUuid()) - && lun.getLocation() != null - && lun.getLocation().getVolume() != null - && "flexvol1".equals(lun.getLocation().getVolume().getName()) + && lun.getLocation() == null + && lun.getSvm() == null )); } } From 5348e786173ea368570bb67836e73ff8f6aff0c7 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 18:32:41 +0530 Subject: [PATCH 16/24] CSTACKEX-198: removing unwanted flags from file clone also --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 1 - .../storage/feign/model/FileCloneRequest.java | 11 ----------- .../storage/vmsnapshot/OntapVMSnapshotStrategy.java | 3 +-- 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index ae3d0b46c7b5..44a19338cfc6 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 @@ -688,7 +688,6 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback cloneDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT); if (CollectionUtils.isNotEmpty(cloneDetails)) { From 3a3f999a19e8749bd717e6dde0a3b28cc3462704 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 19:26:05 +0530 Subject: [PATCH 17/24] CSTACKEX-198: target field is not required for revert file based snapshots --- .../cloudstack/storage/service/UnifiedNASStrategy.java | 5 +++-- .../cloudstack/storage/service/UnifiedNASStrategyTest.java | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) 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 4bca8382f1c4..bc768528ade7 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 @@ -467,9 +467,10 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String } String authHeader = getAuthHeader(); + // Keep PATCH-based revert. ONTAP in this environment rejects "target", so send + // only accepted fields and use "path" to carry source clone file reference. FileInfo filePatchRequest = new FileInfo(); - filePatchRequest.setPath(volumePath); - filePatchRequest.setTarget(snapshotName); + filePatchRequest.setPath(snapshotName); filePatchRequest.setOverwriteEnabled(Boolean.TRUE); logger.debug("revertSnapshotForCloudStackVolume [NFS]: patch file source={} destination={} overwrite=true", 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 8ec218f6a87b..a71c6fa9394f 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 @@ -586,7 +586,7 @@ public void testDeleteCloudStackVolume_AnswerNull() throws Exception { } @Test - public void testRevertSnapshotForCloudStackVolume_UsesFilePatchWithOverwrite() { + public void testRevertSnapshotForCloudStackVolume_UsesFilePatchWithoutTarget() { JobResponse jobResponse = new JobResponse(); Job job = new Job(); job.setUuid("job-uuid-1"); @@ -600,8 +600,8 @@ public void testRevertSnapshotForCloudStackVolume_UsesFilePatchWithOverwrite() { verify(nasFeignClient).updateFile(anyString(), eq("flexvol-uuid-1"), eq("vm-disk.qcow2"), eq(true), argThat(req -> req != null && Boolean.TRUE.equals(req.isOverwriteEnabled()) - && "clone-snap-1".equals(req.getTarget()) - && "vm-disk.qcow2".equals(req.getPath()))); + && "clone-snap-1".equals(req.getPath()) + && req.getTarget() == null)); } @Test From d7f12b9ed7007c38a0808f6bf5d58fe9ad0cf9bb Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Mon, 15 Jun 2026 19:42:44 +0530 Subject: [PATCH 18/24] CSTACKEX-198: adding missing attribute for NFS path --- .../apache/cloudstack/storage/service/UnifiedNASStrategy.java | 3 ++- .../cloudstack/storage/service/UnifiedNASStrategyTest.java | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) 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 bc768528ade7..d48c9518e2bf 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 @@ -472,8 +472,9 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String FileInfo filePatchRequest = new FileInfo(); filePatchRequest.setPath(snapshotName); filePatchRequest.setOverwriteEnabled(Boolean.TRUE); + filePatchRequest.setFillEnabled(Boolean.FALSE); - logger.debug("revertSnapshotForCloudStackVolume [NFS]: patch file source={} destination={} overwrite=true", + logger.debug("revertSnapshotForCloudStackVolume [NFS]: patch file source={} destination={} overwrite=true fill=false", snapshotName, volumePath); return getNasFeignClient().updateFile(authHeader, flexVolUuid, volumePath, true, filePatchRequest); } 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 a71c6fa9394f..1d5c1f7eb947 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 @@ -600,6 +600,7 @@ public void testRevertSnapshotForCloudStackVolume_UsesFilePatchWithoutTarget() { verify(nasFeignClient).updateFile(anyString(), eq("flexvol-uuid-1"), eq("vm-disk.qcow2"), eq(true), argThat(req -> req != null && Boolean.TRUE.equals(req.isOverwriteEnabled()) + && Boolean.FALSE.equals(req.isFillEnabled()) && "clone-snap-1".equals(req.getPath()) && req.getTarget() == null)); } From edf5ade2c3a4e1c4413ff435e96b49969096e430 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 16 Jun 2026 12:37:48 +0530 Subject: [PATCH 19/24] CSTACKEX-198: revert of CS-voluem faced some issues --- .../storage/feign/client/NASFeignClient.java | 10 +++++----- .../storage/feign/client/SANFeignClient.java | 2 +- .../storage/service/UnifiedNASStrategy.java | 3 ++- .../storage/service/UnifiedSANStrategy.java | 3 ++- .../storage/service/UnifiedNASStrategyTest.java | 11 +---------- .../storage/service/UnifiedSANStrategyTest.java | 9 +-------- 6 files changed, 12 insertions(+), 26 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java index 099b63bdb037..2f30dfd92514 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/NASFeignClient.java @@ -48,11 +48,11 @@ void deleteFile(@Param("authHeader") String authHeader, @RequestLine("PATCH /api/storage/volumes/{volumeUuid}/files/{path}?return_records={returnRecords}") @Headers({"Authorization: {authHeader}"}) - JobResponse updateFile(@Param("authHeader") String authHeader, - @Param("volumeUuid") String volumeUUID, - @Param("path") String filePath, - @Param("returnRecords") boolean returnRecords, - FileInfo fileInfo); + void updateFile(@Param("authHeader") String authHeader, + @Param("volumeUuid") String volumeUUID, + @Param("path") String filePath, + @Param("returnRecords") boolean returnRecords, + FileInfo fileInfo); @RequestLine("POST /api/storage/volumes/{volumeUuid}/files/{path}") @Headers({"Authorization: {authHeader}"}) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java index f08a17685cf7..b97367cc2df6 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SANFeignClient.java @@ -55,7 +55,7 @@ public interface SANFeignClient { @RequestLine("PATCH /api/storage/luns/{uuid}") @Headers({"Authorization: {authHeader}"}) - JobResponse updateLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid, Lun lun); + void updateLun(@Param("authHeader") String authHeader, @Param("uuid") String uuid, Lun lun); @RequestLine("DELETE /api/storage/luns/{uuid}") @Headers({"Authorization: {authHeader}"}) 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 d48c9518e2bf..8442e2414392 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 @@ -476,6 +476,7 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String logger.debug("revertSnapshotForCloudStackVolume [NFS]: patch file source={} destination={} overwrite=true fill=false", snapshotName, volumePath); - return getNasFeignClient().updateFile(authHeader, flexVolUuid, volumePath, true, filePatchRequest); + getNasFeignClient().updateFile(authHeader, flexVolUuid, volumePath, true, filePatchRequest); + return null; } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 63d6bd23bbe3..2f6a7416e1ad 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 @@ -607,7 +607,8 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: patch lun destinationUuid={} sourcePath={} sourceUuid={} destinationLun={}", destinationLunUuid, sourceLunPath, lunUuid, destinationLunPath); - return sanFeignClient.updateLun(authHeader, destinationLunUuid, revertCloneRequest); + sanFeignClient.updateLun(authHeader, destinationLunUuid, revertCloneRequest); + return null; } private String resolveLunUuidByName(String authHeader, String svmName, String lunName) { 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 1d5c1f7eb947..e41020a5ce6f 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 @@ -37,7 +37,6 @@ import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.ExportPolicy; -import org.apache.cloudstack.storage.feign.model.FileInfo; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.response.JobResponse; @@ -587,16 +586,8 @@ public void testDeleteCloudStackVolume_AnswerNull() throws Exception { @Test public void testRevertSnapshotForCloudStackVolume_UsesFilePatchWithoutTarget() { - JobResponse jobResponse = new JobResponse(); - Job job = new Job(); - job.setUuid("job-uuid-1"); - jobResponse.setJob(job); - when(nasFeignClient.updateFile(anyString(), anyString(), anyString(), eq(true), any(FileInfo.class))).thenReturn(jobResponse); - - JobResponse result = strategy.revertSnapshotForCloudStackVolume( + strategy.revertSnapshotForCloudStackVolume( "clone-snap-1", "flexvol-uuid-1", "snap-uuid-1", "vm-disk.qcow2", null, "flexvol1"); - - assertNotNull(result); verify(nasFeignClient).updateFile(anyString(), eq("flexvol-uuid-1"), eq("vm-disk.qcow2"), eq(true), argThat(req -> req != null && Boolean.TRUE.equals(req.isOverwriteEnabled()) 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 08b06b35ee52..f2df1865afad 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 @@ -30,7 +30,6 @@ import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunMap; import org.apache.cloudstack.storage.feign.model.OntapStorage; -import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -1807,16 +1806,11 @@ void testEnsureLunMapped_ExistingMapping_ReturnsExistingNumber() { @Test void testRevertSnapshotForCloudStackVolume_UsesLunPatchWithCloneSource() { - JobResponse jobResponse = new JobResponse(); - org.apache.cloudstack.storage.feign.model.Job job = new org.apache.cloudstack.storage.feign.model.Job(); - job.setUuid("job-uuid-1"); - jobResponse.setJob(job); OntapResponse destinationLunResponse = new OntapResponse<>(); Lun destinationLun = new Lun(); destinationLun.setUuid("dest-lun-uuid-1"); destinationLunResponse.setRecords(List.of(destinationLun)); when(sanFeignClient.getLunResponse(eq(authHeader), anyMap())).thenReturn(destinationLunResponse); - when(sanFeignClient.updateLun(eq(authHeader), eq("dest-lun-uuid-1"), any(Lun.class))).thenReturn(jobResponse); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.generateAuthHeader("admin", "password")) @@ -1826,10 +1820,9 @@ void testRevertSnapshotForCloudStackVolume_UsesLunPatchWithCloneSource() { utilityMock.when(() -> OntapStorageUtils.getLunName("flexvol1", "dest-lun-1")) .thenReturn("/vol/flexvol1/dest-lun-1"); - JobResponse result = unifiedSANStrategy.revertSnapshotForCloudStackVolume( + unifiedSANStrategy.revertSnapshotForCloudStackVolume( "clone-snap-1", "flexvol-uuid-1", "clone-lun-uuid-1", "dest-lun-1", "clone-lun-uuid-1", "flexvol1"); - assertNotNull(result); verify(sanFeignClient).updateLun(eq(authHeader), eq("dest-lun-uuid-1"), argThat(lun -> lun != null && lun.getIsOverride() == null From bbe29aeecfecd489336a95e4a5bd0cc0cba86fcf Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 16 Jun 2026 15:06:28 +0530 Subject: [PATCH 20/24] CSTACKEX-198: patch API return has to be void --- .../driver/OntapPrimaryDatastoreDriver.java | 15 ++++----------- .../storage/service/StorageStrategy.java | 4 ++-- .../storage/service/UnifiedNASStrategy.java | 6 +++--- .../storage/service/UnifiedSANStrategy.java | 6 ++---- .../vmsnapshot/OntapVMSnapshotStrategy.java | 19 ++++++++----------- .../storage/service/StorageStrategyTest.java | 3 +-- 6 files changed, 20 insertions(+), 33 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 44a19338cfc6..77dda31f93a7 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 @@ -914,20 +914,13 @@ public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snaps } // Delegate to strategy class for protocol-specific restore - JobResponse jobResponse = storageStrategy.revertSnapshotForCloudStackVolume( + storageStrategy.revertSnapshotForCloudStackVolume( ontapCloneName, flexVolUuid, ontapCloneId, volumePath, lunUuid, flexVolName); - if (jobResponse == null || jobResponse.getJob() == null) { - throw new CloudRuntimeException("Failed to initiate restore from snapshot [" + - ontapCloneName + "]"); - } - // Poll for job completion (use longer timeout for large LUNs/files) - Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 60, 2000); - if (!jobSucceeded) { - throw new CloudRuntimeException("Restore job failed for snapshot [" + - ontapCloneName + "]"); - } + logger.info("revertSnapshot: iSCSI restore for [{}] completed without async job response; treating as synchronous success", volumePath); + + callback.complete(result); logger.info("revertSnapshot: Successfully restored {} [{}] from clone [{}]", ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) ? "LUN" : "file", 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 91fdb94c2b25..f004cc0e5444 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 @@ -540,9 +540,9 @@ public String getNetworkInterface() { * @param volumePath The path of the file/LUN within the FlexVolume * @param lunUuid The LUN UUID (only for iSCSI, null for NFS) * @param flexVolName The FlexVolume name (only for iSCSI, for constructing destination path) - * @return JobResponse for the async restore operation + * @return void */ - public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, + public abstract void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName); 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 8442e2414392..6e28cad4736d 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 @@ -447,10 +447,10 @@ private FileInfo getFile(String volumeUuid, String filePath) { * @param volumePath The file path within the FlexVolume * @param lunUuid Not used for NFS (null) * @param flexVolName The FlexVolume name (required for CLI API) - * @return JobResponse for the async restore operation + * @return void */ @Override - public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, + public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName) { logger.info("revertSnapshotForCloudStackVolume [NFS]: Reverting file [{}] using clone [{}] on FlexVol [{}]", @@ -477,6 +477,6 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String logger.debug("revertSnapshotForCloudStackVolume [NFS]: patch file source={} destination={} overwrite=true fill=false", snapshotName, volumePath); getNasFeignClient().updateFile(authHeader, flexVolUuid, volumePath, true, filePatchRequest); - return null; + } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 2f6a7416e1ad..7650ccb4d691 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -29,7 +29,6 @@ import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Lun; import org.apache.cloudstack.storage.feign.model.LunMap; -import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.model.AccessGroup; import org.apache.cloudstack.storage.service.model.CloudStackVolume; @@ -556,10 +555,10 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup * @param volumePath The LUN name (used to construct the path) * @param lunUuid The LUN UUID (not used in CLI API, kept for interface consistency) * @param flexVolName The FlexVolume name (required for CLI API) - * @return JobResponse for the async restore operation + * @return void */ @Override - public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, + public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName) { logger.trace("revertSnapshotForCloudStackVolume [iSCSI]: Reverting LUN [{}] from clone [{}] on FlexVol [{}]", @@ -608,7 +607,6 @@ public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: patch lun destinationUuid={} sourcePath={} sourceUuid={} destinationLun={}", destinationLunUuid, sourceLunPath, lunUuid, destinationLunPath); sanFeignClient.updateLun(authHeader, destinationLunUuid, revertCloneRequest); - return null; } private String resolveLunUuidByName(String authHeader, String svmName, String lunName) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index af6fb4b4c5ec..e18d12099d57 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -588,7 +588,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { DeleteVMSnapshotCommand deleteSnapshotCommand = new DeleteVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs, guestOS.getDisplayName()); - // Check for FlexVolume snapshots (new approach) + // Check for FlexVolume snapshots List flexVolDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT); if (CollectionUtils.isNotEmpty(flexVolDetails)) { deleteFlexVolSnapshots(flexVolDetails); @@ -888,16 +888,13 @@ void revertCloneBackedSnapshots(List cloneDetails) { logger.info("revertCloneBackedSnapshots: Reverting volume [{}] using clone source [{}] on FlexVol [{}] (protocol={})", detail.volumePath, detail.snapshotName, flexVolName, detail.protocol); String lunUuid = ProtocolType.ISCSI.name().equalsIgnoreCase(detail.protocol) ? detail.snapshotUuid : null; - JobResponse jobResponse = storageStrategy.revertSnapshotForCloudStackVolume( - detail.snapshotName, detail.flexVolUuid, detail.snapshotUuid, detail.volumePath, lunUuid, flexVolName); - - if (jobResponse != null && jobResponse.getJob() != null) { - Boolean success = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 60, 2000); - if (!success) { - throw new CloudRuntimeException("Clone-backed revert failed for volume path [" + - detail.volumePath + "] from clone [" + detail.snapshotName + - "] on FlexVol [" + flexVolName + "]"); - } + try { + storageStrategy.revertSnapshotForCloudStackVolume( + detail.snapshotName, detail.flexVolUuid, detail.snapshotUuid, detail.volumePath, lunUuid, flexVolName); + } catch (Exception e) { + logger.error("revertCloneBackedSnapshots: Revert of FlexVol snapshot failed: {}", e.getMessage(), e); + throw new CloudRuntimeException("Failed to revert volume [" + detail.volumePath + "] from clone [" + + detail.snapshotName + "] on FlexVol [" + flexVolName + "]: " + e.getMessage(), e); } logger.info("revertCloneBackedSnapshots: Successfully reverted volume [{}] from clone [{}] on FlexVol [{}]", 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 86ef1d7c79b6..2c9060b46de2 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 @@ -144,8 +144,7 @@ public CloudStackVolume getCloudStackVolume(Map cloudStackVolume } @Override - public JobResponse revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName) { - return null; + public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName) { } @Override From 010941bb6a20d943427d5bdd4a7ecc85a54710a8 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 16 Jun 2026 15:19:12 +0530 Subject: [PATCH 21/24] CSTACKEX-198: corected compilation errors --- .../driver/OntapPrimaryDatastoreDriverTest.java | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java index 89f1892bb1df..2d849296d5f6 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 @@ -657,13 +657,7 @@ void testRevertSnapshot_UsesCloneMetadata() { storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - JobResponse jobResponse = new JobResponse(); - Job job = new Job(); - job.setUuid("job-uuid-2"); - jobResponse.setJob(job); - when(storageStrategy.revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString(), anyString())) - .thenReturn(jobResponse); - when(storageStrategy.jobPollForSuccess("job-uuid-2", 60, 2000)).thenReturn(true); + doNothing().when(storageStrategy).revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString(), anyString()); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) @@ -698,13 +692,7 @@ void testRevertSnapshot_FallbacksToLegacySnapshotNameWhenCloneNameMissing() { storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - JobResponse jobResponse = new JobResponse(); - Job job = new Job(); - job.setUuid("job-uuid-legacy"); - jobResponse.setJob(job); - when(storageStrategy.revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString(), anyString())) - .thenReturn(jobResponse); - when(storageStrategy.jobPollForSuccess("job-uuid-legacy", 60, 2000)).thenReturn(true); + doNothing().when(storageStrategy).revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString(), anyString()); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) From 26140dd017bda0e9fe971f6b1f5bebe834006f8e Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 16 Jun 2026 18:00:40 +0530 Subject: [PATCH 22/24] CSTACKEX-198: logger update --- .../storage/driver/OntapPrimaryDatastoreDriver.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 77dda31f93a7..5c56604fb76d 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 @@ -917,10 +917,8 @@ public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snaps storageStrategy.revertSnapshotForCloudStackVolume( ontapCloneName, flexVolUuid, ontapCloneId, volumePath, lunUuid, flexVolName); - - logger.info("revertSnapshot: iSCSI restore for [{}] completed without async job response; treating as synchronous success", volumePath); - - callback.complete(result); + logger.info("revertSnapshot: {} restore for [{}] completed using synchronous PATCH semantics", + ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) ? "iSCSI" : "NFS", volumePath); logger.info("revertSnapshot: Successfully restored {} [{}] from clone [{}]", ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) ? "LUN" : "file", From 2d0a7a334d8d5b2ba474013b91fdaeb5a0c46e43 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 16 Jun 2026 18:53:58 +0530 Subject: [PATCH 23/24] CSTACKEX-198: fixed NFs revert functional issue --- .../cloudstack/storage/service/UnifiedNASStrategy.java | 8 ++++---- .../storage/service/UnifiedNASStrategyTest.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) 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 6e28cad4736d..b2432be9f53c 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 @@ -467,16 +467,16 @@ public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVo } String authHeader = getAuthHeader(); - // Keep PATCH-based revert. ONTAP in this environment rejects "target", so send - // only accepted fields and use "path" to carry source clone file reference. + // Keep PATCH-based revert. ONTAP in this environment rejects "target"; use the + // URL path as source clone file and body "path" as destination live file. FileInfo filePatchRequest = new FileInfo(); - filePatchRequest.setPath(snapshotName); + filePatchRequest.setPath(volumePath); filePatchRequest.setOverwriteEnabled(Boolean.TRUE); filePatchRequest.setFillEnabled(Boolean.FALSE); logger.debug("revertSnapshotForCloudStackVolume [NFS]: patch file source={} destination={} overwrite=true fill=false", snapshotName, volumePath); - getNasFeignClient().updateFile(authHeader, flexVolUuid, volumePath, true, filePatchRequest); + getNasFeignClient().updateFile(authHeader, flexVolUuid, snapshotName, true, filePatchRequest); } } 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 e41020a5ce6f..ccb9908eaf06 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 @@ -588,11 +588,11 @@ public void testDeleteCloudStackVolume_AnswerNull() throws Exception { public void testRevertSnapshotForCloudStackVolume_UsesFilePatchWithoutTarget() { strategy.revertSnapshotForCloudStackVolume( "clone-snap-1", "flexvol-uuid-1", "snap-uuid-1", "vm-disk.qcow2", null, "flexvol1"); - verify(nasFeignClient).updateFile(anyString(), eq("flexvol-uuid-1"), eq("vm-disk.qcow2"), eq(true), argThat(req -> + verify(nasFeignClient).updateFile(anyString(), eq("flexvol-uuid-1"), eq("clone-snap-1"), eq(true), argThat(req -> req != null && Boolean.TRUE.equals(req.isOverwriteEnabled()) && Boolean.FALSE.equals(req.isFillEnabled()) - && "clone-snap-1".equals(req.getPath()) + && "vm-disk.qcow2".equals(req.getPath()) && req.getTarget() == null)); } From dc0b10bb7b963260baa549514c75d4a302628f8c Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Wed, 17 Jun 2026 14:12:47 +0530 Subject: [PATCH 24/24] CSTACKEX-198: kept code as per the protocol based class hierarchy and removing foor prints of flexvolume --- .../driver/OntapPrimaryDatastoreDriver.java | 226 ++++------------ .../storage/service/StorageStrategy.java | 48 +++- .../storage/service/UnifiedNASStrategy.java | 158 +++++++++-- .../storage/service/UnifiedSANStrategy.java | 98 +++++-- .../storage/utils/OntapStorageConstants.java | 7 +- .../vmsnapshot/OntapVMSnapshotStrategy.java | 251 +++++------------- .../OntapPrimaryDatastoreDriverTest.java | 46 +--- .../storage/service/StorageStrategyTest.java | 11 +- .../service/UnifiedNASStrategyTest.java | 33 ++- .../service/UnifiedSANStrategyTest.java | 6 +- .../OntapVMSnapshotStrategyTest.java | 14 +- 11 files changed, 439 insertions(+), 459 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 5c56604fb76d..fdc21fd7d00f 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 @@ -54,11 +54,7 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.Svm; -import org.apache.cloudstack.storage.feign.model.response.JobResponse; -import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.SANStrategy; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; @@ -210,7 +206,7 @@ private CloudStackVolume createCloudStackVolume(StoragePoolVO storagePool, Volum * Deletes a volume or snapshot from the ONTAP storage system. * *

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

+ * For snapshots, deletes the clone artifact from ONTAP that was created by takeSnapshot.

*/ @Override public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallback callback) { @@ -237,7 +233,7 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac commandResult.setSuccess(true); } else if (data.getType() == DataObjectType.SNAPSHOT) { // Delete the clone object (file/LUN) that was created by takeSnapshot - deleteOntapSnapshot((SnapshotInfo) data, commandResult); + deleteCloneBackedSnapshot((SnapshotInfo) data, commandResult); } else { throw new CloudRuntimeException("Unsupported data object type: " + data.getType()); } @@ -253,9 +249,9 @@ public void deleteAsync(DataStore store, DataObject data, AsyncCompletionCallbac /** * Deletes a clone-backed ONTAP snapshot object (NFS file clone or iSCSI LUN clone). */ - private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult commandResult) { + private void deleteCloneBackedSnapshot(SnapshotInfo snapshotInfo, CommandResult commandResult) { long snapshotId = snapshotInfo.getId(); - logger.info("deleteOntapSnapshot: Deleting clone-backed ONTAP snapshot object for CloudStack snapshot [{}]", snapshotId); + logger.info("deleteCloneBackedSnapshot: Deleting clone-backed ONTAP object for CloudStack snapshot [{}]", snapshotId); try { String flexVolUuid = getSnapshotDetail(snapshotId, OntapStorageConstants.BASE_ONTAP_FV_ID); @@ -265,7 +261,7 @@ private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult comman String protocol = getSnapshotDetail(snapshotId, OntapStorageConstants.PROTOCOL); if (poolIdStr == null || protocol == null || cloneName == null) { - logger.warn("deleteOntapSnapshot: Missing clone metadata for snapshot [{}]. " + + logger.warn("deleteCloneBackedSnapshot: Missing clone metadata for snapshot [{}]. " + "poolId={}, protocol={}, cloneName={}. Treating as success.", snapshotId, poolIdStr, protocol, cloneName); commandResult.setSuccess(true); @@ -277,30 +273,16 @@ private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult comman Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(poolId); StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - String authHeader = storageStrategy.getAuthHeader(); - String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); - - if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { - if (flexVolUuid == null || flexVolUuid.isEmpty()) { - logger.warn("deleteOntapSnapshot: Missing FlexVol UUID for NFS clone delete on snapshot [{}]. Treating as success.", snapshotId); - commandResult.setSuccess(true); - commandResult.setResult(null); - return; - } - logger.info("deleteOntapSnapshot: Deleting NFS clone file [{}] on FlexVol [{}]", cloneName, flexVolUuid); - storageStrategy.getNasFeignClient().deleteFile(authHeader, flexVolUuid, cloneName); - } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { - if (cloneUuid == null || cloneUuid.isEmpty()) { - String cloneLunPath = OntapStorageUtils.getLunName(poolDetails.get(OntapStorageConstants.VOLUME_NAME), cloneName); - cloneUuid = resolveLunUuidByName(storageStrategy, authHeader, svmName, cloneLunPath); - } - logger.info("deleteOntapSnapshot: Deleting iSCSI clone LUN [{}] (uuid={})", cloneName, cloneUuid); - storageStrategy.getSanFeignClient().deleteLun(authHeader, cloneUuid, Map.of("allow_delete_while_mapped", "true")); - } else { - throw new CloudRuntimeException("Unsupported protocol for snapshot delete: " + protocol); + if (flexVolUuid == null || flexVolUuid.isEmpty()) { + logger.warn("deleteCloneBackedSnapshot: Missing FlexVol UUID for clone delete on snapshot [{}]. Treating as success.", snapshotId); + commandResult.setSuccess(true); + commandResult.setResult(null); + return; } + storageStrategy.deleteSnapshotClone(flexVolUuid, + poolDetails.get(OntapStorageConstants.VOLUME_NAME), cloneName, cloneUuid); - logger.info("deleteOntapSnapshot: Successfully deleted clone object [{}] for CloudStack snapshot [{}]", + logger.info("deleteCloneBackedSnapshot: Successfully deleted clone object [{}] for CloudStack snapshot [{}]", cloneName, snapshotId); commandResult.setSuccess(true); @@ -311,12 +293,12 @@ private void deleteOntapSnapshot(SnapshotInfo snapshotInfo, CommandResult comman String errorMsg = e.getMessage(); if (errorMsg != null && (errorMsg.contains("404") || errorMsg.contains("not found") || errorMsg.contains("does not exist"))) { - logger.warn("deleteOntapSnapshot: Snapshot clone object for CloudStack snapshot [{}] not found, " + + logger.warn("deleteCloneBackedSnapshot: Snapshot clone object for CloudStack snapshot [{}] not found, " + "may have been already deleted. Treating as success.", snapshotId); commandResult.setSuccess(true); commandResult.setResult(null); } else { - logger.error("deleteOntapSnapshot: Failed to delete ONTAP snapshot for CloudStack snapshot [{}]: {}", + logger.error("deleteCloneBackedSnapshot: Failed to delete clone-backed snapshot object for CloudStack snapshot [{}]: {}", snapshotId, e.getMessage(), e); commandResult.setSuccess(false); commandResult.setResult(e.getMessage()); @@ -618,16 +600,12 @@ public long getUsedIops(StoragePool storagePool) { } /** - * Takes a snapshot by creating an ONTAP FlexVolume-level snapshot. - * - *

This method creates a point-in-time, space-efficient snapshot of the entire - * FlexVolume containing the CloudStack volume. FlexVolume snapshots are atomic - * and capture all files/LUNs within the volume at the moment of creation.

+ * Takes a snapshot by creating an ONTAP clone artifact. * - *

Both NFS and iSCSI protocols use the same FlexVolume snapshot approach: + *

Both NFS and iSCSI protocols use clone-backed snapshot semantics: *

    - *
  • NFS: The QCOW2 file is captured within the FlexVolume snapshot
    • - *
  • iSCSI: The LUN is captured within the FlexVolume snapshot
    • + *
  • NFS: Create a file clone from the live QCOW2 file
    • + *
  • iSCSI: Create a LUN clone from the live LUN
    • *
*

* @@ -639,12 +617,11 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback poolDetails = null; try { VolumeInfo volumeInfo = snapshot.getBaseVolume(); @@ -660,103 +637,36 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(volumeVO.getPoolId()); + poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(volumeVO.getPoolId()); protocol = poolDetails.get(OntapStorageConstants.PROTOCOL); flexVolUuid = poolDetails.get(OntapStorageConstants.VOLUME_UUID); - svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); if (flexVolUuid == null || flexVolUuid.isEmpty()) { throw new CloudRuntimeException("FlexVolume UUID not found in pool details for pool " + volumeVO.getPoolId()); } storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - authHeader = storageStrategy.getAuthHeader(); SnapshotObjectTO snapshotObjectTo = (SnapshotObjectTO) snapshot.getTO(); String cloudStackSnapshotName = snapshot.getName(); cloneName = OntapStorageUtils.getOntapCloneName(cloudStackSnapshotName); String volumePath = resolveVolumePathOnOntap(volumeVO, protocol, poolDetails); - String cloneId = null; String lunUuid = null; - JobResponse nfsJobResponse = null; - - if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { - FileCloneRequest fileCloneRequest = new FileCloneRequest(); - FileCloneRequest.VolumeRef volumeRef = new FileCloneRequest.VolumeRef(); - volumeRef.setUuid(flexVolUuid); - volumeRef.setName(poolDetails.get(OntapStorageConstants.VOLUME_NAME)); - fileCloneRequest.setVolume(volumeRef); - fileCloneRequest.setSourcePath(volumePath); - fileCloneRequest.setDestinationPath(cloneName); - logger.info("takeSnapshot: Creating NFS file clone [{}] from source [{}] on FlexVol UUID [{}]", - cloneName, volumePath, flexVolUuid); - nfsJobResponse = storageStrategy.getNasFeignClient().cloneFile(authHeader, fileCloneRequest); - cloneId = cloneName; - } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { + String sourceObjectUuid = null; + + if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_UUID); lunUuid = lunDetail != null ? lunDetail.getValue() : null; if (lunUuid == null) { throw new CloudRuntimeException("LUN UUID not found for iSCSI volume " + volumeVO.getId()); } - if (volumePath == null || volumePath.isEmpty()) { - throw new CloudRuntimeException("Source LUN path is missing for iSCSI volume " + volumeVO.getId()); - } - if (!volumePath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { - throw new CloudRuntimeException("Invalid source LUN path (must start with " + - OntapStorageConstants.VOLUME_PATH_PREFIX + "): " + volumePath); - } - cloneLunPath = OntapStorageUtils.getLunName( - poolDetails.get(OntapStorageConstants.VOLUME_NAME), cloneName); - if (!cloneLunPath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { - throw new CloudRuntimeException("Invalid iSCSI clone LUN path generated: " + cloneLunPath); - } - String svmNameForClone = poolDetails.get(OntapStorageConstants.SVM_NAME); - String flexVolNameForClone = poolDetails.get(OntapStorageConstants.VOLUME_NAME); - if (svmNameForClone == null || svmNameForClone.isEmpty()) { - throw new CloudRuntimeException("SVM name is mandatory for iSCSI clone request"); - } - if (flexVolNameForClone == null || flexVolNameForClone.isEmpty()) { - throw new CloudRuntimeException("FlexVolume name is mandatory for iSCSI clone request"); - } - Lun cloneRequest = new Lun(); - cloneRequest.setName(cloneLunPath); - Svm svm = new Svm(); - svm.setName(svmNameForClone); - cloneRequest.setSvm(svm); - Lun.Location location = new Lun.Location(); - Lun.LocationVolume locationVolume = new Lun.LocationVolume(); - locationVolume.setName(flexVolNameForClone); - location.setVolume(locationVolume); - cloneRequest.setLocation(location); - Lun.Clone clone = new Lun.Clone(); - Lun.Source source = new Lun.Source(); - source.setName(volumePath); - source.setUuid(lunUuid); - clone.setSource(source); - cloneRequest.setClone(clone); - logger.info("takeSnapshot: Creating iSCSI LUN clone [{}] from source LUN UUID [{}]", cloneName, lunUuid); - OntapResponse createCloneResponse = storageStrategy.getSanFeignClient().createLun(authHeader, true, cloneRequest); - if (createCloneResponse == null || createCloneResponse.getRecords() == null || createCloneResponse.getRecords().isEmpty()) { - throw new CloudRuntimeException("Failed to create iSCSI clone LUN for volume " + volumeVO.getId()); - } - cloneId = createCloneResponse.getRecords().get(0).getUuid(); - if (cloneId == null || cloneId.isEmpty()) { - cloneId = resolveLunUuidByName(storageStrategy, authHeader, svmNameForClone, cloneLunPath); - } - } else { - throw new CloudRuntimeException("Unsupported protocol for snapshot clone: " + protocol); + sourceObjectUuid = lunUuid; } - if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { - if (nfsJobResponse == null || nfsJobResponse.getJob() == null) { - throw new CloudRuntimeException("Failed to initiate clone-backed snapshot for volume " + volumeVO.getId()); - } - // Poll for async NFS clone completion - Boolean jobSucceeded = storageStrategy.jobPollForSuccess(nfsJobResponse.getJob().getUuid(), 30, 2000); - if (!jobSucceeded) { - throw new CloudRuntimeException("Clone create job failed for snapshot " + cloudStackSnapshotName); - } - } + logger.info("takeSnapshot: Creating {} clone [{}] from source [{}] on FlexVol [{}]", + protocol, cloneName, volumePath, flexVolUuid); + cloneId = storageStrategy.createSnapshotClone( + flexVolUuid, poolDetails.get(OntapStorageConstants.VOLUME_NAME), volumePath, cloneName, sourceObjectUuid); snapshotObjectTo.setPath(OntapStorageConstants.ONTAP_CLONE_NAME + "=" + cloneName); @@ -772,8 +682,8 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback lunResponse = storageStrategy.getSanFeignClient().getLunResponse(authHeader, - Map.of(OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.NAME, lunName)); - if (lunResponse == null || lunResponse.getRecords() == null || lunResponse.getRecords().isEmpty()) { - throw new CloudRuntimeException("Failed to resolve LUN UUID for clone " + lunName); - } - return lunResponse.getRecords().get(0).getUuid(); - } - /** - * Reverts a volume to a snapshot using protocol-specific ONTAP restore APIs. + * Reverts a volume using protocol-specific ONTAP clone restore APIs. * *

This method delegates to the appropriate StorageStrategy to restore the - * specific file (NFS) or LUN (iSCSI) from the FlexVolume snapshot directly - * via ONTAP REST API, without involving the hypervisor agent.

+ * specific file (NFS) or LUN (iSCSI) from clone artifacts directly via ONTAP + * REST API, without involving the hypervisor agent.

* *

Protocol-specific handling (delegated to strategy classes):

*
    - *
  • NFS (UnifiedNASStrategy): Uses the single-file restore API: - * {@code POST /api/storage/volumes/{volume_uuid}/snapshots/{snapshot_uuid}/files/{file_path}/restore} - * Restores the QCOW2 file from the FlexVolume snapshot to its original location.
    • - *
  • iSCSI (UnifiedSANStrategy): Uses the LUN restore API: - * {@code POST /api/storage/luns/{lun.uuid}/restore} - * Restores the LUN data from the snapshot to the specified destination path.
    • + *
  • NFS (UnifiedNASStrategy): Creates/restores data from file clone artifacts.
    • + *
  • iSCSI (UnifiedSANStrategy): Restores data from LUN clone artifacts via LUN patch clone source.
    • *
*/ @Override @@ -901,21 +789,15 @@ public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snaps StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - // Get the FlexVolume name (required for CLI-based restore API for all protocols) + // Get the FlexVolume name required by strategy clone-restore operations. String flexVolName = poolDetails.get(OntapStorageConstants.VOLUME_NAME); if (flexVolName == null || flexVolName.isEmpty()) { throw new CloudRuntimeException("FlexVolume name not found in pool details for pool " + poolId); } - // Prepare protocol-specific parameters (lunUuid is only needed for backward compatibility) - String lunUuid = null; - if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { - lunUuid = ontapCloneId; - } - - // Delegate to strategy class for protocol-specific restore + // Delegate protocol-specific restore behavior to SAN/NAS strategy implementations storageStrategy.revertSnapshotForCloudStackVolume( - ontapCloneName, flexVolUuid, ontapCloneId, volumePath, lunUuid, flexVolName); + ontapCloneName, flexVolUuid, ontapCloneId, volumePath, flexVolName); logger.info("revertSnapshot: {} restore for [{}] completed using synchronous PATCH semantics", ProtocolType.ISCSI.name().equalsIgnoreCase(protocol) ? "iSCSI" : "NFS", volumePath); @@ -1041,17 +923,17 @@ private CloudStackVolume createDeleteCloudStackVolumeRequest(StoragePool storage * @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 ontapCloneId ONTAP clone artifact UUID + * @param snapshotName CloudStack snapshot display name + * @param volumePath Path of the source volume file/LUN on ONTAP * @param storagePoolId Primary storage pool ID * @param protocol Storage protocol (NFS3 or ISCSI) * @param lunUuid LUN UUID (only for iSCSI, null for NFS) */ private void updateSnapshotDetails(long csSnapshotId, long csVolumeId, String flexVolUuid, - String ontapCloneId, String snapshotName, String ontapCloneName, - String volumePath, long storagePoolId, String protocol, - String lunUuid) { + String ontapCloneId, String snapshotName, String ontapCloneName, + String volumePath, long storagePoolId, String protocol, + String lunUuid) { SnapshotDetailsVO snapshotDetail = new SnapshotDetailsVO(csSnapshotId, OntapStorageConstants.SRC_CS_VOLUME_ID, String.valueOf(csVolumeId), false); snapshotDetailsDao.persist(snapshotDetail); @@ -1088,7 +970,7 @@ private void updateSnapshotDetails(long csSnapshotId, long csVolumeId, String fl OntapStorageConstants.PROTOCOL, protocol, false); snapshotDetailsDao.persist(snapshotDetail); - // Store LUN UUID for iSCSI volumes (required for LUN restore API) + // Store source/live LUN UUID for iSCSI volumes (used during clone-based restore). if (lunUuid != null && !lunUuid.isEmpty()) { snapshotDetail = new SnapshotDetailsVO(csSnapshotId, OntapStorageConstants.LUN_DOT_UUID, lunUuid, 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 f004cc0e5444..8abdaac55bb2 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 @@ -524,27 +524,51 @@ public String getNetworkInterface() { abstract public CloudStackVolume getCloudStackVolume(Map cloudStackVolumeMap); /** - * Reverts a CloudStack volume to a snapshot using protocol-specific ONTAP APIs. + * Reverts a CloudStack volume from a clone artifact using protocol-specific ONTAP APIs. * - *

This method encapsulates the snapshot revert behavior based on protocol:

+ *

This method encapsulates clone-based restore behavior based on protocol:

*
    - *
  • iSCSI/FC: Uses {@code POST /api/storage/luns/{lun.uuid}/restore} - * to restore LUN data from the FlexVolume snapshot.
    • - *
  • NFS: Uses {@code POST /api/storage/volumes/{vol.uuid}/snapshots/{snap.uuid}/files/{path}/restore} - * to restore a single file from the FlexVolume snapshot.
    • + *
  • iSCSI/FC: restores destination LUN data from a source clone LUN.
    • + *
  • NFS: restores destination file data from a source clone file.
    • *
* - * @param snapshotName The ONTAP FlexVolume snapshot name - * @param flexVolUuid The FlexVolume UUID containing the snapshot - * @param snapshotUuid The ONTAP snapshot UUID (used for NFS file restore) - * @param volumePath The path of the file/LUN within the FlexVolume - * @param lunUuid The LUN UUID (only for iSCSI, null for NFS) + * @param snapshotName The ONTAP source clone name/path token + * @param flexVolUuid The FlexVolume UUID containing the source clone + * @param snapshotUuid The ONTAP clone artifact UUID (used by SAN restore, ignored by NAS) + * @param volumePath The destination file/LUN path in the FlexVolume * @param flexVolName The FlexVolume name (only for iSCSI, for constructing destination path) * @return void */ public abstract void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, - String lunUuid, String flexVolName); + String flexVolName); + + /** + * Creates a protocol-specific clone artifact used to represent a snapshot point. + * + *

Implementations own all protocol-specific ONTAP interactions including any + * required async job polling.

+ * + * @param flexVolUuid FlexVolume UUID that contains the source object + * @param flexVolName FlexVolume name for path construction when needed + * @param sourcePath source file/LUN path + * @param cloneName destination clone name/path token + * @param sourceObjectUuid source object UUID (required by SAN, ignored by NAS) + * @return clone artifact UUID (or stable clone identifier for NAS) + */ + public abstract String createSnapshotClone(String flexVolUuid, String flexVolName, String sourcePath, + String cloneName, String sourceObjectUuid); + + /** + * Deletes a protocol-specific clone artifact created for snapshot workflows. + * + * @param flexVolUuid FlexVolume UUID containing the clone + * @param flexVolName FlexVolume name for path construction when needed + * @param cloneName clone name/path token + * @param cloneObjectUuid clone UUID (required by SAN, ignored by NAS) + */ + public abstract void deleteSnapshotClone(String flexVolUuid, String flexVolName, String cloneName, + String cloneObjectUuid); /** 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 b2432be9f53c..e9cdfe50ac10 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 @@ -34,6 +34,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.feign.model.ExportPolicy; import org.apache.cloudstack.storage.feign.model.ExportRule; +import org.apache.cloudstack.storage.feign.model.FileCloneRequest; import org.apache.cloudstack.storage.feign.model.FileInfo; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.Nas; @@ -433,26 +434,16 @@ private FileInfo getFile(String volumeUuid, String filePath) { } /** - * Reverts a file to a snapshot using the ONTAP CLI-based snapshot file restore API. + * Reverts an NFS file from clone artifact data. * - *

ONTAP REST API (CLI passthrough): - * {@code POST /api/private/cli/volume/snapshot/restore-file}

- * - *

This method uses the CLI native API which is more reliable and works - * consistently for both NFS files and iSCSI LUNs.

- * - * @param snapshotName The ONTAP FlexVolume snapshot name - * @param flexVolUuid The FlexVolume UUID (not used in CLI API, kept for interface consistency) - * @param snapshotUuid The ONTAP snapshot UUID (not used in CLI API, kept for interface consistency) - * @param volumePath The file path within the FlexVolume - * @param lunUuid Not used for NFS (null) - * @param flexVolName The FlexVolume name (required for CLI API) - * @return void + *

The source clone file is copied back into the destination live file path. + * If destination already exists, this method uses a temporary backup path to + * preserve rollback safety.

*/ @Override public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, - String lunUuid, String flexVolName) { + String flexVolName) { logger.info("revertSnapshotForCloudStackVolume [NFS]: Reverting file [{}] using clone [{}] on FlexVol [{}]", volumePath, snapshotName, flexVolName); @@ -467,16 +458,135 @@ public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVo } String authHeader = getAuthHeader(); - // Keep PATCH-based revert. ONTAP in this environment rejects "target"; use the - // URL path as source clone file and body "path" as destination live file. - FileInfo filePatchRequest = new FileInfo(); - filePatchRequest.setPath(volumePath); - filePatchRequest.setOverwriteEnabled(Boolean.TRUE); - filePatchRequest.setFillEnabled(Boolean.FALSE); + FileCloneRequest cloneRequest = new FileCloneRequest(); + FileCloneRequest.VolumeRef volumeRef = new FileCloneRequest.VolumeRef(); + volumeRef.setUuid(flexVolUuid); + volumeRef.setName(flexVolName); + cloneRequest.setVolume(volumeRef); + cloneRequest.setSourcePath(snapshotName); + cloneRequest.setDestinationPath(volumePath); + + try { + logger.debug("revertSnapshotForCloudStackVolume [NFS]: clone file source={} destination={}", + snapshotName, volumePath); + JobResponse cloneJobResponse = getNasFeignClient().cloneFile(authHeader, cloneRequest); + if (cloneJobResponse == null || cloneJobResponse.getJob() == null || + cloneJobResponse.getJob().getUuid() == null || cloneJobResponse.getJob().getUuid().isEmpty()) { + throw new CloudRuntimeException(String.format( + "cloneFile did not return a valid job response for source [%s] and destination [%s]", + snapshotName, volumePath)); + } + String cloneJobUuid = cloneJobResponse.getJob().getUuid(); + Boolean jobSucceeded = jobPollForSuccess(cloneJobUuid, 30, 2000); + if (jobSucceeded == null || !jobSucceeded) { + throw new CloudRuntimeException(String.format( + "cloneFile job [%s] failed for source [%s] and destination [%s]", + cloneJobUuid, snapshotName, volumePath)); + } + } catch (FeignException cloneEx) { + if (!isFileAlreadyExistsConflict(cloneEx)) { + throw cloneEx; + } + + String backupPath = volumePath + ".pre_revert_" + System.currentTimeMillis(); + logger.info("revertSnapshotForCloudStackVolume [NFS]: Destination [{}] exists, using backup path [{}] for safe restore", + volumePath, backupPath); + + FileInfo renameToBackup = new FileInfo(); + renameToBackup.setPath(backupPath); + getNasFeignClient().updateFile(authHeader, flexVolUuid, volumePath, true, renameToBackup); + + try { + JobResponse cloneJobResponse = getNasFeignClient().cloneFile(authHeader, cloneRequest); + if (cloneJobResponse == null || cloneJobResponse.getJob() == null || + cloneJobResponse.getJob().getUuid() == null || cloneJobResponse.getJob().getUuid().isEmpty()) { + throw new CloudRuntimeException(String.format( + "cloneFile did not return a valid job response for source [%s] and destination [%s]", + snapshotName, volumePath)); + } + String cloneJobUuid = cloneJobResponse.getJob().getUuid(); + Boolean jobSucceeded = jobPollForSuccess(cloneJobUuid, 30, 2000); + if (jobSucceeded == null || !jobSucceeded) { + throw new CloudRuntimeException(String.format( + "cloneFile job [%s] failed for source [%s] and destination [%s]", + cloneJobUuid, snapshotName, volumePath)); + } + try { + getNasFeignClient().deleteFile(authHeader, flexVolUuid, backupPath); + } catch (Exception cleanupEx) { + logger.warn("revertSnapshotForCloudStackVolume [NFS]: Backup cleanup failed for [{}]: {}", + backupPath, cleanupEx.getMessage()); + } + } catch (Exception restoreEx) { + try { + FileInfo rollbackRename = new FileInfo(); + rollbackRename.setPath(volumePath); + getNasFeignClient().updateFile(authHeader, flexVolUuid, backupPath, true, rollbackRename); + } catch (Exception rollbackEx) { + logger.error("revertSnapshotForCloudStackVolume [NFS]: Failed to roll back backup rename [{}] -> [{}]: {}", + backupPath, volumePath, rollbackEx.getMessage(), rollbackEx); + } + throw restoreEx; + } + } + + } + + @Override + public String createSnapshotClone(String flexVolUuid, String flexVolName, String sourcePath, + String cloneName, String sourceObjectUuid) { + if (flexVolUuid == null || flexVolUuid.isEmpty()) { + throw new CloudRuntimeException("FlexVolume UUID is required for NFS snapshot clone create"); + } + if (sourcePath == null || sourcePath.isEmpty()) { + throw new CloudRuntimeException("Source file path is required for NFS snapshot clone create"); + } + if (cloneName == null || cloneName.isEmpty()) { + throw new CloudRuntimeException("Clone file name is required for NFS snapshot clone create"); + } + + FileCloneRequest fileCloneRequest = new FileCloneRequest(); + FileCloneRequest.VolumeRef volumeRef = new FileCloneRequest.VolumeRef(); + volumeRef.setUuid(flexVolUuid); + volumeRef.setName(flexVolName); + fileCloneRequest.setVolume(volumeRef); + fileCloneRequest.setSourcePath(sourcePath); + fileCloneRequest.setDestinationPath(cloneName); + + JobResponse jobResponse = getNasFeignClient().cloneFile(getAuthHeader(), fileCloneRequest); + if (jobResponse == null || jobResponse.getJob() == null || + jobResponse.getJob().getUuid() == null || jobResponse.getJob().getUuid().isEmpty()) { + throw new CloudRuntimeException("Failed to initiate NFS clone create for " + cloneName); + } + Boolean jobSucceeded = jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); + if (jobSucceeded == null || !jobSucceeded) { + throw new CloudRuntimeException("NFS clone create job failed for " + cloneName); + } + return cloneName; + } - logger.debug("revertSnapshotForCloudStackVolume [NFS]: patch file source={} destination={} overwrite=true fill=false", - snapshotName, volumePath); - getNasFeignClient().updateFile(authHeader, flexVolUuid, snapshotName, true, filePatchRequest); + @Override + public void deleteSnapshotClone(String flexVolUuid, String flexVolName, String cloneName, String cloneObjectUuid) { + if (flexVolUuid == null || flexVolUuid.isEmpty()) { + throw new CloudRuntimeException("FlexVolume UUID is required for NFS snapshot clone delete"); + } + if (cloneName == null || cloneName.isEmpty()) { + throw new CloudRuntimeException("Clone file name is required for NFS snapshot clone delete"); + } + getNasFeignClient().deleteFile(getAuthHeader(), flexVolUuid, cloneName); + } + private boolean isFileAlreadyExistsConflict(FeignException e) { + if (e.status() == 409) { + return true; + } + String message = e.getMessage(); + if (message == null) { + return false; + } + String lower = message.toLowerCase(); + return lower.contains("already exists") + || lower.contains("entry exists") + || lower.contains("duplicate"); } } 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 7650ccb4d691..ab9eb84826b9 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 @@ -541,26 +541,15 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup return response; } /** - * Reverts a LUN to a snapshot using the ONTAP CLI-based snapshot file restore API. + * Reverts a destination LUN from source clone LUN data. * - *

ONTAP REST API (CLI passthrough): - * {@code POST /api/private/cli/volume/snapshot/restore-file}

- * - *

This method uses the CLI native API which is more reliable and works - * consistently for both NFS files and iSCSI LUNs.

- * - * @param snapshotName The ONTAP FlexVolume snapshot name - * @param flexVolUuid The FlexVolume UUID (not used in CLI API, kept for interface consistency) - * @param snapshotUuid The ONTAP snapshot UUID (not used in CLI API, kept for interface consistency) - * @param volumePath The LUN name (used to construct the path) - * @param lunUuid The LUN UUID (not used in CLI API, kept for interface consistency) - * @param flexVolName The FlexVolume name (required for CLI API) - * @return void + *

Uses LUN PATCH clone-source semantics to overwrite the destination LUN + * contents while retaining clone metadata and strategy-driven path resolution.

*/ @Override public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, - String lunUuid, String flexVolName) { + String flexVolName) { logger.trace("revertSnapshotForCloudStackVolume [iSCSI]: Reverting LUN [{}] from clone [{}] on FlexVol [{}]", volumePath, snapshotName, flexVolName); @@ -573,7 +562,7 @@ public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVo if (flexVolName == null || flexVolName.isEmpty()) { throw new CloudRuntimeException("FlexVolume name is required for iSCSI snapshot revert"); } - if (lunUuid == null || lunUuid.isEmpty()) { + if (snapshotUuid == null || snapshotUuid.isEmpty()) { throw new CloudRuntimeException("Source clone LUN UUID is required for iSCSI snapshot revert"); } if (storage.getSvmName() == null || storage.getSvmName().isEmpty()) { @@ -600,15 +589,88 @@ public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVo Lun.Clone clone = new Lun.Clone(); Lun.Source source = new Lun.Source(); source.setName(sourceLunPath); - source.setUuid(lunUuid); + source.setUuid(snapshotUuid); clone.setSource(source); revertCloneRequest.setClone(clone); logger.debug("revertSnapshotForCloudStackVolume [iSCSI]: patch lun destinationUuid={} sourcePath={} sourceUuid={} destinationLun={}", - destinationLunUuid, sourceLunPath, lunUuid, destinationLunPath); + destinationLunUuid, sourceLunPath, snapshotUuid, destinationLunPath); sanFeignClient.updateLun(authHeader, destinationLunUuid, revertCloneRequest); } + @Override + public String createSnapshotClone(String flexVolUuid, String flexVolName, String sourcePath, + String cloneName, String sourceObjectUuid) { + if (sourceObjectUuid == null || sourceObjectUuid.isEmpty()) { + throw new CloudRuntimeException("Source LUN UUID is required for iSCSI snapshot clone create"); + } + if (sourcePath == null || sourcePath.isEmpty()) { + throw new CloudRuntimeException("Source LUN path is required for iSCSI snapshot clone create"); + } + if (flexVolName == null || flexVolName.isEmpty()) { + throw new CloudRuntimeException("FlexVolume name is required for iSCSI snapshot clone create"); + } + + String sourceLunPath = sourcePath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX) + ? sourcePath : OntapStorageUtils.getLunName(flexVolName, sourcePath); + String cloneLunPath = cloneName.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX) + ? cloneName : OntapStorageUtils.getLunName(flexVolName, cloneName); + + if (!sourceLunPath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { + throw new CloudRuntimeException("Invalid source LUN path for iSCSI snapshot clone create: " + sourceLunPath); + } + if (!cloneLunPath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { + throw new CloudRuntimeException("Invalid clone LUN path for iSCSI snapshot clone create: " + cloneLunPath); + } + if (storage.getSvmName() == null || storage.getSvmName().isEmpty()) { + throw new CloudRuntimeException("SVM name is mandatory for iSCSI snapshot clone create"); + } + + Lun cloneRequest = new Lun(); + cloneRequest.setName(cloneLunPath); + Svm svm = new Svm(); + svm.setName(storage.getSvmName()); + cloneRequest.setSvm(svm); + Lun.Location location = new Lun.Location(); + Lun.LocationVolume locationVolume = new Lun.LocationVolume(); + locationVolume.setName(flexVolName); + location.setVolume(locationVolume); + cloneRequest.setLocation(location); + Lun.Clone clone = new Lun.Clone(); + Lun.Source source = new Lun.Source(); + source.setName(sourceLunPath); + source.setUuid(sourceObjectUuid); + clone.setSource(source); + cloneRequest.setClone(clone); + + OntapResponse createCloneResponse = sanFeignClient.createLun(getAuthHeader(), true, cloneRequest); + if (createCloneResponse == null || createCloneResponse.getRecords() == null || createCloneResponse.getRecords().isEmpty()) { + throw new CloudRuntimeException("Failed to create iSCSI clone LUN for source path " + sourceLunPath); + } + String cloneUuid = createCloneResponse.getRecords().get(0).getUuid(); + if (cloneUuid == null || cloneUuid.isEmpty()) { + cloneUuid = resolveLunUuidByName(getAuthHeader(), storage.getSvmName(), cloneLunPath); + } + return cloneUuid; + } + + @Override + public void deleteSnapshotClone(String flexVolUuid, String flexVolName, String cloneName, String cloneObjectUuid) { + String cloneUuid = cloneObjectUuid; + if (cloneUuid == null || cloneUuid.isEmpty()) { + if (cloneName == null || cloneName.isEmpty()) { + throw new CloudRuntimeException("Clone LUN name is required to resolve UUID for iSCSI snapshot clone delete"); + } + if (flexVolName == null || flexVolName.isEmpty()) { + throw new CloudRuntimeException("FlexVolume name is required to resolve clone UUID for iSCSI snapshot clone delete"); + } + String cloneLunPath = cloneName.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX) + ? cloneName : OntapStorageUtils.getLunName(flexVolName, cloneName); + cloneUuid = resolveLunUuidByName(getAuthHeader(), storage.getSvmName(), cloneLunPath); + } + sanFeignClient.deleteLun(getAuthHeader(), cloneUuid, Map.of("allow_delete_while_mapped", "true")); + } + private String resolveLunUuidByName(String authHeader, String svmName, String lunName) { OntapResponse response = sanFeignClient.getLunResponse(authHeader, Map.of(OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.NAME, lunName)); 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 f6b5f25a0e4f..dd2844852823 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 @@ -108,6 +108,9 @@ public class OntapStorageConstants { public static final String FILE_PATH = "file_path"; public static final int MAX_SNAPSHOT_NAME_LENGTH = 256; - /** vm_snapshot_details key for ONTAP FlexVolume-level VM snapshots. */ - public static final String ONTAP_FLEXVOL_SNAPSHOT = "ontapFlexVolSnapshot"; + /** vm_snapshot_details key for ONTAP clone-backed VM snapshot volume entries. */ + public static final String ONTAP_CLONE_SNAPSHOT_DETAIL = "ontapFlexVolSnapshot"; + /** @deprecated Use {@link #ONTAP_CLONE_SNAPSHOT_DETAIL}. Kept for compatibility. */ + @Deprecated + public static final String ONTAP_FLEXVOL_SNAPSHOT = ONTAP_CLONE_SNAPSHOT_DETAIL; } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index e18d12099d57..3e6bee299e0a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -32,9 +32,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.response.JobResponse; -import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.model.ProtocolType; import org.apache.cloudstack.storage.to.VolumeObjectTO; @@ -70,36 +67,34 @@ import org.apache.cloudstack.storage.utils.OntapStorageConstants; /** - * VM Snapshot strategy for NetApp ONTAP managed storage using FlexVolume-level snapshots. + * VM Snapshot strategy for NetApp ONTAP managed storage using clone artifacts. * *

This strategy handles VM-level (instance) snapshots for VMs whose volumes - * reside on ONTAP managed primary storage. Instead of creating per-file clones - * (the old approach), it takes ONTAP FlexVolume-level snapshots via the - * ONTAP REST API ({@code POST /api/storage/volumes/{uuid}/snapshots}).

+ * reside on ONTAP managed primary storage by creating per-volume clone artifacts + * (file clones for NAS, LUN clones for SAN).

* - *

Key Advantage:

- *

When multiple CloudStack disks (ROOT + DATA) reside on the same ONTAP - * FlexVolume, a single FlexVolume snapshot atomically captures all of them. - * This is both faster and more storage-efficient than per-file clones.

+ *

Key Behavior:

+ *

Each CloudStack volume in the VM snapshot is represented by a dedicated ONTAP + * clone artifact. The workflow stores one detail row per CloudStack volume so + * revert/delete can operate precisely per volume.

* *

Flow:

*
    *
  1. Group all VM volumes by their parent FlexVolume UUID
    2. *
  2. Freeze the VM via QEMU guest agent ({@code fsfreeze}) — if quiesce requested
    3. - *
  3. For each unique FlexVolume, create one ONTAP snapshot
    4. + *
  4. For each volume, create a protocol-specific clone artifact
    5. *
  5. Thaw the VM
    6. - *
  6. Record FlexVolume → snapshot UUID mappings in {@code vm_snapshot_details}
    7. + *
  7. Record clone artifact metadata in {@code vm_snapshot_details}
    8. *
* *

Metadata in vm_snapshot_details:

- *

Each FlexVolume snapshot is stored as a detail row with: + *

Each clone artifact is stored as a detail row with: *

    - *
  • name = {@value OntapStorageConstants#ONTAP_FLEXVOL_SNAPSHOT}
    • - *
  • value = {@code "::::::::::"}
    • + *
  • name = {@value OntapStorageConstants#ONTAP_CLONE_SNAPSHOT_DETAIL}
    • + *
  • value = {@code "::::::::::"}
    • *
- * One row is persisted per CloudStack volume (not per FlexVolume) so that the - * revert operation can restore individual files/LUNs using the ONTAP Snapshot - * File Restore API ({@code POST /api/storage/volumes/{vol}/snapshots/{snap}/files/{path}/restore}).

+ * One row is persisted per CloudStack volume so that the revert operation can + * restore from clone artifacts using protocol-specific APIs.

* *

Strategy Selection:

*

Returns {@code StrategyPriority.HIGHEST} when:

@@ -113,7 +108,7 @@ public class OntapVMSnapshotStrategy extends StorageVMSnapshotStrategy { private static final Logger logger = LogManager.getLogger(OntapVMSnapshotStrategy.class); - /** Separator used in the vm_snapshot_details value to delimit FlexVol UUID, snapshot UUID, snapshot name, and pool ID. */ + /** Separator used in vm_snapshot_details values for clone metadata serialization. */ static final String DETAIL_SEPARATOR = "::"; @Inject @@ -137,9 +132,9 @@ public StrategyPriority canHandle(VMSnapshot vmSnapshot) { // For existing (non-Allocated) snapshots, check if we created them if (!VMSnapshot.State.Allocated.equals(vmSnapshotVO.getState())) { - // Check for our FlexVolume snapshot details first - List flexVolDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT); - if (CollectionUtils.isNotEmpty(flexVolDetails)) { + // Check for our clone snapshot details first + List cloneSnapshotDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_CLONE_SNAPSHOT_DETAIL); + if (CollectionUtils.isNotEmpty(cloneSnapshotDetails)) { // Verify the volumes are still on ONTAP storage if (allVolumesOnOntapManagedStorage(vmSnapshot.getVmId())) { return StrategyPriority.HIGHEST; @@ -164,7 +159,7 @@ public StrategyPriority canHandle(VMSnapshot vmSnapshot) { @Override public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMemory) { - // ONTAP FlexVolume snapshots only support disk-only (crash-consistent) snapshots. + // ONTAP clone-backed snapshots only support disk-only (crash-consistent) snapshots. // Memory snapshots (snapshotMemory=true) are not supported because: // 1. ONTAP snapshots capture disk state only, not VM memory // 2. Allowing memory snapshots would require falling back to libvirt snapshots, @@ -239,21 +234,19 @@ boolean allVolumesOnOntapManagedStorage(long vmId) { } // ────────────────────────────────────────────────────────────────────────── - // Take VM Snapshot (FlexVolume-level) + // Take VM Snapshot (clone-backed) // ────────────────────────────────────────────────────────────────────────── /** - * Takes a VM-level snapshot by freezing the VM, creating ONTAP FlexVolume-level - * snapshots (one per unique FlexVolume), and then thawing the VM. + * Takes a VM-level snapshot by freezing the VM, creating ONTAP clone artifacts + * (one per CloudStack volume), and then thawing the VM. * - *

Volumes are grouped by their parent FlexVolume UUID (from storage pool details). - * For each unique FlexVolume, exactly one ONTAP snapshot is created via - * {@code POST /api/storage/volumes/{uuid}/snapshots}. This means if a VM has - * ROOT and DATA disks on the same FlexVolume, only one snapshot is created.

+ *

Volumes are grouped by FlexVolume UUID for efficient metadata lookup, but a + * dedicated clone artifact is created per CloudStack volume.

* *

Memory Snapshots Not Supported: This strategy only supports disk-only * (crash-consistent) snapshots. Memory snapshots (snapshotmemory=true) are rejected - * with a clear error message. This is because ONTAP FlexVolume snapshots capture disk + * with a clear error message. This is because ONTAP clone-backed snapshots capture disk * state only, and allowing mixed snapshot chains (ONTAP disk + libvirt memory) would * cause issues during revert operations.

* @@ -279,8 +272,8 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { FreezeThawVMAnswer thawAnswer = null; long startFreeze = 0; - // Track which FlexVolume snapshots were created (for rollback) - List createdSnapshots = new ArrayList<>(); + // Track which clone artifacts were created (for rollback) + List createdSnapshots = new ArrayList<>(); boolean result = false; try { @@ -310,7 +303,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { if (!vmIsRunning) { logger.info("takeVMSnapshot: VM [{}] is in state [{}] (not Running). Skipping freeze/thaw - " + - "FlexVolume snapshot will be taken directly.", userVm.getInstanceName(), userVm.getState()); + "clone artifacts will be created directly.", userVm.getInstanceName(), userVm.getState()); } else if (quiesceVm) { logger.info("takeVMSnapshot: Quiesce option is enabled for ONTAP VM Snapshot of VM [{}]. " + "VM file systems will be frozen/thawed for application-consistent snapshots.", userVm.getInstanceName()); @@ -384,78 +377,18 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { for (Long volumeId : groupInfo.volumeIds) { String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails); String cloneName = buildPerVolumeCloneName(snapshotNameBase, vmSnapshot.getId(), volumeId); - String cloneUuid = cloneName; - if (ProtocolType.NFS3.name().equalsIgnoreCase(protocol)) { - org.apache.cloudstack.storage.feign.model.FileCloneRequest cloneRequest = new org.apache.cloudstack.storage.feign.model.FileCloneRequest(); - org.apache.cloudstack.storage.feign.model.FileCloneRequest.VolumeRef volumeRef = new org.apache.cloudstack.storage.feign.model.FileCloneRequest.VolumeRef(); - volumeRef.setUuid(flexVolUuid); - volumeRef.setName(groupInfo.poolDetails.get(OntapStorageConstants.VOLUME_NAME)); - cloneRequest.setVolume(volumeRef); - cloneRequest.setSourcePath(volumePath); - cloneRequest.setDestinationPath(cloneName); - JobResponse fileJobResponse = storageStrategy.getNasFeignClient().cloneFile(authHeader, cloneRequest); - if (fileJobResponse == null || fileJobResponse.getJob() == null) { - throw new CloudRuntimeException("Failed to submit clone-backed VM snapshot for volume " + volumeId); - } - Boolean jobSucceeded = storageStrategy.jobPollForSuccess(fileJobResponse.getJob().getUuid(), 30, 2000); - if (!jobSucceeded) { - throw new CloudRuntimeException("Clone-backed VM snapshot job failed for volume " + volumeId); - } - } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { + String sourceObjectUuid = null; + if (ProtocolType.ISCSI.name().equalsIgnoreCase(protocol)) { VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeId, OntapStorageConstants.LUN_DOT_UUID); String sourceLunUuid = lunDetail != null ? lunDetail.getValue() : null; if (sourceLunUuid == null || sourceLunUuid.isEmpty()) { throw new CloudRuntimeException("Source LUN UUID missing for volume " + volumeId); } - if (volumePath == null || volumePath.isEmpty()) { - throw new CloudRuntimeException("Source LUN path is missing for volume " + volumeId); - } - if (!volumePath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { - throw new CloudRuntimeException("Invalid source LUN path (must start with " + - OntapStorageConstants.VOLUME_PATH_PREFIX + "): " + volumePath); - } - String cloneLunPath = OntapStorageUtils.getLunName( - groupInfo.poolDetails.get(OntapStorageConstants.VOLUME_NAME), cloneName); - if (!cloneLunPath.startsWith(OntapStorageConstants.VOLUME_PATH_PREFIX)) { - throw new CloudRuntimeException("Invalid iSCSI clone LUN path generated: " + cloneLunPath); - } - String svmName = groupInfo.poolDetails.get(OntapStorageConstants.SVM_NAME); - String flexVolName = groupInfo.poolDetails.get(OntapStorageConstants.VOLUME_NAME); - if (svmName == null || svmName.isEmpty()) { - throw new CloudRuntimeException("SVM name is mandatory for iSCSI clone request"); - } - if (flexVolName == null || flexVolName.isEmpty()) { - throw new CloudRuntimeException("FlexVolume name is mandatory for iSCSI clone request"); - } - org.apache.cloudstack.storage.feign.model.Lun cloneRequest = new org.apache.cloudstack.storage.feign.model.Lun(); - cloneRequest.setName(cloneLunPath); - org.apache.cloudstack.storage.feign.model.Svm svm = new org.apache.cloudstack.storage.feign.model.Svm(); - svm.setName(svmName); - cloneRequest.setSvm(svm); - org.apache.cloudstack.storage.feign.model.Lun.Location location = new org.apache.cloudstack.storage.feign.model.Lun.Location(); - org.apache.cloudstack.storage.feign.model.Lun.LocationVolume locationVolume = new org.apache.cloudstack.storage.feign.model.Lun.LocationVolume(); - locationVolume.setName(flexVolName); - location.setVolume(locationVolume); - cloneRequest.setLocation(location); - org.apache.cloudstack.storage.feign.model.Lun.Clone clone = new org.apache.cloudstack.storage.feign.model.Lun.Clone(); - org.apache.cloudstack.storage.feign.model.Lun.Source source = new org.apache.cloudstack.storage.feign.model.Lun.Source(); - source.setName(volumePath); - source.setUuid(sourceLunUuid); - clone.setSource(source); - cloneRequest.setClone(clone); - logger.info("CloneRequest: {}", cloneRequest); - OntapResponse createCloneResponse = storageStrategy.getSanFeignClient().createLun(authHeader, true, cloneRequest); - if (createCloneResponse == null || createCloneResponse.getRecords() == null || createCloneResponse.getRecords().isEmpty()) { - throw new CloudRuntimeException("Failed to create iSCSI clone LUN for volume " + volumeId); - } - cloneUuid = createCloneResponse.getRecords().get(0).getUuid(); - if (cloneUuid == null || cloneUuid.isEmpty()) { - cloneUuid = resolveLunUuid(storageStrategy, authHeader, svmName, cloneLunPath); - } - } else { - throw new CloudRuntimeException("Unsupported protocol for VM snapshot clone: " + protocol); + sourceObjectUuid = sourceLunUuid; } - FlexVolSnapshotDetail detail = new FlexVolSnapshotDetail( + String cloneUuid = storageStrategy.createSnapshotClone(flexVolUuid, + groupInfo.poolDetails.get(OntapStorageConstants.VOLUME_NAME), volumePath, cloneName, sourceObjectUuid); + CSVolSnapshotDetail detail = new CSVolSnapshotDetail( flexVolUuid, cloneUuid, cloneName, volumePath, groupInfo.poolId, protocol); createdSnapshots.add(detail); } @@ -484,10 +417,10 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { } } - // ── Step 4: Persist FlexVolume snapshot details (one row per CloudStack volume) ── - for (FlexVolSnapshotDetail detail : createdSnapshots) { + // ── Step 4: Persist clone snapshot details (one row per CloudStack volume) ── + for (CSVolSnapshotDetail detail : createdSnapshots) { vmSnapshotDetailsDao.persist(new VMSnapshotDetailsVO( - vmSnapshot.getId(), OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT, detail.toString(), true)); + vmSnapshot.getId(), OntapStorageConstants.ONTAP_CLONE_SNAPSHOT_DETAIL, detail.toString(), true)); } // ── Step 5: Finalize via parent processAnswer ── @@ -495,7 +428,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { answer.setVolumeTOs(volumeTOs); processAnswer(vmSnapshotVO, userVm, answer, null); - logger.info("takeVMSnapshot: ONTAP FlexVolume VM Snapshot [{}] created successfully for VM [{}] ({} FlexVol snapshot(s))", + logger.info("takeVMSnapshot: ONTAP VM Snapshot [{}] created successfully for VM [{}] ({} snapshot(s))", vmSnapshot.getName(), userVm.getInstanceName(), createdSnapshots.size()); long newChainSize = 0; @@ -521,13 +454,13 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { } finally { if (!result) { - // Rollback all FlexVolume snapshots created so far (deduplicate by FlexVol+Snapshot) + // Rollback all created clone artifacts so far (deduplicate by FlexVol+Clone) Map rolledBack = new HashMap<>(); - for (FlexVolSnapshotDetail detail : createdSnapshots) { + for (CSVolSnapshotDetail detail : createdSnapshots) { String dedupeKey = detail.flexVolUuid + "::" + detail.snapshotUuid; if (!rolledBack.containsKey(dedupeKey)) { try { - rollbackFlexVolSnapshot(detail); + rollbackCloudStackVolSnapshot(detail); rolledBack.put(dedupeKey, Boolean.TRUE); } catch (Exception rollbackEx) { logger.error("takeVMSnapshot: Failed to rollback FlexVol snapshot [{}] on FlexVol [{}]: {}", @@ -550,7 +483,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { try { List vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshot.getId()); for (VMSnapshotDetailsVO detail : vmSnapshotDetails) { - if (OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT.equals(detail.getName())) { + if (OntapStorageConstants.ONTAP_CLONE_SNAPSHOT_DETAIL.equals(detail.getName())) { vmSnapshotDetailsDao.remove(detail.getId()); } } @@ -588,10 +521,10 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { DeleteVMSnapshotCommand deleteSnapshotCommand = new DeleteVMSnapshotCommand(vmInstanceName, vmSnapshotTO, volumeTOs, guestOS.getDisplayName()); - // Check for FlexVolume snapshots - List flexVolDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT); - if (CollectionUtils.isNotEmpty(flexVolDetails)) { - deleteFlexVolSnapshots(flexVolDetails); + // Check for clone snapshot details + List cloneSnapshotDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_CLONE_SNAPSHOT_DETAIL); + if (CollectionUtils.isNotEmpty(cloneSnapshotDetails)) { + deleteCloneSnapshotArtifacts(cloneSnapshotDetails); } processAnswer(vmSnapshotVO, userVm, new DeleteVMSnapshotAnswer(deleteSnapshotCommand, volumeTOs), null); @@ -640,7 +573,7 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { // Revert clone-backed snapshot artifacts per volume: // - NFS: patch file(source=clone, destination=live file, overwrite=true) // - iSCSI: patch LUN (clone.source=clone LUN, destination=live LUN) - List cloneDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_FLEXVOL_SNAPSHOT); + List cloneDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), OntapStorageConstants.ONTAP_CLONE_SNAPSHOT_DETAIL); if (CollectionUtils.isNotEmpty(cloneDetails)) { revertCloneBackedSnapshots(cloneDetails); } @@ -665,7 +598,7 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { } // ────────────────────────────────────────────────────────────────────────── - // FlexVolume Snapshot Helpers + // Clone Snapshot Helpers // ────────────────────────────────────────────────────────────────────────── /** @@ -716,15 +649,6 @@ String buildPerVolumeCloneName(String snapshotNameBase, Long vmSnapshotId, Long return OntapStorageUtils.getOntapCloneName(snapshotNameBase + "_s" + vmSnapshotId + "_v" + volumeId); } - String resolveLunUuid(StorageStrategy strategy, String authHeader, String svmName, String lunName) { - OntapResponse response = strategy.getSanFeignClient() - .getLunResponse(authHeader, Map.of(OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.NAME, lunName)); - if (response == null || response.getRecords() == null || response.getRecords().isEmpty()) { - throw new CloudRuntimeException("Could not resolve LUN UUID for clone " + lunName); - } - return response.getRecords().get(0).getUuid(); - } - /** * Resolves the ONTAP-side path of a CloudStack volume within its FlexVolume. * @@ -760,74 +684,44 @@ String resolveVolumePathOnOntap(Long volumeId, String protocol, Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - String authHeader = storageStrategy.getAuthHeader(); - - if (ProtocolType.NFS3.name().equalsIgnoreCase(detail.protocol)) { - logger.info("rollbackFlexVolSnapshot: Deleting NFS clone file [{}] on FlexVol [{}]", - detail.snapshotName, detail.flexVolUuid); - storageStrategy.getNasFeignClient().deleteFile(authHeader, detail.flexVolUuid, detail.snapshotName); - } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(detail.protocol)) { - logger.info("rollbackFlexVolSnapshot: Deleting iSCSI clone LUN [{}] (uuid={})", - detail.snapshotName, detail.snapshotUuid); - String cloneUuid = detail.snapshotUuid; - if (cloneUuid == null || cloneUuid.isEmpty()) { - String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); - String cloneLunPath = OntapStorageUtils.getLunName(poolDetails.get(OntapStorageConstants.VOLUME_NAME), detail.snapshotName); - cloneUuid = resolveLunUuid(storageStrategy, authHeader, svmName, cloneLunPath); - } - storageStrategy.getSanFeignClient().deleteLun(authHeader, cloneUuid, Map.of("allow_delete_while_mapped", "true")); - } + storageStrategy.deleteSnapshotClone(detail.flexVolUuid, poolDetails.get(OntapStorageConstants.VOLUME_NAME), + detail.snapshotName, detail.snapshotUuid); } catch (Exception e) { - logger.error("rollbackFlexVolSnapshot: Rollback of FlexVol snapshot failed: {}", e.getMessage(), e); + logger.error("rollbackCloudStackVolSnapshot: Rollback of CloudStack Vol snapshot failed: {}", e.getMessage(), e); } } /** - * Deletes all FlexVolume snapshots associated with a VM snapshot. + * Deletes all CS-Volume snapshots associated with a VM snapshot. * *

Since there is one detail row per CloudStack volume, multiple rows may reference - * the same FlexVol + snapshot combination. This method deduplicates to delete each - * underlying ONTAP snapshot only once.

+ * the respective CS Vol + snapshot combination.

*/ - void deleteFlexVolSnapshots(List flexVolDetails) { + void deleteCloneSnapshotArtifacts(List cloneSnapshotDetails) { // Track which FlexVol+Snapshot pairs have already been deleted Map deletedSnapshots = new HashMap<>(); - for (VMSnapshotDetailsVO detailVO : flexVolDetails) { - FlexVolSnapshotDetail detail = FlexVolSnapshotDetail.parse(detailVO.getValue()); + for (VMSnapshotDetailsVO detailVO : cloneSnapshotDetails) { + CSVolSnapshotDetail detail = CSVolSnapshotDetail.parse(detailVO.getValue()); String dedupeKey = detail.flexVolUuid + "::" + detail.snapshotUuid; // Only delete the ONTAP snapshot once per FlexVol+Snapshot pair if (!deletedSnapshots.containsKey(dedupeKey)) { Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); - String authHeader = storageStrategy.getAuthHeader(); try { - if (ProtocolType.NFS3.name().equalsIgnoreCase(detail.protocol)) { - logger.info("deleteFlexVolSnapshots: Deleting NFS clone file [{}] on FlexVol [{}]", - detail.snapshotName, detail.flexVolUuid); - storageStrategy.getNasFeignClient().deleteFile(authHeader, detail.flexVolUuid, detail.snapshotName); - } else if (ProtocolType.ISCSI.name().equalsIgnoreCase(detail.protocol)) { - logger.info("deleteFlexVolSnapshots: Deleting iSCSI clone LUN [{}] (uuid={})", - detail.snapshotName, detail.snapshotUuid); - String cloneUuid = detail.snapshotUuid; - if (cloneUuid == null || cloneUuid.isEmpty()) { - String svmName = poolDetails.get(OntapStorageConstants.SVM_NAME); - String cloneLunPath = OntapStorageUtils.getLunName(poolDetails.get(OntapStorageConstants.VOLUME_NAME), detail.snapshotName); - cloneUuid = resolveLunUuid(storageStrategy, authHeader, svmName, cloneLunPath); - } - storageStrategy.getSanFeignClient().deleteLun(authHeader, cloneUuid, Map.of("allow_delete_while_mapped", "true")); - } + storageStrategy.deleteSnapshotClone(detail.flexVolUuid, poolDetails.get(OntapStorageConstants.VOLUME_NAME), + detail.snapshotName, detail.snapshotUuid); } catch (Exception e) { if (isSnapshotAlreadyMissing(e)) { - logger.warn("deleteFlexVolSnapshots: Clone [{}] on FlexVol [{}] is already missing. " + + logger.warn("deleteCloneSnapshotArtifacts: Clone [{}] on FlexVol [{}] is already missing. " + "Treating as success.", detail.snapshotName, detail.flexVolUuid); } else { throw e; @@ -835,7 +729,7 @@ void deleteFlexVolSnapshots(List flexVolDetails) { } deletedSnapshots.put(dedupeKey, Boolean.TRUE); - logger.info("deleteFlexVolSnapshots: Deleted clone [{}] on FlexVol [{}]", detail.snapshotName, detail.flexVolUuid); + logger.info("deleteCloneSnapshotArtifacts: Deleted clone [{}] on FlexVol [{}]", detail.snapshotName, detail.flexVolUuid); } // Always remove the DB detail row @@ -869,7 +763,7 @@ private boolean isSnapshotAlreadyMissing(Exception e) { */ void revertCloneBackedSnapshots(List cloneDetails) { for (VMSnapshotDetailsVO detailVO : cloneDetails) { - FlexVolSnapshotDetail detail = FlexVolSnapshotDetail.parse(detailVO.getValue()); + CSVolSnapshotDetail detail = CSVolSnapshotDetail.parse(detailVO.getValue()); if (detail.volumePath == null || detail.volumePath.isEmpty()) { // Legacy detail row without volumePath – cannot do single-file restore @@ -887,10 +781,9 @@ void revertCloneBackedSnapshots(List cloneDetails) { logger.info("revertCloneBackedSnapshots: Reverting volume [{}] using clone source [{}] on FlexVol [{}] (protocol={})", detail.volumePath, detail.snapshotName, flexVolName, detail.protocol); - String lunUuid = ProtocolType.ISCSI.name().equalsIgnoreCase(detail.protocol) ? detail.snapshotUuid : null; try { storageStrategy.revertSnapshotForCloudStackVolume( - detail.snapshotName, detail.flexVolUuid, detail.snapshotUuid, detail.volumePath, lunUuid, flexVolName); + detail.snapshotName, detail.flexVolUuid, detail.snapshotUuid, detail.volumePath, flexVolName); } catch (Exception e) { logger.error("revertCloneBackedSnapshots: Revert of FlexVol snapshot failed: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to revert volume [" + detail.volumePath + "] from clone [" + @@ -921,7 +814,7 @@ static class FlexVolGroupInfo { } /** - * Holds the metadata for a single volume's FlexVolume snapshot entry (used during create and for + * Holds the metadata for a single volume's clone snapshot entry (used during create and for * serialization/deserialization to/from vm_snapshot_details). * *

One row is persisted per CloudStack volume. Multiple volumes may share the same @@ -929,7 +822,7 @@ static class FlexVolGroupInfo { * *

Serialized format: {@code "::::::::::"}

*/ - static class FlexVolSnapshotDetail { + static class CSVolSnapshotDetail { final String flexVolUuid; final String snapshotUuid; final String snapshotName; @@ -939,8 +832,8 @@ static class FlexVolSnapshotDetail { /** Storage protocol: NFS3, ISCSI, etc. */ final String protocol; - FlexVolSnapshotDetail(String flexVolUuid, String snapshotUuid, String snapshotName, - String volumePath, long poolId, String protocol) { + CSVolSnapshotDetail(String flexVolUuid, String snapshotUuid, String snapshotName, + String volumePath, long poolId, String protocol) { this.flexVolUuid = flexVolUuid; this.snapshotUuid = snapshotUuid; this.snapshotName = snapshotName; @@ -950,18 +843,18 @@ static class FlexVolSnapshotDetail { } /** - * Parses a vm_snapshot_details value string back into a FlexVolSnapshotDetail. + * Parses a vm_snapshot_details value string back into a CSVolSnapshotDetail. */ - static FlexVolSnapshotDetail parse(String value) { + static CSVolSnapshotDetail parse(String value) { String[] parts = value.split(DETAIL_SEPARATOR); if (parts.length == 4) { // Legacy format without volumePath and protocol: flexVolUuid::snapshotUuid::snapshotName::poolId - return new FlexVolSnapshotDetail(parts[0], parts[1], parts[2], null, Long.parseLong(parts[3]), null); + return new CSVolSnapshotDetail(parts[0], parts[1], parts[2], null, Long.parseLong(parts[3]), null); } if (parts.length != 6) { throw new CloudRuntimeException("Invalid ONTAP FlexVol snapshot detail format: " + value); } - return new FlexVolSnapshotDetail(parts[0], parts[1], parts[2], parts[3], Long.parseLong(parts[4]), parts[5]); + return new CSVolSnapshotDetail(parts[0], parts[1], parts[2], parts[3], Long.parseLong(parts[4]), parts[5]); } @Override 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 2d849296d5f6..8e104827a68e 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 @@ -42,10 +42,7 @@ import org.apache.cloudstack.storage.feign.client.NASFeignClient; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.model.Igroup; -import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.Lun; -import org.apache.cloudstack.storage.feign.model.response.JobResponse; -import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.service.UnifiedSANStrategy; import org.apache.cloudstack.storage.service.model.AccessGroup; @@ -64,7 +61,6 @@ import org.mockito.junit.jupiter.MockitoExtension; import java.util.HashMap; -import java.util.List; import java.util.Map; import static com.cloud.agent.api.to.DataObjectType.SNAPSHOT; @@ -79,6 +75,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; @@ -616,14 +613,8 @@ void testTakeSnapshot_NfsCloneSuccess() { when(volumeDao.findById(100L)).thenReturn(volumeVO); when(storagePoolDao.findById(1L)).thenReturn(storagePool); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - when(storageStrategy.getAuthHeader()).thenReturn("Basic auth"); - when(storageStrategy.getNasFeignClient()).thenReturn(nasFeignClient); - JobResponse jobResponse = new JobResponse(); - Job job = new Job(); - job.setUuid("job-uuid-1"); - jobResponse.setJob(job); - when(nasFeignClient.cloneFile(anyString(), any())).thenReturn(jobResponse); - when(storageStrategy.jobPollForSuccess("job-uuid-1", 30, 2000)).thenReturn(true); + when(storageStrategy.createSnapshotClone(eq("flexvol-uuid-1"), eq("flexvol1"), + eq("vol-100.qcow2"), eq("UI_Snapshot_Name"), isNull())).thenReturn("UI_Snapshot_Name"); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) @@ -633,7 +624,8 @@ void testTakeSnapshot_NfsCloneSuccess() { driver.takeSnapshot(snapshotInfo, createCallback); - verify(nasFeignClient).cloneFile(anyString(), any()); + verify(storageStrategy).createSnapshotClone(eq("flexvol-uuid-1"), eq("flexvol1"), + eq("vol-100.qcow2"), eq("UI_Snapshot_Name"), isNull()); verify(snapshotDetailsDao, atLeastOnce()).persist(any(SnapshotDetailsVO.class)); verify(createCallback).complete(any(CreateCmdResult.class)); } @@ -657,7 +649,7 @@ void testRevertSnapshot_UsesCloneMetadata() { storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - doNothing().when(storageStrategy).revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString(), anyString()); + doNothing().when(storageStrategy).revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString()); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) @@ -667,7 +659,7 @@ void testRevertSnapshot_UsesCloneMetadata() { verify(storageStrategy).revertSnapshotForCloudStackVolume( eq("UI_Snapshot_Name"), eq("flexvol-uuid-1"), eq("clone-lun-uuid-1"), - eq("dest-lun-1"), eq("clone-lun-uuid-1"), eq("flexvol1")); + eq("dest-lun-1"), eq("flexvol1")); verify(commandCallback).complete(any(CommandResult.class)); } } @@ -692,7 +684,7 @@ void testRevertSnapshot_FallbacksToLegacySnapshotNameWhenCloneNameMissing() { storagePoolDetails.put(OntapStorageConstants.VOLUME_NAME, "flexvol1"); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - doNothing().when(storageStrategy).revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString(), anyString()); + doNothing().when(storageStrategy).revertSnapshotForCloudStackVolume(anyString(), anyString(), anyString(), anyString(), anyString()); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) @@ -702,7 +694,7 @@ void testRevertSnapshot_FallbacksToLegacySnapshotNameWhenCloneNameMissing() { verify(storageStrategy).revertSnapshotForCloudStackVolume( eq("Legacy_UI_Snapshot"), eq("flexvol-uuid-1"), eq("clone-lun-uuid-2"), - eq("dest-lun-1"), eq("clone-lun-uuid-2"), eq("flexvol1")); + eq("dest-lun-1"), eq("flexvol1")); verify(commandCallback).complete(any(CommandResult.class)); } } @@ -724,8 +716,7 @@ void testDeleteAsync_SnapshotNfsClone_UsesDeleteFile() { .thenReturn(new SnapshotDetailsVO(700L, OntapStorageConstants.PROTOCOL, ProtocolType.NFS3.name(), false)); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - when(storageStrategy.getAuthHeader()).thenReturn("Basic auth"); - when(storageStrategy.getNasFeignClient()).thenReturn(nasFeignClient); + doNothing().when(storageStrategy).deleteSnapshotClone("flexvol-uuid-nfs", null, "clone-file-nfs.qcow2", "clone-id-nfs"); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) @@ -733,7 +724,7 @@ void testDeleteAsync_SnapshotNfsClone_UsesDeleteFile() { driver.deleteAsync(dataStore, snapshotInfo, commandCallback); - verify(nasFeignClient).deleteFile("Basic auth", "flexvol-uuid-nfs", "clone-file-nfs.qcow2"); + verify(storageStrategy).deleteSnapshotClone("flexvol-uuid-nfs", null, "clone-file-nfs.qcow2", "clone-id-nfs"); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); verify(commandCallback).complete(resultCaptor.capture()); assertTrue(resultCaptor.getValue().isSuccess()); @@ -759,25 +750,14 @@ void testDeleteAsync_SnapshotIscsiClone_ResolvesUuidAndUsesDeleteLun() { .thenReturn(new SnapshotDetailsVO(701L, OntapStorageConstants.PROTOCOL, ProtocolType.ISCSI.name(), false)); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails); - when(storageStrategy.getAuthHeader()).thenReturn("Basic auth"); - when(storageStrategy.getSanFeignClient()).thenReturn(sanFeignClient); - - OntapResponse lunResponse = new OntapResponse<>(); - Lun lun = new Lun(); - lun.setUuid("resolved-clone-uuid"); - lunResponse.setRecords(List.of(lun)); - when(sanFeignClient.getLunResponse(eq("Basic auth"), any())).thenReturn(lunResponse); + doNothing().when(storageStrategy).deleteSnapshotClone("flexvol-uuid-iscsi", "flexvol1", "clone-lun-name", null); try (MockedStatic utilityMock = mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails)) .thenReturn(storageStrategy); - utilityMock.when(() -> OntapStorageUtils.getLunName("flexvol1", "clone-lun-name")) - .thenReturn("/vol/flexvol1/clone-lun-name"); - driver.deleteAsync(dataStore, snapshotInfo, commandCallback); - verify(sanFeignClient).deleteLun(eq("Basic auth"), eq("resolved-clone-uuid"), - argThat(map -> "true".equals(map.get("allow_delete_while_mapped")))); + verify(storageStrategy).deleteSnapshotClone("flexvol-uuid-iscsi", "flexvol1", "clone-lun-name", null); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(CommandResult.class); verify(commandCallback).complete(resultCaptor.capture()); assertTrue(resultCaptor.getValue().isSuccess()); 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 2c9060b46de2..fb80101a8850 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 @@ -144,7 +144,16 @@ public CloudStackVolume getCloudStackVolume(Map cloudStackVolume } @Override - public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String lunUuid, String flexVolName) { + public void revertSnapshotForCloudStackVolume(String snapshotName, String flexVolUuid, String snapshotUuid, String volumePath, String flexVolName) { + } + + @Override + public String createSnapshotClone(String flexVolUuid, String flexVolName, String sourcePath, String cloneName, String sourceObjectUuid) { + return null; + } + + @Override + public void deleteSnapshotClone(String flexVolUuid, String flexVolName, String cloneName, String cloneObjectUuid) { } @Override diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java index ccb9908eaf06..3baa5c284261 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 @@ -138,6 +138,9 @@ private void injectField(String fieldName, Object mockedField) throws Exception } private class TestableUnifiedNASStrategy extends UnifiedNASStrategy { + private boolean jobPollResult = true; + private String lastPolledJobUuid; + public TestableUnifiedNASStrategy(OntapStorage ontapStorage, NASFeignClient nasFeignClient, VolumeFeignClient volumeFeignClient, @@ -166,6 +169,12 @@ private void injectParentMockedClient(String fieldName, Object mockedClient) { throw new RuntimeException("Failed to inject parent mocked client: " + fieldName, e); } } + + @Override + public Boolean jobPollForSuccess(String jobUUID, int maxRetries, int sleepTimeInMilliSecs) { + this.lastPolledJobUuid = jobUUID; + return jobPollResult; + } } // Test createCloudStackVolume - Success @@ -585,20 +594,28 @@ public void testDeleteCloudStackVolume_AnswerNull() throws Exception { } @Test - public void testRevertSnapshotForCloudStackVolume_UsesFilePatchWithoutTarget() { + public void testRevertSnapshotForCloudStackVolume_UsesCloneFileRestore() { + JobResponse cloneJobResponse = new JobResponse(); + Job job = new Job(); + job.setUuid("job-uuid-1"); + cloneJobResponse.setJob(job); + when(nasFeignClient.cloneFile(anyString(), any())).thenReturn(cloneJobResponse); + strategy.revertSnapshotForCloudStackVolume( - "clone-snap-1", "flexvol-uuid-1", "snap-uuid-1", "vm-disk.qcow2", null, "flexvol1"); - verify(nasFeignClient).updateFile(anyString(), eq("flexvol-uuid-1"), eq("clone-snap-1"), eq(true), argThat(req -> + "clone-snap-1", "flexvol-uuid-1", "snap-uuid-1", "vm-disk.qcow2", "flexvol1"); + verify(nasFeignClient).cloneFile(anyString(), argThat(req -> req != null - && Boolean.TRUE.equals(req.isOverwriteEnabled()) - && Boolean.FALSE.equals(req.isFillEnabled()) - && "vm-disk.qcow2".equals(req.getPath()) - && req.getTarget() == null)); + && "clone-snap-1".equals(req.getSourcePath()) + && "vm-disk.qcow2".equals(req.getDestinationPath()) + && req.getVolume() != null + && "flexvol-uuid-1".equals(req.getVolume().getUuid()) + && "flexvol1".equals(req.getVolume().getName()))); + assertEquals("job-uuid-1", strategy.lastPolledJobUuid); } @Test public void testRevertSnapshotForCloudStackVolume_MissingFlexVolUuid_Throws() { assertThrows(CloudRuntimeException.class, () -> strategy.revertSnapshotForCloudStackVolume( - "clone-snap-1", null, "snap-uuid-1", "vm-disk.qcow2", null, "flexvol1")); + "clone-snap-1", null, "snap-uuid-1", "vm-disk.qcow2", "flexvol1")); } } 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 f2df1865afad..9b886b7e2965 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 @@ -1821,7 +1821,7 @@ void testRevertSnapshotForCloudStackVolume_UsesLunPatchWithCloneSource() { .thenReturn("/vol/flexvol1/dest-lun-1"); unifiedSANStrategy.revertSnapshotForCloudStackVolume( - "clone-snap-1", "flexvol-uuid-1", "clone-lun-uuid-1", "dest-lun-1", "clone-lun-uuid-1", "flexvol1"); + "clone-snap-1", "flexvol-uuid-1", "clone-lun-uuid-1", "dest-lun-1", "flexvol1"); verify(sanFeignClient).updateLun(eq(authHeader), eq("dest-lun-uuid-1"), argThat(lun -> lun != null @@ -1838,8 +1838,8 @@ void testRevertSnapshotForCloudStackVolume_UsesLunPatchWithCloneSource() { } @Test - void testRevertSnapshotForCloudStackVolume_MissingLunUuid_Throws() { + void testRevertSnapshotForCloudStackVolume_MissingSnapshotUuid_Throws() { assertThrows(CloudRuntimeException.class, () -> unifiedSANStrategy.revertSnapshotForCloudStackVolume( - "clone-snap-1", "flexvol-uuid-1", "clone-lun-uuid-1", "dest-lun-1", null, "flexvol1")); + "clone-snap-1", "flexvol-uuid-1", null, "dest-lun-1", "flexvol1")); } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java index 60796ac0ff51..ff88d0740f97 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java @@ -535,8 +535,8 @@ void testGroupVolumesByFlexVol_VolumeNotFound_ThrowsException() { @Test void testFlexVolSnapshotDetail_ParseAndToString_NewFormat() { String value = "flexvol-uuid-1::snap-uuid-1::vmsnap_200_1234567890::root-disk.qcow2::401::NFS3"; - OntapVMSnapshotStrategy.FlexVolSnapshotDetail detail = - OntapVMSnapshotStrategy.FlexVolSnapshotDetail.parse(value); + OntapVMSnapshotStrategy.CSVolSnapshotDetail detail = + OntapVMSnapshotStrategy.CSVolSnapshotDetail.parse(value); assertEquals("flexvol-uuid-1", detail.flexVolUuid); assertEquals("snap-uuid-1", detail.snapshotUuid); @@ -551,8 +551,8 @@ void testFlexVolSnapshotDetail_ParseAndToString_NewFormat() { void testFlexVolSnapshotDetail_ParseLegacy4FieldFormat() { // Legacy format without volumePath and protocol String value = "flexvol-uuid-1::snap-uuid-1::vmsnap_200_1234567890::401"; - OntapVMSnapshotStrategy.FlexVolSnapshotDetail detail = - OntapVMSnapshotStrategy.FlexVolSnapshotDetail.parse(value); + OntapVMSnapshotStrategy.CSVolSnapshotDetail detail = + OntapVMSnapshotStrategy.CSVolSnapshotDetail.parse(value); assertEquals("flexvol-uuid-1", detail.flexVolUuid); assertEquals("snap-uuid-1", detail.snapshotUuid); @@ -565,20 +565,20 @@ void testFlexVolSnapshotDetail_ParseLegacy4FieldFormat() { @Test void testFlexVolSnapshotDetail_ParseInvalidFormat_ThrowsException() { assertThrows(CloudRuntimeException.class, - () -> OntapVMSnapshotStrategy.FlexVolSnapshotDetail.parse("invalid-format")); + () -> OntapVMSnapshotStrategy.CSVolSnapshotDetail.parse("invalid-format")); } @Test void testFlexVolSnapshotDetail_ParseTooFewParts_ThrowsException() { assertThrows(CloudRuntimeException.class, - () -> OntapVMSnapshotStrategy.FlexVolSnapshotDetail.parse("a::b::c")); + () -> OntapVMSnapshotStrategy.CSVolSnapshotDetail.parse("a::b::c")); } @Test void testFlexVolSnapshotDetail_Parse5Parts_ThrowsException() { // 5 parts is neither legacy (4) nor current (6) format assertThrows(CloudRuntimeException.class, - () -> OntapVMSnapshotStrategy.FlexVolSnapshotDetail.parse("a::b::c::d::e")); + () -> OntapVMSnapshotStrategy.CSVolSnapshotDetail.parse("a::b::c::d::e")); } // ══════════════════════════════════════════════════════════════════════════