From 07a63a72161691562df38296412c60216d1894a6 Mon Sep 17 00:00:00 2001 From: sandeeplocharla Date: Fri, 10 Jul 2026 13:04:48 +0530 Subject: [PATCH 1/4] Resolved conflicts --- .../cloudstack/storage/service/StorageStrategy.java | 7 ++++++- .../cloudstack/storage/service/UnifiedNASStrategy.java | 9 ++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) 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 2d2b0d29839f..11c46c2b9b09 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 @@ -19,6 +19,7 @@ package org.apache.cloudstack.storage.service; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -389,7 +390,11 @@ public void deleteStorageVolume(Volume volume) { throw new CloudRuntimeException("Volume deletion job failed for volume: " + volume.getName()); } logger.info("Volume deleted successfully: " + volume.getName()); - } catch (FeignException.FeignClientException e) { + } catch (FeignException e) { + if (e.status() == 404) { + logger.warn("deleteStorageVolume: Volume '{}' not found in ONTAP (may not have been created), treating as no-op", volume.getName()); + return; + } logger.error("Exception while deleting volume: ", e); throw new CloudRuntimeException("Failed to delete volume: " + e.getMessage()); } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java index 0a257a29527b..07773691a909 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java @@ -181,12 +181,15 @@ public void deleteAccessGroup(AccessGroup accessGroup) { String exportPolicyId = details.get(OntapStorageConstants.EXPORT_POLICY_ID); try { - nasFeignClient.deleteExportPolicyById(authHeader,exportPolicyId); + nasFeignClient.deleteExportPolicyById(authHeader, exportPolicyId); logger.info("deleteAccessGroup: Successfully deleted export policy '{}'", exportPolicyName); - } catch (Exception e) { + } catch (FeignException e) { + if (e.status() == 404) { + logger.warn("deleteAccessGroup: Export policy '{}' not found in ONTAP, treating as no-op", exportPolicyName); + return; + } logger.error("deleteAccessGroup: Failed to delete export policy. Exception: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to delete export policy: " + e.getMessage(), e); - } } catch (Exception e) { logger.error("deleteAccessGroup: Failed to delete export policy. Exception: {}", e.getMessage(), e); From f4524b4a5b4d85896daa0e19b91f5e48209c5060 Mon Sep 17 00:00:00 2001 From: sandeeplocharla Date: Fri, 10 Jul 2026 13:05:54 +0530 Subject: [PATCH 2/4] Resolved conflicts and comments --- .../org/apache/cloudstack/storage/service/StorageStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 11c46c2b9b09..1a02d46dfc55 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 @@ -392,7 +392,7 @@ public void deleteStorageVolume(Volume volume) { logger.info("Volume deleted successfully: " + volume.getName()); } catch (FeignException e) { if (e.status() == 404) { - logger.warn("deleteStorageVolume: Volume '{}' not found in ONTAP (may not have been created), treating as no-op", volume.getName()); + logger.warn("deleteStorageVolume: Volume '{}' not found in ONTAP, treating as no-op", volume.getName()); return; } logger.error("Exception while deleting volume: ", e); From 6a285ddcbeb92231643fa2d3425ae73bc50bf7c1 Mon Sep 17 00:00:00 2001 From: sandeeplocharla Date: Tue, 14 Jul 2026 10:46:16 +0530 Subject: [PATCH 3/4] Addressed comments and fixed checkstyle issue --- .../storage/service/StorageStrategy.java | 1 - .../storage/service/StorageStrategyTest.java | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) 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 1a02d46dfc55..4e827bb516ef 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 @@ -19,7 +19,6 @@ package org.apache.cloudstack.storage.service; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; 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 d3d5e22285ba..33e9da474554 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 @@ -644,6 +644,25 @@ public void testDeleteStorageVolume_feignException() { assertTrue(ex.getMessage().contains("Failed to delete volume")); } + @Test + public void testDeleteStorageVolume_notFound_404_returnsWithoutThrowing() { + // Setup + Volume volume = new Volume(); + volume.setName("test-volume"); + volume.setUuid("vol-uuid-1"); + + FeignException feignEx = mock(FeignException.class); + when(feignEx.status()).thenReturn(404); + when(volumeFeignClient.deleteVolume(anyString(), eq("vol-uuid-1"))) + .thenThrow(feignEx); + + // Execute - 404 means volume already gone on ONTAP, treated as no-op + storageStrategy.deleteStorageVolume(volume); + + // Verify the delete was attempted + verify(volumeFeignClient).deleteVolume(anyString(), eq("vol-uuid-1")); + } + // ========== getStoragePath() Tests ========== @Test From ca63c9c318987d6faec9ecf78924f0b7fd73bb5d Mon Sep 17 00:00:00 2001 From: sandeeplocharla Date: Fri, 31 Jul 2026 08:30:40 +0530 Subject: [PATCH 4/4] Added more UTs --- .../storage/service/StorageStrategyTest.java | 13 +++++++++++ .../service/UnifiedNASStrategyTest.java | 22 +++++++++++++++++++ 2 files changed, 35 insertions(+) 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 33e9da474554..070c352a7620 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 @@ -969,4 +969,17 @@ void testDeleteFlexVolSnapshotForCloudStackVolume_AlreadyAbsentOnOntap() { verify(snapshotFeignClient).deleteSnapshot(anyString(), eq("fv-uuid-1"), eq("snap-uuid-1")); } + + @Test + void testDeleteFlexVolSnapshotForCloudStackVolume_Feign404_TreatedAsSuccess() { + FeignException notFoundException = mock(FeignException.class); + when(notFoundException.status()).thenReturn(404); + when(snapshotFeignClient.deleteSnapshot(anyString(), eq("fv-uuid-1"), eq("snap-uuid-1"))) + .thenThrow(notFoundException); + + storageStrategy.deleteFlexVolSnapshotForCloudStackVolume("fv-uuid-1", "snap-uuid-1", "snap-name-1"); + + verify(snapshotFeignClient).deleteSnapshot(anyString(), eq("fv-uuid-1"), eq("snap-uuid-1")); + verify(jobFeignClient, never()).getJobByUUID(anyString(), anyString()); + } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java index b04e9c0b1b26..f0eb5f0ccced 100755 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedNASStrategyTest.java @@ -79,6 +79,8 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import feign.FeignException; + @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) public class UnifiedNASStrategyTest { @@ -516,6 +518,26 @@ public void testDeleteAccessGroup_Failed() { }); } + // Test deleteAccessGroup - Export policy not found should be treated as no-op + @Test + public void testDeleteAccessGroup_NotFound404_NoThrow() { + AccessGroup accessGroup = mock(AccessGroup.class); + Map details = new HashMap<>(); + details.put(OntapStorageConstants.EXPORT_POLICY_NAME, "export-policy-1"); + details.put(OntapStorageConstants.EXPORT_POLICY_ID, "1"); + + when(accessGroup.getStoragePoolId()).thenReturn(1L); + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(details); + + FeignException feignException = mock(FeignException.class); + when(feignException.status()).thenReturn(404); + doThrow(feignException).when(nasFeignClient).deleteExportPolicyById(anyString(), eq("1")); + + strategy.deleteAccessGroup(accessGroup); + + verify(nasFeignClient).deleteExportPolicyById(anyString(), eq("1")); + } + // Test deleteCloudStackVolume - Success @Test public void testDeleteCloudStackVolume_Success() throws Exception {