CSTACKEX:200 - added temp CG logic for VM snapshots if VM span across… by rajiv-jain-netapp · Pull Request #67 · NetApp/cloudstack

rajiv-jain-netapp · 2026-06-26T06:27:05Z

… multiple volumes at storage

Description

This PR...

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

rajiv-jain-netapp · 2026-06-28T15:53:33Z

Testing:

create snapshot at VM and cloudstack volume level
Able to perform snapshot revert for both vm level and cloudstack volume level
created VM with respective volumes at 2 flexvolumes with same protocol.
tested with snapshot create and snapshot revert.
tested for snapshot delete for all above 4 cases

Copilot

Pull request overview

This PR extends CloudStack’s snapshot handling for ONTAP-managed storage by adding temporary consistency-group (CG) orchestration for VM snapshots that span multiple FlexVols, while also tightening snapshot archival behavior so managed-primary snapshots remain on array/primary storage.

Changes:

Add temporary ONTAP consistency-group (two-phase start+commit) flow for VM snapshots spanning multiple FlexVols; use direct FlexVol snapshots for single-FlexVol cases.
Adjust snapshot archival-to-secondary logic to avoid secondary bookkeeping/copy for managed primary snapshots (e.g., ONTAP volume snapshots kept on PRIMARY).
Refactor ONTAP strategy utilities: introduce “operations-only” connect mode, add job polling helpers, and centralize delete/revert logic in StorageStrategy.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java	Avoids archiving managed-primary snapshots to secondary; clarifies behavior and logging.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java	Implements temporary CG orchestration for multi-FlexVol VM snapshots; refactors snapshot naming and strategy resolution.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java	Changes strategy connection defaults and adds overload to control aggregate validation.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java	Adds CG/SVM UUID constants and polling configuration knobs.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java	Adds operations-only connect path, job polling helpers, and shared snapshot delete orchestration.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java	Persists resolved SVM UUID into pool details during initialization.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java	Adds Feign bindings for consistency-group and CG snapshot APIs.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java	Delegates volume-snapshot delete and revert orchestration into `StorageStrategy`; updates capability flags and snapshot naming.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java	Adds unit tests covering single-FlexVol vs CG flows, scope validation, and snapshot naming updates.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java	Extends tests for operations-only connect and new polling/delete helpers.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java	Updates capability expectations (revert supported).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    /**
+     * Returns a connected {@link StorageStrategy} for operations on an existing pool (snapshots,
+     * delete, revert, grant/revoke). Does not require aggregate free space for the full pool size.
+     */
    public static StorageStrategy getStrategyByStoragePoolDetails(Map<String, String> details) {
+        return getStrategyByStoragePoolDetails(details, false);
+    }


+    /**
+     * Single GET attempt: match by name, then fall back to listing all CG snapshots in this group (And it would one
+     *  always since workflow is keep deleting the CG).
+     */


github-actions · 2026-06-29T05:15:48Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

suryag1201 · 2026-06-30T07:24:20Z

                if (lunUUID == null) {
                    throw new CloudRuntimeException("LUN UUID not found for iSCSI volume " + volumeVO.getId());
                }
+                lunUuid = lunUUID;


no need of this extra lunUuid variable assignment with temp lunUUID var, can be done directly also

suryag1201 · 2026-06-30T07:28:11Z

-            name = StringUtils.left(volumeName, volumeName.length() - trimRequired) + "-" + snapshotUuid;
-        }
-        return name;
+    private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId) {


as discussed, we have to check for two cloudstack using same ontap.

suryag1201 · 2026-06-30T07:30:21Z

-        }
-        return name;
+    private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId) {
+        return OntapStorageUtils.buildOntapSnapshotName(cloudStackSnapshotName, "cs" + snapshotId);


we can move this cs to constant
Also, for better readability use - (hyphen)
Please add example for snapshot name

suryag1201 · 2026-06-30T07:40:31Z

+    @RequestLine("POST /api/application/consistency-groups")
+    @Headers({"Authorization: {authHeader}", "Content-Type: application/json"})
+    JobResponse createConsistencyGroup(@Param("authHeader") String authHeader,
+                                       Map<String, Object> request);


to create CG, request should be model type why map?

suryag1201 · 2026-06-30T07:41:22Z

+     */
+    @RequestLine("GET /api/application/consistency-groups")
+    @Headers({"Authorization: {authHeader}"})
+    OntapResponse<Map<String, Object>> getConsistencyGroups(@Param("authHeader") String authHeader,


the return type should be OntapResponse

suryag1201 · 2026-06-30T07:41:52Z

+     */
+    @RequestLine("POST /api/application/consistency-groups/{cgUuid}/snapshots")
+    @Headers({"Authorization: {authHeader}", "Content-Type: application/json"})
+    JobResponse createConsistencyGroupSnapshot(@Param("authHeader") String authHeader,


same here for request

suryag1201 · 2026-06-30T07:42:04Z

+     */
+    @RequestLine("GET /api/application/consistency-groups/{cgUuid}/snapshots")
+    @Headers({"Authorization: {authHeader}"})
+    OntapResponse<Map<String, Object>> getConsistencyGroupSnapshots(@Param("authHeader") String authHeader,


same here for return type

suryag1201 · 2026-06-30T07:45:34Z

     * Presents aggregate object for the unified storage, not eligible for disaggregated
     */
    private List<Aggregate> aggregates;
+    private String resolvedSvmUuid;


svmUUID should be fine instead resolvedSvmUuid

suryag1201 · 2026-06-30T09:39:35Z

-                            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) {


can we also test, VM span across mutiple storage pool where svms are also diff

…uild

rajiv-jain-netapp requested review from piyush5netapp, sandeeplocharla and suryag1201 as code owners June 26, 2026 06:27

rajiv-jain-netapp requested a review from Copilot June 29, 2026 04:02

Copilot started reviewing on behalf of rajiv-jain-netapp June 29, 2026 04:02 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

github-actions Bot added the status:has-conflicts label Jun 29, 2026

suryag1201 reviewed Jun 30, 2026

View reviewed changes

Fix for testInitialize_clusterNotKVM UT failure during ontap plugin b…

3a04b7e

…uild

sandeeplocharla force-pushed the feature/CSTACKEX-200 branch from 26f20d7 to 3a04b7e Compare July 1, 2026 10:14

github-actions Bot removed the status:has-conflicts label Jul 1, 2026

Uh oh!

Conversation

rajiv-jain-netapp commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

rajiv-jain-netapp commented Jun 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

github-actions Bot commented Jun 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rajiv-jain-netapp commented Jun 26, 2026 •

edited

Loading