From 8c452cab96b4fa375d1c708d3d42fc43855f6b58 Mon Sep 17 00:00:00 2001 From: Brad House Date: Fri, 6 Nov 2026 21:20:25 +0000 Subject: [PATCH] Don't fail snapshot strategy selection when a snapshot's volume row is gone Picking a snapshot strategy asks every strategy whether it can handle the snapshot (StorageStrategyFactoryImpl.bestMatch). The Ceph, ScaleIO and storage-system strategies look up the snapshot's volume with findByIdIncludingRemoved() and dereference the result without a null check: VolumeVO volumeVO = volumeDao.findByIdIncludingRemoved(volumeId); boolean baseVolumeExists = volumeVO.getRemoved() == null; // NPE findByIdIncludingRemoved() still returns expunged volumes, but returns null once the volume row itself has been deleted from the volumes table. A snapshot that still references such a volume then makes canHandle() throw, and because every strategy is asked, the whole operation fails for every storage type, not just Ceph or ScaleIO. The most visible case is listSnapshots: the response's "revertable" flag comes from SnapshotObject.isRevertable() -> getSnapshotStrategy(REVERT), so a single such snapshot makes the list call fail with Cannot invoke "com.cloud.storage.VolumeVO.getRemoved()" because "volumeVO" is null Treat a missing volume row the same as a removed volume: Ceph and ScaleIO return CANT_HANDLE, as they already do for a removed volume, and StorageSystemSnapshotStrategy declines REVERT. DefaultSnapshotStrategy already handles a null volume for REVERT. StorageSystemSnapshotStrategy also unboxed the volume's pool id (pool_id, else last_pool_id) before its REVERT branch checked whether the volume was removed. A removed volume can have neither, e.g. a local-storage volume whose host was deleted (VolumeDaoImpl.updateAndRemoveVolume clears pool_id), which hit the same NPE on listSnapshots. The pool id is now only read where it is used, and the REVERT checks skip the volume-pool fallback when there is none. TAKE and BACKUP still need the volume's pool and are unchanged. Signed-off-by: Brad House --- .../snapshot/CephSnapshotStrategy.java | 2 +- .../snapshot/ScaleIOSnapshotStrategy.java | 2 +- .../StorageSystemSnapshotStrategy.java | 22 +- .../snapshot/CephSnapshotStrategyTest.java | 18 +- .../snapshot/ScaleIOSnapshotStrategyTest.java | 90 ++++++++ .../StorageSystemSnapshotStrategyTest.java | 196 ++++++++++++++++++ 6 files changed, 318 insertions(+), 12 deletions(-) create mode 100644 engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/ScaleIOSnapshotStrategyTest.java create mode 100644 engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategyTest.java diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/CephSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/CephSnapshotStrategy.java index d9d028d4d085..50a5173cb7ec 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/CephSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/CephSnapshotStrategy.java @@ -52,7 +52,7 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat } long volumeId = snapshot.getVolumeId(); VolumeVO volumeVO = volumeDao.findByIdIncludingRemoved(volumeId); - boolean baseVolumeExists = volumeVO.getRemoved() == null; + boolean baseVolumeExists = volumeVO != null && volumeVO.getRemoved() == null; if (!baseVolumeExists) { return StrategyPriority.CANT_HANDLE; } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/ScaleIOSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/ScaleIOSnapshotStrategy.java index c1e38fc92512..c585f09a9c6d 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/ScaleIOSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/ScaleIOSnapshotStrategy.java @@ -50,7 +50,7 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat } long volumeId = snapshot.getVolumeId(); VolumeVO volumeVO = volumeDao.findByIdIncludingRemoved(volumeId); - boolean baseVolumeExists = volumeVO.getRemoved() == null; + boolean baseVolumeExists = volumeVO != null && volumeVO.getRemoved() == null; if (!baseVolumeExists) { return StrategyPriority.CANT_HANDLE; } diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java index 560bb4b2fc12..4b52d6a79605 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java @@ -951,10 +951,8 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat VolumeVO volumeVO = volumeDao.findByIdIncludingRemoved(volumeId); - long volumeStoragePoolId = (volumeVO.getPoolId() != null ? volumeVO.getPoolId() : volumeVO.getLastPoolId()); - if (SnapshotOperation.REVERT.equals(op)) { - boolean baseVolumeExists = volumeVO.getRemoved() == null; + boolean baseVolumeExists = volumeVO != null && volumeVO.getRemoved() == null; if (baseVolumeExists) { boolean acceptableFormat = isAcceptableRevertFormat(volumeVO); @@ -964,6 +962,8 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat boolean usingBackendSnapshot = usingBackendSnapshotFor(snapshot.getId()); + Long volumeStoragePoolId = getVolumeStoragePoolId(volumeVO); + if (usingBackendSnapshot) { if (snapshotStore != null) { long snapshotStoragePoolId = snapshotStore.getDataStoreId(); @@ -975,7 +975,7 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat return StrategyPriority.HIGHEST; } - storageSystemSupportsCapability = storageSystemSupportsCapability(volumeStoragePoolId, + storageSystemSupportsCapability = volumeStoragePoolId != null && storageSystemSupportsCapability(volumeStoragePoolId, DataStoreCapabilities.CAN_REVERT_VOLUME_TO_SNAPSHOT.toString()); if (storageSystemSupportsCapability) { @@ -994,10 +994,12 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat } } - StoragePoolVO storagePoolVO = storagePoolDao.findById(volumeStoragePoolId); + if (volumeStoragePoolId != null) { + StoragePoolVO storagePoolVO = storagePoolDao.findById(volumeStoragePoolId); - if (storagePoolVO.isManaged()) { - return StrategyPriority.HIGHEST; + if (storagePoolVO.isManaged()) { + return StrategyPriority.HIGHEST; + } } } } @@ -1006,9 +1008,15 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat return StrategyPriority.CANT_HANDLE; } + long volumeStoragePoolId = getVolumeStoragePoolId(volumeVO); + boolean storageSystemSupportsCapability = storageSystemSupportsCapability(volumeStoragePoolId, DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()); return storageSystemSupportsCapability ? StrategyPriority.HIGHEST : StrategyPriority.CANT_HANDLE; } + private Long getVolumeStoragePoolId(VolumeVO volumeVO) { + return volumeVO.getPoolId() != null ? volumeVO.getPoolId() : volumeVO.getLastPoolId(); + } + } diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/CephSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/CephSnapshotStrategyTest.java index 0a8bba821f87..890c51a0db84 100644 --- a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/CephSnapshotStrategyTest.java +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/CephSnapshotStrategyTest.java @@ -65,12 +65,24 @@ public void canHandleTestNotReomvedAndSnapshotNotStoredOnRbd() { @Test public void canHandleTestReomvedAndSnapshotNotStoredOnRbd() { - configureAndVerifyCanHandle(null, false); + configureAndVerifyCanHandle(new Date(), false); } @Test public void canHandleTestReomvedAndSnapshotStoredOnRbd() { - configureAndVerifyCanHandle(null, true); + configureAndVerifyCanHandle(new Date(), true); + } + + @Test + public void canHandleTestVolumeRowMissing() { + Snapshot snapshot = Mockito.mock(Snapshot.class); + Mockito.when(snapshot.getVolumeId()).thenReturn(1L); + Mockito.when(volumeDao.findByIdIncludingRemoved(1L)).thenReturn(null); + + for (SnapshotOperation op : SnapshotOperation.values()) { + Assert.assertEquals(StrategyPriority.CANT_HANDLE, cephSnapshotStrategy.canHandle(snapshot, null, op)); + } + Mockito.verify(cephSnapshotStrategy, Mockito.never()).isSnapshotStoredOnRbdStoragePoolAndOperationForSameZone(Mockito.any(), Mockito.any()); } private void configureAndVerifyCanHandle(Date removed, boolean isSnapshotStoredOnRbdStoragePool) { @@ -85,7 +97,7 @@ private void configureAndVerifyCanHandle(Date removed, boolean isSnapshotStoredO for (int i = 0; i < snapshotOps.length - 1; i++) { StrategyPriority strategyPriority = cephSnapshotStrategy.canHandle(snapshot, null, snapshotOps[i]); - if (snapshotOps[i] == SnapshotOperation.REVERT && isSnapshotStoredOnRbdStoragePool) { + if (snapshotOps[i] == SnapshotOperation.REVERT && isSnapshotStoredOnRbdStoragePool && removed == null) { Assert.assertEquals(StrategyPriority.HIGHEST, strategyPriority); } else { Assert.assertEquals(StrategyPriority.CANT_HANDLE, strategyPriority); diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/ScaleIOSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/ScaleIOSnapshotStrategyTest.java new file mode 100644 index 000000000000..a189a6231312 --- /dev/null +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/ScaleIOSnapshotStrategyTest.java @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.storage.snapshot; + +import java.util.Date; + +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.storage.Snapshot; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; + +@RunWith(MockitoJUnitRunner.class) +public class ScaleIOSnapshotStrategyTest { + + @Spy + @InjectMocks + private ScaleIOSnapshotStrategy scaleIOSnapshotStrategy; + @Mock + private SnapshotDataStoreDao snapshotStoreDao; + @Mock + private PrimaryDataStoreDao primaryDataStoreDao; + @Mock + private VolumeDao volumeDao; + + private void verifyCantHandleAnyOperation(VolumeVO volumeVO) { + Snapshot snapshot = Mockito.mock(Snapshot.class); + Mockito.when(snapshot.getVolumeId()).thenReturn(1L); + Mockito.when(volumeDao.findByIdIncludingRemoved(1L)).thenReturn(volumeVO); + + for (SnapshotOperation op : SnapshotOperation.values()) { + Assert.assertEquals(StrategyPriority.CANT_HANDLE, scaleIOSnapshotStrategy.canHandle(snapshot, null, op)); + } + Mockito.verifyNoInteractions(snapshotStoreDao, primaryDataStoreDao); + } + + @Test + public void canHandleTestVolumeExistsAndSnapshotStoredOnScaleIO() { + Snapshot snapshot = Mockito.mock(Snapshot.class); + Mockito.when(snapshot.getVolumeId()).thenReturn(1L); + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + Mockito.when(volumeVO.getRemoved()).thenReturn(null); + Mockito.when(volumeDao.findByIdIncludingRemoved(1L)).thenReturn(volumeVO); + Mockito.doReturn(true).when(scaleIOSnapshotStrategy).isSnapshotStoredOnScaleIOStoragePoolAndOperationForSameZone(Mockito.any(), Mockito.any()); + + for (SnapshotOperation op : SnapshotOperation.values()) { + StrategyPriority expected = (op == SnapshotOperation.REVERT || op == SnapshotOperation.DELETE) ? StrategyPriority.HIGHEST : StrategyPriority.CANT_HANDLE; + Assert.assertEquals(expected, scaleIOSnapshotStrategy.canHandle(snapshot, null, op)); + } + } + + @Test + public void canHandleTestVolumeRowMissing() { + verifyCantHandleAnyOperation(null); + } + + @Test + public void canHandleTestVolumeRemoved() { + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + Mockito.when(volumeVO.getRemoved()).thenReturn(new Date()); + verifyCantHandleAnyOperation(volumeVO); + } +} diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategyTest.java new file mode 100644 index 000000000000..14ed3ed4d37a --- /dev/null +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategyTest.java @@ -0,0 +1,196 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.storage.snapshot; + +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation; +import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.Snapshot; +import com.cloud.storage.Storage.ImageFormat; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.SnapshotDetailsVO; +import com.cloud.storage.dao.SnapshotDetailsDao; +import com.cloud.storage.dao.VolumeDao; + +@RunWith(MockitoJUnitRunner.class) +public class StorageSystemSnapshotStrategyTest { + + @InjectMocks + private StorageSystemSnapshotStrategy storageSystemSnapshotStrategy; + @Mock + private DataStoreManager dataStoreMgr; + @Mock + private PrimaryDataStoreDao storagePoolDao; + @Mock + private SnapshotDataStoreDao snapshotStoreDao; + @Mock + private SnapshotDetailsDao snapshotDetailsDao; + @Mock + private VolumeDao volumeDao; + + private Snapshot mockSnapshotOfVolume(VolumeVO volumeVO) { + Snapshot snapshot = Mockito.mock(Snapshot.class); + Mockito.when(snapshot.getVolumeId()).thenReturn(1L); + Mockito.when(volumeDao.findByIdIncludingRemoved(1L)).thenReturn(volumeVO); + return snapshot; + } + + @Test + public void canHandleRevertTestVolumeRowMissing() { + Snapshot snapshot = mockSnapshotOfVolume(null); + + Assert.assertEquals(StrategyPriority.CANT_HANDLE, storageSystemSnapshotStrategy.canHandle(snapshot, null, SnapshotOperation.REVERT)); + Mockito.verifyNoInteractions(dataStoreMgr, storagePoolDao, snapshotStoreDao); + } + + @Test + public void canHandleRevertTestRemovedVolume() { + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + Mockito.when(volumeVO.getRemoved()).thenReturn(new Date()); + Snapshot snapshot = mockSnapshotOfVolume(volumeVO); + + Assert.assertEquals(StrategyPriority.CANT_HANDLE, storageSystemSnapshotStrategy.canHandle(snapshot, null, SnapshotOperation.REVERT)); + Mockito.verifyNoInteractions(dataStoreMgr, storagePoolDao, snapshotStoreDao); + } + + @Test + public void canHandleRevertTestVolumeWithoutPool() { + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + Mockito.when(volumeVO.getRemoved()).thenReturn(null); + Mockito.when(volumeVO.getFormat()).thenReturn(ImageFormat.RAW); + Mockito.when(volumeVO.getPoolId()).thenReturn(null); + Mockito.when(volumeVO.getLastPoolId()).thenReturn(null); + Snapshot snapshot = mockSnapshotOfVolume(volumeVO); + + Assert.assertEquals(StrategyPriority.CANT_HANDLE, storageSystemSnapshotStrategy.canHandle(snapshot, null, SnapshotOperation.REVERT)); + Mockito.verifyNoInteractions(dataStoreMgr, storagePoolDao); + } + + @Test + public void canHandleRevertTestBackendSnapshotOfVolumeWithoutPool() { + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + Mockito.when(volumeVO.getRemoved()).thenReturn(null); + Mockito.when(volumeVO.getFormat()).thenReturn(ImageFormat.RAW); + Mockito.when(volumeVO.getPoolId()).thenReturn(null); + Mockito.when(volumeVO.getLastPoolId()).thenReturn(null); + Snapshot snapshot = mockSnapshotOfVolume(volumeVO); + Mockito.when(snapshot.getId()).thenReturn(2L); + + SnapshotDetailsVO takeSnapshot = Mockito.mock(SnapshotDetailsVO.class); + Mockito.when(takeSnapshot.getValue()).thenReturn("true"); + Mockito.when(snapshotDetailsDao.findDetail(2L, "takeSnapshot")).thenReturn(takeSnapshot); + + SnapshotDataStoreVO snapshotStore = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(snapshotStore.getDataStoreId()).thenReturn(10L); + Mockito.when(snapshotStoreDao.findOneBySnapshotAndDatastoreRole(2L, DataStoreRole.Primary)).thenReturn(snapshotStore); + + DataStore dataStore = Mockito.mock(DataStore.class); + DataStoreDriver driver = Mockito.mock(DataStoreDriver.class); + Mockito.when(driver.getCapabilities()).thenReturn(new HashMap<>()); + Mockito.when(dataStore.getDriver()).thenReturn(driver); + Mockito.when(dataStoreMgr.getDataStore(10L, DataStoreRole.Primary)).thenReturn(dataStore); + + Assert.assertEquals(StrategyPriority.CANT_HANDLE, storageSystemSnapshotStrategy.canHandle(snapshot, null, SnapshotOperation.REVERT)); + Mockito.verify(dataStoreMgr, Mockito.times(1)).getDataStore(Mockito.anyLong(), Mockito.any()); + } + + @Test + public void canHandleRevertTestBackendSnapshotOnRevertCapablePoolOfVolumeWithoutPool() { + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + Mockito.when(volumeVO.getRemoved()).thenReturn(null); + Mockito.when(volumeVO.getFormat()).thenReturn(ImageFormat.RAW); + Mockito.when(volumeVO.getPoolId()).thenReturn(null); + Mockito.when(volumeVO.getLastPoolId()).thenReturn(null); + Snapshot snapshot = mockSnapshotOfVolume(volumeVO); + Mockito.when(snapshot.getId()).thenReturn(2L); + + SnapshotDetailsVO takeSnapshot = Mockito.mock(SnapshotDetailsVO.class); + Mockito.when(takeSnapshot.getValue()).thenReturn("true"); + Mockito.when(snapshotDetailsDao.findDetail(2L, "takeSnapshot")).thenReturn(takeSnapshot); + + SnapshotDataStoreVO snapshotStore = Mockito.mock(SnapshotDataStoreVO.class); + Mockito.when(snapshotStore.getDataStoreId()).thenReturn(10L); + Mockito.when(snapshotStoreDao.findOneBySnapshotAndDatastoreRole(2L, DataStoreRole.Primary)).thenReturn(snapshotStore); + + mockDataStoreCapability(10L, DataStoreCapabilities.CAN_REVERT_VOLUME_TO_SNAPSHOT.toString()); + + Assert.assertEquals(StrategyPriority.HIGHEST, storageSystemSnapshotStrategy.canHandle(snapshot, null, SnapshotOperation.REVERT)); + } + + @Test + public void canHandleTakeAndBackupTestVolumeOnLastPool() { + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + Mockito.when(volumeVO.getPoolId()).thenReturn(null); + Mockito.when(volumeVO.getLastPoolId()).thenReturn(20L); + Snapshot snapshot = mockSnapshotOfVolume(volumeVO); + + mockDataStoreCapability(20L, DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString()); + + for (SnapshotOperation op : List.of(SnapshotOperation.TAKE, SnapshotOperation.BACKUP)) { + Assert.assertEquals(StrategyPriority.HIGHEST, storageSystemSnapshotStrategy.canHandle(snapshot, null, op)); + } + } + + private void mockDataStoreCapability(long storeId, String capability) { + DataStore dataStore = Mockito.mock(DataStore.class); + DataStoreDriver driver = Mockito.mock(DataStoreDriver.class); + Map capabilities = new HashMap<>(); + capabilities.put(capability, Boolean.TRUE.toString()); + Mockito.when(driver.getCapabilities()).thenReturn(capabilities); + Mockito.when(dataStore.getDriver()).thenReturn(driver); + Mockito.when(dataStoreMgr.getDataStore(storeId, DataStoreRole.Primary)).thenReturn(dataStore); + } + + @Test + public void canHandleRevertTestVolumeOnManagedLastPool() { + VolumeVO volumeVO = Mockito.mock(VolumeVO.class); + Mockito.when(volumeVO.getRemoved()).thenReturn(null); + Mockito.when(volumeVO.getFormat()).thenReturn(ImageFormat.RAW); + Mockito.when(volumeVO.getPoolId()).thenReturn(null); + Mockito.when(volumeVO.getLastPoolId()).thenReturn(20L); + Snapshot snapshot = mockSnapshotOfVolume(volumeVO); + + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.when(pool.isManaged()).thenReturn(true); + Mockito.when(storagePoolDao.findById(20L)).thenReturn(pool); + + Assert.assertEquals(StrategyPriority.HIGHEST, storageSystemSnapshotStrategy.canHandle(snapshot, null, SnapshotOperation.REVERT)); + } +}