Skip to content

Commit b2a499e

Browse files
abh1sarabh1sar
committed
Handle backup offering remove without deleting backups
1 parent d76cb40 commit b2a499e

9 files changed

Lines changed: 119 additions & 38 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
@@ -187,4 +187,5 @@ List<VMInstanceVO> searchRemovedByRemoveDate(final Date startDate, final Date en
187187

188188
Map<String, Long> getNameIdMapForVmIds(Collection<Long> ids);
189189

190+
List<VMInstanceVO> listByIds(List<Long> ids);
190191
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,4 +1224,14 @@ public Map<String, Long> getNameIdMapForVmIds(Collection<Long> ids) {
12241224
return vms.stream()
12251225
.collect(Collectors.toMap(VMInstanceVO::getInstanceName, VMInstanceVO::getId));
12261226
}
1227+
1228+
@Override
1229+
public List<VMInstanceVO> listByIds(List<Long> ids) {
1230+
SearchBuilder<VMInstanceVO> idsSearch = createSearchBuilder();
1231+
idsSearch.and("ids", idsSearch.entity().getId(), SearchCriteria.Op.IN);
1232+
idsSearch.done();
1233+
SearchCriteria<VMInstanceVO> sc = idsSearch.create();
1234+
sc.setParameters("ids", ids.toArray());
1235+
return listIncludingRemovedBy(sc);
1236+
}
12271237
}

