Refactoring retention of backup schedules

[bernardodemarco]

Description

PR #10017 introduced, among other features, the ability to configure retention for backup schedules. This PR aims to fix some minor inconsistencies in that feature, refactor redundant workflows and behaviors, and improve code readability and maintainability by adding logs and more granular unit tests.

Below, I have listed all the points that this PR addresses.

---

Exposed scheduleid parameter

When scheduling a backup creation, the ID of the schedule responsible for the backup is passed as a parameter to the createBackup API:

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

Lines 1307 to 1322 in f52e058

	final Long eventId = ActionEventUtils.onScheduledActionEvent(User.UID_SYSTEM, vm.getAccountId(),
	EventTypes.EVENT_VM_BACKUP_CREATE, "creating backup for VM ID:" + vm.getUuid(),
	vmId, ApiCommandResourceType.VirtualMachine.toString(),
	true, 0);
	final Map<String, String> params = new HashMap<String, String>();
	params.put(ApiConstants.VIRTUAL_MACHINE_ID, "" + vmId);
	params.put(ApiConstants.SCHEDULE_ID, "" + backupScheduleId);
	params.put("ctxUserId", "1");
	params.put("ctxAccountId", "" + vm.getAccountId());
	params.put("ctxStartEventId", String.valueOf(eventId));
	final CreateBackupCmd cmd = new CreateBackupCmd();
	ComponentContext.inject(cmd);
	apiDispatcher.dispatchCreateCmd(cmd, params);
	params.put("id", "" + vmId);
	params.put("ctxStartEventId", "1");

This parameter is intended solely for use by the scheduling workflow. End users should neither access nor be aware of it. If a backup is manually created while specifying a schedule ID, inconsistencies may occur later in the scheduling process. To prevent this, changes were made to ensure the parameter is not exposed to end users.

---

Relationship between cloud.backup and cloud.backup_schedule

To determine whether backups should be deleted to meet the retention requirements, the backup creation workflow must be able to identify whether the backup is scheduled and which schedule it belongs to. If the backup is scheduled, retention validation should be performed. On the other hand, if it is a manual backup, retention validation should be skipped.

Currently, the cloud.backup table uses the backup_interval_type column for this purpose:

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

Lines 91 to 92 in f52e058

	@Column(name = "backup_interval_type")
	private short backupIntervalType;

However, the cloud.backup_schedule table already contains a schedule_type column, leading to data redundancy.

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

Lines 45 to 46 in f52e058

	@Column(name = "schedule_type")
	private Short scheduleType;

Because of this, the backup creation logic requires back-and-forth conversions between DateUtil.IntervalType and org.apache.cloudstack.backup.Backup.Type. For example:

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

Lines 819 to 840 in f52e058

	private Backup.Type getBackupType(Long scheduleId) {
	if (scheduleId.equals(Snapshot.MANUAL_POLICY_ID)) {
	return Backup.Type.MANUAL;
	} else {
	BackupScheduleVO scheduleVO = backupScheduleDao.findById(scheduleId);
	DateUtil.IntervalType intvType = scheduleVO.getScheduleType();
	return getBackupType(intvType);
	}
	}
	private Backup.Type getBackupType(DateUtil.IntervalType intvType) {
	if (intvType.equals(DateUtil.IntervalType.HOURLY)) {
	return Backup.Type.HOURLY;
	} else if (intvType.equals(DateUtil.IntervalType.DAILY)) {
	return Backup.Type.DAILY;
	} else if (intvType.equals(DateUtil.IntervalType.WEEKLY)) {
	return Backup.Type.WEEKLY;
	} else if (intvType.equals(DateUtil.IntervalType.MONTHLY)) {
	return Backup.Type.MONTHLY;
	}
	return null;
	}

Moreover, since the relationship is based solely on the interval type, deleting backups to comply with a schedule's retention policy may unintentionally remove backups from previously deleted schedules of the same type. For instance:

1. Create an HOURLY schedule
2. 10 backups are created from this schedule
3. The schedule is deleted
4. A new HOURLY schedule is created with a retention of 3
5. On the next backup execution, 8 backups are deleted

This happens because the system currently treats all backups with the same interval type as belonging to the same schedule.

Therefore, to address this, the relationship between cloud.backup and cloud.backup_schedule was refactored by removing the schedule_type from the cloud.backup table, and adding the backup_schedule_id column.

This change makes it possible to associate each backup with a specific schedule, eliminating ambiguity and data redundancy. Manual backups will have a NULL value for backup_schedule_id, while scheduled backups will reference their respective schedule, from which the interval type can be determined.

---

Removal of maximum allowed retention configurations

The backup.max.hourly, backup.max.daily, backup.max.weekly, and backup.max.monthly settings are currently used to define the maximum retention values that end users can configure. These are validated during backup schedule creation, and an exception is thrown if the specified retention exceeds the configured maximum.

However, since administrators can already define backup and allocated backup storage limits per account and domain, these settings have limited practical use. This PR proposes removing them, allowing users to omit retention entirely. In this approach, backup limits are enforced solely through account and domain-level control limits.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• build/CI
• test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Screenshots (if appropriate):

How Has This Been Tested?

• Verified that manual backups are not associated with any schedule
• Verified that for manual backups, the backup deletion flow is not executed after backups are created
• Verified that when the retention is greater than 0 and the number of backups in a schedule is less than the retention, no backup is deleted
• Verified that when the retention is greater than 0 and the number of backups in a schedule is greater than the retention, backups are correctly deleted
• Verified that when the retention is increased, the updated retention value is respected
• Verified that when the retention is reduced, the necessary number of backups is deleted to comply with the new retention value
• Verified that when the backup schedule associated with a backup has a retention value of 0, the backup deletion flow is not executed
• Deleted a schedule, created a new one with the same interval type, and verified that backups from the old schedule are not considered in the retention calculation

[codecov]

Codecov Report

Attention: Patch coverage is 70.32967% with 27 lines in your changes missing coverage. Please review.

Project coverage is 16.57%. Comparing base (f52e058) to head (6dbc1c0).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
...rg/apache/cloudstack/backup/dao/BackupDaoImpl.java	0.00%	9 Missing ⚠️
...rg/apache/cloudstack/backup/BackupManagerImpl.java	88.73%	3 Missing and 5 partials ⚠️
...in/java/org/apache/cloudstack/backup/BackupVO.java	0.00%	6 Missing ⚠️
...org/apache/cloudstack/backup/BackupScheduleVO.java	0.00%	3 Missing ⚠️
...stack/api/command/user/backup/CreateBackupCmd.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #11223      +/-   ##
============================================
- Coverage     16.58%   16.57%   -0.01%     
- Complexity    14038    14075      +37     
============================================
  Files          5758     5772      +14     
  Lines        511733   512910    +1177     
  Branches      62216    62305      +89     
============================================
+ Hits          84871    85033     +162     
- Misses       417389   418409    +1020     
+ Partials       9473     9468       -5     

Flag	Coverage Δ
uitests	3.89% <ø> (-0.02%)	⬇️
unittests	17.47% <70.32%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bernardodemarco]

@blueorangutan package

[blueorangutan]

@bernardodemarco a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14215

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.