Skip to content

Commit d04fa02

Browse files
Pearl1594Pearl1594
authored
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
1 parent fbc8610 commit d04fa02

7 files changed

Lines changed: 37 additions & 15 deletions

File tree

‎engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,4 +158,5 @@ public interface VMInstanceDao extends GenericDao<VMInstanceVO, Long>, StateDao<
158158
boolean isPowerStateUpToDate(long instanceId);
159159

160160
List<VMInstanceVO> listNonMigratingVmsByHostEqualsLastHost(long hostId);
161+
161162
}

‎engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ protected void init() {
300300
LastHostAndStatesSearch.and("lastHost", LastHostAndStatesSearch.entity().getLastHostId(), Op.EQ);
301301
LastHostAndStatesSearch.and("states", LastHostAndStatesSearch.entity().getState(), Op.IN);
302302
LastHostAndStatesSearch.done();
303+
303304
}
304305

305306
@Override

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@
8181
import org.apache.cloudstack.api.command.user.vmgroup.CreateVMGroupCmd;
8282
import org.apache.cloudstack.api.command.user.vmgroup.DeleteVMGroupCmd;
8383
import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
84+
import com.cloud.agent.api.to.deployasis.OVFNetworkTO;
85+
import org.apache.cloudstack.backup.Backup;
86+
import org.apache.cloudstack.backup.BackupManager;
87+
import org.apache.cloudstack.backup.dao.BackupDao;
8488
import org.apache.cloudstack.context.CallContext;
8589
import org.apache.cloudstack.engine.cloud.entity.api.VirtualMachineEntity;
8690
import org.apache.cloudstack.engine.cloud.entity.api.db.dao.VMNetworkMapDao;
@@ -141,7 +145,6 @@
141145
import com.cloud.agent.api.to.DiskTO;
142146
import com.cloud.agent.api.to.NicTO;
143147
import com.cloud.agent.api.to.VirtualMachineTO;
144-
import com.cloud.agent.api.to.deployasis.OVFNetworkTO;
145148
import com.cloud.agent.api.to.deployasis.OVFPropertyTO;
146149
import com.cloud.agent.manager.Commands;
147150
import com.cloud.alert.AlertManager;
@@ -519,6 +522,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
519522
private StorageManager storageMgr;
520523
@Inject
521524
private ServiceOfferingJoinDao serviceOfferingJoinDao;
525+
@Inject
526+
private BackupDao backupDao;
527+
@Inject
528+
private BackupManager backupManager;
522529

523530
private ScheduledExecutorService _executor = null;
524531
private ScheduledExecutorService _vmIpFetchExecutor = null;
@@ -2261,6 +2268,13 @@ public boolean expunge(UserVmVO vm, long callerUserId, Account caller) {
22612268
}
22622269
try {
22632270

2271+
if (vm.getBackupOfferingId() != null) {
2272+
List<Backup> backupsForVm = backupDao.listByVmId(vm.getDataCenterId(), vm.getId());
2273+
if (CollectionUtils.isEmpty(backupsForVm)) {
2274+
backupManager.removeVMFromBackupOffering(vm.getId(), true);
2275+
}
2276+
}
2277+
22642278
releaseNetworkResourcesOnExpunge(vm.getId());
22652279

22662280
List<VolumeVO> rootVol = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT);

‎server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.TimeZone;
2727
import java.util.Timer;
2828
import java.util.TimerTask;
29+
import java.util.stream.Collectors;
2930

3031
import javax.inject.Inject;
3132
import javax.naming.ConfigurationException;
@@ -61,6 +62,7 @@
6162
import org.apache.cloudstack.poll.BackgroundPollTask;
6263
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
6364
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
65+
import org.apache.commons.collections.CollectionUtils;
6466
import org.apache.log4j.Logger;
6567

6668
import com.cloud.api.ApiDispatcher;
@@ -333,35 +335,29 @@ public boolean removeVMFromBackupOffering(final Long vmId, final boolean forced)
333335
}
334336

