Skip to content

Commit bdeb62d

Browse files
abh1sarabh1sar
committed
Remove the concept of orphaned backup as vm with backups cannot be purged.
1 parent b2a499e commit bdeb62d

10 files changed

Lines changed: 21 additions & 110 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,6 @@ public class ApiConstants {
368368
public static final String OP = "op";
369369
public static final String OPTION = "option";
370370
public static final String OPTIONS = "options";
371-
public static final String IS_ORPHAN = "isorphan";
372371
public static final String OS_CATEGORY_ID = "oscategoryid";
373372
public static final String OS_CATEGORY_NAME = "oscategoryname";
374373
public static final String OS_NAME = "osname";

‎api/src/main/java/org/apache/cloudstack/api/response/BackupResponse.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,6 @@ public class BackupResponse extends BaseResponse {
119119
@Param(description = "the interval type of the backup schedule", since = "4.21.0")
120120
private String intervalType;
121121

122-
@SerializedName(ApiConstants.IS_ORPHAN)
123-
@Param(description = "The backups is orphaned and not associated with an existing VM. A new VM can still be created using the backup", since = "4.21.0")
124-
private Boolean isOrphan;
125-
126122
public String getId() {
127123
return id;
128124
}
@@ -298,12 +294,4 @@ public String getIntervalType() {
298294
public void setIntervalType(String intervalType) {
299295
this.intervalType = intervalType;
300296
}
301-
302-
public Boolean getIsOrphan() {
303-
return isOrphan;
304-
}
305-
306-
public void setIsOrphan(Boolean isOrphan) {
307-
this.isOrphan = isOrphan;
308-
}
309297
}

‎api/src/main/java/org/apache/cloudstack/backup/Backup.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,7 @@ public String toString() {
158158
}
159159

160160
Long getVmId();
161-
String getVmName();
162161
long getBackupOfferingId();
163-
void setVmName(String vmName);
164162
String getExternalId();
165163
String getType();
166164
Date getDate();

‎api/src/main/java/org/apache/cloudstack/backup/BackupManager.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,6 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
264264

265265
String getBackupNameFromVM(VirtualMachine vm);
266266

267-
void updateOrphanedBackups(VirtualMachine vm);
268-
269267
Capacity getBackupStorageUsedStats(Long zoneId);
270268

271269
void checkAndRemoveBackupOfferingBeforeExpunge(VirtualMachine vm);

‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -656,8 +656,6 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti
656656
throw new CloudRuntimeException("Unable to expunge " + vm, e);
657657
}
658658

659-
backupManager.updateOrphanedBackups(vm);
660-
661659
logger.debug("Expunging vm " + vm);
662660

663661
final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);

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

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,6 @@ public class BackupVO implements Backup {
6262
@Column(name = "vm_id")
6363
private Long vmId;
6464

65-
@Column(name = "vm_name")
66-
private String vmName;
67-
6865
@Column(name = "external_id")
6966
private String externalId;
7067

@@ -116,7 +113,7 @@ public BackupVO() {
116113
@Override
117114
public String toString() {
118115
return String.format("Backup %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(
119-
this, "id", "uuid", "vmId", "vmName", "backupType", "externalId"));
116+
this, "id", "uuid", "vmId", "backupType", "externalId"));
120117
}
121118

122119
@Override
@@ -138,16 +135,6 @@ public void setVmId(Long vmId) {
138135
this.vmId = vmId;
139136
}
140137

141-
@Override
142-
public String getVmName() {
143-
return vmName;
144-
}
145-
146-
@Override
147-
public void setVmName(String vmName) {
148-
this.vmName = vmName;
149-
}
150-
151138
@Override
152139
public String getExternalId() {
153140
return externalId;

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,6 @@ public BackupResponse newBackupResponse(Backup backup, Boolean listVmDetails) {
300300
if (vm != null) {
301301
response.setVmId(vm.getUuid());
302302
response.setVmName(vm.getHostName());
303-
} else {
304-
response.setVmName(backup.getVmName());
305-
response.setIsOrphan(true);
306303
}
307304
response.setExternalId(backup.getExternalId());
308305
response.setType(backup.getType());

‎engine/schema/src/main/resources/META-INF/db/schema-42010to42100.sql‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@ ALTER TABLE `cloud`.`backup_schedule` ADD COLUMN `max_backups` int(8) default NU
4141
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'name', 'VARCHAR(255) NOT NULL COMMENT "name of the backup"');
4242
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'description', 'VARCHAR(1024) COMMENT "description for the backup"');
4343
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'backup_interval_type', 'int(5) COMMENT "type of backup, e.g. manual, recurring - hourly, daily, weekly or monthly"');
44-
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'vm_name', 'VARCHAR(255) COMMENT "name of the vm for which backup is taken, set only for orphaned backups"');
45-
UPDATE `cloud`.`backups` JOIN `cloud`.`vm_instance` ON `backups`.`vm_id` = `vm_instance`.`id` SET `backups`.`name` = `vm_instance`.`name`;
4644

