CSTACKEX-158: if ontap snapshot are already delete from ontap side, d… (#82)

…Deletion of CS side of snapshot should not fail on not finding ontap
snapshot.

This PR...
<!--- Describe your changes in DETAIL - And how has behaviour
functionally changed. -->

<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to
reproduce. -->

<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically
be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->

<!---
*******************************************************************************
-->
<!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE
DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!---
*******************************************************************************
-->

- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] 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)

- [ ] Major
- [x] Minor

- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial

Test -1: Ran VM snapshot delete operation when the respective snapshot
is not available at ONTAP, it passed.
Test -2: Ran VM snapshot delete operation when the respective snapshot
is available at ONTAP; it passed
Test -3: Ran cloudstack volume snapshot delete workflow when the
respective snapshot is not available at ONTAP, it passed.
Test -4: Ran cloudstack volume snapshot delete workflow when the
respective snapshot is available at ONTAP, it passed.

<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran
to -->

change?

<!-- see how your change affects other areas of the code, etc. -->

<!-- Please read the
[CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
document -->

Author: rajiv-jain-netapp

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java

@@ -316,7 +316,7 @@ private void deleteCloudStackVolumeSnapshot(SnapshotInfo snapshotInfo, CommandRe
             commandResult.setSuccess(true);
             commandResult.setResult(null);
         } catch (Exception e) {
-            if (isSnapshotNotFoundError(e)) {
+            if (OntapStorageUtils.isOntapObjectNotFoundError(e)) {
                 logger.warn("deleteCloudStackVolumeSnapshot: ONTAP snapshot for CloudStack snapshot [{}] "
                         + "already absent (idempotent success): {}", snapshotId, e.getMessage());
                 commandResult.setSuccess(true);
@@ -330,25 +330,6 @@ private void deleteCloudStackVolumeSnapshot(SnapshotInfo snapshotInfo, CommandRe
         }
     }
 
-    /**
-     * Returns true when the exception indicates the ONTAP snapshot was already removed.
-     * Delete is idempotent: a missing backend snapshot is treated as success.
-     */
-    private boolean isSnapshotNotFoundError(Throwable error) {
-        if (error == null) {
-            return false;
-        }
-        String message = error.getMessage();
-        if (message != null) {
-            String lower = message.toLowerCase();
-            if (lower.contains("404") || lower.contains("not found") || lower.contains("does not exist")
-                    || lower.contains("entry doesn't exist")) {
-                return true;
-            }
-        }
-        return isSnapshotNotFoundError(error.getCause());
-    }
-
     private long resolveSnapshotPoolId(String poolIdStr, long snapshotId) {
         if (poolIdStr != null && !poolIdStr.isEmpty()) {
             return Long.parseLong(poolIdStr);

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java

@@ -889,21 +889,35 @@ public void deleteFlexVolSnapshotForCloudStackVolume(String flexVolUuid, String
         logger.info("deleteFlexVolSnapshotForCloudStackVolume: issuing ONTAP REST delete for snapshot [{}] "
                 + "(uuid={}) on FlexVol [{}]", snapshotName, snapshotUuid, flexVolUuid);
 
-        JobResponse jobResponse = snapshotFeignClient.deleteSnapshot(getAuthHeader(), flexVolUuid, snapshotUuid);
-
-        if (jobResponse == null || jobResponse.getJob() == null) {
-            logger.debug("deleteFlexVolSnapshotForCloudStackVolume: no async job returned for snapshot [{}] "
-                    + "(uuid={}); treating HTTP success as completion", snapshotName, snapshotUuid);
-        } else {
-            logger.debug("deleteFlexVolSnapshotForCloudStackVolume: polling ONTAP delete job [{}] for snapshot [{}]",
-                    jobResponse.getJob().getUuid(), snapshotName);
-        }
+        try {
+            JobResponse jobResponse = snapshotFeignClient.deleteSnapshot(getAuthHeader(), flexVolUuid, snapshotUuid);
+
+            if (jobResponse == null || jobResponse.getJob() == null) {
+                logger.debug("deleteFlexVolSnapshotForCloudStackVolume: no async job returned for snapshot [{}] "
+                        + "(uuid={}); treating HTTP success as completion", snapshotName, snapshotUuid);
+            } else {
+                logger.debug("deleteFlexVolSnapshotForCloudStackVolume: polling ONTAP delete job [{}] for snapshot [{}]",
+                        jobResponse.getJob().getUuid(), snapshotName);
+            }
 
-        pollJobIfPresent(jobResponse, "delete FlexVol snapshot [" + snapshotName + "] uuid [" + snapshotUuid + "]",
-                OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_MAX_RETRIES,
-                OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_POLL_INTERVAL_MS);
+            pollJobIfPresent(jobResponse, "delete FlexVol snapshot [" + snapshotName + "] uuid [" + snapshotUuid + "]",
+                    OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_MAX_RETRIES,
+                    OntapStorageConstants.ONTAP_SNAPSHOT_DELETE_JOB_POLL_INTERVAL_MS);
 
-        logger.info("deleteFlexVolSnapshotForCloudStackVolume: ONTAP FlexVol snapshot [{}] (uuid={}) removed from [{}]",
-                snapshotName, snapshotUuid, flexVolUuid);
+            logger.info("deleteFlexVolSnapshotForCloudStackVolume: ONTAP FlexVol snapshot [{}] (uuid={}) removed from [{}]",
+                    snapshotName, snapshotUuid, flexVolUuid);
+        } catch (Exception e) {
+            if (OntapStorageUtils.isOntapObjectNotFoundError(e)) {
+                logger.warn("deleteFlexVolSnapshotForCloudStackVolume: ONTAP snapshot [{}] (uuid={}) on FlexVol [{}] "
+                        + "already absent; treating delete as success: {}", snapshotName, snapshotUuid, flexVolUuid,
+                        e.getMessage());
+                return;
+            }
+            if (e instanceof CloudRuntimeException) {
+                throw (CloudRuntimeException) e;
+            }
+            throw new CloudRuntimeException("Failed to delete ONTAP FlexVol snapshot [" + snapshotName + "]: "
+                    + e.getMessage(), e);
+        }
     }
 }

plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java

@@ -248,4 +248,32 @@ public static String extractUuidFromOntapJobDescription(String description, Stri
         return remainder.isEmpty() ? null : remainder;
     }
 
+    /**
+     * Returns true when the exception indicates the ONTAP Object was already removed.
+     * Delete workflows treat a missing backend object as idempotent success.
+     */
+    public static boolean isOntapObjectNotFoundError(Throwable error) {
+        if (error == null) {
+            return false;
+        }
+        if(error instanceof FeignException) {
+            FeignException feignException = (FeignException) error;
+            if (feignException.status() == 404) {
+                return true;
+            }
+        }
+        String message = error.getMessage();
+        if (message != null) {
+            String lower = message.toLowerCase();
+            if (lower.contains("404") || lower.contains("not found") || lower.contains("does not exist")
+                    || lower.contains("entry doesn't exist")) {
+                return true;
+            }
+        } else {
+            logger.warn("Error message is null for exception: {}", error.getClass().getName());
+            return false;
+        }
+        return false;
+    }
+
 }

plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java

@@ -18,9 +18,11 @@
  */
 package org.apache.cloudstack.storage.utils;
 
+import com.cloud.utils.exception.CloudRuntimeException;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class OntapStorageUtilsTest {
@@ -79,4 +81,16 @@ public void getIgroupName_truncates_whenOneCharOverMaxLength() {
 
         assertEquals(OntapStorageConstants.IGROUP_NAME_MAX_LENGTH, result.length());
     }
+
+    @Test
+    public void isOntapSnapshotNotFoundError_matchesEntryDoesNotExist() {
+        CloudRuntimeException ex = new CloudRuntimeException("Job failed with error: entry doesn't exist");
+        assertTrue(OntapStorageUtils.isOntapObjectNotFoundError(ex));
+    }
+
+    @Test
+    public void isOntapSnapshotNotFoundError_rejectsUnrelatedErrors() {
+        assertFalse(OntapStorageUtils.isOntapObjectNotFoundError(
+                new CloudRuntimeException("Job failed with error: permission denied")));
+    }
 }