Repository navigation
Add new backup report feature - #13939
JoaoJandre wants to merge 8 commits into
Conversation
boring-cyborg
Bot
added
component:api
component:backup
component:build
component:database
labels
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13939 +/- ##
============================================
- Coverage 20.66% 20.64% -0.03%
+ Complexity 21720 21719 -1
============================================
Files 6492 6507 +15
Lines 590728 591624 +896
Branches 72448 72484 +36
============================================
+ Hits 122101 122113 +12
- Misses 455355 456235 +880
- Partials 13272 13276 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
GaOrtiga
left a comment
GaOrtiga
left a comment
There was a problem hiding this comment.
@JoaoJandre I ran some tests, overall LGTM, just a few minor nits
If a backup was created within the gathered period, but has been removed after, it appears as a deleted backup and shows 0 total storage. Given that the deletion happened after the period, it should show as a succesful backup. Same for a failed backup, where the failure reason is not showed.
API return
(admin) 🐱 > get backupreport startdate='2026-08-01' enddate='2026-08-23T11:59:59-0300' domainid=517e680a-dc14-4cac-98ab-5b6a21a2ac36
{
"backupreport": {
"backupstoragetotal": 0,
"domainreport": [
{
"accountreport": [
{
"account": "userc",
"accountid": "62f2b1f0-4373-4983-84a0-5e74b1558002",
"backupstoragetotal": 0,
"deletedbackup": [
{
"backupofferingname": "a",
"created": "2026-08-23T11:25:46-0300",
"id": "05c18465-4986-4d85-a119-00a469c35cb7",
"name": "VM-75e8ffef-9ae6-47fb-bca8-b1724fab8f63-2026-08-24T11:25:45.937948704-03:00",
"removed": "2026-08-24T11:30:21-0300",
"virtualmachineid": "75e8ffef-9ae6-47fb-bca8-b1724fab8f63",
"virtualmachinename": "VM-75e8ffef-9ae6-47fb-bca8-b1724fab8f63",
"zone": "zone",
"zoneid": "b25d0ef2-b171-4885-beec-21a27044f07c"
}
],
"failedbackup": [],
"sucessfulbackup": []
}
],
"backupstoragetotal": 0,
"domain": "c",
"domainid": "517e680a-dc14-4cac-98ab-5b6a21a2ac36"
}
],
"enddate": "2026-08-23T11:59:59-0300",
"providerinfo": [
{
"compressionreport": []
},
{
"validationreport": []
}
],
"schedulereport": [],
"startdate": "2026-08-01T00:00:00-0300"
}
}
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
|
||
| public class Upgrade42210to42300 extends DbUpgradeAbstractImpl implements DbUpgrade, DbUpgradeSystemVmTemplate { | ||
|
|
||
| // This must be moved to the new upgrade class when 4.23 is released |
There was a problem hiding this comment.
Please update/resolve conflicts and fix the upgrade path
There was a problem hiding this comment.
the upgrade path to 24 is not created still. #14033. I'll update again when the PR is merged
Should be fixed now, can you test it? |
aaaf43a to
56c14a8
Compare
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@blueorangutan package |
|
@JoaoJandre a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19381 |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
abh1sar
left a comment
abh1sar
left a comment
There was a problem hiding this comment.
Please add some Unit tests for code coverage
| backupResponse.setZone(backupInfo.getZoneName()); | ||
| backupResponse.setZoneId(backupInfo.getZoneUuid()); | ||
|
|
||
| if (removedBackupStates.contains(backupInfo.getStatus()) && backupInfo.getRemoved().before(endDate)) { |
There was a problem hiding this comment.
getRemoved() will NPE here for Hidden backups
| if (removedBackupStates.contains(backupInfo.getStatus()) && backupInfo.getRemoved().before(endDate)) { | ||
| backupResponse.setRemoved(backupInfo.getRemoved()); | ||
| currentAccountResponse.addDeletedBackup(backupResponse); | ||
| } else if (aliveBackupStates.contains(backupInfo.getStatus()) || backupInfo.getFailureReason() == null) { |
There was a problem hiding this comment.
NAS doesn't set the failure reason. This will make all those backups to be counted as successful. better to look at backupStates, something like:
if (removed != null && removed.before(endDate)) {
// deleted
} else if (removedBackupStates.contains(status)) {
return; // tombstone (hidden) with no removal timestamp
} else if (errorBackupStates.contains(status)) {
// failed
} else if (aliveBackupStates.contains(status)) {
// successful
}
| " \n" + | ||
| "\n" + | ||
| " <#list account.successfulBackups>\n" + | ||
| " <p>Successfully created backups for the ${(account.accountName??)?then(\"project\",\"account\")}:</p>\n" + |
There was a problem hiding this comment.
| " <p>Successfully created backups for the ${(account.accountName??)?then(\"project\",\"account\")}:</p>\n" + | |
| " <p>Successfully created backups for the ${(account.accountName??)?then(\"account\",\"project\")}:</p>\n" + |
| " </#list>\n" + | ||
| "\n" + | ||
| " <#list account.failedBackups>\n" + | ||
| " <p>Failed backups for the ${(account.accountName??)?then(\"project\",\"account\")}:</p>\n" + |
There was a problem hiding this comment.
| " <p>Failed backups for the ${(account.accountName??)?then(\"project\",\"account\")}:</p>\n" + | |
| " <p>Failed backups for the ${(account.accountName??)?then(\"account\",\"project\")}:</p>\n" + |
| " </#list>\n" + | ||
| "\n" + | ||
| " <#list account.deletedBackups>\n" + | ||
| " <p>Removed backups for the ${(account.accountName??)?then(\"project\",\"account\")}:</p> \n" + |
There was a problem hiding this comment.
| " <p>Removed backups for the ${(account.accountName??)?then(\"project\",\"account\")}:</p> \n" + | |
| " <p>Removed backups for the ${(account.accountName??)?then(\"account\",\"project\")}:</p> \n" + |
|
|
||
| -- Backup report | ||
|
|
||
| CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.backups', 'failure_reason', 'varchar(255)'); |
There was a problem hiding this comment.
255 might be too small to store the value, can we use TEXT?
| } | ||
|
|
||
| private boolean isTaskDisabled(BackupReportVO latestReport) { | ||
| if (backupReportTaskEnabled.value()) { |
There was a problem hiding this comment.
This is a zone scoped setting, right? what is the config is disabled in global but enabled in zone?
| logger.debug("Starting backup report task."); | ||
|
|
||
| try { | ||
| Transaction.execute(TransactionLegacy.CLOUD_DB, new TransactionCallbackNoReturn() { |
There was a problem hiding this comment.
Do we really need a transaction here? at least the sendMail should be out of it.
| public static final String DOMAIN_REPORT = "domainreport"; | ||
| public static final String ACCOUNT_REPORT = "accountreport"; | ||
| public static final String FAILURE_REASON = "failurereason"; | ||
| public static final String SUCCESSFUL_BACKUP = "sucessfulbackup"; |
There was a problem hiding this comment.
| public static final String SUCCESSFUL_BACKUP = "sucessfulbackup"; | |
| public static final String SUCCESSFUL_BACKUP = "successfulbackup"; |
Description
This PR adds a feature for periodically sending basic backup reports to operators. To enable backup reports, the environment's alert email settings must be configured beforehand. To configure the report task, the following settings have been added:
backup.report.task.enabledfalsebackup.report.periodbackup.report.periodminutes, as well as a forecast of the next execution of backup schedules that may potentially run during the nextbackup.report.periodminutes.1440backup.report.timeout10Additionally, the
getBackupReportAPI has been added to generate reports through the API. This API returns the report in the API response and does not send emails to operators. The API supports the following parameters:zoneiddomainidaccountidprojectid.projectidaccountid.startdateenddateThis API is only available to
Adminaccounts.API Usage Example
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
Example generated email:
How Has This Been Tested?
getBackupReportAPI passing only startdate and enddategetBackupReportAPI specifying the domaingetBackupReportAPI specifying the accountgetBackupReportAPI specifying the project