diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java index 702a3aa5e414..b27e4ba6db9a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java @@ -1053,16 +1053,12 @@ void rollbackFlexVolSnapshot(FlexVolSnapshotDetail detail) { try { Map poolDetails = storagePoolDetailsDao.listDetailsKeyPairs(detail.poolId); StorageStrategy storageStrategy = resolveStorageStrategy(poolDetails); - SnapshotFeignClient client = storageStrategy.getSnapshotFeignClient(); - String authHeader = storageStrategy.getAuthHeader(); logger.info("rollbackFlexVolSnapshot: Rolling back FlexVol snapshot [{}] (uuid={}) on FlexVol [{}]", detail.snapshotName, detail.snapshotUuid, detail.flexVolUuid); - JobResponse jobResponse = client.deleteSnapshot(authHeader, detail.flexVolUuid, detail.snapshotUuid); - if (jobResponse != null && jobResponse.getJob() != null) { - storageStrategy.jobPollForSuccess(jobResponse.getJob().getUuid(), 10, 2000); - } + storageStrategy.deleteFlexVolSnapshotForCloudStackVolume( + detail.flexVolUuid, detail.snapshotUuid, detail.snapshotName); } catch (Exception e) { logger.error("rollbackFlexVolSnapshot: Rollback of FlexVol snapshot failed: {}", e.getMessage(), e); } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java index a3ffed1fb0f7..aa0d48c2e850 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java @@ -18,11 +18,13 @@ */ package org.apache.cloudstack.storage.vmsnapshot; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doReturn; @@ -58,6 +60,9 @@ import org.apache.cloudstack.storage.feign.model.response.OntapResponse; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.Appender; +import org.apache.logging.log4j.core.Logger; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -686,6 +691,44 @@ void testFlexVolSnapshotDetail_Parse5Parts_ThrowsException() { () -> OntapVMSnapshotStrategy.FlexVolSnapshotDetail.parse("a::b::c::d::e")); } + @Test + void testRollbackFlexVolSnapshot_UsesSharedDelete() { + OntapVMSnapshotStrategy.FlexVolSnapshotDetail detail = new OntapVMSnapshotStrategy.FlexVolSnapshotDetail( + "flexvol-uuid-1", "snapshot-uuid-1", "snapshot-name-1", "volume-301.qcow2", POOL_ID_1, "NFS3"); + doReturn(storageStrategy).when(strategy).resolveStorageStrategy(any()); + + assertDoesNotThrow(() -> strategy.rollbackFlexVolSnapshot(detail)); + + verify(storageStrategy).deleteFlexVolSnapshotForCloudStackVolume( + "flexvol-uuid-1", "snapshot-uuid-1", "snapshot-name-1"); + verify(storageStrategy, never()).getSnapshotFeignClient(); + verify(storageStrategy, never()).getAuthHeader(); + verify(storageStrategy, never()).jobPollForSuccess(any(), anyInt(), anyInt()); + } + + @Test + void testRollbackFlexVolSnapshot_DeleteFailureIsLoggedAndNotThrown() { + OntapVMSnapshotStrategy.FlexVolSnapshotDetail detail = new OntapVMSnapshotStrategy.FlexVolSnapshotDetail( + "flexvol-uuid-1", "snapshot-uuid-1", "snapshot-name-1", "volume-301.qcow2", POOL_ID_1, "NFS3"); + doReturn(storageStrategy).when(strategy).resolveStorageStrategy(any()); + doThrow(new CloudRuntimeException("delete failed")).when(storageStrategy) + .deleteFlexVolSnapshotForCloudStackVolume(any(), any(), any()); + + Logger logger = (Logger) LogManager.getLogger(OntapVMSnapshotStrategy.class); + Appender appender = mock(Appender.class); + when(appender.getName()).thenReturn("rollback-test-appender"); + when(appender.isStarted()).thenReturn(true); + logger.addAppender(appender); + try { + assertDoesNotThrow(() -> strategy.rollbackFlexVolSnapshot(detail)); + + verify(appender).append(argThat(event -> event.getMessage().getFormattedMessage() + .contains("Rollback of FlexVol snapshot failed: delete failed"))); + } finally { + logger.removeAppender(appender); + } + } + // ══════════════════════════════════════════════════════════════════════════ // Tests: buildSnapshotName // ══════════════════════════════════════════════════════════════════════════ @@ -861,6 +904,36 @@ void testTakeVMSnapshot_SingleFlexVolSuccess_UsesDirectSnapshotNotCg() throws Ex verify(vmSnapshotDetailsDao, atLeastOnce()).persist(any(VMSnapshotDetailsVO.class)); } + @Test + void testTakeVMSnapshot_RollbackDeleteFailurePreservesOriginalError() throws Exception { + VMSnapshotVO vmSnapshot = createTakeSnapshotVmSnapshot(); + setupTakeSnapshotCommon(vmSnapshot); + setupSingleVolumeForTakeSnapshot(); + String snapshotName = strategy.buildSnapshotName(vmSnapshot); + setupSingleFlexVolFlowMocks(snapshotName); + + FreezeThawVMAnswer freezeAnswer = mock(FreezeThawVMAnswer.class); + when(freezeAnswer.getResult()).thenReturn(true); + FreezeThawVMAnswer thawAnswer = mock(FreezeThawVMAnswer.class); + when(thawAnswer.getResult()).thenReturn(true); + when(agentMgr.send(eq(HOST_ID), any(FreezeThawVMCommand.class))) + .thenReturn(freezeAnswer) + .thenReturn(thawAnswer); + doThrow(new CloudRuntimeException("snapshot persistence failed")).when(vmSnapshotDetailsDao) + .persist(any(VMSnapshotDetailsVO.class)); + doThrow(new CloudRuntimeException("rollback delete failed")).when(storageStrategy) + .deleteFlexVolSnapshotForCloudStackVolume(any(), any(), any()); + when(vmSnapshotDetailsDao.listDetails(SNAPSHOT_ID)).thenReturn(Collections.emptyList()); + doReturn(true).when(vmSnapshotHelper).vmSnapshotStateTransitTo(any(), eq(VMSnapshot.Event.OperationFailed)); + + CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, + () -> strategy.takeVMSnapshot(vmSnapshot)); + + assertEquals("snapshot persistence failed", exception.getMessage()); + verify(storageStrategy).deleteFlexVolSnapshotForCloudStackVolume( + eq("flexvol-uuid-1"), any(), eq(snapshotName)); + } + @Test void testTakeVMSnapshot_TemporaryCgTwoPhaseSuccess_PersistsDetailsAndCleansUpCg() throws Exception { VMSnapshotVO vmSnapshot = createTakeSnapshotVmSnapshot();