diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java index 2001cf05ab9..3cdaa3b05ad 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java @@ -147,4 +147,6 @@ public interface VolumeDao extends GenericDao, StateDao findByDiskOfferingId(long diskOfferingId); VolumeVO getInstanceRootVolume(long instanceId); + + void updateAndRemoveVolume(VolumeVO volume); } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java index 3c865bac663..eb3206fc42e 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java @@ -766,4 +766,15 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol sc.setParameters("vType", Volume.Type.ROOT); return findOneBy(sc); } + + @Override + public void updateAndRemoveVolume(VolumeVO volume) { + if (volume.getState() != Volume.State.Destroy) { + volume.setState(Volume.State.Destroy); + volume.setPoolId(null); + volume.setInstanceId(null); + update(volume.getId(), volume); + remove(volume.getId()); + } + } } diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 34629082748..d552c12e1a7 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -38,6 +38,9 @@ import javax.naming.ConfigurationException; import com.cloud.exception.StorageConflictException; import com.cloud.exception.StorageUnavailableException; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; @@ -294,6 +297,8 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, private UserVmDetailsDao userVmDetailsDao; @Inject private AnnotationDao annotationDao; + @Inject + private VolumeDao volumeDao; private final long _nodeId = ManagementServerNode.getManagementServerId(); @@ -979,6 +984,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, final Long poolId = pool.getPoolId(); final StoragePoolVO storagePool = _storagePoolDao.findById(poolId); if (storagePool.isLocal() && isForceDeleteStorage) { + destroyLocalStoragePoolVolumes(poolId); storagePool.setUuid(null); storagePool.setClusterId(null); _storagePoolDao.update(poolId, storagePool); @@ -1011,6 +1017,27 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, return true; } + private void addVolumesToList(List volumes, List volumesToAdd) { + if (CollectionUtils.isNotEmpty(volumesToAdd)) { + volumes.addAll(volumesToAdd); + } + } + + protected void destroyLocalStoragePoolVolumes(long poolId) { + List rootDisks = volumeDao.findByPoolId(poolId); + List dataVolumes = volumeDao.findByPoolId(poolId, Volume.Type.DATADISK); + + List volumes = new ArrayList<>(); + addVolumesToList(volumes, rootDisks); + addVolumesToList(volumes, dataVolumes); + + if (CollectionUtils.isNotEmpty(volumes)) { + for (VolumeVO volume : volumes) { + volumeDao.updateAndRemoveVolume(volume); + } + } + } + /** * Returns true if host can be deleted.
* A host can be deleted either if it is in Maintenance or "Degraded" state. diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 646b81960d6..fd234b24794 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -36,6 +36,7 @@ import javax.inject.Inject; import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy; import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.InternalIdentity; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd; import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd; @@ -88,6 +89,7 @@ import org.apache.cloudstack.storage.command.AttachAnswer; import org.apache.cloudstack.storage.command.AttachCommand; import org.apache.cloudstack.storage.command.DettachCommand; import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; @@ -273,6 +275,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Inject private PrimaryDataStoreDao _storagePoolDao; @Inject + private ImageStoreDao imageStoreDao; + @Inject private DiskOfferingDao _diskOfferingDao; @Inject private ServiceOfferingDao _serviceOfferingDao; @@ -1619,6 +1623,12 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } private void expungeVolumesInPrimaryOrSecondary(VolumeVO volume, DataStoreRole role) throws InterruptedException, ExecutionException { + if (!canAccessVolumeStore(volume, role)) { + s_logger.debug(String.format("Cannot access the storage pool with role: %s " + + "for the volume: %s, skipping expunge from storage", + role.name(), volume.getName())); + return; + } VolumeInfo volOnStorage = volFactory.getVolume(volume.getId(), role); if (volOnStorage != null) { s_logger.info("Expunging volume " + volume.getId() + " from " + role + " data store"); @@ -1638,6 +1648,17 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } } } + + private boolean canAccessVolumeStore(VolumeVO volume, DataStoreRole role) { + if (volume == null) { + throw new CloudRuntimeException("No volume given, cannot check access to volume store"); + } + InternalIdentity pool = role == DataStoreRole.Primary ? + _storagePoolDao.findById(volume.getPoolId()) : + imageStoreDao.findById(volume.getPoolId()); + return pool != null; + } + /** * Clean volumes cache entries (if they exist). */ diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 1793456ab91..a7ddd16462e 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -34,9 +34,13 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.UUID; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; import org.apache.cloudstack.api.command.admin.host.CancelHostAsDegradedCmd; import org.apache.cloudstack.api.command.admin.host.DeclareHostAsDegradedCmd; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; @@ -98,6 +102,8 @@ public class ResourceManagerImplTest { private VMInstanceDao vmInstanceDao; @Mock private ConfigurationDao configurationDao; + @Mock + private VolumeDao volumeDao; @Spy @InjectMocks @@ -119,6 +125,13 @@ public class ResourceManagerImplTest { @Mock private GetVncPortCommand getVncPortCommandVm2; + @Mock + private VolumeVO rootDisk1; + @Mock + private VolumeVO rootDisk2; + @Mock + private VolumeVO dataDisk; + @Mock private Connection sshConnection; @@ -138,6 +151,10 @@ public class ResourceManagerImplTest { private static String vm2VncAddress = "10.2.2.2"; private static int vm2VncPort = 5901; + private static long poolId = 1L; + private List rootDisks; + private List dataDisks; + @Before public void setup() throws Exception { MockitoAnnotations.initMocks(this); @@ -179,6 +196,11 @@ public class ResourceManagerImplTest { willReturn(new SSHCmdHelper.SSHCmdResult(0,"","")); when(configurationDao.getValue(ResourceManager.KvmSshToAgentEnabled.key())).thenReturn("true"); + + rootDisks = Arrays.asList(rootDisk1, rootDisk2); + dataDisks = Collections.singletonList(dataDisk); + when(volumeDao.findByPoolId(poolId)).thenReturn(rootDisks); + when(volumeDao.findByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(dataDisks); } @Test @@ -527,4 +549,33 @@ public class ResourceManagerImplTest { return new HostVO(1L, "host01", Host.Type.Routing, "192.168.1.1", "255.255.255.0", null, null, null, null, null, null, null, null, null, null, UUID.randomUUID().toString(), hostStatus, "1.0", null, null, 1L, null, 0, 0, null, 0, null); } + + @Test + public void testDestroyLocalStoragePoolVolumesBothRootDisksAndDataDisks() { + resourceManager.destroyLocalStoragePoolVolumes(poolId); + verify(volumeDao, times(rootDisks.size() + dataDisks.size())) + .updateAndRemoveVolume(any(VolumeVO.class)); + } + + @Test + public void testDestroyLocalStoragePoolVolumesOnlyRootDisks() { + when(volumeDao.findByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(null); + resourceManager.destroyLocalStoragePoolVolumes(poolId); + verify(volumeDao, times(rootDisks.size())).updateAndRemoveVolume(any(VolumeVO.class)); + } + + @Test + public void testDestroyLocalStoragePoolVolumesOnlyDataDisks() { + when(volumeDao.findByPoolId(poolId)).thenReturn(null); + resourceManager.destroyLocalStoragePoolVolumes(poolId); + verify(volumeDao, times(dataDisks.size())).updateAndRemoveVolume(any(VolumeVO.class)); + } + + @Test + public void testDestroyLocalStoragePoolVolumesNoDisks() { + when(volumeDao.findByPoolId(poolId)).thenReturn(null); + when(volumeDao.findByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(null); + resourceManager.destroyLocalStoragePoolVolumes(poolId); + verify(volumeDao, never()).updateAndRemoveVolume(any(VolumeVO.class)); + } } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 16c7887ef91..e5b85c829e4 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -52,6 +52,8 @@ import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext; import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; +import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao; @@ -143,6 +145,12 @@ public class VolumeApiServiceImplTest { @Mock private PrimaryDataStoreDao primaryDataStoreDaoMock; @Mock + private ImageStoreDao imageStoreDao; + @Mock + private ImageStoreVO imageStoreVO; + @Mock + private StoragePoolVO storagePoolVO; + @Mock private VMSnapshotDao _vmSnapshotDao; @Mock private AsyncJobManager _jobMgr; @@ -214,6 +222,7 @@ public class VolumeApiServiceImplTest { private long volumeMockId = 12313l; private long vmInstanceMockId = 1123l; private long volumeSizeMock = 456789921939l; + private static long imageStoreId = 10L; private String projectMockUuid = "projectUuid"; private long projecMockId = 13801801923810L; @@ -239,6 +248,10 @@ public class VolumeApiServiceImplTest { Mockito.when(storagePoolMock.getId()).thenReturn(storagePoolMockId); + Mockito.when(volumeVoMock.getPoolId()).thenReturn(storagePoolMockId); + Mockito.when(imageStoreDao.findById(imageStoreId)).thenReturn(imageStoreVO); + Mockito.when(primaryDataStoreDaoMock.findById(storagePoolMockId)).thenReturn(storagePoolVO); + volumeApiServiceImpl._gson = GsonHelper.getGsonLogger(); // mock caller context @@ -914,7 +927,7 @@ public class VolumeApiServiceImplTest { @Test public void expungeVolumesInSecondaryStorageIfNeededTestVolumeNotFoundInSecondaryStorage() throws InterruptedException, ExecutionException { Mockito.lenient().doReturn(asyncCallFutureVolumeapiResultMock).when(volumeServiceMock).expungeVolumeAsync(volumeInfoMock); - Mockito.doReturn(null).when(volumeDataFactoryMock).getVolume(volumeMockId, DataStoreRole.Image); + Mockito.lenient().doReturn(null).when(imageStoreDao).findById(imageStoreId); Mockito.lenient().doNothing().when(resourceLimitServiceMock).decrementResourceCount(accountMockId, ResourceType.secondary_storage, volumeSizeMock); Mockito.lenient().doReturn(accountMockId).when(volumeInfoMock).getAccountId(); Mockito.lenient().doReturn(volumeSizeMock).when(volumeInfoMock).getSize(); @@ -933,6 +946,7 @@ public class VolumeApiServiceImplTest { Mockito.doNothing().when(resourceLimitServiceMock).decrementResourceCount(accountMockId, ResourceType.secondary_storage, volumeSizeMock); Mockito.doReturn(accountMockId).when(volumeInfoMock).getAccountId(); Mockito.doReturn(volumeSizeMock).when(volumeInfoMock).getSize(); + Mockito.doReturn(imageStoreId).when(volumeVoMock).getPoolId(); volumeApiServiceImpl.expungeVolumesInSecondaryStorageIfNeeded(volumeVoMock); @@ -948,6 +962,7 @@ public class VolumeApiServiceImplTest { Mockito.lenient().doNothing().when(resourceLimitServiceMock).decrementResourceCount(accountMockId, ResourceType.secondary_storage, volumeSizeMock); Mockito.lenient().doReturn(accountMockId).when(volumeInfoMock).getAccountId(); Mockito.lenient().doReturn(volumeSizeMock).when(volumeInfoMock).getSize(); + Mockito.doReturn(imageStoreId).when(volumeVoMock).getPoolId(); Mockito.doThrow(InterruptedException.class).when(asyncCallFutureVolumeapiResultMock).get(); @@ -962,6 +977,7 @@ public class VolumeApiServiceImplTest { Mockito.lenient().doNothing().when(resourceLimitServiceMock).decrementResourceCount(accountMockId, ResourceType.secondary_storage, volumeSizeMock); Mockito.lenient().doReturn(accountMockId).when(volumeInfoMock).getAccountId(); Mockito.lenient().doReturn(volumeSizeMock).when(volumeInfoMock).getSize(); + Mockito.doReturn(imageStoreId).when(volumeVoMock).getPoolId(); Mockito.doThrow(ExecutionException.class).when(asyncCallFutureVolumeapiResultMock).get();