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 5e7a80b1af7c..7c959065d96b 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 @@ -313,7 +313,7 @@ private void deleteCloudStackVolumeSnapshot(SnapshotInfo snapshotInfo, CommandRe commandResult.setSuccess(true); commandResult.setResult(null); } catch (Exception e) { - if (isSnapshotNotFoundError(e)) { + if (OntapStorageUtils.isOntapSnapshotNotFoundError(e)) { logger.warn("deleteCloudStackVolumeSnapshot: ONTAP snapshot for CloudStack snapshot [{}] " + "already absent (idempotent success): {}", snapshotId, e.getMessage()); commandResult.setSuccess(true); @@ -327,25 +327,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); 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 d349305e5e43..8ae6021a4310 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 @@ -805,21 +805,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.isOntapSnapshotNotFoundError(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); + } } } 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 1ada832ea952..971390e381cf 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 @@ -239,4 +239,23 @@ public static String extractUuidFromOntapJobDescription(String description, Stri return remainder.isEmpty() ? null : remainder; } + /** + * Returns true when the exception indicates the ONTAP snapshot was already removed. + * Delete workflows treat a missing backend snapshot as idempotent success. + */ + public static boolean isOntapSnapshotNotFoundError(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 isOntapSnapshotNotFoundError(error.getCause()); + } + } 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 6f09c3649757..0c3df6c376a6 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 @@ -929,4 +929,24 @@ void testDeleteFlexVolSnapshotForCloudStackVolume_PollsJobAndSucceeds() { verify(snapshotFeignClient).deleteSnapshot(anyString(), eq("fv-uuid-1"), eq("snap-uuid-1")); } + + @Test + void testDeleteFlexVolSnapshotForCloudStackVolume_AlreadyAbsentOnOntap() { + Job job = new Job(); + job.setUuid("delete-job-missing"); + JobResponse response = new JobResponse(); + response.setJob(job); + when(snapshotFeignClient.deleteSnapshot(anyString(), eq("fv-uuid-1"), eq("snap-uuid-1"))) + .thenReturn(response); + + Job failedJob = new Job(); + failedJob.setUuid("delete-job-missing"); + failedJob.setState(OntapStorageConstants.JOB_FAILURE); + failedJob.setMessage("entry doesn't exist"); + when(jobFeignClient.getJobByUUID(anyString(), eq("delete-job-missing"))).thenReturn(failedJob); + + storageStrategy.deleteFlexVolSnapshotForCloudStackVolume("fv-uuid-1", "snap-uuid-1", "snap-name-1"); + + verify(snapshotFeignClient).deleteSnapshot(anyString(), eq("fv-uuid-1"), eq("snap-uuid-1")); + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java index 372a75ad257d..db083783d9c2 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/utils/OntapStorageUtilsTest.java +++ b/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.isOntapSnapshotNotFoundError(ex)); + } + + @Test + public void isOntapSnapshotNotFoundError_rejectsUnrelatedErrors() { + assertFalse(OntapStorageUtils.isOntapSnapshotNotFoundError( + new CloudRuntimeException("Job failed with error: permission denied"))); + } }