This is an automated email from the ASF dual-hosted git repository.
gutoveronezi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 7c61d8aeaf Set root volume as destroyed when destroying a VM (#6868)
7c61d8aeaf is described below
commit 7c61d8aeaff7ecd496ec6e7f4a6d2885a3dfe174
Author: João Jandre <[email protected]>
AuthorDate: Tue Dec 6 17:48:35 2022 -0300
Set root volume as destroyed when destroying a VM (#6868)
* Set root volume as destroyed when destroying a VM
* Address review
* Address review
Co-authored-by: João Jandre <[email protected]>
---
.../main/java/com/cloud/storage/dao/VolumeDao.java | 1 +
.../java/com/cloud/storage/dao/VolumeDaoImpl.java | 7 +++++
.../java/com/cloud/storage/StorageManagerImpl.java | 12 +++++++++
.../src/main/java/com/cloud/vm/UserVmManager.java | 6 +++++
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 30 +++++++++++++++++++---
.../java/com/cloud/vm/UserVmManagerImplTest.java | 18 +++++++++++++
6 files changed, 71 insertions(+), 3 deletions(-)
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 64151d6068..2001cf05ab 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
@@ -146,4 +146,5 @@ public interface VolumeDao extends GenericDao<VolumeVO,
Long>, StateDao<Volume.S
* @return the list of volumes that uses that disk offering.
*/
List<VolumeVO> findByDiskOfferingId(long diskOfferingId);
+ VolumeVO getInstanceRootVolume(long instanceId);
}
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 d27faa3a78..3c865bac66 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
@@ -759,4 +759,11 @@ public class VolumeDaoImpl extends
GenericDaoBase<VolumeVO, Long> implements Vol
throw new CloudRuntimeException(e);
}
}
+ @Override
+ public VolumeVO getInstanceRootVolume(long instanceId) {
+ SearchCriteria<VolumeVO> sc = RootDiskStateSearch.create();
+ sc.setParameters("instanceId", instanceId);
+ sc.setParameters("vType", Volume.Type.ROOT);
+ return findOneBy(sc);
+ }
}
diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
index 00a15e6e32..0018cc49d1 100644
--- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
@@ -46,6 +46,7 @@ import java.util.stream.Collectors;
import javax.inject.Inject;
import com.google.common.collect.Sets;
+import com.cloud.vm.UserVmManager;
import org.apache.cloudstack.annotation.AnnotationService;
import org.apache.cloudstack.annotation.dao.AnnotationDao;
import org.apache.cloudstack.api.ApiConstants;
@@ -344,6 +345,8 @@ public class StorageManagerImpl extends ManagerBase
implements StorageManager, C
@Inject
private AnnotationDao annotationDao;
+ @Inject
+ protected UserVmManager userVmManager;
protected List<StoragePoolDiscoverer> _discoverers;
public List<StoragePoolDiscoverer> getDiscoverers() {
@@ -1325,6 +1328,15 @@ public class StorageManagerImpl extends ManagerBase
implements StorageManager, C
List<VolumeVO> vols =
_volsDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() -
((long)StorageCleanupDelay.value() << 10)));
for (VolumeVO vol : vols) {
+ if (Type.ROOT.equals(vol.getVolumeType())) {
+ VMInstanceVO vmInstanceVO =
_vmInstanceDao.findById(vol.getInstanceId());
+ if (vmInstanceVO != null &&
vmInstanceVO.getState() == State.Destroyed) {
+ s_logger.debug(String.format("ROOT volume
[%s] will not be expunged because the VM is [%s], therefore this volume will be
expunged with the VM"
+ + " cleanup job.", vol.getUuid(),
vmInstanceVO.getState()));
+ continue;
+ }
+ }
+
try {
// If this fails, just log a warning. It's ideal
if we clean up the host-side clustered file
// system, but not necessary.
diff --git a/server/src/main/java/com/cloud/vm/UserVmManager.java
b/server/src/main/java/com/cloud/vm/UserVmManager.java
index 082c36f518..5029615b24 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManager.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManager.java
@@ -57,6 +57,10 @@ public interface UserVmManager extends UserVmService {
ConfigKey<Boolean> DisplayVMOVFProperties = new
ConfigKey<Boolean>("Advanced", Boolean.class, "vm.display.ovf.properties",
"false",
"Set display of VMs OVF properties as part of VM details", true);
+ ConfigKey<Boolean> DestroyRootVolumeOnVmDestruction = new
ConfigKey<Boolean>("Advanced", Boolean.class,
"destroy.root.volume.on.vm.destruction", "false",
+ "Destroys the VM's root volume when the VM is destroyed.",
+ true, ConfigKey.Scope.Domain);
+
static final int MAX_USER_DATA_LENGTH_BYTES = 2048;
public static final String CKS_NODE = "cksnode";
@@ -136,4 +140,6 @@ public interface UserVmManager extends UserVmService {
boolean checkIfDynamicScalingCanBeEnabled(VirtualMachine vm,
ServiceOffering offering, VirtualMachineTemplate template, Long zoneId);
+ Boolean getDestroyRootVolumeOnVmDestruction(Long domainId);
+
}
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 4e4f38e1af..caa2459f5a 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -596,6 +596,8 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
private int _scaleRetry;
private Map<Long, VmAndCountDetails> vmIdCountMap = new
ConcurrentHashMap<>();
+ protected static long ROOT_DEVICE_ID = 0;
+
private static final int MAX_HTTP_GET_LENGTH = 2 *
MAX_USER_DATA_LENGTH_BYTES;
private static final int NUM_OF_2K_BLOCKS = 512;
private static final int MAX_HTTP_POST_LENGTH = NUM_OF_2K_BLOCKS *
MAX_USER_DATA_LENGTH_BYTES;
@@ -2333,8 +2335,8 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
for (VolumeVO volume : volumes) {
if (volume.getVolumeType().equals(Volume.Type.ROOT)) {
- // Create an event
- _volumeService.publishVolumeCreationUsageEvent(volume);
+ recoverRootVolume(volume, vmId);
+ break;
}
}
@@ -2346,6 +2348,15 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
return _vmDao.findById(vmId);
}
+ protected void recoverRootVolume(VolumeVO volume, Long vmId) {
+ if (Volume.State.Destroy.equals(volume.getState())) {
+ _volumeService.recoverVolume(volume.getId());
+ _volsDao.attachVolume(volume.getId(), vmId, ROOT_DEVICE_ID);
+ } else {
+ _volumeService.publishVolumeCreationUsageEvent(volume);
+ }
+ }
+
@Override
public boolean configure(String name, Map<String, Object> params) throws
ConfigurationException {
_name = name;
@@ -3330,6 +3341,15 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
deleteVolumesFromVm(volumesToBeDeleted, expunge);
+ if (getDestroyRootVolumeOnVmDestruction(vm.getDomainId())) {
+ VolumeVO rootVolume = _volsDao.getInstanceRootVolume(vm.getId());
+ if (rootVolume != null) {
+ _volService.destroyVolume(rootVolume.getId());
+ } else {
+ s_logger.warn(String.format("Tried to destroy ROOT volume for
VM [%s], but couldn't retrieve it.", vm.getUuid()));
+ }
+ }
+
return destroyedVm;
}
@@ -8004,7 +8024,7 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {EnableDynamicallyScaleVm,
AllowDiskOfferingChangeDuringScaleVm, AllowUserExpungeRecoverVm,
VmIpFetchWaitInterval, VmIpFetchTrialMax,
VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers,
AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties,
- KvmAdditionalConfigAllowList,
XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList};
+ KvmAdditionalConfigAllowList,
XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList,
DestroyRootVolumeOnVmDestruction};
}
@Override
@@ -8334,4 +8354,8 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
s_logger.warn(String.format("Skip collecting vm %s disk and
network statistics as the expected vm state is %s but actual state is %s", vm,
expectedState, vm.getState()));
}
}
+
+ public Boolean getDestroyRootVolumeOnVmDestruction(Long domainId){
+ return DestroyRootVolumeOnVmDestruction.valueIn(domainId);
+ }
}
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index 2e6f72e67b..06a56e45f1 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -16,6 +16,8 @@
// under the License.
package com.cloud.vm;
+import com.cloud.storage.Volume;
+import com.cloud.storage.dao.VolumeDao;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -166,6 +168,12 @@ public class UserVmManagerImplTest {
@Mock
UserDataDao userDataDao;
+ @Mock
+ private VolumeVO volumeVOMock;
+
+ @Mock
+ private VolumeDao volumeDaoMock;
+
private long vmId = 1l;
private static final long GiB_TO_BYTES = 1024 * 1024 * 1024;
@@ -856,4 +864,14 @@ public class UserVmManagerImplTest {
Assert.assertEquals("testUserdata", userVmVO.getUserData());
Assert.assertEquals(1L, (long)userVmVO.getUserDataId());
}
+
+ @Test
+ public void recoverRootVolumeTestDestroyState() {
+ Mockito.doReturn(Volume.State.Destroy).when(volumeVOMock).getState();
+
+ userVmManagerImpl.recoverRootVolume(volumeVOMock, vmId);
+
+ Mockito.verify(volumeApiService).recoverVolume(volumeVOMock.getId());
+ Mockito.verify(volumeDaoMock).attachVolume(volumeVOMock.getId(), vmId,
UserVmManagerImpl.ROOT_DEVICE_ID);
+ }
}
