Fix deletion of backup schedules (#11222)

This commit is contained in:
Bernardo De Marco Gonçalves 2025-07-23 04:31:17 -03:00 committed by GitHub
parent e7172161dd
commit 9d049707d5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 174 additions and 27 deletions

View File

@ -43,7 +43,7 @@ import com.cloud.utils.exception.CloudRuntimeException;
description = "Deletes the backup schedule of a VM",
responseObject = SuccessResponse.class, since = "4.14.0",
authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
public class DeleteBackupScheduleCmd extends BaseCmd {
public class DeleteBackupScheduleCmd extends BaseCmd {
@Inject
private BackupManager backupManager;
@ -52,17 +52,13 @@ public class DeleteBackupScheduleCmd extends BaseCmd {
//////////////// API parameters /////////////////////
/////////////////////////////////////////////////////
@Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID,
type = CommandType.UUID,
entityType = UserVmResponse.class,
description = "ID of the VM")
@Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID, type = CommandType.UUID, entityType = UserVmResponse.class,
description = "ID of the VM from which all backup schedules will be deleted.")
private Long vmId;
@Parameter(name = ApiConstants.ID,
type = CommandType.UUID,
entityType = BackupScheduleResponse.class,
description = "ID of the schedule",
since = "4.20.1")
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = BackupScheduleResponse.class,
since = "4.20.1", description = "ID of the backup schedule to be deleted. It has precedence over the 'virtualmachineid' parameter, " +
"i.e., when the 'id' parameter is specified, the 'virtualmachineid' parameter will be ignored.")
private Long id;
/////////////////////////////////////////////////////

View File

@ -28,6 +28,9 @@ import com.google.gson.annotations.SerializedName;
@EntityReference(value = BackupSchedule.class)
public class BackupScheduleResponse extends BaseResponse {
@SerializedName(ApiConstants.ID)
@Param(description = "ID of the backup schedule.")
private String id;
@SerializedName(ApiConstants.VIRTUAL_MACHINE_NAME)
@Param(description = "name of the VM")
@ -51,7 +54,11 @@ public class BackupScheduleResponse extends BaseResponse {
@SerializedName(ApiConstants.MAX_BACKUPS)
@Param(description = "maximum number of backups retained")
private Integer maxBakups;
private Integer maxBackups;
public void setId(String id) {
this.id = id;
}
public String getVmName() {
return vmName;
@ -93,7 +100,7 @@ public class BackupScheduleResponse extends BaseResponse {
this.timezone = timezone;
}
public void setMaxBakups(Integer maxBakups) {
this.maxBakups = maxBakups;
public void setMaxBackups(Integer maxBackups) {
this.maxBackups = maxBackups;
}
}

View File

@ -31,4 +31,5 @@ public interface BackupSchedule extends InternalIdentity {
Date getScheduledTimestamp();
Long getAsyncJobId();
Integer getMaxBackups();
String getUuid();
}

View File

@ -18,6 +18,7 @@
package org.apache.cloudstack.backup;
import java.util.Date;
import java.util.UUID;
import javax.persistence.Column;
import javax.persistence.Entity;
@ -39,6 +40,9 @@ public class BackupScheduleVO implements BackupSchedule {
@Column(name = "id")
private long id;
@Column(name = "uuid", nullable = false)
private String uuid = UUID.randomUUID().toString();
@Column(name = "vm_id")
private Long vmId;
@ -84,6 +88,11 @@ public class BackupScheduleVO implements BackupSchedule {
return id;
}
@Override
public String getUuid() {
return uuid;
}
public Long getVmId() {
return vmId;
}

View File

@ -92,12 +92,13 @@ public class BackupScheduleDaoImpl extends GenericDaoBase<BackupScheduleVO, Long
public BackupScheduleResponse newBackupScheduleResponse(BackupSchedule schedule) {
VMInstanceVO vm = vmInstanceDao.findByIdIncludingRemoved(schedule.getVmId());
BackupScheduleResponse response = new BackupScheduleResponse();
response.setId(schedule.getUuid());
response.setVmId(vm.getUuid());
response.setVmName(vm.getHostName());
response.setIntervalType(schedule.getScheduleType());
response.setSchedule(schedule.getSchedule());
response.setTimezone(schedule.getTimezone());
response.setMaxBakups(schedule.getMaxBackups());
response.setMaxBackups(schedule.getMaxBackups());
response.setObjectName("backupschedule");
return response;
}

View File

@ -233,3 +233,6 @@ CREATE TABLE IF NOT EXISTS `cloud`.`gui_themes_details` (
PRIMARY KEY (`id`),
CONSTRAINT `fk_gui_themes_details__gui_theme_id` FOREIGN KEY (`gui_theme_id`) REFERENCES `gui_themes`(`id`)
);
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backup_schedule', 'uuid', 'VARCHAR(40) NOT NULL');
UPDATE `cloud`.`backup_schedule` SET uuid = UUID();

View File

@ -71,6 +71,7 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import com.amazonaws.util.CollectionUtils;
@ -515,24 +516,44 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager {
public boolean deleteBackupSchedule(DeleteBackupScheduleCmd cmd) {
Long vmId = cmd.getVmId();
Long id = cmd.getId();
if (Objects.isNull(vmId) && Objects.isNull(id)) {
throw new InvalidParameterValueException("Either instance ID or ID of backup schedule needs to be specified");
if (ObjectUtils.allNull(vmId, id)) {
throw new InvalidParameterValueException("Either instance ID or ID of backup schedule needs to be specified.");
}
if (Objects.nonNull(vmId)) {
final VMInstanceVO vm = findVmById(vmId);
validateForZone(vm.getDataCenterId());
accountManager.checkAccess(CallContext.current().getCallingAccount(), null, true, vm);
return deleteAllVMBackupSchedules(vm.getId());
} else {
final BackupSchedule schedule = backupScheduleDao.findById(id);
if (Objects.nonNull(id)) {
BackupSchedule schedule = backupScheduleDao.findById(id);
if (schedule == null) {
throw new CloudRuntimeException("Could not find the requested backup schedule.");
throw new InvalidParameterValueException("Could not find the requested backup schedule.");
}
checkCallerAccessToBackupScheduleVm(schedule.getVmId());
return backupScheduleDao.remove(schedule.getId());
}
checkCallerAccessToBackupScheduleVm(vmId);
return deleteAllVmBackupSchedules(vmId);
}
private boolean deleteAllVMBackupSchedules(long vmId) {
/**
* Checks if the backup framework is enabled for the zone in which the VM with specified ID is allocated and
* if the caller has access to the VM.
*
* @param vmId The ID of the virtual machine to check access for
* @throws PermissionDeniedException if the caller doesn't have access to the VM
* @throws CloudRuntimeException if the backup framework is disabled
*/
protected void checkCallerAccessToBackupScheduleVm(long vmId) {
VMInstanceVO vm = findVmById(vmId);
validateForZone(vm.getDataCenterId());
accountManager.checkAccess(CallContext.current().getCallingAccount(), null, true, vm);
}
/**
* Deletes all backup schedules associated with a specific VM.
*
* @param vmId The ID of the virtual machine whose backup schedules should be deleted
* @return true if all backup schedules were successfully deleted, false if any deletion failed
*/
protected boolean deleteAllVmBackupSchedules(long vmId) {
List<BackupScheduleVO> vmBackupSchedules = backupScheduleDao.listByVM(vmId);
boolean success = true;
for (BackupScheduleVO vmBackupSchedule : vmBackupSchedules) {

View File

@ -47,6 +47,7 @@ import com.cloud.vm.dao.VMInstanceDao;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.command.admin.backup.UpdateBackupOfferingCmd;
import org.apache.cloudstack.api.command.user.backup.CreateBackupScheduleCmd;
import org.apache.cloudstack.api.command.user.backup.DeleteBackupScheduleCmd;
import org.apache.cloudstack.backup.dao.BackupDao;
import org.apache.cloudstack.backup.dao.BackupOfferingDao;
import org.apache.cloudstack.backup.dao.BackupScheduleDao;
@ -75,6 +76,8 @@ import java.util.Map;
import java.util.TimeZone;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
@ -127,7 +130,21 @@ public class BackupManagerTest {
@Mock
AlertManager alertManager;
private AccountVO account;
@Mock
private VMInstanceVO vmInstanceVOMock;
@Mock
private CallContext callContextMock;
@Mock
private AccountVO accountVOMock;
@Mock
private DeleteBackupScheduleCmd deleteBackupScheduleCmdMock;
@Mock
private BackupScheduleVO backupScheduleVOMock;
private UserVO user;
private String[] hostPossibleValues = {"127.0.0.1", "hostname"};
@ -657,4 +674,96 @@ public class BackupManagerTest {
}
}
}
@Test
public void checkCallerAccessToBackupScheduleVmTestExecuteAccessCheckMethods() {
long vmId = 1L;
long dataCenterId = 2L;
try (MockedStatic<CallContext> mockedCallContext = Mockito.mockStatic(CallContext.class)) {
Mockito.when(vmInstanceDao.findById(vmId)).thenReturn(vmInstanceVOMock);
Mockito.when(vmInstanceVOMock.getDataCenterId()).thenReturn(dataCenterId);
Mockito.when(backupManager.isDisabled(dataCenterId)).thenReturn(false);
mockedCallContext.when(CallContext::current).thenReturn(callContextMock);
Mockito.when(callContextMock.getCallingAccount()).thenReturn(accountVOMock);
Mockito.doNothing().when(accountManager).checkAccess(accountVOMock, null, true, vmInstanceVOMock);
backupManager.checkCallerAccessToBackupScheduleVm(vmId);
verify(accountManager, times(1)).checkAccess(accountVOMock, null, true, vmInstanceVOMock);
}
}
@Test
public void deleteAllVmBackupSchedulesTestReturnSuccessWhenAllSchedulesAreDeleted() {
long vmId = 1L;
List<BackupScheduleVO> backupSchedules = List.of(Mockito.mock(BackupScheduleVO.class), Mockito.mock(BackupScheduleVO.class));
Mockito.when(backupScheduleDao.listByVM(vmId)).thenReturn(backupSchedules);
Mockito.when(backupSchedules.get(0).getId()).thenReturn(2L);
Mockito.when(backupSchedules.get(1).getId()).thenReturn(3L);
Mockito.when(backupScheduleDao.remove(Mockito.anyLong())).thenReturn(true);
boolean success = backupManager.deleteAllVmBackupSchedules(vmId);
assertTrue(success);
Mockito.verify(backupScheduleDao, times(2)).remove(Mockito.anyLong());
}
@Test
public void deleteAllVmBackupSchedulesTestReturnFalseWhenAnyDeletionFails() {
long vmId = 1L;
List<BackupScheduleVO> backupSchedules = List.of(Mockito.mock(BackupScheduleVO.class), Mockito.mock(BackupScheduleVO.class));
Mockito.when(backupScheduleDao.listByVM(vmId)).thenReturn(backupSchedules);
Mockito.when(backupSchedules.get(0).getId()).thenReturn(2L);
Mockito.when(backupSchedules.get(1).getId()).thenReturn(3L);
Mockito.when(backupScheduleDao.remove(2L)).thenReturn(true);
Mockito.when(backupScheduleDao.remove(3L)).thenReturn(false);
boolean success = backupManager.deleteAllVmBackupSchedules(vmId);
assertFalse(success);
Mockito.verify(backupScheduleDao, times(2)).remove(Mockito.anyLong());
}
@Test(expected = InvalidParameterValueException.class)
public void deleteBackupScheduleTestThrowExceptionWhenVmIdAndScheduleIdAreNull() {
when(deleteBackupScheduleCmdMock.getVmId()).thenReturn(null);
when(deleteBackupScheduleCmdMock.getId()).thenReturn(null);
backupManager.deleteBackupSchedule(deleteBackupScheduleCmdMock);
}
@Test
public void deleteBackupScheduleTestDeleteVmSchedulesWhenVmIdIsSpecified() {
long vmId = 1L;
when(deleteBackupScheduleCmdMock.getId()).thenReturn(null);
when(deleteBackupScheduleCmdMock.getVmId()).thenReturn(vmId);
Mockito.doNothing().when(backupManager).checkCallerAccessToBackupScheduleVm(vmId);
Mockito.doReturn(true).when(backupManager).deleteAllVmBackupSchedules(vmId);
boolean success = backupManager.deleteBackupSchedule(deleteBackupScheduleCmdMock);
assertTrue(success);
}
@Test(expected = InvalidParameterValueException.class)
public void deleteBackupScheduleTestThrowExceptionWhenSpecificScheduleIsNotFound() {
long id = 1L;
when(deleteBackupScheduleCmdMock.getId()).thenReturn(id);
backupManager.deleteBackupSchedule(deleteBackupScheduleCmdMock);
}
@Test
public void deleteBackupScheduleTestDeleteSpecificScheduleWhenItsIdIsSpecified() {
long id = 1L;
long vmId = 2L;
when(deleteBackupScheduleCmdMock.getId()).thenReturn(id);
when(deleteBackupScheduleCmdMock.getVmId()).thenReturn(null);
when(backupScheduleDao.findById(id)).thenReturn(backupScheduleVOMock);
when(backupScheduleVOMock.getVmId()).thenReturn(vmId);
Mockito.doNothing().when(backupManager).checkCallerAccessToBackupScheduleVm(vmId);
when(backupScheduleVOMock.getId()).thenReturn(id);
when(backupScheduleDao.remove(id)).thenReturn(true);
boolean success = backupManager.deleteBackupSchedule(deleteBackupScheduleCmdMock);
assertTrue(success);
}
}

View File

@ -167,7 +167,7 @@ export default {
methods: {
handleClickDelete (record) {
const params = {}
params.virtualmachineid = record.virtualmachineid
params.id = record.id
this.actionLoading = true
postAPI('deleteBackupSchedule', params).then(json => {
if (json.deletebackupscheduleresponse.success) {