Refactoring retention of backup schedules

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

@@ -1260,6 +1260,10 @@ public class ApiConstants {
             "however, the following formats are also accepted: \"yyyy-MM-dd HH:mm:ss\" (e.g.: \"2023-01-01 12:00:00\") and \"yyyy-MM-dd\" (e.g.: \"2023-01-01\" - if the time is not " +
             "added, it will be interpreted as \"23:59:59\"). If the recommended format is not used, the date will be considered in the server timezone.";
 
+    public static final String PARAMETER_DESCRIPTION_MAX_BACKUPS = "The maximum number of backups to keep for a VM. " +
+            "If \"0\", no retention policy will be applied and, thus, no backups from the schedule will be automatically deleted. " +
+            "This parameter is only supported for the Dummy, NAS and EMC Networker backup provider.";
+
     public static final String VMWARE_DC = "vmwaredc";
 
     public static final String CSS = "css";

api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupCmd.java

@@ -19,7 +19,6 @@
 
 import javax.inject.Inject;
 
-import com.cloud.storage.Snapshot;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiCommandResourceType;
@@ -28,7 +27,6 @@
 import org.apache.cloudstack.api.BaseAsyncCreateCmd;
 import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.ServerApiException;
-import org.apache.cloudstack.api.response.BackupScheduleResponse;
 import org.apache.cloudstack.api.response.SuccessResponse;
 import org.apache.cloudstack.api.response.UserVmResponse;
 import org.apache.cloudstack.backup.BackupManager;
@@ -62,13 +60,6 @@ public class CreateBackupCmd extends BaseAsyncCreateCmd {
             description = "ID of the VM")
     private Long vmId;
 
-    @Parameter(name = ApiConstants.SCHEDULE_ID,
-            type = CommandType.LONG,
-            entityType = BackupScheduleResponse.class,
-            description = "backup schedule ID of the VM, if this is null, it indicates that it is a manual backup.",
-            since = "4.21.0")
-    private Long scheduleId;
-
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -77,22 +68,14 @@ public Long getVmId() {
         return vmId;
     }
 
-    public Long getScheduleId() {
-        if (scheduleId != null) {
-            return scheduleId;
-        } else {
-            return Snapshot.MANUAL_POLICY_ID;
-        }
-    }
-
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
 
     @Override
     public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
         try {
-            boolean result = backupManager.createBackup(getVmId(), getScheduleId());
+            boolean result = backupManager.createBackup(getVmId(), getJob());
             if (result) {
                 SuccessResponse response = new SuccessResponse(getCommandName());
                 response.setResponseName(getCommandName());

api/src/main/java/org/apache/cloudstack/api/command/user/backup/CreateBackupScheduleCmd.java

@@ -75,10 +75,8 @@ public class CreateBackupScheduleCmd extends BaseCmd {
             description = "Specifies a timezone for this command. For more information on the timezone parameter, see TimeZone Format.")
     private String timezone;
 
-    @Parameter(name = ApiConstants.MAX_BACKUPS,
-            type = CommandType.INTEGER,
-            description = "maximum number of backups to retain",
-            since = "4.21.0")
+    @Parameter(name = ApiConstants.MAX_BACKUPS, type = CommandType.INTEGER,
+            since = "4.21.0", description = ApiConstants.PARAMETER_DESCRIPTION_MAX_BACKUPS)
     private Integer maxBackups;
 
     /////////////////////////////////////////////////////

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

@@ -33,28 +33,6 @@ enum Status {
         Allocated, Queued, BackingUp, BackedUp, Error, Failed, Restoring, Removed, Expunged
     }
 
-    public enum Type {
-        MANUAL, HOURLY, DAILY, WEEKLY, MONTHLY;
-        private int max = 8;
-
-        public void setMax(int max) {
-            this.max = max;
-        }
-
-        public int getMax() {
-            return max;
-        }
-
-        @Override
-        public String toString() {
-            return this.name();
-        }
-
-        public boolean equals(String snapshotType) {
-            return this.toString().equalsIgnoreCase(snapshotType);
-        }
-    }
-
     class Metric {
         private Long backupSize = 0L;
         private Long dataSize = 0L;
@@ -166,4 +144,5 @@ public String toString() {
     Long getProtectedSize();
     List<VolumeInfo> getBackedUpVolumes();
     long getZoneId();
+    Long getBackupScheduleId();
 }

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

@@ -58,38 +58,6 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
             "false",
             "Enable volume attach/detach operations for VMs that are assigned to Backup Offerings.", true);
 
-    ConfigKey<Integer> BackupHourlyMax = new ConfigKey<Integer>("Advanced", Integer.class,
-            "backup.max.hourly",
-            "8",
-            "Maximum recurring hourly backups to be retained for an instance. If the limit is reached, early backups from the start of the hour are deleted so that newer ones can be saved. This limit does not apply to manual backups. If set to 0, recurring hourly backups can not be scheduled.",
-            false,
-            ConfigKey.Scope.Global,
-            null);
-
-    ConfigKey<Integer> BackupDailyMax = new ConfigKey<Integer>("Advanced", Integer.class,
-            "backup.max.daily",
-            "8",
-            "Maximum recurring daily backups to be retained for an instance. If the limit is reached, backups from the start of the day are deleted so that newer ones can be saved. This limit does not apply to manual backups. If set to 0, recurring daily backups can not be scheduled.",
-            false,
-            ConfigKey.Scope.Global,
-            null);
-
-    ConfigKey<Integer> BackupWeeklyMax = new ConfigKey<Integer>("Advanced", Integer.class,
-            "backup.max.weekly",
-            "8",
-            "Maximum recurring weekly backups to be retained for an instance. If the limit is reached, backups from the beginning of the week are deleted so that newer ones can be saved. This limit does not apply to manual backups. If set to 0, recurring weekly backups can not be scheduled.",
-            false,
-            ConfigKey.Scope.Global,
-            null);
-
-    ConfigKey<Integer> BackupMonthlyMax = new ConfigKey<Integer>("Advanced", Integer.class,
-            "backup.max.monthly",
-            "8",
-            "Maximum recurring monthly backups to be retained for an instance. If the limit is reached, backups from the beginning of the month are deleted so that newer ones can be saved. This limit does not apply to manual backups. If set to 0, recurring monthly backups can not be scheduled.",
-            false,
-             ConfigKey.Scope.Global,
-            null);
-
     ConfigKey<Long> DefaultMaxAccountBackups = new ConfigKey<Long>("Account Defaults", Long.class,
             "max.account.backups",
             "20",
@@ -201,10 +169,10 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
     /**
      * Creates backup of a VM
      * @param vmId Virtual Machine ID
-     * @param scheduleId Virtual Machine Backup Schedule ID
+     * @param job The async job associated with the backup retention
      * @return returns operation success
      */
-    boolean createBackup(final Long vmId, final Long scheduleId) throws ResourceAllocationException;
+    boolean createBackup(final Long vmId, Object job) throws ResourceAllocationException;
 
     /**
      * List existing backups for a VM

api/src/main/java/org/apache/cloudstack/backup/BackupSchedule.java

@@ -30,6 +30,6 @@ public interface BackupSchedule extends InternalIdentity {
     String getTimezone();
     Date getScheduledTimestamp();
     Long getAsyncJobId();
-    Integer getMaxBackups();
+    int getMaxBackups();
     String getUuid();
 }

engine/schema/src/main/java/org/apache/cloudstack/backup/BackupScheduleVO.java

@@ -63,12 +63,12 @@ public class BackupScheduleVO implements BackupSchedule {
     Long asyncJobId;
 
     @Column(name = "max_backups")
-    Integer maxBackups = 0;
+    private int maxBackups = 0;
 
     public BackupScheduleVO() {
     }
 
-    public BackupScheduleVO(Long vmId, DateUtil.IntervalType scheduleType, String schedule, String timezone, Date scheduledTimestamp, Integer maxBackups) {
+    public BackupScheduleVO(Long vmId, DateUtil.IntervalType scheduleType, String schedule, String timezone, Date scheduledTimestamp, int maxBackups) {
         this.vmId = vmId;
         this.scheduleType = (short) scheduleType.ordinal();
         this.schedule = schedule;
@@ -142,11 +142,11 @@ public void setAsyncJobId(Long asyncJobId) {
         this.asyncJobId = asyncJobId;
     }
 
-    public Integer getMaxBackups() {
+    public int getMaxBackups() {
         return maxBackups;
     }
 
-    public void setMaxBackups(Integer maxBackups) {
+    public void setMaxBackups(int maxBackups) {
         this.maxBackups = maxBackups;
     }
 }

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

@@ -88,12 +88,12 @@ public class BackupVO implements Backup {
     @Column(name = "zone_id")
     private long zoneId;
 
-    @Column(name = "backup_interval_type")
-    private short backupIntervalType;
-
     @Column(name = "backed_volumes", length = 65535)
     protected String backedUpVolumes;
 
+    @Column(name = "backup_schedule_id")
+    private Long backupScheduleId;
+
     public BackupVO() {
         this.uuid = UUID.randomUUID().toString();
     }
@@ -211,14 +211,6 @@ public void setZoneId(long zoneId) {
         this.zoneId = zoneId;
     }
 
-    public short getBackupIntervalType() {
-        return backupIntervalType;
-    }
-
-    public void setBackupIntervalType(short backupIntervalType) {
-        this.backupIntervalType = backupIntervalType;
-    }
-
     @Override
     public Class<?> getEntityType() {
         return Backup.class;
@@ -247,4 +239,13 @@ public Date getRemoved() {
     public void setRemoved(Date removed) {
         this.removed = removed;
     }
+
+    @Override
+    public Long getBackupScheduleId() {
+        return backupScheduleId;
+    }
+
+    public void setBackupScheduleId(Long backupScheduleId) {
+        this.backupScheduleId = backupScheduleId;
+    }
 }

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

@@ -36,9 +36,8 @@ public interface BackupDao extends GenericDao<BackupVO, Long> {
     BackupVO getBackupVO(Backup backup);
     List<Backup> listByOfferingId(Long backupOfferingId);
 
-    List<BackupVO> listBackupsByVMandIntervalType(Long vmId, Backup.Type backupType);
-
     BackupResponse newBackupResponse(Backup backup);
     public Long countBackupsForAccount(long accountId);
     public Long calculateBackupStorageForAccount(long accountId);
+    List<BackupVO> listBySchedule(Long backupScheduleId);
 }

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

@@ -24,6 +24,7 @@
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 
+import com.cloud.utils.db.Filter;
 import com.cloud.utils.db.GenericSearchBuilder;
 import org.apache.cloudstack.api.response.BackupResponse;
 import org.apache.cloudstack.backup.Backup;
@@ -63,7 +64,7 @@ public class BackupDaoImpl extends GenericDaoBase<BackupVO, Long> implements Bac
     private SearchBuilder<BackupVO> backupSearch;
     private GenericSearchBuilder<BackupVO, Long> CountBackupsByAccount;
     private GenericSearchBuilder<BackupVO, SumCount> CalculateBackupStorageByAccount;
-    private SearchBuilder<BackupVO> ListBackupsByVMandIntervalType;
+    private SearchBuilder<BackupVO> listBackupsBySchedule;
 
     public BackupDaoImpl() {
     }
@@ -91,12 +92,11 @@ protected void init() {
         CalculateBackupStorageByAccount.and("removed", CalculateBackupStorageByAccount.entity().getRemoved(), SearchCriteria.Op.NULL);
         CalculateBackupStorageByAccount.done();
 
-        ListBackupsByVMandIntervalType = createSearchBuilder();
-        ListBackupsByVMandIntervalType.and("vmId", ListBackupsByVMandIntervalType.entity().getVmId(), SearchCriteria.Op.EQ);
-        ListBackupsByVMandIntervalType.and("intervalType", ListBackupsByVMandIntervalType.entity().getBackupIntervalType(), SearchCriteria.Op.EQ);
-        ListBackupsByVMandIntervalType.and("status", ListBackupsByVMandIntervalType.entity().getStatus(), SearchCriteria.Op.EQ);
-        ListBackupsByVMandIntervalType.and("removed", ListBackupsByVMandIntervalType.entity().getRemoved(), SearchCriteria.Op.NULL);
-        ListBackupsByVMandIntervalType.done();
+        listBackupsBySchedule = createSearchBuilder();
+        listBackupsBySchedule.and("backup_schedule_id", listBackupsBySchedule.entity().getBackupScheduleId(), SearchCriteria.Op.EQ);
+        listBackupsBySchedule.and("status", listBackupsBySchedule.entity().getStatus(), SearchCriteria.Op.EQ);
+        listBackupsBySchedule.and("removed", listBackupsBySchedule.entity().getRemoved(), SearchCriteria.Op.NULL);
+        listBackupsBySchedule.done();
     }
 
     @Override
@@ -184,12 +184,11 @@ public Long calculateBackupStorageForAccount(long accountId) {
     }
 
     @Override
-    public List<BackupVO> listBackupsByVMandIntervalType(Long vmId, Backup.Type backupType) {
-        SearchCriteria<BackupVO> sc = ListBackupsByVMandIntervalType.create();
-        sc.setParameters("vmId", vmId);
-        sc.setParameters("type", backupType.ordinal());
+    public List<BackupVO> listBySchedule(Long backupScheduleId) {
+        SearchCriteria<BackupVO> sc = listBackupsBySchedule.create();
+        sc.setParameters("backup_schedule_id", backupScheduleId);
         sc.setParameters("status", Backup.Status.BackedUp);
-        return listBy(sc, null);
+        return listBy(sc, new Filter(BackupVO.class, "date", true));
     }
 
     @Override

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

@@ -19,9 +19,8 @@
 -- Schema upgrade from 4.20.1.0 to 4.21.0.0
 --;
 
--- Add columns max_backup and backup_interval_type to backup table
-ALTER TABLE `cloud`.`backup_schedule` ADD COLUMN `max_backups` int(8) default NULL COMMENT 'maximum number of backups to maintain';
-ALTER TABLE `cloud`.`backups` ADD COLUMN `backup_interval_type` int(5) COMMENT 'type of backup, e.g. manual, recurring - hourly, daily, weekly or monthly';
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backup_schedule', 'max_backups', 'INT(8) UNSIGNED NOT NULL DEFAULT 0 COMMENT ''Maximum number of backups to be retained''');
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'backup_schedule_id', 'BIGINT(20) UNSIGNED');
 
 -- Add console_endpoint_creator_address column to cloud.console_session table
 CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.console_session', 'console_endpoint_creator_address', 'VARCHAR(45)');

server/src/main/java/com/cloud/api/dispatch/ParamGenericValidationWorker.java

@@ -69,6 +69,7 @@ public class ParamGenericValidationWorker implements DispatchWorker {
         defaultParamNames.add(ApiConstants.ID);
         defaultParamNames.add(ApiConstants.SIGNATURE_VERSION);
         defaultParamNames.add(ApiConstants.EXPIRES);
+        defaultParamNames.add(ApiConstants.SCHEDULE_ID);
         defaultParamNames.add("_");
     }