‎engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDao.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,12 @@ public interface BackupDao extends GenericDao<BackupVO, Long> {
3333
List<Backup> listByVmId(Long zoneId, Long vmId);
3434
List<Backup> listByAccountId(Long accountId);
3535
List<Backup> syncBackups(Long zoneId, Long vmId, List<Backup> externalBackups);
36+
List<Backup> listByVmIdAndOffering(Long zoneId, Long vmId, Long offeringId);
37+
List<BackupVO> searchByVmIds(List<Long> vmIds);
3638
BackupVO getBackupVO(Backup backup);
3739
List<Backup> listByOfferingId(Long backupOfferingId);
38-
3940
List<BackupVO> listBackupsByVMandIntervalType(Long vmId, Backup.Type backupType);
40-
41+
List<Long> listVmIdsWithBackupsInZone(Long zoneId);
4142
BackupResponse newBackupResponse(Backup backup, Boolean listVmDetails);
4243
public Long countBackupsForAccount(long accountId);
4344
public Long calculateBackupStorageForAccount(long accountId);

‎engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.List;
2323
import java.util.Map;
2424
import java.util.Objects;
25+
import java.util.stream.Collectors;
2526

2627
import javax.annotation.PostConstruct;
2728
import javax.inject.Inject;
@@ -33,6 +34,7 @@
3334
import org.apache.cloudstack.backup.BackupDetailVO;
3435
import org.apache.cloudstack.backup.BackupOffering;
3536
import org.apache.cloudstack.backup.BackupVO;
37+
import org.apache.commons.collections.CollectionUtils;
3638

3739
import com.cloud.dc.DataCenterVO;
3840
import com.cloud.dc.dao.DataCenterDao;
@@ -70,6 +72,7 @@ public class BackupDaoImpl extends GenericDaoBase<BackupVO, Long> implements Bac
7072
BackupDetailsDao backupDetailsDao;
7173

7274
private SearchBuilder<BackupVO> backupSearch;
75+
private SearchBuilder<BackupVO> backupVmSearchInZone;
7376
private GenericSearchBuilder<BackupVO, Long> CountBackupsByAccount;
7477
private GenericSearchBuilder<BackupVO, SumCount> CalculateBackupStorageByAccount;
7578
private SearchBuilder<BackupVO> ListBackupsByVMandIntervalType;
@@ -86,6 +89,11 @@ protected void init() {
8689
backupSearch.and("zone_id", backupSearch.entity().getZoneId(), SearchCriteria.Op.EQ);
8790
backupSearch.done();
8891

92+
backupVmSearchInZone = createSearchBuilder();
93+
backupVmSearchInZone.and("zone_id", backupSearch.entity().getZoneId(), SearchCriteria.Op.EQ);
94+
backupVmSearchInZone.select("vm_id", SearchCriteria.Func.DISTINCT, backupVmSearchInZone.entity().getVmId());
95+
backupSearch.done();
96+
8997
CountBackupsByAccount = createSearchBuilder(Long.class);
9098
CountBackupsByAccount.select(null, SearchCriteria.Func.COUNT, null);
9199
CountBackupsByAccount.and("account", CountBackupsByAccount.entity().getAccountId(), SearchCriteria.Op.EQ);
@@ -139,13 +147,36 @@ public List<Backup> listByVmId(Long zoneId, Long vmId) {
139147
return new ArrayList<>(listBy(sc));
140148
}
141149

150+
@Override
151+
public List<Backup> listByVmIdAndOffering(Long zoneId, Long vmId, Long offeringId) {
152+
SearchCriteria<BackupVO> sc = backupSearch.create();
153+
sc.setParameters("vm_id", vmId);
154+
if (zoneId != null) {
155+
sc.setParameters("zone_id", zoneId);
156+
}
157+
sc.setParameters("backup_offering_id", offeringId);
158+
return new ArrayList<>(listBy(sc));
159+
}
160+
142161
private Backup findByExternalId(Long zoneId, String externalId) {
143162
SearchCriteria<BackupVO> sc = backupSearch.create();
144163
sc.setParameters("external_id", externalId);
145164
sc.setParameters("zone_id", zoneId);
146165
return findOneBy(sc);
147166
}
148167

168+
@Override
169+
public List<BackupVO> searchByVmIds(List<Long> vmIds) {
170+
if (CollectionUtils.isEmpty(vmIds)) {
171+
return new ArrayList<>();
172+
}
173+
SearchBuilder<BackupVO> sb = createSearchBuilder();
174+
sb.and("vmIds", sb.entity().getVmId(), SearchCriteria.Op.IN);
175+
SearchCriteria<BackupVO> sc = sb.create();
176+
sc.setParameters("vmIds", vmIds.toArray());
177+
return search(sc, null);
178+
}
179+
149180
public BackupVO getBackupVO(Backup backup) {
150181
BackupVO backupVO = new BackupVO();
151182
backupVO.setExternalId(backup.getExternalId());
@@ -242,6 +273,14 @@ public void saveDetails(BackupVO backup) {
242273
backupDetailsDao.saveDetails(details);
243274
}
244275

276+
@Override
277+
public List<Long> listVmIdsWithBackupsInZone(Long zoneId) {
278+
SearchCriteria<BackupVO> sc = backupVmSearchInZone.create();
279+
sc.setParameters("zone_id", zoneId);
280+
List<BackupVO> backups = customSearch(sc, null);
281+
return backups.stream().map(BackupVO::getVmId).collect(Collectors.toList());
282+
}
283+
245284
@Override
246285
public BackupResponse newBackupResponse(Backup backup, Boolean listVmDetails) {
247286
VMInstanceVO vm = null;

‎plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ private boolean restoreVMBackup(VirtualMachine vm, Backup backup) {
243243
List<VolumeVO> restoreVolumes = volumeDao.findByInstance(vm.getId());
244244

245245
LOG.debug("Restoring vm {} from backup {} on the NAS Backup Provider", vm, backup);
246-
BackupRepository backupRepository = getBackupRepository(vm, backup);
246+
BackupRepository backupRepository = getBackupRepository(backup);
247247

248248
final Host host = getLastVMHypervisorHost(vm);
249249
RestoreBackupCommand restoreCommand = new RestoreBackupCommand();
@@ -295,7 +295,7 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeU
295295
Long backedUpVolumeSize = matchingVolume.isPresent() ? matchingVolume.get().getSize() : 0L;
296296

297297
LOG.debug("Restoring vm volume {} from backup {} on the NAS Backup Provider", volume, backup);
298-
BackupRepository backupRepository = getBackupRepository(backupSourceVm, backup);
298+
BackupRepository backupRepository = getBackupRepository(backup);
299299

300300
VolumeVO restoredVolume = new VolumeVO(Volume.Type.DATADISK, null, backup.getZoneId(),
301301
backup.getDomainId(), backup.getAccountId(), 0, null,
@@ -345,15 +345,10 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeU
345345
return new Pair<>(answer.getResult(), answer.getDetails());
346346
}
347347

348-
private BackupRepository getBackupRepository(VirtualMachine vm, Backup backup) {
349-
BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(vm.getBackupOfferingId());
350-
final String errorMessage = "No valid backup repository found for the VM, please check the attached backup offering";
348+
private BackupRepository getBackupRepository(Backup backup) {
349+
BackupRepository backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
351350
if (backupRepository == null) {
352-
logger.warn(errorMessage + "Re-attempting with the backup offering associated with the backup");
353-
}
354-
backupRepository = backupRepositoryDao.findByBackupOfferingId(backup.getBackupOfferingId());
355-
if (backupRepository == null) {
356-
throw new CloudRuntimeException(errorMessage);
351+
throw new CloudRuntimeException(String.format("No valid backup repository found for the backup %s, please check the attached backup offering", backup.getUuid()));
357352
}
358353
return backupRepository;
359354
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1645,7 +1645,8 @@ protected void runInContext() {
16451645

16461646
backupProvider.syncBackupStorageStats(dataCenter.getId());
16471647

1648-
List<VMInstanceVO> vms = vmInstanceDao.listByZoneWithBackups(dataCenter.getId(), null);
1648+
List<Long> vmIds = backupDao.listVmIdsWithBackupsInZone(dataCenter.getId());
1649+
List<VMInstanceVO> vms = vmInstanceDao.listByIds(vmIds);
16491650
if (vms == null || vms.isEmpty()) {
16501651
logger.debug("Can't find any VM to sync backups in zone {}", dataCenter);
16511652
continue;
@@ -1830,7 +1831,7 @@ public void checkAndRemoveBackupOfferingBeforeExpunge(VirtualMachine vm) {
18301831
if (vm.getBackupOfferingId() == null) {
18311832
return;
18321833
}
1833-
List<Backup> backupsForVm = backupDao.listByVmId(vm.getDataCenterId(), vm.getId());
1834+
List<Backup> backupsForVm = backupDao.listByVmIdAndOffering(vm.getDataCenterId(), vm.getId(), vm.getBackupOfferingId());
18341835
if (org.apache.commons.collections.CollectionUtils.isEmpty(backupsForVm)) {
18351836
removeVMFromBackupOffering(vm.getId(), true);
18361837
} else {

‎server/src/main/java/org/apache/cloudstack/resource/ResourceCleanupServiceImpl.java‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636

3737
import org.apache.cloudstack.api.ApiConstants;
3838
import org.apache.cloudstack.api.command.admin.resource.PurgeExpungedResourcesCmd;
39+
import org.apache.cloudstack.backup.BackupVO;
40+
import org.apache.cloudstack.backup.dao.BackupDao;
3941
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
4042
import org.apache.cloudstack.framework.async.AsyncCallFuture;
4143
import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher;
@@ -158,6 +160,8 @@ public class ResourceCleanupServiceImpl extends ManagerBase implements ResourceC
158160
ManagementServerHostDao managementServerHostDao;
159161
@Inject
160162
ServiceOfferingDetailsDao serviceOfferingDetailsDao;
163+
@Inject
164+
BackupDao backupDao;
161165

162166
private ScheduledExecutorService expungedResourcesCleanupExecutor;
163167
private ExecutorService purgeExpungedResourcesJobExecutor;
@@ -300,7 +304,7 @@ protected HashSet<Long> getVmIdsWithActiveVolumeSnapshots(List<Long> vmIds) {
300304
.collect(Collectors.toCollection(HashSet::new));
301305
}
302306

303-
protected Pair<List<Long>, List<Long>> getFilteredVmIdsForSnapshots(List<Long> vmIds) {
307+
protected Pair<List<Long>, List<Long>> getFilteredVmIdsForSnapshotsAndBackups(List<Long> vmIds) {
304308
HashSet<Long> currentSkippedVmIds = new HashSet<>();
305309
List<VMSnapshotVO> activeSnapshots = vmSnapshotDao.searchByVms(vmIds);
306310
if (CollectionUtils.isNotEmpty(activeSnapshots)) {
@@ -323,17 +327,32 @@ protected Pair<List<Long>, List<Long>> getFilteredVmIdsForSnapshots(List<Long> v
323327
if (CollectionUtils.isNotEmpty(currentSkippedVmIds)) {
324328
vmIds.removeAll(currentSkippedVmIds);
325329
}
330+
331+
List<BackupVO> backups = backupDao.searchByVmIds(vmIds);
332+
if (CollectionUtils.isNotEmpty(activeSnapshots)) {
333+
HashSet<Long> vmIdsWithBackups = backups.stream().map(BackupVO::getVmId)
334+
.collect(Collectors.toCollection(HashSet::new));
335+
if (logger.isDebugEnabled()) {
336+
logger.debug(String.format("Skipping purging VMs with IDs %s as they have backups",
337+
StringUtils.join(vmIdsWithActiveVolumeSnapshots)));
338+
}
339+
currentSkippedVmIds.addAll(vmIdsWithBackups);
340+
}
341+
if (CollectionUtils.isNotEmpty(currentSkippedVmIds)) {
342+
vmIds.removeAll(currentSkippedVmIds);
343+
}
344+
326345
return new Pair<>(vmIds, new ArrayList<>(currentSkippedVmIds));
327346
}
328347

329-
protected Pair<List<Long>, List<Long>> getVmIdsWithNoActiveSnapshots(final Date startDate, final Date endDate,
330-
final Long batchSize, final List<Long> skippedVmIds) {
348+
protected Pair<List<Long>, List<Long>> getVmIdsWithNoActiveSnapshotsAndBackups(final Date startDate, final Date endDate,
349+
final Long batchSize, final List<Long> skippedVmIds) {
331350
List<VMInstanceVO> vms = vmInstanceDao.searchRemovedByRemoveDate(startDate, endDate, batchSize, skippedVmIds);
332351
if (CollectionUtils.isEmpty(vms)) {
333352
return new Pair<>(new ArrayList<>(), new ArrayList<>());
334353
}
335354
List<Long> vmIds = vms.stream().map(VMInstanceVO::getId).collect(Collectors.toList());
336-
return getFilteredVmIdsForSnapshots(vmIds);
355+
return getFilteredVmIdsForSnapshotsAndBackups(vmIds);
337356
}
338357

339358
protected long purgeVMEntities(final Long batchSize, final Date startDate, final Date endDate) {
@@ -344,7 +363,7 @@ protected long purgeVMEntities(final Long batchSize, final Date startDate, final
344363
List<Long> skippedVmIds = new ArrayList<>();
345364
do {
346365
Pair<List<Long>, List<Long>> allVmIds =
347-
getVmIdsWithNoActiveSnapshots(startDate, endDate, batchSize, skippedVmIds);
366+
getVmIdsWithNoActiveSnapshotsAndBackups(startDate, endDate, batchSize, skippedVmIds);
348367
List<Long> vmIds = allVmIds.first();
349368
List<Long> currentSkippedVmIds = allVmIds.second();
350369
count = vmIds.size() + currentSkippedVmIds.size();
@@ -364,7 +383,7 @@ protected boolean purgeVMEntity(final long vmId) {
364383
final Long batchSize = ExpungedResourcesPurgeBatchSize.value().longValue();
365384
List<Long> vmIds = new ArrayList<>();
366385
vmIds.add(vmId);
367-
Pair<List<Long>, List<Long>> allVmIds = getFilteredVmIdsForSnapshots(vmIds);
386+
Pair<List<Long>, List<Long>> allVmIds = getFilteredVmIdsForSnapshotsAndBackups(vmIds);
368387
if (CollectionUtils.isEmpty(allVmIds.first())) {
369388
return false;
370389
}

‎server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,9 @@ public void testBackupSyncTask() {
665665
VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
666666
when(vm.getId()).thenReturn(vmId);
667667
when(vm.getAccountId()).thenReturn(accountId);
668-
when(vmInstanceDao.listByZoneWithBackups(dataCenterId, null)).thenReturn(List.of(vm));
668+
List<Long> vmIds = List.of(vmId);
669+
when(backupDao.listVmIdsWithBackupsInZone(dataCenterId)).thenReturn(vmIds);
670+
when(vmInstanceDao.listByIds(vmIds)).thenReturn(List.of(vm));
669671
Backup.Metric metric = new Backup.Metric(metricSize, null);
670672
Map<VirtualMachine, Backup.Metric> metricMap = new HashMap<>();
671673
metricMap.put(vm, metric);

0 commit comments

Comments
 (0)