server: usage generated for destroyed VMs with no backups (#5017)

Fixes: #4990
When a VM associated with a backup offering is destroyed/expunged, the backup offering isn't unassigned, and despite the VM having no backups present, backup usage is generated. This PR prevent usage record generation when there are no backups present for a VM with a backup offering associated to it. This is done by ensuring that usage event for backups is generated only when a the backup size > 0
This commit is contained in:
Pearl Dsilva 2021-05-31 18:59:48 +05:30 committed by GitHub
parent fbc8610f6e
commit d04fa0201d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 37 additions and 15 deletions

View File

@ -158,4 +158,5 @@ public interface VMInstanceDao extends GenericDao<VMInstanceVO, Long>, StateDao<
boolean isPowerStateUpToDate(long instanceId);
List<VMInstanceVO> listNonMigratingVmsByHostEqualsLastHost(long hostId);
}

View File

@ -300,6 +300,7 @@ public class VMInstanceDaoImpl extends GenericDaoBase<VMInstanceVO, Long> implem
LastHostAndStatesSearch.and("lastHost", LastHostAndStatesSearch.entity().getLastHostId(), Op.EQ);
LastHostAndStatesSearch.and("states", LastHostAndStatesSearch.entity().getState(), Op.IN);
LastHostAndStatesSearch.done();
}
@Override

View File

@ -81,6 +81,10 @@ import org.apache.cloudstack.api.command.user.vm.UpgradeVMCmd;
import org.apache.cloudstack.api.command.user.vmgroup.CreateVMGroupCmd;
import org.apache.cloudstack.api.command.user.vmgroup.DeleteVMGroupCmd;
import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
import com.cloud.agent.api.to.deployasis.OVFNetworkTO;
import org.apache.cloudstack.backup.Backup;
import org.apache.cloudstack.backup.BackupManager;
import org.apache.cloudstack.backup.dao.BackupDao;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.cloud.entity.api.VirtualMachineEntity;
import org.apache.cloudstack.engine.cloud.entity.api.db.dao.VMNetworkMapDao;
@ -141,7 +145,6 @@ import com.cloud.agent.api.VolumeStatsEntry;
import com.cloud.agent.api.to.DiskTO;
import com.cloud.agent.api.to.NicTO;
import com.cloud.agent.api.to.VirtualMachineTO;
import com.cloud.agent.api.to.deployasis.OVFNetworkTO;
import com.cloud.agent.api.to.deployasis.OVFPropertyTO;
import com.cloud.agent.manager.Commands;
import com.cloud.alert.AlertManager;
@ -519,6 +522,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
private StorageManager storageMgr;
@Inject
private ServiceOfferingJoinDao serviceOfferingJoinDao;
@Inject
private BackupDao backupDao;
@Inject
private BackupManager backupManager;
private ScheduledExecutorService _executor = null;
private ScheduledExecutorService _vmIpFetchExecutor = null;
@ -2261,6 +2268,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
}
try {
if (vm.getBackupOfferingId() != null) {
List<Backup> backupsForVm = backupDao.listByVmId(vm.getDataCenterId(), vm.getId());
if (CollectionUtils.isEmpty(backupsForVm)) {
backupManager.removeVMFromBackupOffering(vm.getId(), true);
}
}
releaseNetworkResourcesOnExpunge(vm.getId());
List<VolumeVO> rootVol = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT);

View File

