Skip stale destroyed volumes in local storage checks

Author: andrijapanicsb

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -7660,11 +7660,27 @@ private boolean isImplicitPlannerUsedByOffering(long offeringId) {
 
     protected boolean isAnyVmVolumeUsingLocalStorage(final List<VolumeVO> volumes) {
         for (VolumeVO vol : volumes) {
+            if (vol == null || vol.getRemoved() != null ||
+                    Volume.State.Destroy.equals(vol.getState()) ||
+                    Volume.State.Expunged.equals(vol.getState())) {
+                logger.debug("Skipping non-active volume while checking local storage usage: {}", vol);
+                continue;
+            }
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(vol.getDiskOfferingId());
-            if (diskOffering.isUseLocalStorage()) {
+            if (diskOffering != null && diskOffering.isUseLocalStorage()) {
                 return true;
             }
-            StoragePoolVO storagePool = _storagePoolDao.findById(vol.getPoolId());
+            Long poolId = vol.getPoolId();
+            if (poolId == null) {
+                logger.debug("Skipping volume without storage pool while checking local storage usage: {}", vol);
+                continue;
+            }
+            StoragePoolVO storagePool = _storagePoolDao.findById(poolId);
+            if (storagePool == null || storagePool.getRemoved() != null) {
+                throw new CloudRuntimeException(String.format(
+                        "Cannot determine local storage usage for active volume %s because storage pool ID %s is missing or removed",
+                        vol, poolId));
+            }
             if (storagePool.isLocal()) {
                 return true;
             }

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -1264,6 +1264,58 @@ public void testIsAnyVmVolumeUsingLocalStorage() {
         }
     }
 
+    @Test
+    public void testIsAnyVmVolumeUsingLocalStorageSkipsDestroyedVolumeWithMissingPool() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getState()).thenReturn(Volume.State.Destroy);
+
+        Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(Collections.singletonList(volume)));
+        Mockito.verify(primaryDataStoreDao, never()).findById(anyLong());
+    }
+
+    @Test
+    public void testIsAnyVmVolumeUsingLocalStorageSkipsRemovedVolume() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getRemoved()).thenReturn(new Date());
+
+        Assert.assertFalse(userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(Collections.singletonList(volume)));
+        Mockito.verify(primaryDataStoreDao, never()).findById(anyLong());
+    }
+
+    @Test
+    public void testIsAnyVmVolumeUsingLocalStorageFailsForActiveVolumeWithMissingPool() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getState()).thenReturn(Volume.State.Ready);
+        Mockito.when(volume.getDiskOfferingId()).thenReturn(1L);
+        Mockito.when(volume.getPoolId()).thenReturn(2L);
+        DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class);
+        Mockito.when(diskOfferingDao.findById(1L)).thenReturn(diskOffering);
+        Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false);
+        Mockito.when(primaryDataStoreDao.findById(2L)).thenReturn(null);
+
+        CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () ->
+                userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(Collections.singletonList(volume)));
+        Assert.assertTrue(exception.getMessage().contains("storage pool ID 2 is missing or removed"));
+    }
+
+    @Test
+    public void testIsAnyVmVolumeUsingLocalStorageFailsForActiveVolumeWithRemovedPool() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getState()).thenReturn(Volume.State.Ready);
+        Mockito.when(volume.getDiskOfferingId()).thenReturn(1L);
+        Mockito.when(volume.getPoolId()).thenReturn(2L);
+        DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class);
+        Mockito.when(diskOfferingDao.findById(1L)).thenReturn(diskOffering);
+        Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false);
+        StoragePoolVO storagePool = Mockito.mock(StoragePoolVO.class);
+        Mockito.when(storagePool.getRemoved()).thenReturn(new Date());
+        Mockito.when(primaryDataStoreDao.findById(2L)).thenReturn(storagePool);
+
+        CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () ->
+                userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(Collections.singletonList(volume)));
+        Assert.assertTrue(exception.getMessage().contains("storage pool ID 2 is missing or removed"));
+    }
+
     private List<VolumeVO> mockVolumesForIsAllVmVolumesOnZoneWideStore(int nullPoolIdVolumes, int nullPoolVolumes, int zoneVolumes, int nonZoneVolumes) {
         List<VolumeVO> volumes = new ArrayList<>();
         for (int i=0; i< nullPoolIdVolumes + nullPoolVolumes + zoneVolumes + nonZoneVolumes; ++i) {