Uh oh!
There was an error while loading. Please reload this page.
Fix deletion of backup schedules - #11222
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #11222 +/- ##
=========================================
Coverage 16.57% 16.58% - Complexity 14054 14067 +13
=========================================
Files 5772 5772 Lines 512926 512951 +25 Branches 62306 62304 -2 =========================================
+ Hits 85014 85051 +37 + Misses 418426 418418 -8 + Partials 9486 9482 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.
bernardodemarco
commented
Jul 16, 2025
@blueorangutan package |
blueorangutan
commented
Jul 16, 2025
@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
commented
Jul 16, 2025
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14216 |
abh1sar
left a comment
There was a problem hiding this comment.
Very useful change. thanks @bernardodemarco.
Just one comment. cmk is not listing backup schedules automatically. I believe it is because listBackupSchedule doesn't have an id parameter.
(localhost) 🐱 > list backupschedule virtualmachineid=c5dd93a7-2060-48db-983e-d6d318e6bafd {
"backupschedule": [
{
"id": "65790631-2654-4743-bc16-2652420f73d1",
"intervaltype": "HOURLY",
"maxbackups": 5,
"schedule": "3",
"timezone": "ART",
"virtualmachineid": "c5dd93a7-2060-48db-983e-d6d318e6bafd",
"virtualmachinename": "VM-c5dd93a7-2060-48db-983e-d6d318e6bafd"
},
{
"id": "5f5d1155-a935-4db3-b28d-7b637634e244",
"intervaltype": "WEEKLY",
"maxbackups": 4,
"schedule": "00:12:1",
"timezone": "ACT",
"virtualmachineid": "c5dd93a7-2060-48db-983e-d6d318e6bafd",
"virtualmachinename": "VM-c5dd93a7-2060-48db-983e-d6d318e6bafd"
}
],
"count": 2
}
(localhost) 🐱 > delete backupschedule virtualmachineid=c5dd93a7-2060-48db-983e-d6d318e6bafd id=
winterhazel
left a comment
There was a problem hiding this comment.
Looks good, tested by:
- Deleting specific backup schedules of a VM by passing the
idparameter via both the UI and CMK - Deleting all backup schedules of a VM by passing the
virtualmachineidparameter via CMK - Checking that users can only delete backup schedules associated to VMs that they have access to
Some remarks:
- I noticed that when a user deletes the last backup schedule of a VM via the UI, the schedule listing does not update because
listBackupSchedulereturns a 530 instead of an empty response - @abh1sar's comment would also be a nice addition
Both remarks can be addressed in a separate PR, as they are related to listBackupSchedule, and not deleteBackupSchedule.
bernardodemarco
commented
Jul 17, 2025
@abh1sar thanks for your review and @winterhazel thanks for testing!
Yes, I also noticed those behaviors while working on this PR, definitely something important for us to address. I'm on vacation for the next 10 days and will be taking some time off during this period. So, I suggest we handle these two minor issues in a separate PR. |
winterhazel
commented
Jul 22, 2025
@sureshanaparti can we run the CI here? |
sureshanaparti
commented
Jul 22, 2025
@blueorangutan package |
blueorangutan
commented
Jul 22, 2025
@sureshanaparti 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
commented
Jul 22, 2025
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14287 |
sureshanaparti
commented
Jul 22, 2025
@blueorangutan test |
blueorangutan
commented
Jul 22, 2025
@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Uh oh!
There was an error while loading. Please reload this page.
blueorangutan
commented
Jul 22, 2025
[SF] Trillian test result (tid-13836)
|
Uh oh!
There was an error while loading. Please reload this page.
Description
PR #9666 introduced changes to fix issues with the deletion of backup schedules. Prior to that PR, the
deleteBackupScheduleAPI accepted only one parameter (virtualmachineid) and would delete a random backup schedule associated with the specified VM. The update modified the behavior so that, instead of randomly deleting one schedule, the API deletes all backup schedules for the given VM (when thevirtualmachineidparameter is specified).Additionally, the PR added support for deleting a specific schedule by introducing the
idparameter to thedeleteBackupScheduleAPI. However, theBackupScheduleVOclass currently does not have auuidattribute, and theBackupScheduleResponsedoes not have theidfield. As a result, the API currently does not correctly recognize theidparameter in the API calls, preventing the deletion of a specific schedule.This PR addresses that issue by enabling the deletion of individual backup schedules via their ID. It also updates the UI to send the
idparameter when the user selects a specific schedule to delete, rather than using thevirtualmachineidparameter.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
Before applying the PR changes, I verified that is was not possible to delete specific schedules:
Backup Schedules
After applying the changes:
listBackupScheduleAPI return