Clean up backup references to their schedules when the schedules are deleted (#12401)

* clean up backup schedule references after their deletion

* drop unused column

* address reviews

Author: bernardodemarco

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

@@ -21,20 +21,14 @@
 import java.util.List;
 
 import com.cloud.utils.DateUtil;
-import org.apache.cloudstack.api.response.BackupScheduleResponse;
-import org.apache.cloudstack.backup.BackupSchedule;
 import org.apache.cloudstack.backup.BackupScheduleVO;
 
 import com.cloud.utils.db.GenericDao;
 
 public interface BackupScheduleDao extends GenericDao<BackupScheduleVO, Long> {
-    BackupScheduleVO findByVM(Long vmId);
-
     List<BackupScheduleVO> listByVM(Long vmId);
 
     BackupScheduleVO findByVMAndIntervalType(Long vmId, DateUtil.IntervalType intervalType);
 
     List<BackupScheduleVO> getSchedulesToExecute(Date currentTimestamp);
-
-    BackupScheduleResponse newBackupScheduleResponse(BackupSchedule schedule);
 }

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

@@ -17,28 +17,23 @@
 
 package org.apache.cloudstack.backup.dao;
 
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
 import java.util.Date;
 import java.util.List;
 
 import javax.annotation.PostConstruct;
-import javax.inject.Inject;
 
 import com.cloud.utils.DateUtil;
-import org.apache.cloudstack.api.response.BackupScheduleResponse;
-import org.apache.cloudstack.backup.BackupSchedule;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.TransactionLegacy;
 import org.apache.cloudstack.backup.BackupScheduleVO;
 
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
-import com.cloud.vm.VMInstanceVO;
-import com.cloud.vm.dao.VMInstanceDao;
 
 public class BackupScheduleDaoImpl extends GenericDaoBase<BackupScheduleVO, Long> implements BackupScheduleDao {
-
-    @Inject
-    VMInstanceDao vmInstanceDao;
-
     private SearchBuilder<BackupScheduleVO> backupScheduleSearch;
     private SearchBuilder<BackupScheduleVO> executableSchedulesSearch;
 
@@ -59,13 +54,6 @@ protected void init() {
         executableSchedulesSearch.done();
     }
 
-    @Override
-    public BackupScheduleVO findByVM(Long vmId) {
-        SearchCriteria<BackupScheduleVO> sc = backupScheduleSearch.create();
-        sc.setParameters("vm_id", vmId);
-        return findOneBy(sc);
-    }
-
     @Override
     public List<BackupScheduleVO> listByVM(Long vmId) {
         SearchCriteria<BackupScheduleVO> sc = backupScheduleSearch.create();
@@ -88,21 +76,19 @@ public List<BackupScheduleVO> getSchedulesToExecute(Date currentTimestamp) {
         return listBy(sc);
     }
 
+    @DB
     @Override
-    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.setMaxBackups(schedule.getMaxBackups());
-        if (schedule.getQuiesceVM() != null) {
-            response.setQuiesceVM(schedule.getQuiesceVM());
+    public boolean remove(Long id) {
+        String sql = "UPDATE backups SET backup_schedule_id = NULL WHERE backup_schedule_id = ?";
+        TransactionLegacy transaction = TransactionLegacy.currentTxn();
+        try {
+            PreparedStatement preparedStatement = transaction.prepareAutoCloseStatement(sql);
+            preparedStatement.setLong(1, id);
+            preparedStatement.executeUpdate();
+            return super.remove(id);
+        } catch (SQLException e) {
+            logger.warn("Unable to clean up backup schedules references from the backups table.", e);
+            return false;
         }
-        response.setObjectName("backupschedule");
-        return response;
     }
 }

engine/schema/src/main/resources/META-INF/db/schema-42200to42210.sql

@@ -38,3 +38,6 @@ UPDATE `cloud`.`vm_template` SET guest_os_id = 99 WHERE name = 'kvm-default-vm-i
 
 -- Update existing vm_template records with NULL type to "USER"
 UPDATE `cloud`.`vm_template` SET `type` = 'USER' WHERE `type` IS NULL;
+
+-- Drops the unused "backup_interval_type" column of the "cloud.backups" table
+ALTER TABLE `cloud`.`backups` DROP COLUMN `backup_interval_type`;

server/src/main/java/com/cloud/api/ApiDBUtils.java

@@ -45,7 +45,6 @@
 import org.apache.cloudstack.api.response.AccountResponse;
 import org.apache.cloudstack.api.response.AsyncJobResponse;
 import org.apache.cloudstack.api.response.BackupOfferingResponse;
-import org.apache.cloudstack.api.response.BackupScheduleResponse;
 import org.apache.cloudstack.api.response.DiskOfferingResponse;
 import org.apache.cloudstack.api.response.DomainResponse;
 import org.apache.cloudstack.api.response.DomainRouterResponse;
@@ -76,7 +75,6 @@
 import org.apache.cloudstack.api.response.ZoneResponse;
 import org.apache.cloudstack.backup.BackupOffering;
 import org.apache.cloudstack.backup.BackupRepository;
-import org.apache.cloudstack.backup.BackupSchedule;
 import org.apache.cloudstack.backup.dao.BackupDao;
 import org.apache.cloudstack.backup.dao.BackupOfferingDao;
 import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
@@ -2299,10 +2297,6 @@ public static ResourceIconVO getResourceIconByResourceUUID(String resourceUUID,
         return s_resourceIconDao.findByResourceUuid(resourceUUID, resourceType);
     }
 
-    public static BackupScheduleResponse newBackupScheduleResponse(BackupSchedule schedule) {
-        return s_backupScheduleDao.newBackupScheduleResponse(schedule);
-    }
-
     public static BackupOfferingResponse newBackupOfferingResponse(BackupOffering offering) {
         BackupRepository repository = s_backupRepositoryDao.findByUuid(offering.getExternalId());
         Boolean crossZoneInstanceCreationEnabled = repository != null ? Boolean.TRUE.equals(repository.crossZoneInstanceCreationEnabled()) : false;

server/src/main/java/com/cloud/api/ApiResponseHelper.java

@@ -5042,7 +5042,25 @@ public UserDataResponse createUserDataResponse(UserData userData) {
 
     @Override
     public BackupScheduleResponse createBackupScheduleResponse(BackupSchedule schedule) {
-        return ApiDBUtils.newBackupScheduleResponse(schedule);
+        BackupScheduleResponse response = new BackupScheduleResponse();
+        response.setId(schedule.getUuid());
+        response.setIntervalType(schedule.getScheduleType());
+        response.setSchedule(schedule.getSchedule());
+        response.setTimezone(schedule.getTimezone());
+        response.setMaxBackups(schedule.getMaxBackups());
+
+        if (schedule.getQuiesceVM() != null) {
+            response.setQuiesceVM(schedule.getQuiesceVM());
+        }
+
+        VMInstanceVO vm = ApiDBUtils.findVMInstanceById(schedule.getVmId());
+        if (vm != null) {
+            response.setVmId(vm.getUuid());
+            response.setVmName(vm.getHostName());
+        }
+
+        response.setObjectName("backupschedule");
+        return response;
     }
 
     @Override