4745
-- Make the column vm_id in backups table nullable to handle orphan backups
4846
ALTER TABLE `cloud`.`backups` MODIFY COLUMN `vm_id` BIGINT UNSIGNED NULL;

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

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,12 +1235,11 @@ public boolean deleteBackup(final Long backupId, final Boolean forced) {
12351235
}
12361236

12371237
final Long vmId = backup.getVmId();
1238-
if (vmId == null) {
1239-
logger.debug("Deleting orphaned backup with ID {}", backupId);
1240-
} else {
1241-
logger.debug("Deleting backup {} belonging to instance {}", backupId, vmId);
1242-
}
12431238
final VMInstanceVO vm = vmInstanceDao.findByIdIncludingRemoved(vmId);
1239+
if (vm == null) {
1240+
logger.warn("Instance {} not found for backup {} during delete backup", vmId, backup.toString());
1241+
}
1242+
logger.debug("Deleting backup {} belonging to instance {}", backup.toString(), vmId);
12441243

12451244
validateBackupForZone(backup.getZoneId());
12461245
accountManager.checkAccess(CallContext.current().getCallingAccount(), null, true, vm == null ? backup : vm);
@@ -1673,15 +1672,15 @@ private Backup checkAndUpdateIfBackupEntryExistsForRestorePoint(Backup.RestorePo
16731672
for (final Backup backupInDb : backupsInDb) {
16741673
logger.debug(String.format("Checking if Backup %s with external ID %s for VM %s is valid", backupsInDb, backupInDb.getName(), vm));
16751674
if (restorePoint.getId().equals(backupInDb.getExternalId())) {
1676-
logger.debug(String.format("Found Backup %s in both Database and Networker", backupInDb));
1675+
logger.debug(String.format("Found Backup %s in both Database and Provider", backupInDb));
16771676
if (metric != null) {
16781677
logger.debug(String.format("Update backup [%s] from [size: %s, protected size: %s] to [size: %s, protected size: %s].",
16791678
backupInDb, backupInDb.getSize(), backupInDb.getProtectedSize(), metric.getBackupSize(), metric.getDataSize()));
16801679

1681-
resourceLimitMgr.decrementResourceCount(vm.getAccountId(), Resource.ResourceType.backup_storage, backupInDb.getSize());
1680+
resourceLimitMgr.decrementResourceCount(backupInDb.getAccountId(), Resource.ResourceType.backup_storage, backupInDb.getSize());
16821681
((BackupVO) backupInDb).setSize(metric.getBackupSize());
16831682
((BackupVO) backupInDb).setProtectedSize(metric.getDataSize());
1684-
resourceLimitMgr.incrementResourceCount(vm.getAccountId(), Resource.ResourceType.backup_storage, backupInDb.getSize());
1683+
resourceLimitMgr.incrementResourceCount(backupInDb.getAccountId(), Resource.ResourceType.backup_storage, backupInDb.getSize());
16851684

16861685
backupDao.update(backupInDb.getId(), ((BackupVO) backupInDb));
16871686
}
@@ -1691,6 +1690,16 @@ private Backup checkAndUpdateIfBackupEntryExistsForRestorePoint(Backup.RestorePo
16911690
return null;
16921691
}
16931692

1693+
private void processRemoveList(List<Long> removeList) {
1694+
for (final Long backupIdToRemove : removeList) {
1695+
logger.warn(String.format("Removing backup with ID: [%s].", backupIdToRemove));
1696+
Backup backup = backupDao.findById(backupIdToRemove);
1697+
resourceLimitMgr.decrementResourceCount(backup.getAccountId(), Resource.ResourceType.backup);
1698+
resourceLimitMgr.decrementResourceCount(backup.getAccountId(), Resource.ResourceType.backup_storage, backup.getSize());
1699+
backupDao.remove(backupIdToRemove);
1700+
}
1701+
}
1702+
16941703
private void syncBackups(BackupProvider backupProvider, VirtualMachine vm, Backup.Metric metric) {
16951704
Transaction.execute(new TransactionCallbackNoReturn() {
16961705
@Override
@@ -1726,13 +1735,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
17261735
vm.getId(), ApiCommandResourceType.VirtualMachine.toString(),0);
17271736
}
17281737
}
1729-
for (final Long backupIdToRemove : removeList) {
1730-
logger.warn(String.format("Removing backup with ID: [%s].", backupIdToRemove));
1731-
Backup backup = backupDao.findById(backupIdToRemove);
1732-
resourceLimitMgr.decrementResourceCount(vm.getAccountId(), Resource.ResourceType.backup);
1733-
resourceLimitMgr.decrementResourceCount(vm.getAccountId(), Resource.ResourceType.backup_storage, backup.getSize());
1734-
backupDao.remove(backupIdToRemove);
1735-
}
1738+
processRemoveList(removeList);
17361739
}
17371740
});
17381741
}
@@ -1805,20 +1808,6 @@ public BackupOffering updateBackupOffering(UpdateBackupOfferingCmd updateBackupO
18051808
return response;
18061809
}
18071810

