From 3efaa7a18cbc71fb751d9db9d1ee288d7e154d6a Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Tue, 23 Jun 2026 19:06:06 +0530 Subject: [PATCH 1/2] CSTACKEX:200 - added temp CG logic for VM snapshots if VM span across multiple volumes at storage --- .../driver/OntapPrimaryDatastoreDriver.java | 19 +- .../feign/client/SnapshotFeignClient.java | 92 +++++ .../storage/utils/OntapStorageConstants.java | 3 + .../storage/utils/OntapStorageUtils.java | 42 ++ .../vmsnapshot/OntapVMSnapshotStrategy.java | 369 ++++++++++++++---- .../OntapVMSnapshotStrategyTest.java | 269 +++++++++++-- 6 files changed, 671 insertions(+), 123 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 04996b74d2a5..ac9b7e818747 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 @@ -67,7 +67,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; @@ -670,8 +669,8 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback-} + * Builds an ONTAP-safe snapshot name from the CloudStack UI name with uniqueness suffix. */ - private String buildSnapshotName(String volumeName, String snapshotUuid) { - String name = volumeName + "-" + snapshotUuid; - int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH; - int trimRequired = name.length() - maxLength; - - if (trimRequired > 0) { - name = StringUtils.left(volumeName, volumeName.length() - trimRequired) + "-" + snapshotUuid; - } - return name; + private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId) { + return OntapStorageUtils.buildOntapSnapshotName(cloudStackSnapshotName, "cs" + snapshotId); } /** 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..d9566b422e38 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 @@ -181,4 +181,96 @@ JobResponse restoreFileFromSnapshot(@Param("authHeader") String authHeader, @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) JobResponse restoreFileFromSnapshotCli(@Param("authHeader") String authHeader, CliSnapshotRestoreRequest request); + + /** + * Creates a consistency group. + * + *

ONTAP REST: {@code POST /api/application/consistency-groups}

+ * + * @param authHeader Basic auth header + * @param request consistency group create request body + * @return JobResponse containing the async job reference + */ + @RequestLine("POST /api/application/consistency-groups") + @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) + JobResponse createConsistencyGroup(@Param("authHeader") String authHeader, + Map request); + + /** + * Lists consistency groups. + * + *

ONTAP REST: {@code GET /api/application/consistency-groups}

+ * + * @param authHeader Basic auth header + * @param queryParams Optional query parameters + * @return Paginated consistency group records + */ + @RequestLine("GET /api/application/consistency-groups") + @Headers({"Authorization: {authHeader}"}) + OntapResponse> getConsistencyGroups(@Param("authHeader") String authHeader, + @QueryMap Map queryParams); + + /** + * Creates (starts) a consistency group snapshot. + * + *

ONTAP REST: {@code POST /api/application/consistency-groups/{cgUuid}/snapshots}

+ * + * @param authHeader Basic auth header + * @param cgUuid consistency group UUID + * @param request snapshot start request body + * @return JobResponse containing the async job reference + */ + @RequestLine("POST /api/application/consistency-groups/{cgUuid}/snapshots") + @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) + JobResponse createConsistencyGroupSnapshot(@Param("authHeader") String authHeader, + @Param("cgUuid") String cgUuid, + Map request); + + /** + * Lists snapshots for a consistency group. + * + *

ONTAP REST: {@code GET /api/application/consistency-groups/{cgUuid}/snapshots}

+ * + * @param authHeader Basic auth header + * @param cgUuid consistency group UUID + * @param queryParams Optional query parameters + * @return Paginated consistency group snapshot records + */ + @RequestLine("GET /api/application/consistency-groups/{cgUuid}/snapshots") + @Headers({"Authorization: {authHeader}"}) + OntapResponse> getConsistencyGroupSnapshots(@Param("authHeader") String authHeader, + @Param("cgUuid") String cgUuid, + @QueryMap Map queryParams); + + /** + * Commits a started consistency group snapshot. + * + *

ONTAP REST: {@code PATCH /api/application/consistency-groups/{cgUuid}/snapshots/{snapshotUuid}}

+ * + * @param authHeader Basic auth header + * @param cgUuid consistency group UUID + * @param snapshotUuid consistency group snapshot UUID + * @param request commit request body + * @return JobResponse containing the async job reference + */ + @RequestLine("PATCH /api/application/consistency-groups/{cgUuid}/snapshots/{snapshotUuid}") + @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) + JobResponse commitConsistencyGroupSnapshot(@Param("authHeader") String authHeader, + @Param("cgUuid") String cgUuid, + @Param("snapshotUuid") String snapshotUuid, + Map request); + + /** + * Deletes a consistency group. + * + *

ONTAP REST: {@code DELETE /api/application/consistency-groups/{cgUuid}}

+ * + * @param authHeader Basic auth header + * @param cgUuid consistency group UUID + * @return JobResponse containing the async job reference + */ + @RequestLine("DELETE /api/application/consistency-groups/{cgUuid}") + @Headers({"Authorization: {authHeader}"}) + JobResponse deleteConsistencyGroup(@Param("authHeader") String authHeader, + @Param("cgUuid") String cgUuid); } 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..39ffb60633e7 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 @@ -105,6 +105,9 @@ public class OntapStorageConstants { 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 String ONTAP_TEMP_CG_PREFIX = "cs-temp-cg-"; + public static final int ONTAP_CG_JOB_MAX_RETRIES = 60; + public static final int ONTAP_CG_JOB_POLL_INTERVAL_MS = 2000; /** 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..b05100a4886c 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,46 @@ public static String getLunName(String volName, String lunName) { return OntapStorageConstants.VOLUME_PATH_PREFIX + volName + OntapStorageConstants.SLASH + lunName; } + /** + * Builds an ONTAP-safe name token from user-provided snapshot text. + */ + public static String getOntapCloneName(String cloudStackSnapshotName) { + if (cloudStackSnapshotName == null || cloudStackSnapshotName.trim().isEmpty()) { + throw new InvalidParameterValueException("Snapshot name cannot be null or blank"); + } + String normalized = cloudStackSnapshotName.replaceAll("[^a-zA-Z0-9_]", "_"); + if (normalized.isEmpty()) { + normalized = "snapshot"; + } + if (!Character.isLetter(normalized.charAt(0))) { + normalized = "s_" + normalized; + } + if (normalized.length() > OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH) { + normalized = normalized.substring(0, OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH); + } + return normalized; + } + + /** + * Builds an ONTAP-safe snapshot name that preserves the CloudStack UI snapshot name + * and appends a uniqueness suffix. + */ + public static String buildOntapSnapshotName(String cloudStackSnapshotName, String uniquenessSuffix) { + String normalizedBase = (cloudStackSnapshotName == null || cloudStackSnapshotName.trim().isEmpty()) + ? "snapshot" + : getOntapCloneName(cloudStackSnapshotName); + String suffix = (uniquenessSuffix == null || uniquenessSuffix.isEmpty()) + ? "" + : "_" + uniquenessSuffix.replaceAll("[^a-zA-Z0-9_]", "_"); + int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH; + int maxBaseLength = maxLength - suffix.length(); + if (maxBaseLength <= 0) { + return normalizedBase.substring(0, maxLength); + } + if (normalizedBase.length() > maxBaseLength) { + normalizedBase = normalizedBase.substring(0, maxBaseLength); + } + return normalizedBase + suffix; + } + } 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..d07d2aebe604 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 @@ -20,8 +20,10 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import javax.inject.Inject; @@ -72,25 +74,22 @@ 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 temporary consistency-group orchestration. * *

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}).

