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 <joao@scclouds.com.br>
This commit is contained in:
João Jandre 2022-12-06 17:48:35 -03:00 committed by GitHub
parent a63b2aba7a
commit 7c61d8aeaf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 71 additions and 3 deletions

View File

@ -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);
}

View File

@ -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);
}
}

View File

@ -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.

View File

@ -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);
}

View File

@ -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);
}
}

View File

@ -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);
}
}