Clean up backup references to their schedules when the schedules are deleted by bernardodemarco · Pull Request #12401 · apache/cloudstack

bernardodemarco · 2026-01-09T19:53:34Z

Description

Currently, the cloud.backups table references a backup schedule through the backup_schedule_id field. It is only populated when a backup is created from a schedule. For ad hoc backups, the field's value is kept as NULL.

Additionally, when backup schedules are deleted via the deleteBackupSchedule API, their corresponding entries are expunged from the DB. However, the backups' references to the schedules are not cleaned up and, thus, these references remain indefinitely in the cloud.backups table. It is important to highlight that, although the clean up process is not performed, it does not cause any bugs/side effects.

Therefore, this PR proposes to clean up such references before deleting backup schedules from the DB. The PR also cleans up some pieces of the code related with backup schedules, removing unused methods, moving methods' definitions to appropriate layers of the project and dropping the unused backup_interval_type column from the cloud.backups table (see #11223).

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

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Created a backup schedule and waited for some backups to be created from it
Deleted the backup schedule
Verified that the schedule's backups did not have references pointing to the schedule anymore

bernardodemarco · 2026-01-09T20:06:35Z

@blueorangutan package

blueorangutan · 2026-01-09T20:08:05Z

@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.

codecov · 2026-01-09T20:11:15Z

Codecov Report

❌ Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.60%. Comparing base (03de62b) to head (9e3df76).

Files with missing lines	Patch %	Lines
...src/main/java/com/cloud/api/ApiResponseHelper.java	0.00%	14 Missing ⚠️
...e/cloudstack/backup/dao/BackupScheduleDaoImpl.java	0.00%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.22   #12401   +/-   ##
=========================================
  Coverage     17.60%   17.60%           
- Complexity    15674    15676    +2     
=========================================
  Files          5918     5918           
  Lines        531727   531729    +2     
  Branches      65016    65017    +1     
=========================================
+ Hits          93595    93619   +24     
+ Misses       427570   427548   -22     
  Partials      10562    10562