- * - *

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.

+ * reside on ONTAP managed primary storage. When VM volumes span multiple FlexVols, + * snapshot creation is coordinated through a temporary ONTAP consistency group (CG) + * and two-phase snapshot flow (start + commit). When all volumes share a single FlexVol, + * a direct FlexVol snapshot is used instead.

* *

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. If VM spans multiple FlexVolumes: create temporary CG, start + commit CG snapshot (two-phase)
    5. + *
  5. If VM spans a single FlexVolume: create one FlexVol snapshot directly (no CG overhead)
    6. *
  6. Thaw the VM
    7. - *
  7. Record FlexVolume → snapshot UUID mappings in {@code vm_snapshot_details}
    8. + *
  8. Resolve FlexVolume → snapshot UUID mappings and persist in {@code vm_snapshot_details}
    9. *
* *

Metadata in vm_snapshot_details:

@@ -251,12 +250,14 @@ boolean allVolumesOnOntapManagedStorage(long vmId) { /** * Takes a VM-level snapshot by freezing the VM, creating ONTAP FlexVolume-level - * snapshots (one per unique FlexVolume), and then thawing the VM. + * snapshot(s), 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.

+ * When the VM spans more than one unique FlexVolume, a temporary ONTAP + * consistency group is used with two-phase snapshot semantics (start + commit) so + * all FlexVols are captured at the same point in time. When all VM volumes reside + * on a single FlexVolume, a direct per-FlexVol snapshot is taken instead — + * CG orchestration is unnecessary in that case.

* *

Memory Snapshots Not Supported: This strategy only supports disk-only * (crash-consistent) snapshots. Memory snapshots (snapshotmemory=true) are rejected @@ -286,7 +287,7 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { FreezeThawVMAnswer thawAnswer = null; long startFreeze = 0; - // Track which FlexVolume snapshots were created (for rollback) + // Track which FlexVolume snapshots were created (for rollback and detail persistence) List createdSnapshots = new ArrayList<>(); boolean result = false; @@ -338,7 +339,8 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { CreateVMSnapshotCommand ccmd = new CreateVMSnapshotCommand( userVm.getInstanceName(), userVm.getUuid(), target, volumeTOs, guestOS.getDisplayName()); - logger.info("takeVMSnapshot: Creating ONTAP FlexVolume VM Snapshot for VM [{}] with quiesce={}", userVm.getInstanceName(), quiesceVm); + logger.info("takeVMSnapshot: Creating ONTAP VM snapshot for VM [{}] with quiesce={}", + userVm.getInstanceName(), quiesceVm); // Prepare volume info list and calculate sizes for (VolumeObjectTO volumeObjectTO : volumeTOs) { @@ -375,56 +377,20 @@ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { userVm.getInstanceName(), quiesceVm, vmIsRunning); } - // ── Step 2: Create FlexVolume-level snapshots ── + // ── Step 2: Create FlexVolume-level snapshot(s) ── try { String snapshotNameBase = buildSnapshotName(vmSnapshot); - for (Map.Entry entry : flexVolGroups.entrySet()) { - String flexVolUuid = entry.getKey(); - FlexVolGroupInfo groupInfo = entry.getValue(); - long startSnapshot = System.nanoTime(); - - // 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) - for (Long volumeId : groupInfo.volumeIds) { - String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails); - FlexVolSnapshotDetail detail = new FlexVolSnapshotDetail( - flexVolUuid, snapshotUuid, snapshotNameBase, volumePath, groupInfo.poolId, protocol); - createdSnapshots.add(detail); - } - - logger.info("takeVMSnapshot: ONTAP FlexVolume snapshot [{}] (uuid={}) on FlexVol [{}] completed in {} ms. Covers volumes: {}", - snapshotNameBase, snapshotUuid, flexVolUuid, - TimeUnit.MILLISECONDS.convert(System.nanoTime() - startSnapshot, TimeUnit.NANOSECONDS), - groupInfo.volumeIds); + // CG orchestration is only required when VM disks span multiple FlexVols. + // A single FlexVol already provides atomic capture for all volumes on that FlexVol. + if (flexVolGroups.size() > 1) { + logger.info("takeVMSnapshot: VM [{}] spans {} FlexVol(s); using temporary CG two-phase snapshot flow", + userVm.getInstanceName(), flexVolGroups.size()); + createVmSnapshotsViaTemporaryCg(vmSnapshot, userVm, flexVolGroups, snapshotNameBase, createdSnapshots); + } else { + logger.info("takeVMSnapshot: VM [{}] spans a single FlexVol; using direct FlexVol snapshot flow", + userVm.getInstanceName()); + createVmSnapshotsViaSingleFlexVol(vmSnapshot, userVm, flexVolGroups, snapshotNameBase, createdSnapshots); } } finally { // ── Step 3: Thaw the VM (only if it was frozen, always even on error) ── @@ -456,7 +422,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 [{}] ({} detail row(s))", vmSnapshot.getName(), userVm.getInstanceName(), createdSnapshots.size()); long newChainSize = 0; @@ -668,16 +634,140 @@ Map groupVolumesByFlexVol(List volumeT } /** - * Builds a deterministic, ONTAP-safe snapshot name for a VM snapshot. - * Format: {@code vmsnap__} + * Creates VM snapshot artifacts via direct FlexVol snapshot API. + * + *

Used when all VM volumes map to a single FlexVol. In that case a CG is not + * needed because one FlexVol snapshot already captures every disk atomically.

*/ - 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); + void createVmSnapshotsViaSingleFlexVol(VMSnapshot vmSnapshot, UserVm userVm, + Map flexVolGroups, + String snapshotNameBase, + List createdSnapshots) { + for (Map.Entry entry : flexVolGroups.entrySet()) { + String flexVolUuid = entry.getKey(); + FlexVolGroupInfo groupInfo = entry.getValue(); + long startSnapshot = System.nanoTime(); + + StorageStrategy storageStrategy = resolveStorageStrategy(groupInfo.poolDetails); + SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient(); + String authHeader = storageStrategy.getAuthHeader(); + + FlexVolSnapshot snapshotRequest = new FlexVolSnapshot(snapshotNameBase, + "CloudStack VM snapshot " + vmSnapshot.getName() + " for VM " + userVm.getInstanceName()); + + logger.info("takeVMSnapshot: [FlexVol] Creating 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 + "]"); + } + + Boolean jobSucceeded = storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 30, 2000); + if (!jobSucceeded) { + throw new CloudRuntimeException("FlexVolume snapshot job failed on FlexVol UUID [" + flexVolUuid + "]"); + } + + String snapshotUuid = resolveSnapshotUuid(snapshotClient, authHeader, flexVolUuid, snapshotNameBase); + String protocol = groupInfo.poolDetails.get(OntapStorageConstants.PROTOCOL); + + for (Long volumeId : groupInfo.volumeIds) { + String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails); + createdSnapshots.add(new FlexVolSnapshotDetail( + flexVolUuid, snapshotUuid, snapshotNameBase, volumePath, groupInfo.poolId, protocol)); + } + + logger.info("takeVMSnapshot: [FlexVol] Snapshot [{}] (uuid={}) on FlexVol [{}] completed in {} ms. Covers volumes: {}", + snapshotNameBase, snapshotUuid, flexVolUuid, + TimeUnit.MILLISECONDS.convert(System.nanoTime() - startSnapshot, TimeUnit.NANOSECONDS), + groupInfo.volumeIds); } - return name; + } + + /** + * Creates VM snapshot artifacts via temporary consistency-group two-phase flow. + * + *

Used when VM volumes span multiple FlexVols and require a consistent + * point-in-time capture across all participating FlexVolumes.

+ */ + void createVmSnapshotsViaTemporaryCg(VMSnapshot vmSnapshot, UserVm userVm, + Map flexVolGroups, + String snapshotNameBase, + List createdSnapshots) { + String tempCgName = buildTemporaryConsistencyGroupName(vmSnapshot); + String tempCgUuid = null; + String cgSnapshotUuid = null; + long cgFlowStart = System.nanoTime(); + + // All volumes in a VM snapshot belong to ONTAP-managed pools and share the same ONTAP credentials. + // Use any one FlexVol group to obtain strategy/client objects for this operation. + FlexVolGroupInfo referenceGroup = flexVolGroups.values().iterator().next(); + StorageStrategy storageStrategy = resolveStorageStrategy(referenceGroup.poolDetails); + SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient(); + String authHeader = storageStrategy.getAuthHeader(); + + try { + logger.info("takeVMSnapshot: [CG:Create] Creating temporary consistency group [{}] for VM [{}] over {} FlexVol(s)", + tempCgName, userVm.getInstanceName(), flexVolGroups.size()); + tempCgUuid = createTemporaryConsistencyGroup(snapshotClient, storageStrategy, authHeader, tempCgName, flexVolGroups.keySet()); + + logger.info("takeVMSnapshot: [CG:Start] Starting phase-1 snapshot [{}] for temporary consistency group [{}]", + snapshotNameBase, tempCgUuid); + startConsistencyGroupSnapshot(snapshotClient, storageStrategy, authHeader, tempCgUuid, snapshotNameBase); + + cgSnapshotUuid = resolveConsistencyGroupSnapshotUuid(snapshotClient, authHeader, tempCgUuid, snapshotNameBase); + + logger.info("takeVMSnapshot: [CG:Commit] Committing phase-2 snapshot [{}] (uuid={}) for temporary consistency group [{}]", + snapshotNameBase, cgSnapshotUuid, tempCgUuid); + commitConsistencyGroupSnapshot(snapshotClient, storageStrategy, authHeader, tempCgUuid, cgSnapshotUuid); + + // Resolve per-FlexVol snapshot UUIDs and build one detail entry per CloudStack volume. + for (Map.Entry entry : flexVolGroups.entrySet()) { + String flexVolUuid = entry.getKey(); + FlexVolGroupInfo groupInfo = entry.getValue(); + String snapshotUuid = resolveSnapshotUuid(snapshotClient, authHeader, flexVolUuid, snapshotNameBase); + String protocol = groupInfo.poolDetails.get(OntapStorageConstants.PROTOCOL); + + for (Long volumeId : groupInfo.volumeIds) { + String volumePath = resolveVolumePathOnOntap(volumeId, protocol, groupInfo.poolDetails); + createdSnapshots.add(new FlexVolSnapshotDetail( + flexVolUuid, snapshotUuid, snapshotNameBase, volumePath, groupInfo.poolId, protocol)); + } + + logger.debug("takeVMSnapshot: [CG:Resolve] Snapshot [{}] resolved to FlexVol snapshot uuid [{}] for FlexVol [{}], volumes={}", + snapshotNameBase, snapshotUuid, flexVolUuid, groupInfo.volumeIds); + } + } finally { + // CG is only a transaction boundary; remove it after commit/failure and keep snapshots intact. + if (tempCgUuid != null) { + try { + logger.info("takeVMSnapshot: [CG:Cleanup] Deleting temporary consistency group [{}]", tempCgUuid); + deleteTemporaryConsistencyGroup(snapshotClient, storageStrategy, authHeader, tempCgUuid); + } catch (Exception cleanupEx) { + logger.warn("takeVMSnapshot: Failed to delete temporary consistency group [{}]: {}", + tempCgUuid, cleanupEx.getMessage()); + } + } + } + + logger.info("takeVMSnapshot: Temporary consistency-group two-phase flow completed for VM [{}] in {} ms. CG snapshot uuid={}, detail rows={}", + userVm.getInstanceName(), + TimeUnit.MILLISECONDS.convert(System.nanoTime() - cgFlowStart, TimeUnit.NANOSECONDS), + cgSnapshotUuid, createdSnapshots.size()); + } + + /** + * Builds an ONTAP-safe snapshot name from the CloudStack VM snapshot UI name. + */ + String buildSnapshotName(VMSnapshot vmSnapshot) { + return OntapStorageUtils.buildOntapSnapshotName(vmSnapshot.getName(), "vm" + vmSnapshot.getId()); + } + + /** + * Wrapper for static utility to simplify unit testing and keep call sites explicit. + */ + StorageStrategy resolveStorageStrategy(Map poolDetails) { + return OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); } /** @@ -695,6 +785,133 @@ String resolveSnapshotUuid(SnapshotFeignClient client, String authHeader, return response.getRecords().get(0).getUuid(); } + /** + * Builds a deterministic temporary CG name for the VM snapshot transaction. + */ + String buildTemporaryConsistencyGroupName(VMSnapshot vmSnapshot) { + return OntapStorageUtils.buildOntapSnapshotName( + OntapStorageConstants.ONTAP_TEMP_CG_PREFIX + vmSnapshot.getId(), + "cg" + vmSnapshot.getId()); + } + + /** + * Creates a temporary consistency group for the involved FlexVol UUIDs and returns its UUID. + */ + String createTemporaryConsistencyGroup(SnapshotFeignClient client, StorageStrategy storageStrategy, + String authHeader, String cgName, Set flexVolUuids) { + List> volumeRefs = new ArrayList<>(); + for (String flexVolUuid : flexVolUuids) { + Map volumeRef = new LinkedHashMap<>(); + volumeRef.put("uuid", flexVolUuid); + volumeRefs.add(volumeRef); + } + + Map payload = new LinkedHashMap<>(); + payload.put("name", cgName); + payload.put("volumes", volumeRefs); + + JobResponse response = client.createConsistencyGroup(authHeader, payload); + pollJobIfPresent(storageStrategy, response, "create temporary consistency group " + cgName); + + String cgUuid = resolveConsistencyGroupUuidByName(client, authHeader, cgName); + if (cgUuid == null || cgUuid.isEmpty()) { + throw new CloudRuntimeException("Unable to resolve temporary consistency group UUID for [" + cgName + "]"); + } + return cgUuid; + } + + /** + * Starts phase-1 of the two-phase CG snapshot. + */ + void startConsistencyGroupSnapshot(SnapshotFeignClient client, StorageStrategy storageStrategy, + String authHeader, String cgUuid, String snapshotName) { + Map payload = new LinkedHashMap<>(); + payload.put("name", snapshotName); + payload.put("action", "start"); + JobResponse response = client.createConsistencyGroupSnapshot(authHeader, cgUuid, payload); + pollJobIfPresent(storageStrategy, response, "start CG snapshot " + snapshotName + " for " + cgUuid); + } + + /** + * Commits phase-2 of the started CG snapshot. + */ + void commitConsistencyGroupSnapshot(SnapshotFeignClient client, StorageStrategy storageStrategy, + String authHeader, String cgUuid, String snapshotUuid) { + Map payload = new LinkedHashMap<>(); + payload.put("action", "commit"); + JobResponse response = client.commitConsistencyGroupSnapshot(authHeader, cgUuid, snapshotUuid, payload); + pollJobIfPresent(storageStrategy, response, "commit CG snapshot " + snapshotUuid + " for " + cgUuid); + } + + /** + * Deletes the temporary consistency group used as a transaction boundary. + */ + void deleteTemporaryConsistencyGroup(SnapshotFeignClient client, StorageStrategy storageStrategy, + String authHeader, String cgUuid) { + JobResponse response = client.deleteConsistencyGroup(authHeader, cgUuid); + pollJobIfPresent(storageStrategy, response, "delete temporary consistency group " + cgUuid); + } + + /** + * Resolves consistency group UUID by name. + */ + String resolveConsistencyGroupUuidByName(SnapshotFeignClient client, String authHeader, String cgName) { + Map query = new HashMap<>(); + query.put("name", cgName); + query.put("fields", "uuid,name"); + OntapResponse> response = client.getConsistencyGroups(authHeader, query); + if (response == null || response.getRecords() == null || response.getRecords().isEmpty()) { + return null; + } + return getStringField(response.getRecords().get(0), "uuid"); + } + + /** + * Resolves consistency group snapshot UUID by name. + */ + String resolveConsistencyGroupSnapshotUuid(SnapshotFeignClient client, String authHeader, + String cgUuid, String snapshotName) { + Map query = new HashMap<>(); + query.put("name", snapshotName); + query.put("fields", "uuid,name"); + OntapResponse> response = client.getConsistencyGroupSnapshots(authHeader, cgUuid, query); + if (response == null || response.getRecords() == null || response.getRecords().isEmpty()) { + throw new CloudRuntimeException("Unable to resolve consistency group snapshot UUID for snapshot [" + + snapshotName + "] in CG [" + cgUuid + "]"); + } + String snapshotUuid = getStringField(response.getRecords().get(0), "uuid"); + if (snapshotUuid == null || snapshotUuid.isEmpty()) { + throw new CloudRuntimeException("Invalid consistency group snapshot UUID for snapshot [" + + snapshotName + "] in CG [" + cgUuid + "]"); + } + return snapshotUuid; + } + + /** + * Polls ONTAP job only when the endpoint returns a job reference. + */ + void pollJobIfPresent(StorageStrategy storageStrategy, JobResponse response, String operationName) { + if (response == null || response.getJob() == null || response.getJob().getUuid() == null) { + logger.debug("pollJobIfPresent: No async job returned for operation [{}], continuing without polling", operationName); + return; + } + Boolean success = storageStrategy.jobPollForSuccess( + response.getJob().getUuid(), + OntapStorageConstants.ONTAP_CG_JOB_MAX_RETRIES, + OntapStorageConstants.ONTAP_CG_JOB_POLL_INTERVAL_MS); + if (!Boolean.TRUE.equals(success)) { + throw new CloudRuntimeException("ONTAP operation failed: " + operationName); + } + } + + private String getStringField(Map record, String key) { + if (record == null) { + return null; + } + Object value = record.get(key); + return value != null ? value.toString() : null; + } + /** * Resolves the ONTAP-side path of a CloudStack volume within its FlexVolume. * @@ -735,7 +952,7 @@ String resolveVolumePathOnOntap(Long volumeId, String protocol, Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); - StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); + StorageStrategy storageStrategy = resolveStorageStrategy(poolDetails); SnapshotFeignClient client = storageStrategy.getSnapshotFeignClient(); String authHeader = storageStrategy.getAuthHeader(); @@ -769,7 +986,7 @@ void deleteFlexVolSnapshots(List flexVolDetails) { // 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); + StorageStrategy storageStrategy = resolveStorageStrategy(poolDetails); SnapshotFeignClient client = storageStrategy.getSnapshotFeignClient(); String authHeader = storageStrategy.getAuthHeader(); @@ -818,7 +1035,7 @@ void revertFlexVolSnapshots(List flexVolDetails) { } Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); - StorageStrategy storageStrategy = OntapStorageUtils.getStrategyByStoragePoolDetails(poolDetails); + StorageStrategy storageStrategy = resolveStorageStrategy(poolDetails); SnapshotFeignClient snapshotClient = storageStrategy.getSnapshotFeignClient(); String authHeader = storageStrategy.getAuthHeader(); 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..9d7989d65e7a 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 @@ -21,12 +21,17 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -44,6 +49,12 @@ 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.FlexVolSnapshot; +import org.apache.cloudstack.storage.feign.model.Job; +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.to.VolumeObjectTO; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -127,6 +138,10 @@ class OntapVMSnapshotStrategyTest { private VolumeDataFactory volumeDataFactory; @Mock private VolumeDetailsDao volumeDetailsDao; + @Mock + private StorageStrategy storageStrategy; + @Mock + private SnapshotFeignClient snapshotFeignClient; @Spy @InjectMocks @@ -226,14 +241,18 @@ void testCanHandle_AllocatedDiskType_VmxenHypervisor_ReturnsCantHandle() { } @Test - void testCanHandle_AllocatedDiskType_VmNotRunning_ReturnsCantHandle() { + void testCanHandle_AllocatedDiskType_VmStopped_ReturnsHighest() { UserVmVO userVm = createMockUserVm(Hypervisor.HypervisorType.KVM, VirtualMachine.State.Stopped); when(userVmDao.findById(VM_ID)).thenReturn(userVm); VMSnapshotVO vmSnapshot = createMockVmSnapshot(VMSnapshot.State.Allocated, VMSnapshot.Type.Disk); + VolumeVO vol = createMockVolume(VOLUME_ID_1, POOL_ID_1); + when(volumeDao.findByInstance(VM_ID)).thenReturn(Collections.singletonList(vol)); + StoragePoolVO pool = createOntapManagedPool(POOL_ID_1); + when(storagePool.findById(POOL_ID_1)).thenReturn(pool); StrategyPriority result = strategy.canHandle(vmSnapshot); - assertEquals(StrategyPriority.CANT_HANDLE, result); + assertEquals(StrategyPriority.HIGHEST, result); } @Test @@ -593,10 +612,11 @@ void testFlexVolSnapshotDetail_Parse5Parts_ThrowsException() { void testBuildSnapshotName_Format() { VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); when(vmSnapshot.getId()).thenReturn(SNAPSHOT_ID); + when(vmSnapshot.getName()).thenReturn("UI VM Snapshot"); String name = strategy.buildSnapshotName(vmSnapshot); - assertEquals(true, name.startsWith("vmsnap_200_")); + assertEquals(true, name.startsWith("UI_VM_Snapshot_vm200")); assertEquals(true, name.length() <= OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH); } @@ -732,6 +752,86 @@ void testTakeVMSnapshot_OperationTimeout_ThrowsCloudRuntimeException() throws Ex assertEquals(true, ex.getMessage().contains("timed out")); } + @Test + void testTakeVMSnapshot_SingleFlexVolSuccess_UsesDirectSnapshotNotCg() throws Exception { + VMSnapshotVO vmSnapshot = createTakeSnapshotVmSnapshot(); + setupTakeSnapshotCommon(vmSnapshot); + setupSingleVolumeForTakeSnapshot(); + + String snapshotName = strategy.buildSnapshotName(vmSnapshot); + setupSingleFlexVolFlowMocks(snapshotName); + + FreezeThawVMAnswer freezeAnswer = mock(FreezeThawVMAnswer.class); + when(freezeAnswer.getResult()).thenReturn(true); + FreezeThawVMAnswer thawAnswer = mock(FreezeThawVMAnswer.class); + when(thawAnswer.getResult()).thenReturn(true); + when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class))) + .thenReturn(freezeAnswer) + .thenReturn(thawAnswer); + + strategy.takeVMSnapshot(vmSnapshot); + + verify(snapshotFeignClient, times(1)).createSnapshot(any(), eq("flexvol-uuid-1"), any()); + verify(snapshotFeignClient, never()).createConsistencyGroup(any(), any()); + verify(snapshotFeignClient, never()).createConsistencyGroupSnapshot(any(), any(), any()); + verify(snapshotFeignClient, never()).commitConsistencyGroupSnapshot(any(), any(), any(), any()); + verify(snapshotFeignClient, never()).deleteConsistencyGroup(any(), any()); + verify(vmSnapshotDetailsDao, atLeastOnce()).persist(any(VMSnapshotDetailsVO.class)); + } + + @Test + void testTakeVMSnapshot_TemporaryCgTwoPhaseSuccess_PersistsDetailsAndCleansUpCg() throws Exception { + VMSnapshotVO vmSnapshot = createTakeSnapshotVmSnapshot(); + setupTakeSnapshotCommon(vmSnapshot); + setupMultiFlexVolForTakeSnapshot(); + + String snapshotName = strategy.buildSnapshotName(vmSnapshot); + setupTemporaryCgFlowMocks(snapshotName); + + FreezeThawVMAnswer freezeAnswer = mock(FreezeThawVMAnswer.class); + when(freezeAnswer.getResult()).thenReturn(true); + FreezeThawVMAnswer thawAnswer = mock(FreezeThawVMAnswer.class); + when(thawAnswer.getResult()).thenReturn(true); + when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class))) + .thenReturn(freezeAnswer) + .thenReturn(thawAnswer); + + strategy.takeVMSnapshot(vmSnapshot); + + verify(snapshotFeignClient, times(1)).createConsistencyGroup(any(), any()); + verify(snapshotFeignClient, times(1)).createConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), any()); + verify(snapshotFeignClient, times(1)).commitConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), eq("cg-snap-uuid-1"), any()); + verify(snapshotFeignClient, times(1)).deleteConsistencyGroup(any(), eq("cg-uuid-1")); + verify(vmSnapshotDetailsDao, atLeastOnce()).persist(any(VMSnapshotDetailsVO.class)); + } + + @Test + void testTakeVMSnapshot_TemporaryCgStartFails_TransitionsToOperationFailed() throws Exception { + VMSnapshotVO vmSnapshot = createTakeSnapshotVmSnapshot(); + setupTakeSnapshotCommon(vmSnapshot); + setupMultiFlexVolForTakeSnapshot(); + + String snapshotName = strategy.buildSnapshotName(vmSnapshot); + setupTemporaryCgFlowMocks(snapshotName); + when(snapshotFeignClient.createConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), any())) + .thenThrow(new CloudRuntimeException("start phase failed")); + + FreezeThawVMAnswer freezeAnswer = mock(FreezeThawVMAnswer.class); + when(freezeAnswer.getResult()).thenReturn(true); + FreezeThawVMAnswer thawAnswer = mock(FreezeThawVMAnswer.class); + when(thawAnswer.getResult()).thenReturn(true); + when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class))) + .thenReturn(freezeAnswer) + .thenReturn(thawAnswer); + when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList()); + doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed)); + + assertThrows(CloudRuntimeException.class, () -> strategy.takeVMSnapshot(vmSnapshot)); + + verify(snapshotFeignClient, times(1)).deleteConsistencyGroup(any(), eq("cg-uuid-1")); + verify(vmSnapshotHelper, atLeastOnce()).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed)); + } + // ══════════════════════════════════════════════════════════════════════════ // Tests: Quiesce Behavior // ══════════════════════════════════════════════════════════════════════════ @@ -746,20 +846,9 @@ void testTakeVMSnapshot_QuiesceFalse_SkipsFreezeThaw() throws Exception { setupTakeSnapshotCommon(vmSnapshot); setupSingleVolumeForTakeSnapshot(); + setupSingleFlexVolFlowMocks(strategy.buildSnapshotName(vmSnapshot)); - // The FlexVolume snapshot flow will try to call Utility.getStrategyByStoragePoolDetails - // which is a static method that makes real connections. We expect this to fail in unit tests. - // The important thing is that freeze/thaw was NOT called before the failure. - when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList()); - doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed)); - - // Since Utility.getStrategyByStoragePoolDetails is static and creates real Feign clients, - // this will fail. We just verify that freeze was never called. - try { - strategy.takeVMSnapshot(vmSnapshot); - } catch (Exception e) { - // Expected — static utility can't be mocked in unit test - } + strategy.takeVMSnapshot(vmSnapshot); // No freeze/thaw commands should be sent when quiesce is false verify(agentMgr, never()).send(eq(HOST_ID), any(FreezeThawVMCommand.class)); @@ -790,16 +879,9 @@ void testTakeVMSnapshot_WithParentSnapshot_SetsParentId() throws Exception { when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class))) .thenReturn(freezeAnswer) .thenReturn(thawAnswer); + setupSingleFlexVolFlowMocks(strategy.buildSnapshotName(vmSnapshot)); - when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList()); - doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed)); - - // FlexVol snapshot flow will fail on static method, but parent should already be set - try { - strategy.takeVMSnapshot(vmSnapshot); - } catch (Exception e) { - // Expected - } + strategy.takeVMSnapshot(vmSnapshot); // Verify parent was set on the VM snapshot before the FlexVol snapshot attempt verify(vmSnapshot).setParent(199L); @@ -820,15 +902,9 @@ void testTakeVMSnapshot_WithNoParentSnapshot_SetsParentNull() throws Exception { when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class))) .thenReturn(freezeAnswer) .thenReturn(thawAnswer); + setupSingleFlexVolFlowMocks(strategy.buildSnapshotName(vmSnapshot)); - when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList()); - doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed)); - - try { - strategy.takeVMSnapshot(vmSnapshot); - } catch (Exception e) { - // Expected - } + strategy.takeVMSnapshot(vmSnapshot); verify(vmSnapshot).setParent(null); } @@ -866,6 +942,9 @@ private UserVmVO setupTakeSnapshotCommon(VMSnapshotVO vmSnapshot) throws Excepti when(vmSnapshotDao.findCurrentSnapshotByVmId(VM_ID)).thenReturn(null); doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.CreateRequested); + doNothing().when(strategy).processAnswer(any(), any(), any(), any()); + doNothing().when(strategy).publishUsageEvent(any(), any(), any(), any()); + doNothing().when(strategy).publishUsageEvent(any(), any(), any(), anyLong(), anyLong()); return userVm; } @@ -880,6 +959,7 @@ private void setupSingleVolumeForTakeSnapshot() { VolumeVO volumeVO = mock(VolumeVO.class); when(volumeVO.getId()).thenReturn(VOLUME_ID_1); when(volumeVO.getPoolId()).thenReturn(POOL_ID_1); + when(volumeVO.getPath()).thenReturn("volume-301.qcow2"); when(volumeVO.getVmSnapshotChainSize()).thenReturn(null); when(volumeDao.findById(VOLUME_ID_1)).thenReturn(volumeVO); @@ -899,4 +979,127 @@ private void setupSingleVolumeForTakeSnapshot() { when(volumeInfo.getName()).thenReturn("vol-1"); when(volumeDataFactory.getVolume(VOLUME_ID_1)).thenReturn(volumeInfo); } + + private void setupMultiFlexVolForTakeSnapshot() { + VolumeObjectTO volumeTO1 = mock(VolumeObjectTO.class); + when(volumeTO1.getId()).thenReturn(VOLUME_ID_1); + when(volumeTO1.getSize()).thenReturn(10737418240L); + VolumeObjectTO volumeTO2 = mock(VolumeObjectTO.class); + when(volumeTO2.getId()).thenReturn(VOLUME_ID_2); + when(volumeTO2.getSize()).thenReturn(10737418240L); + List volumeTOs = Arrays.asList(volumeTO1, volumeTO2); + when(vmSnapshotHelper.getVolumeTOList(VM_ID)).thenReturn(volumeTOs); + + VolumeVO volumeVO1 = mock(VolumeVO.class); + when(volumeVO1.getId()).thenReturn(VOLUME_ID_1); + when(volumeVO1.getPoolId()).thenReturn(POOL_ID_1); + when(volumeVO1.getPath()).thenReturn("volume-301.qcow2"); + when(volumeVO1.getVmSnapshotChainSize()).thenReturn(null); + when(volumeDao.findById(VOLUME_ID_1)).thenReturn(volumeVO1); + + VolumeVO volumeVO2 = mock(VolumeVO.class); + when(volumeVO2.getId()).thenReturn(VOLUME_ID_2); + when(volumeVO2.getPoolId()).thenReturn(POOL_ID_2); + when(volumeVO2.getPath()).thenReturn("volume-302.qcow2"); + when(volumeVO2.getVmSnapshotChainSize()).thenReturn(null); + when(volumeDao.findById(VOLUME_ID_2)).thenReturn(volumeVO2); + + Map poolDetails1 = new HashMap<>(); + poolDetails1.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-1"); + poolDetails1.put(OntapStorageConstants.USERNAME, "admin"); + poolDetails1.put(OntapStorageConstants.PASSWORD, "pass"); + poolDetails1.put(OntapStorageConstants.STORAGE_IP, "10.0.0.1"); + poolDetails1.put(OntapStorageConstants.SVM_NAME, "svm1"); + poolDetails1.put(OntapStorageConstants.SIZE, "107374182400"); + poolDetails1.put(OntapStorageConstants.PROTOCOL, "NFS3"); + when(storagePoolDetailsDao.listDetailsKeyPairs(POOL_ID_1)).thenReturn(poolDetails1); + + Map poolDetails2 = new HashMap<>(); + poolDetails2.put(OntapStorageConstants.VOLUME_UUID, "flexvol-uuid-2"); + poolDetails2.put(OntapStorageConstants.USERNAME, "admin"); + poolDetails2.put(OntapStorageConstants.PASSWORD, "pass"); + poolDetails2.put(OntapStorageConstants.STORAGE_IP, "10.0.0.1"); + poolDetails2.put(OntapStorageConstants.SVM_NAME, "svm1"); + poolDetails2.put(OntapStorageConstants.SIZE, "107374182400"); + poolDetails2.put(OntapStorageConstants.PROTOCOL, "NFS3"); + when(storagePoolDetailsDao.listDetailsKeyPairs(POOL_ID_2)).thenReturn(poolDetails2); + + VolumeInfo volumeInfo1 = mock(VolumeInfo.class); + when(volumeInfo1.getId()).thenReturn(VOLUME_ID_1); + when(volumeDataFactory.getVolume(VOLUME_ID_1)).thenReturn(volumeInfo1); + VolumeInfo volumeInfo2 = mock(VolumeInfo.class); + when(volumeInfo2.getId()).thenReturn(VOLUME_ID_2); + when(volumeDataFactory.getVolume(VOLUME_ID_2)).thenReturn(volumeInfo2); + } + + private JobResponse createJobResponse(String uuid) { + Job job = new Job(); + job.setUuid(uuid); + JobResponse response = new JobResponse(); + response.setJob(job); + return response; + } + + private void setupSingleFlexVolFlowMocks(String snapshotName) { + doReturn(storageStrategy).when(strategy).resolveStorageStrategy(any()); + when(storageStrategy.getSnapshotFeignClient()).thenReturn(snapshotFeignClient); + when(storageStrategy.getAuthHeader()).thenReturn("Basic dGVzdDp0ZXN0"); + when(storageStrategy.jobPollForSuccess(any(), anyInt(), anyInt())).thenReturn(true); + + when(snapshotFeignClient.createSnapshot(any(), eq("flexvol-uuid-1"), any())) + .thenReturn(createJobResponse("job-fv-snap")); + + OntapResponse flexVolSnapshots = new OntapResponse<>(); + FlexVolSnapshot flexVolSnapshot = new FlexVolSnapshot(); + flexVolSnapshot.setUuid("fv-snap-uuid-1"); + flexVolSnapshot.setName(snapshotName); + flexVolSnapshots.setRecords(Collections.singletonList(flexVolSnapshot)); + when(snapshotFeignClient.getSnapshots(any(), eq("flexvol-uuid-1"), any())) + .thenReturn(flexVolSnapshots); + } + + private void setupTemporaryCgFlowMocks(String snapshotName) { + doReturn(storageStrategy).when(strategy).resolveStorageStrategy(any()); + when(storageStrategy.getSnapshotFeignClient()).thenReturn(snapshotFeignClient); + when(storageStrategy.getAuthHeader()).thenReturn("Basic dGVzdDp0ZXN0"); + when(storageStrategy.jobPollForSuccess(any(), anyInt(), anyInt())).thenReturn(true); + + when(snapshotFeignClient.createConsistencyGroup(any(), any())).thenReturn(createJobResponse("job-cg-create")); + OntapResponse> cgResponse = new OntapResponse<>(); + Map cgRecord = new HashMap<>(); + cgRecord.put("uuid", "cg-uuid-1"); + cgResponse.setRecords(Collections.singletonList(cgRecord)); + when(snapshotFeignClient.getConsistencyGroups(any(), any())).thenReturn(cgResponse); + + when(snapshotFeignClient.createConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), any())) + .thenReturn(createJobResponse("job-cg-start")); + OntapResponse> cgSnapshotResponse = new OntapResponse<>(); + Map cgSnapshotRecord = new HashMap<>(); + cgSnapshotRecord.put("uuid", "cg-snap-uuid-1"); + cgSnapshotRecord.put("name", snapshotName); + cgSnapshotResponse.setRecords(Collections.singletonList(cgSnapshotRecord)); + when(snapshotFeignClient.getConsistencyGroupSnapshots(any(), eq("cg-uuid-1"), any())) + .thenReturn(cgSnapshotResponse); + when(snapshotFeignClient.commitConsistencyGroupSnapshot(any(), eq("cg-uuid-1"), eq("cg-snap-uuid-1"), any())) + .thenReturn(createJobResponse("job-cg-commit")); + + when(snapshotFeignClient.deleteConsistencyGroup(any(), eq("cg-uuid-1"))) + .thenReturn(createJobResponse("job-cg-delete")); + + OntapResponse flexVolSnapshots = new OntapResponse<>(); + FlexVolSnapshot flexVolSnapshot = new FlexVolSnapshot(); + flexVolSnapshot.setUuid("fv-snap-uuid-1"); + flexVolSnapshot.setName(snapshotName); + flexVolSnapshots.setRecords(Collections.singletonList(flexVolSnapshot)); + when(snapshotFeignClient.getSnapshots(any(), eq("flexvol-uuid-1"), any())) + .thenReturn(flexVolSnapshots); + + OntapResponse flexVolSnapshots2 = new OntapResponse<>(); + FlexVolSnapshot flexVolSnapshot2 = new FlexVolSnapshot(); + flexVolSnapshot2.setUuid("fv-snap-uuid-2"); + flexVolSnapshot2.setName(snapshotName); + flexVolSnapshots2.setRecords(Collections.singletonList(flexVolSnapshot2)); + when(snapshotFeignClient.getSnapshots(any(), eq("flexvol-uuid-2"), any())) + .thenReturn(flexVolSnapshots2); + } } From 4ec7c181a2d023df7440509211962cdebe812df5 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Sat, 27 Jun 2026 12:58:30 +0530 Subject: [PATCH 2/2] CSTACKEX-200: added consistency for the VM level snapshots if CSV are at multiple flexvolumes --- .../driver/OntapPrimaryDatastoreDriver.java | 25 ++------- .../storage/service/StorageStrategy.java | 55 ++++++++++++++++--- .../storage/utils/OntapStorageConstants.java | 2 + .../vmsnapshot/OntapVMSnapshotStrategy.java | 53 +++--------------- .../OntapPrimaryDatastoreDriverTest.java | 1 + .../storage/service/StorageStrategyTest.java | 43 +++++++++++++++ 6 files changed, 107 insertions(+), 72 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 ac9b7e818747..5a1bbc9ee41c 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 @@ -97,6 +97,7 @@ public Map getCapabilities() { Map mapCapabilities = new HashMap<>(); mapCapabilities.put(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString(), Boolean.TRUE.toString()); mapCapabilities.put(DataStoreCapabilities.CAN_CREATE_VOLUME_FROM_SNAPSHOT.toString(), Boolean.TRUE.toString()); + mapCapabilities.put(DataStoreCapabilities.CAN_REVERT_VOLUME_TO_SNAPSHOT.toString(), Boolean.TRUE.toString()); return mapCapabilities; } @@ -683,6 +684,7 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback * - *

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.
    • - *
+ *

Both NFS and iSCSI delegate to CLI-based SFSR: + * {@code POST /api/private/cli/volume/snapshot/restore-file}

*/ @Override public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snapshotOnPrimaryStore, @@ -846,17 +841,7 @@ public void revertSnapshot(SnapshotInfo snapshotOnImageStore, SnapshotInfo snaps JobResponse jobResponse = storageStrategy.revertSnapshotForCloudStackVolume( snapshotName, flexVolUuid, ontapSnapshotUuid, volumePath, lunUuid, flexVolName); - if (jobResponse == null || jobResponse.getJob() == null) { - throw new CloudRuntimeException("Failed to initiate restore from snapshot [" + - snapshotName + "]"); - } - - // 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 + "]"); - } + storageStrategy.executeCliSfsrRestore(jobResponse, "revert snapshot [" + snapshotName + "]"); logger.info("revertSnapshot: Successfully restored {} [{}] from snapshot [{}]", 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 bd808a26d6f8..f6b9e57bc0ac 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 @@ -527,15 +527,13 @@ 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 to a snapshot using ONTAP CLI-based Single File Snap Restore (SFSR). * - *

This method encapsulates the snapshot revert 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.
    • - *
+ *

Both NFS and iSCSI use the CLI passthrough API: + * {@code POST /api/private/cli/volume/snapshot/restore-file}

+ * + *

Callers should invoke {@link #executeCliSfsrRestore(JobResponse, String)} after this + * method returns to poll the async job when present, or treat a missing job as synchronous success.

* * @param snapshotName The ONTAP FlexVolume snapshot name * @param flexVolUuid The FlexVolume UUID containing the snapshot @@ -681,4 +679,45 @@ public Boolean jobPollForSuccess(String jobUUID, int maxRetries, int sleepTimeIn } return true; } + + /** + * Polls an ONTAP async job when the API response includes a job reference. + * + *

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

+ * + * @param response ONTAP job response (may be null or without a job) + * @param operationName label for logging and error messages + */ + public void pollJobIfPresent(JobResponse response, String operationName) { + pollJobIfPresent(response, operationName, + OntapStorageConstants.ONTAP_CG_JOB_MAX_RETRIES, + OntapStorageConstants.ONTAP_CG_JOB_POLL_INTERVAL_MS); + } + + /** + * Polls an ONTAP async job when present, using caller-supplied retry settings. + */ + public void pollJobIfPresent(JobResponse response, String operationName, + int maxRetries, int pollIntervalMs) { + if (response == null || response.getJob() == null || response.getJob().getUuid() == null) { + logger.debug("pollJobIfPresent: No async job returned for operation [{}], continuing without polling", + operationName); + return; + } + Boolean success = jobPollForSuccess(response.getJob().getUuid(), maxRetries, pollIntervalMs); + if (!Boolean.TRUE.equals(success)) { + throw new CloudRuntimeException("ONTAP operation failed: " + operationName); + } + } + + /** + * Completes CLI-based SFSR ({@code restore-file}) orchestration: poll job when returned, + * otherwise accept synchronous success. + */ + public void executeCliSfsrRestore(JobResponse response, String operationName) { + pollJobIfPresent(response, operationName, + OntapStorageConstants.ONTAP_SFSR_JOB_MAX_RETRIES, + OntapStorageConstants.ONTAP_SFSR_JOB_POLL_INTERVAL_MS); + } } 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 39ffb60633e7..a5243f35ac05 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,8 @@ public class OntapStorageConstants { public static final String ONTAP_TEMP_CG_PREFIX = "cs-temp-cg-"; public static final int ONTAP_CG_JOB_MAX_RETRIES = 60; public static final int ONTAP_CG_JOB_POLL_INTERVAL_MS = 2000; + public static final int ONTAP_SFSR_JOB_MAX_RETRIES = 60; + public static final int ONTAP_SFSR_JOB_POLL_INTERVAL_MS = 2000; /** 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/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index d07d2aebe604..aae4f289e100 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 @@ -35,7 +35,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; @@ -811,7 +810,7 @@ String createTemporaryConsistencyGroup(SnapshotFeignClient client, StorageStrate payload.put("volumes", volumeRefs); JobResponse response = client.createConsistencyGroup(authHeader, payload); - pollJobIfPresent(storageStrategy, response, "create temporary consistency group " + cgName); + storageStrategy.pollJobIfPresent(response, "create temporary consistency group " + cgName); String cgUuid = resolveConsistencyGroupUuidByName(client, authHeader, cgName); if (cgUuid == null || cgUuid.isEmpty()) { @@ -829,7 +828,7 @@ void startConsistencyGroupSnapshot(SnapshotFeignClient client, StorageStrategy s payload.put("name", snapshotName); payload.put("action", "start"); JobResponse response = client.createConsistencyGroupSnapshot(authHeader, cgUuid, payload); - pollJobIfPresent(storageStrategy, response, "start CG snapshot " + snapshotName + " for " + cgUuid); + storageStrategy.pollJobIfPresent(response, "start CG snapshot " + snapshotName + " for " + cgUuid); } /** @@ -840,7 +839,7 @@ void commitConsistencyGroupSnapshot(SnapshotFeignClient client, StorageStrategy Map payload = new LinkedHashMap<>(); payload.put("action", "commit"); JobResponse response = client.commitConsistencyGroupSnapshot(authHeader, cgUuid, snapshotUuid, payload); - pollJobIfPresent(storageStrategy, response, "commit CG snapshot " + snapshotUuid + " for " + cgUuid); + storageStrategy.pollJobIfPresent(response, "commit CG snapshot " + snapshotUuid + " for " + cgUuid); } /** @@ -849,7 +848,7 @@ void commitConsistencyGroupSnapshot(SnapshotFeignClient client, StorageStrategy void deleteTemporaryConsistencyGroup(SnapshotFeignClient client, StorageStrategy storageStrategy, String authHeader, String cgUuid) { JobResponse response = client.deleteConsistencyGroup(authHeader, cgUuid); - pollJobIfPresent(storageStrategy, response, "delete temporary consistency group " + cgUuid); + storageStrategy.pollJobIfPresent(response, "delete temporary consistency group " + cgUuid); } /** @@ -887,23 +886,6 @@ String resolveConsistencyGroupSnapshotUuid(SnapshotFeignClient client, String au return snapshotUuid; } - /** - * Polls ONTAP job only when the endpoint returns a job reference. - */ - void pollJobIfPresent(StorageStrategy storageStrategy, JobResponse response, String operationName) { - if (response == null || response.getJob() == null || response.getJob().getUuid() == null) { - logger.debug("pollJobIfPresent: No async job returned for operation [{}], continuing without polling", operationName); - return; - } - Boolean success = storageStrategy.jobPollForSuccess( - response.getJob().getUuid(), - OntapStorageConstants.ONTAP_CG_JOB_MAX_RETRIES, - OntapStorageConstants.ONTAP_CG_JOB_POLL_INTERVAL_MS); - if (!Boolean.TRUE.equals(success)) { - throw new CloudRuntimeException("ONTAP operation failed: " + operationName); - } - } - private String getStringField(Map record, String key) { if (record == null) { return null; @@ -1036,40 +1018,23 @@ void revertFlexVolSnapshots(List flexVolDetails) { Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); StorageStrategy storageStrategy = resolveStorageStrategy(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); + JobResponse jobResponse = storageStrategy.revertSnapshotForCloudStackVolume( + detail.snapshotName, detail.flexVolUuid, detail.snapshotUuid, + detail.volumePath, null, 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 + - "] on FlexVol [" + flexVolName + "]"); - } - } + storageStrategy.executeCliSfsrRestore(jobResponse, + "VM snapshot file restore for path [" + ontapFilePath + "] from snapshot [" + detail.snapshotName + "]"); logger.info("revertFlexVolSnapshots: Successfully restored volume [{}] from snapshot [{}] on FlexVol [{}]", ontapFilePath, 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..e176bd3993b3 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 @@ -134,6 +134,7 @@ void testGetCapabilities() { // so StorageSystemSnapshotStrategy handles snapshot backup to secondary storage assertEquals(Boolean.TRUE.toString(), capabilities.get("STORAGE_SYSTEM_SNAPSHOT")); assertEquals(Boolean.TRUE.toString(), capabilities.get("CAN_CREATE_VOLUME_FROM_SNAPSHOT")); + assertEquals(Boolean.TRUE.toString(), capabilities.get("CAN_REVERT_VOLUME_TO_SNAPSHOT")); } @Test diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java index 86ef1d7c79b6..ca952fef0887 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 @@ -823,4 +823,47 @@ private void setupSuccessfulJobCreation() { when(volumeFeignClient.getVolume(anyString(), anyMap())) .thenReturn(volumeResponse); } + + // ========== pollJobIfPresent / executeCliSfsrRestore Tests ========== + + @Test + void testPollJobIfPresent_NoJob_DoesNotPoll() { + storageStrategy.pollJobIfPresent(null, "test operation"); + storageStrategy.pollJobIfPresent(new JobResponse(), "test operation"); + verify(jobFeignClient, times(0)).getJobByUUID(anyString(), anyString()); + } + + @Test + void testPollJobIfPresent_WithJob_PollsUntilSuccess() { + Job job = new Job(); + job.setUuid("sfsr-job-1"); + JobResponse response = new JobResponse(); + response.setJob(job); + + Job completedJob = new Job(); + completedJob.setUuid("sfsr-job-1"); + completedJob.setState(OntapStorageConstants.JOB_SUCCESS); + when(jobFeignClient.getJobByUUID(anyString(), eq("sfsr-job-1"))).thenReturn(completedJob); + + storageStrategy.executeCliSfsrRestore(response, "CLI SFSR restore"); + + verify(jobFeignClient, atLeastOnce()).getJobByUUID(anyString(), eq("sfsr-job-1")); + } + + @Test + void testPollJobIfPresent_JobFailure_ThrowsCloudRuntimeException() { + Job job = new Job(); + job.setUuid("sfsr-job-fail"); + JobResponse response = new JobResponse(); + response.setJob(job); + + Job failedJob = new Job(); + failedJob.setUuid("sfsr-job-fail"); + failedJob.setState(OntapStorageConstants.JOB_FAILURE); + failedJob.setMessage("restore failed"); + when(jobFeignClient.getJobByUUID(anyString(), eq("sfsr-job-fail"))).thenReturn(failedJob); + + assertThrows(CloudRuntimeException.class, + () -> storageStrategy.executeCliSfsrRestore(response, "CLI SFSR restore")); + } }