create temp CG for consistent VM snapshot for the VM which is span across multiple flexvolumes by rajiv-jain-netapp · Pull Request #74 · NetApp/cloudstack

rajiv-jain-netapp · 2026-07-02T13:05:43Z

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?

… multiple volumes at storage

… at multiple flexvolumes

…ective checks by which delete snapshot would be handled from primary only

…t workflows

rajiv-jain-netapp · 2026-07-02T13:06:31Z

testing results and all the executed test cases added under previous PR #67

sandeeplocharla

Partially reviewed. Still need to go through a good portion of OntapVMSnapshotStrategy.java and SnapshotManagerImpl.java

Copilot

Pull request overview

This PR enhances NetApp ONTAP managed-storage snapshot handling by ensuring VM snapshots remain consistent when a VM’s volumes span multiple FlexVols, and by preventing managed PRIMARY (array-native) volume snapshots from being incorrectly archived/bookkept as secondary snapshots.

Changes:

Add a temporary ONTAP Consistency Group (CG) two-phase snapshot flow for VM snapshots spanning multiple FlexVols; keep a direct FlexVol snapshot fast-path for single-FlexVol VMs.
Ensure managed PRIMARY volume snapshots (e.g., ONTAP volume snapshots) remain on primary/array storage and do not use secondary-archive bookkeeping paths.
Extend ONTAP integration utilities/strategy to better separate “provisioning-time” aggregate validation from “operations-only” connectivity, and add CG REST client APIs + related unit tests.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 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; refactors decision into `archiveSnapshotToSecondary`.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java	Implements temporary CG orchestration for multi-FlexVol VM snapshots, plus single-FlexVol direct snapshot path and improved naming.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java	Adds operations-only connect mode, job polling helpers, and centralized FlexVol snapshot delete orchestration.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java	Adds snapshot naming helpers and an overload to connect with/without aggregate validation.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java	Adds SVM UUID support and CG/SFSR/delete polling constants.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java	Persists resolved SVM UUID into pool details when available.
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java	Adds ONTAP CG REST API endpoints (create/list CG, start/commit snapshot, delete CG).
plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java	Delegates volume-snapshot delete/revert to strategy helpers; updates naming and makes delete more robust/idempotent.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java	Adds/updates unit tests for CG flow, naming, scope resolution, and avoids static utility connection issues.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java	Adds tests for operations-only connect, job polling helpers, and FlexVol snapshot delete orchestration.
plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java	Updates capabilities expectations to include revert support.

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

sandeeplocharla · 2026-07-07T14:31:36Z