@ -26,6 +26,7 @@ import java.util.Map;
import java.util.TimeZone;
import java.util.Timer;
import java.util.TimerTask;
import java.util.stream.Collectors;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
@ -61,6 +62,7 @@ import org.apache.cloudstack.poll.BackgroundPollManager;
import org.apache.cloudstack.poll.BackgroundPollTask;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.commons.collections.CollectionUtils;
import org.apache.log4j.Logger;
import com.cloud.api.ApiDispatcher;
@ -333,24 +335,22 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager {
}
boolean result = false;
VMInstanceVO vmInstance = null;
try {
vmInstance = vmInstanceDao.acquireInLockTable(vm.getId());
vmInstance.setBackupOfferingId(null);
vmInstance.setBackupExternalId(null);
vmInstance.setBackupVolumes(null);
result = backupProvider.removeVMFromBackupOffering(vmInstance);
vm.setBackupOfferingId(null);
vm.setBackupExternalId(null);
vm.setBackupVolumes(null);
result = backupProvider.removeVMFromBackupOffering(vm);
if (result && backupProvider.willDeleteBackupsOnOfferingRemoval()) {
final List<Backup> backups = backupDao.listByVmId(null, vm.getId());
for (final Backup backup : backups) {
backupDao.remove(backup.getId());
}
}
if ((result || forced) && vmInstanceDao.update(vmInstance.getId(), vmInstance)) {
if ((result || forced) && vmInstanceDao.update(vm.getId(), vm)) {
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_BACKUP_OFFERING_REMOVE, vm.getAccountId(), vm.getDataCenterId(), vm.getId(),
"Backup-" + vm.getHostName() + "-" + vm.getUuid(), vm.getBackupOfferingId(), null, null,
Backup.class.getSimpleName(), vm.getUuid());
final BackupSchedule backupSchedule = backupScheduleDao.findByVM(vmInstance.getId());
final BackupSchedule backupSchedule = backupScheduleDao.findByVM(vm.getId());
if (backupSchedule != null) {
backupScheduleDao.remove(backupSchedule.getId());
}
@ -358,10 +358,6 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager {
}
} catch (final Exception e) {
LOG.warn("Exception caught when trying to remove VM from the backup offering: ", e);
} finally {
if (vmInstance != null) {
vmInstanceDao.releaseFromLockTable(vmInstance.getId());
}
}
return result;
}
@ -671,6 +667,14 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager {
if (offering == null) {
throw new CloudRuntimeException("VM backup offering ID " + vm.getBackupOfferingId() + " does not exist");
}
List<Backup> backupsForVm = backupDao.listByVmId(vm.getDataCenterId(), vmId);
if (CollectionUtils.isNotEmpty(backupsForVm)) {
backupsForVm = backupsForVm.stream().filter(vmBackup -> vmBackup.getId() != backupId).collect(Collectors.toList());
if (backupsForVm.size() <= 0 && vm.getRemoved() != null) {
removeVMFromBackupOffering(vmId, true);
}
}
final BackupProvider backupProvider = getBackupProvider(offering.getProvider());
boolean result = backupProvider.deleteBackup(backup);
if (result) {

View File

@ -2402,6 +2402,7 @@
"message.action.delete.vpn.user": "Please confirm that you want to delete the VPN user.",
"message.action.delete.zone": "Please confirm that you want to delete this zone.",
"message.action.destroy.instance": "Please confirm that you want to destroy the instance.",
"message.action.destroy.instance.with.backups": "Please confirm that you want to destroy the instance. There may be backups associated with the instance which will not be deleted.",
"message.action.destroy.systemvm": "Please confirm that you want to destroy the System VM.",
"message.action.destroy.volume": "Please confirm that you want to destroy the volume.",
"message.action.disable.cluster": "Please confirm that you want to disable this cluster.",
@ -2420,6 +2421,7 @@
"message.action.enable.pod": "Please confirm that you want to enable this pod.",
"message.action.enable.zone": "Please confirm that you want to enable this zone.",
"message.action.expunge.instance": "Please confirm that you want to expunge this instance.",
"message.action.expunge.instance.with.backups": "Please confirm that you want to expunge this instance. There may be backups associated with the instance which will not be deleted.",
"message.action.force.reconnect": "Your host has been successfully forced to reconnect. This process can take up to several minutes.",
"message.action.host.enable.maintenance.mode": "Enabling maintenance mode will cause a live migration of all running instances on this host to any available host.",
"message.action.instance.reset.password": "Please confirm that you want to change the ROOT password for this virtual machine.",

View File

@ -377,7 +377,7 @@ export default {
api: 'expungeVirtualMachine',
icon: 'delete',
label: 'label.action.expunge.instance',
message: 'message.action.expunge.instance',
message: (record) => { return record.backupofferingid ? 'message.action.expunge.instance.with.backups' : 'message.action.expunge.instance' },
docHelp: 'adminguide/virtual_machines.html#deleting-vms',
dataView: true,
show: (record, store) => { return ['Destroyed', 'Expunging'].includes(record.state) && store.features.allowuserexpungerecovervm }

View File

@ -17,7 +17,7 @@
<template>
<div class="form-layout">
<a-alert type="warning" v-html="$t('message.action.destroy.instance')" /><br/>
<a-alert type="warning" v-html="resource.backupofferingid ? $t('message.action.destroy.instance.with.backups') : $t('message.action.destroy.instance')" /><br/>
<a-spin :spinning="loading">
<a-form
:form="form"