335337
boolean result = false;
336-
VMInstanceVO vmInstance = null;
337338
try {
338-
vmInstance = vmInstanceDao.acquireInLockTable(vm.getId());
339-
vmInstance.setBackupOfferingId(null);
340-
vmInstance.setBackupExternalId(null);
341-
vmInstance.setBackupVolumes(null);
342-
result = backupProvider.removeVMFromBackupOffering(vmInstance);
339+
vm.setBackupOfferingId(null);
340+
vm.setBackupExternalId(null);
341+
vm.setBackupVolumes(null);
342+
result = backupProvider.removeVMFromBackupOffering(vm);
343343
if (result && backupProvider.willDeleteBackupsOnOfferingRemoval()) {
344344
final List<Backup> backups = backupDao.listByVmId(null, vm.getId());
345345
for (final Backup backup : backups) {
346346
backupDao.remove(backup.getId());
347347
}
348348
}
349-
if ((result || forced) && vmInstanceDao.update(vmInstance.getId(), vmInstance)) {
349+
if ((result || forced) && vmInstanceDao.update(vm.getId(), vm)) {
350350
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_BACKUP_OFFERING_REMOVE, vm.getAccountId(), vm.getDataCenterId(), vm.getId(),
351351
"Backup-" + vm.getHostName() + "-" + vm.getUuid(), vm.getBackupOfferingId(), null, null,
352352
Backup.class.getSimpleName(), vm.getUuid());
353-
final BackupSchedule backupSchedule = backupScheduleDao.findByVM(vmInstance.getId());
353+
final BackupSchedule backupSchedule = backupScheduleDao.findByVM(vm.getId());
354354
if (backupSchedule != null) {
355355
backupScheduleDao.remove(backupSchedule.getId());
356356
}
357357
result = true;
358358
}
359359
} catch (final Exception e) {
360360
LOG.warn("Exception caught when trying to remove VM from the backup offering: ", e);
361-
} finally {
362-
if (vmInstance != null) {
363-
vmInstanceDao.releaseFromLockTable(vmInstance.getId());
364-
}
365361
}
366362
return result;
367363
}
@@ -671,6 +667,14 @@ public boolean deleteBackup(final Long backupId) {
671667
if (offering == null) {
672668
throw new CloudRuntimeException("VM backup offering ID " + vm.getBackupOfferingId() + " does not exist");
673669
}
670+
List<Backup> backupsForVm = backupDao.listByVmId(vm.getDataCenterId(), vmId);
671+
if (CollectionUtils.isNotEmpty(backupsForVm)) {
672+
backupsForVm = backupsForVm.stream().filter(vmBackup -> vmBackup.getId() != backupId).collect(Collectors.toList());
673+
if (backupsForVm.size() <= 0 && vm.getRemoved() != null) {
674+
removeVMFromBackupOffering(vmId, true);
675+
}
676+
}
677+
674678
final BackupProvider backupProvider = getBackupProvider(offering.getProvider());
675679
boolean result = backupProvider.deleteBackup(backup);
676680
if (result) {

‎ui/public/locales/en.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2402,6 +2402,7 @@
24022402
"message.action.delete.vpn.user": "Please confirm that you want to delete the VPN user.",
24032403
"message.action.delete.zone": "Please confirm that you want to delete this zone.",
24042404
"message.action.destroy.instance": "Please confirm that you want to destroy the instance.",
2405+
"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.",
24052406
"message.action.destroy.systemvm": "Please confirm that you want to destroy the System VM.",
24062407
"message.action.destroy.volume": "Please confirm that you want to destroy the volume.",
24072408
"message.action.disable.cluster": "Please confirm that you want to disable this cluster.",
@@ -2420,6 +2421,7 @@
24202421
"message.action.enable.pod": "Please confirm that you want to enable this pod.",
24212422
"message.action.enable.zone": "Please confirm that you want to enable this zone.",
24222423
"message.action.expunge.instance": "Please confirm that you want to expunge this instance.",
2424+
"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.",
24232425
"message.action.force.reconnect": "Your host has been successfully forced to reconnect. This process can take up to several minutes.",
24242426
"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.",
24252427
"message.action.instance.reset.password": "Please confirm that you want to change the ROOT password for this virtual machine.",

‎ui/src/config/section/compute.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ export default {
377377
api: 'expungeVirtualMachine',
378378
icon: 'delete',
379379
label: 'label.action.expunge.instance',
380-
message: 'message.action.expunge.instance',
380+
message: (record) => { return record.backupofferingid ? 'message.action.expunge.instance.with.backups' : 'message.action.expunge.instance' },
381381
docHelp: 'adminguide/virtual_machines.html#deleting-vms',
382382
dataView: true,
383383
show: (record, store) => { return ['Destroyed', 'Expunging'].includes(record.state) && store.features.allowuserexpungerecovervm }

‎ui/src/views/compute/DestroyVM.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
<template>
1919
<div class="form-layout">
20-
<a-alert type="warning" v-html="$t('message.action.destroy.instance')" /><br/>
20+
<a-alert type="warning" v-html="resource.backupofferingid ? $t('message.action.destroy.instance.with.backups') : $t('message.action.destroy.instance')" /><br/>
2121
<a-spin :spinning="loading">
2222
<a-form
2323
:form="form"

0 commit comments

Comments
 (0)