+        return null;
+    }
+
+    private String getStringField(Map<String, Object> record, String key) {


This can be in utils, instead.

piyush5netapp · 2026-07-07T14:21:34Z

-            if (eligibleAggregates.isEmpty()) {
-                logger.error("No suitable aggregates found on SVM " + svmName + " for volume operations.");
-                throw new CloudRuntimeException("No suitable aggregates found on SVM " + svmName + " for volume operations.");
+            this.resolvedSvmUuid = svm.getUuid();


Since we are fetching svm uuid now , lets explicitly add uuid in queryParams

I did not get this comment

Copilot

Pull request overview

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

Copilot

Pull request overview

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

                VolumeDetailVO lunDetail = volumeDetailsDao.findDetail(volumeVO.getId(), OntapStorageConstants.LUN_DOT_UUID);
-                String lunUUID = lunDetail != null ? lunDetail.getValue() : null;
-                if (lunUUID == null) {
+                if (lunDetail.getValue() == null) {
                    throw new CloudRuntimeException("LUN UUID not found for iSCSI volume " + volumeVO.getId());
                }
+                lunUuid = lunDetail.getValue();


Copilot

Pull request overview

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

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


            String snapshotName = getSnapshotDetail(snapshotId, OntapStorageConstants.ONTAP_SNAP_NAME);
            String poolIdStr = getSnapshotDetail(snapshotId, OntapStorageConstants.PRIMARY_POOL_ID);

            if (flexVolUuid == null || ontapSnapshotUuid == null) {


+            logger.info("Selected aggregate: " + aggr.getName() + " for volume operations.");
+            this.aggregates = List.of(aggr);


Copilot

Pull request overview

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

+    public static String buildOntapSnapshotName(String cloudStackSnapshotName, String uniquenessSuffix) {
+        String normalizedBase = (cloudStackSnapshotName == null || cloudStackSnapshotName.trim().isEmpty())
+                ? "snapshot"
+                : getOntapSnapshotName(cloudStackSnapshotName);
+        String suffix = (uniquenessSuffix == null || uniquenessSuffix.isEmpty())
+                ? ""
+                : "_" + uniquenessSuffix.replaceAll("[^a-zA-Z0-9_]", "_");
+        int maxLength = OntapStorageConstants.MAX_SNAPSHOT_NAME_LENGTH;
+        int maxBaseLength = maxLength - suffix.length();
+        if (maxBaseLength <= 0) {
+            return normalizedBase.substring(0, maxLength);
+        }
+        if (normalizedBase.length() > maxBaseLength) {
+            normalizedBase = normalizedBase.substring(0, maxBaseLength);
+        }
+        return normalizedBase + suffix;
+    }


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


+            } else if (aggrResp.getSpace() == null || aggrResp.getAvailableBlockStorageSpace() == null ||
+                    aggrResp.getAvailableBlockStorageSpace() <= storage.getSize().doubleValue()) {
+                logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Skipping this aggregate.");
+                continue;
+            }
+            logger.info("Selected aggregate: " + aggr.getName() + " for volume operations.");
+            this.aggregates = List.of(aggr);
+            break;


rajiv-jain-netapp added 8 commits June 23, 2026 19:06

CSTACKEX:200 - added temp CG logic for VM snapshots if VM span across…

3efaa7a

… multiple volumes at storage

CSTACKEX-200: added consistency for the VM level snapshots if CSV are…

4ec7c18

… at multiple flexvolumes

CSTACKEX-200: ONTAP will have snapshot always on primary, adding resp…

4b482df

…ective checks by which delete snapshot would be handled from primary only

CSTACKEX-200: skip aggr space validations for existing volume operations

2724dbb

CSTACKEX-200: CG create must have SVM name and respective validations

4d1377c

CSTACKEX-200: add provisioning options

f72d5cf

CSTACKEX-200 race conditions needs to be handled while taking snapsho…

5d9639e

…t workflows

CSTACKEX-200: state is not a valid field for CG

26f20d7

rajiv-jain-netapp requested review from piyush5netapp, sandeeplocharla and suryag1201 as code owners July 2, 2026 13:05

sandeeplocharla reviewed Jul 6, 2026

View reviewed changes

rajiv-jain-netapp requested a review from Copilot July 7, 2026 09:06

Copilot started reviewing on behalf of rajiv-jain-netapp July 7, 2026 09:06 View session

CSTACKEX-200: incorporate one round of review comments

0d3f4b7

Copilot AI reviewed Jul 7, 2026

View reviewed changes

rajiv-jain-netapp added 2 commits July 7, 2026 16:59

CSTACKEX-200: incorporate copilot comments

605a878

Merge branch 'main' into feature/CSTACKEX-200

61f8105

sandeeplocharla reviewed Jul 7, 2026

View reviewed changes

piyush5netapp reviewed Jul 7, 2026

View reviewed changes

suryag1201 reviewed Jul 7, 2026

View reviewed changes

CSTACKEX-200: fix for the issue added due to merge

e1c44f5

Copilot AI review requested due to automatic review settings July 8, 2026 07:04

Copilot started reviewing on behalf of rajiv-jain-netapp July 8, 2026 07:04 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

CSTACKEX-200: incorporate review comments from team and copilot

b4d6d72

Copilot AI review requested due to automatic review settings July 8, 2026 13:24

Copilot started reviewing on behalf of rajiv-jain-netapp July 8, 2026 13:25 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

suryag1201 reviewed Jul 9, 2026

View reviewed changes

Comment thread ...lume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/SnapshotFeignClient.java

sandeeplocharla previously approved these changes Jul 9, 2026

View reviewed changes

CSTACKEX-200: fix to the copilot comment

d25c9d0

Copilot AI review requested due to automatic review settings July 9, 2026 06:42

rajiv-jain-netapp dismissed sandeeplocharla’s stale review via d25c9d0 July 9, 2026 06:42

Copilot started reviewing on behalf of rajiv-jain-netapp July 9, 2026 06:42 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

suryag1201 previously approved these changes Jul 9, 2026

View reviewed changes

CSTACKEX-200: missed to add new clases

be6a3f2

rajiv-jain-netapp dismissed suryag1201’s stale review via be6a3f2 July 9, 2026 12:42

Copilot AI review requested due to automatic review settings July 9, 2026 12:42

Copilot started reviewing on behalf of rajiv-jain-netapp July 9, 2026 12:45 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

suryag1201 approved these changes Jul 9, 2026

View reviewed changes

sandeeplocharla approved these changes Jul 9, 2026

View reviewed changes

rajiv-jain-netapp merged commit ab25766 into main Jul 9, 2026
20 checks passed

rajiv-jain-netapp deleted the feature/CSTACKEX-200 branch July 9, 2026 15:12

		logger.info("Selected aggregate: " + aggr.getName() + " for volume operations.");
		this.aggregates = List.of(aggr);

Uh oh!

Conversation

rajiv-jain-netapp commented Jul 2, 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 Jul 2, 2026

Uh oh!

sandeeplocharla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sandeeplocharla Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

piyush5netapp Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

rajiv-jain-netapp Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

rajiv-jain-netapp commented Jul 2, 2026 •

edited

Loading