1808-
@Override
1809-
public void updateOrphanedBackups(VirtualMachine vm) {
1810-
List<Backup> backups = backupDao.listByVmId(vm.getDataCenterId(), vm.getId());
1811-
if (backups.size() == 0) {
1812-
return;
1813-
}
1814-
for (Backup backup : backups) {
1815-
BackupVO backupVO = backupDao.findById(backup.getId());
1816-
backupVO.setVmId(null);
1817-
backupVO.setVmName(vm.getHostName());
1818-
backupDao.update(backup.getId(), backupVO);
1819-
}
1820-
}
1821-
18221811
@Override
18231812
public CapacityVO getBackupStorageUsedStats(Long zoneId) {
18241813
final BackupProvider backupProvider = getBackupProvider(zoneId);

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

Lines changed: 2 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -680,10 +680,12 @@ public void testBackupSyncTask() {
680680

681681
BackupVO backupInDb1 = new BackupVO();
682682
backupInDb1.setSize(backup1Size);
683+
backupInDb1.setAccountId(accountId);
683684
backupInDb1.setExternalId(restorePoint1ExternalId);
684685

685686
BackupVO backupInDb2 = new BackupVO();
686687
backupInDb2.setSize(backup2Size);
688+
backupInDb2.setAccountId(accountId);
687689
backupInDb2.setExternalId(null);
688690
ReflectionTestUtils.setField(backupInDb2, "id", backup2Id);
689691
when(backupDao.findById(backup2Id)).thenReturn(backupInDb2);
@@ -990,11 +992,6 @@ public void testRemoveVMFromBackupOffering() {
990992
}
991993
}
992994

993-
private
994-
void setupRestoreBackupToVMMocks() {
995-
996-
}
997-
998995
@Test
999996
public void testRestoreBackupToVM() throws NoTransitionException {
1000997
Long backupId = 1L;
@@ -1003,8 +1000,6 @@ public void testRestoreBackupToVM() throws NoTransitionException {
10031000
Long offeringId = 4L;
10041001
Long poolId = 5L;
10051002

1006-
setupRestoreBackupToVMMocks();
1007-
10081003
BackupVO backup = mock(BackupVO.class);
10091004
when(backup.getBackupOfferingId()).thenReturn(offeringId);
10101005

@@ -1052,42 +1047,6 @@ public void testRestoreBackupToVM() throws NoTransitionException {
10521047
}
10531048
}
10541049

1055-
@Test
1056-
public void testUpdateOrphanedBackups() {
1057-
Long vmId = 1L;
1058-
Long zoneId = 2L;
1059-
Long backupId1 = 3L;
1060-
Long backupId2 = 4L;
1061-
1062-
VirtualMachine vm = mock(VirtualMachine.class);
1063-
when(vm.getDataCenterId()).thenReturn(1L);
1064-
when(vm.getId()).thenReturn(vmId);
1065-
when(vm.getHostName()).thenReturn("test-vm");
1066-
when(vm.getDataCenterId()).thenReturn(zoneId);
1067-
1068-
Backup backup1 = mock(Backup.class);
1069-
Backup backup2 = mock(Backup.class);
1070-
when(backup1.getId()).thenReturn(backupId1);
1071-
when(backup2.getId()).thenReturn(backupId2);
1072-
List<Backup> backups = List.of(backup1, backup2);
1073-
when(backupDao.listByVmId(zoneId, vmId)).thenReturn(backups);
1074-
1075-
BackupVO backupVO1 = new BackupVO();
1076-
BackupVO backupVO2 = new BackupVO();
1077-
when(backupDao.findById(backupId1)).thenReturn(backupVO1);
1078-
when(backupDao.findById(backupId2)).thenReturn(backupVO2);
1079-
1080-
backupManager.updateOrphanedBackups(vm);
1081-
1082-
Assert.assertEquals(null, backupVO1.getVmId());
1083-
Assert.assertEquals("test-vm", backupVO1.getVmName());
1084-
Assert.assertEquals(null, backupVO2.getVmId());
1085-
Assert.assertEquals("test-vm", backupVO2.getVmName());
1086-
for (Backup backup : backups) {
1087-
verify(backupDao).update(Mockito.eq(backup.getId()), any(BackupVO.class));
1088-
}
1089-
}
1090-
10911050
@Test
10921051
public void testGetBackupStorageUsedStats() {
10931052
Long zoneId = 1L;

0 commit comments

Comments
 (0)