Flag	Coverage Δ
uitests	`3.70% <ø> (ø)`
unittests	`18.67% <0.00%> (+<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.

blueorangutan · 2026-01-09T21:23:42Z

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

Copilot

Pull request overview

This PR addresses the cleanup of orphaned backup schedule references in the cloud.backups table. When backup schedules are deleted, backups created from those schedules retain references to non-existent schedule IDs. The PR implements proper cleanup by setting backup_schedule_id to NULL before deleting schedules, and also removes unused code and database columns.

Changes:

Implements cleanup of backup schedule references before schedule deletion
Removes unused backup_interval_type column from cloud.backups table
Refactors backup schedule response creation from DAO layer to API layer

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
engine/schema/src/main/resources/META-INF/db/schema-42200to42210.sql	Adds SQL to drop unused `backup_interval_type` column
engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDaoImpl.java	Implements cleanup logic in `remove()` method and removes unused response builder
engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupScheduleDao.java	Removes unused interface methods
server/src/main/java/com/cloud/api/ApiDBUtils.java	Removes delegation method for backup schedule response creation
server/src/main/java/com/cloud/api/ApiResponseHelper.java	Moves backup schedule response creation logic from DAO to API layer

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

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

github-actions · 2026-01-26T07:21:03Z

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

sureshanaparti · 2026-02-13T05:38:29Z

@bernardodemarco can you check if there any any conflicts. thanks.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

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

DaanHoogland · 2026-03-05T19:31:44Z

@blueorangutan package

blueorangutan · 2026-03-05T19:34:04Z

@DaanHoogland 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 · 2026-03-05T20:42:07Z

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

blueorangutan · 2026-03-06T09:00:14Z

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

blueorangutan · 2026-03-06T12:22:56Z

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

DaanHoogland · 2026-03-09T09:52:28Z

@blueorangutan test

blueorangutan · 2026-03-09T09:54:04Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2026-03-10T02:27:10Z

[SF] Trillian test result (tid-15578)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 56949 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12401-t15578-kvm-ol8.zip
Smoke tests completed. 146 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestListIdsParams>:teardown	`Error`	1.12	test_list_ids_parameter.py
test_01_snapshot_root_disk	`Error`	5.71	test_snapshots.py
test_02_list_snapshots_with_removed_data_store	`Error`	52.53	test_snapshots.py
test_02_list_snapshots_with_removed_data_store	`Error`	52.53	test_snapshots.py
ContextSuite context=TestSnapshotStandaloneBackup>:teardown	`Error`	34.34	test_snapshots.py
test_01_snapshot_usage	`Error`	28.73	test_usage.py
test_01_vpn_usage	`Error`	1.06	test_usage.py

github-actions · 2026-03-24T19:11:58Z

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

bernardodemarco · 2026-04-08T13:23:45Z

@blueorangutan package

blueorangutan · 2026-04-08T13:26:05Z

@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.

Tonitzpp

@bernardodemarco, lgtm. Here are the tests that I've done.

Tests descriptions

First, I created a virtual machine to perform the tests
Assigned a backup offering with the dummy provider
Set up an hourly backup schedule
Checked in the database the backup schedule that was taken in the cloud.backups table

select * from backups \G

*************************** 1. row ***************************
                id: 1
              uuid: 90aacd9d-cd3c-4f1e-b05d-60039c5f2daf
             vm_id: 3
       external_id: dummy-external-id
              type: FULL
              size: 8589934592
    protected_size: 8589934592
            status: BackedUp
backup_offering_id: 1
        account_id: 2
         domain_id: 1
           zone_id: 1
           removed: NULL
              date: 2026-04-08 17:13:59
    backed_volumes: [{"uuid":"1aa850ba-f375-4b95-b0e5-ab53720b2033","type":"ROOT","size":8589934592,"path":"1aa850ba-f375-4b95-b0e5-ab53720b2033","deviceId":0,"diskOfferingId":"fe487456-99a4-41b1-8fdd-65f717985973"}]
backup_schedule_id: 2
              name: test-2026-04-08T17:13:59+0000
       description: NULL
1 row in set (0.000 sec)

Via CMK, I used the deleteBackupSchedule API to remove the backup schedule that had been created
After removing, I checked the cloud.backups table to check if the backup_schedule_id field have been changed

select * from backups \G

*************************** 1. row ***************************
                id: 1
              uuid: 90aacd9d-cd3c-4f1e-b05d-60039c5f2daf
             vm_id: 3
       external_id: dummy-external-id
              type: FULL
              size: 8589934592
    protected_size: 8589934592
            status: BackedUp
backup_offering_id: 1
        account_id: 2
         domain_id: 1
           zone_id: 1
           removed: NULL
              date: 2026-04-08 17:13:59
    backed_volumes: [{"uuid":"1aa850ba-f375-4b95-b0e5-ab53720b2033","type":"ROOT","size":8589934592,"path":"1aa850ba-f375-4b95-b0e5-ab53720b2033","deviceId":0,"diskOfferingId":"fe487456-99a4-41b1-8fdd-65f717985973"}]
backup_schedule_id: NULL
              name: test-2026-04-08T17:13:59+0000
       description: NULL
1 row in set (0.000 sec)

blueorangutan · 2026-04-08T14:24:05Z

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

bernardodemarco added this to the 4.22.1 milestone Jan 9, 2026

DaanHoogland requested review from abh1sar and Copilot January 12, 2026 07:17

Copilot started reviewing on behalf of DaanHoogland January 12, 2026 07:18 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

github-actions bot added the status:has-conflicts label Jan 26, 2026

sureshanaparti added this to Apache CloudStack 4.22.1 Feb 3, 2026

sureshanaparti moved this to In Progress in Apache CloudStack 4.22.1 Feb 3, 2026

sureshanaparti added status:needs-review status:needs-testing labels Feb 13, 2026

sureshanaparti requested a review from Copilot February 27, 2026 05:34

Copilot started reviewing on behalf of sureshanaparti February 27, 2026 05:35 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

bernardodemarco force-pushed the clean-backup-schedule-reference branch from 020f31a to e1009dd Compare March 5, 2026 19:19

github-actions bot removed the status:has-conflicts label Mar 5, 2026

github-actions bot added the status:has-conflicts label Mar 24, 2026

abh1sar removed their request for review April 7, 2026 05:31

bernardodemarco added 3 commits April 8, 2026 10:21

clean up backup schedule references after their deletion

16970c6

drop unused column

ddb7fec

address reviews

9e3df76

bernardodemarco force-pushed the clean-backup-schedule-reference branch from 4250f65 to 9e3df76 Compare April 8, 2026 13:22

bernardodemarco removed the status:has-conflicts label Apr 8, 2026

Tonitzpp approved these changes Apr 8, 2026

View reviewed changes

Conversation

bernardodemarco commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

bernardodemarco commented Jan 9, 2026

Uh oh!

blueorangutan commented Jan 9, 2026

Uh oh!

codecov bot commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Jan 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 26, 2026

Uh oh!

sureshanaparti commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DaanHoogland commented Mar 5, 2026

Uh oh!

blueorangutan commented Mar 5, 2026

Uh oh!

blueorangutan commented Mar 5, 2026

Uh oh!

blueorangutan commented Mar 6, 2026

Uh oh!

blueorangutan commented Mar 6, 2026

Uh oh!

DaanHoogland commented Mar 9, 2026

Uh oh!

blueorangutan commented Mar 9, 2026

Uh oh!

blueorangutan commented Mar 10, 2026

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

bernardodemarco commented Apr 8, 2026

Uh oh!

blueorangutan commented Apr 8, 2026

Uh oh!

Tonitzpp left a comment

Choose a reason for hiding this comment

Tests descriptions

Uh oh!

blueorangutan commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bernardodemarco commented Jan 9, 2026 •

edited

Loading

codecov bot commented Jan 9, 2026 •

edited

Loading