Skip to content

Block backup deletion while create-VM-from-backup or restore jobs are in progress - #12792

Merged
sureshanaparti merged 5 commits into
apache:4.22from
Damans227:fix/11356-block-backup-delete-during-restore
Apr 9, 2026
Merged

Block backup deletion while create-VM-from-backup or restore jobs are in progress#12792
sureshanaparti merged 5 commits into
apache:4.22from
Damans227:fix/11356-block-backup-delete-during-restore

Conversation

@Damans227

@Damans227Damans227 commented Mar 11, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes: #11356

Adds a check in deleteBackup() to block deletion when a createVMFromBackup or restoreBackup async job is in progress for that backup.

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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

Screen.Recording.2026-03-12.at.10.00.41.AM.mp4
Screenshot 2026-03-12 at 10 01 29 AM

How Has This Been Tested?

  • Reproduced the problem: started createVMFromBackup, then attempted deleteBackup on same backup, confirmed it now throws CloudRuntimeException
  • Added unit test testDeleteBackupBlockedByPendingJobs

How did you try to break this feature and the system with this change?

  • Verified deleteBackup still succeeds when no pending jobs exist
  • Verified existing unit tests work

@Damans227

Copy link
Copy Markdown
CollaboratorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

codecovBot commented Mar 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 17.61%. Comparing base (7aa0558) to head (44f8036).
⚠️ Report is 26 commits behind head on 4.22.

Additional details and impacted files
@@ Coverage Diff @@## 4.22 #12792 +/- ##
============================================
- Coverage 17.61% 17.61% -0.01% 
Complexity 15664 15664 ============================================
Files 5917 5917 Lines 531402 531426 +24 Branches 64971 64974 +3 ============================================
+ Hits 93596 93599 +3 - Misses 427252 427271 +19 - Partials 10554 10556 +2 
FlagCoverage Δ
uitests3.70% <ø> (-0.01%)⬇️
unittests18.68% <100.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.

@weizhouapacheweizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

@sureshanapartisureshanaparti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents VM backup deletion when there are in-progress async jobs that depend on the same backup (notably create-VM-from-backup and restore), avoiding failures caused by deleting a backup mid-operation.

Changes:

  • Added a pending-async-job check to BackupManagerImpl.deleteBackup() to block deletion when related jobs are in progress.
  • Added a unit test covering the “delete blocked by pending jobs” scenario.
  • Updated existing test stubbing to include backup.getUuid() where required by the new logic.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

FileDescription
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.javaAdds checkForPendingBackupJobs and calls it from deleteBackup to prevent deletion while dependent jobs are in progress.
server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.javaIntroduces a new unit test ensuring deletion is blocked when countPendingJobs(...) reports pending work.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment threadserver/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java Outdated
Comment threadserver/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java Outdated
Comment threadserver/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java Outdated
Comment threadserver/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java Outdated
Damans227and others added 3 commits March 13, 2026 06:58
Co-authored-by: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com>
@rajujith

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

Copy link
Copy Markdown

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

@rajujithrajujith self-assigned this Mar 27, 2026

@rajujithrajujith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Damans227 Its working based on the changes you made. However I happen to notice this scenario where the backup can be deleted before the async job for the createVMfromBackup is submitted, the the API for the create VM was received first by the backup can be deleted before the async job starts. This may be a corner case.

2026-03-27 11:40:07,695 DEBUG [c.c.a.ApiServlet] (qtp1513608173-24:[ctx-d15e4b38]) (logid:cb5f1a1c) ===START=== 172.30.205.2 -- POST
command=createVMFromBackup
response=json
zoneid=3fd429e2-761f-4c99-8380-2ce101f7afd9
backupid=23ad6f34-9fdd-4084-a923-ddb52e2bc42d
2026-03-27 11:40:12,620 DEBUG [c.c.a.ApiServlet] (qtp1513608173-20:[ctx-62dbf022]) (logid:beb548a8) ===START=== 172.30.205.2 -- POST
command=deleteBackup
response=json
id=23ad6f34-9fdd-4084-a923-ddb52e2bc42d
2026-03-27 11:40:14,383 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (qtp1513608173-20:[ctx-62dbf022, ctx-a53727df]) (logid:beb548a8) submit async job-45, details: AsyncJob {"accountId":2,"cmd":"org.apache.cloudstack.api.command.user.backup.DeleteBackupCmd","cmdInfo":"{\"response\":\"json\",\"ctxUserId\":\"2\",\"sessionkey\":\"bXz3XP467EaOMhhIMWBWsnhuMw0\",\"httpmethod\":\"POST\",\"ctxStartEventId\":\"165\",\"id\":\"23ad6f34-9fdd-4084-a923-ddb52e2bc42d\"
2026-03-27 11:40:19,531 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (qtp1513608173-24:[ctx-d15e4b38, ctx-5d597821]) (logid:cb5f1a1c) submit async job-46, details: AsyncJob {"accountId":2,"cmd":"org.apache.cloudstack.api.command.admin.vm.CreateVMFromBackupCmdByAdmin","cmdInfo":"{\"response\":\"json\",\"ctxUserId\":\"2\",\"backupid\":\"23ad6f34-9fdd-4084-a923-ddb52e2bc42d\",\"sessionkey\":\"bXz3XP467EaOMhhIMWBWsnhuMw0\",\"zoneid\":\"3fd429e2-761f-4c99-8380-2ce101f7afd9\",\"httpmethod\":\"POST\",\"ctxStartEventId\":\"168\",\"id\":\"6\",\"ctxDetails\":\"{\\\"interface com.cloud.vm.VirtualMachine\\\":\\\"564b0bfe-754f-4cf7-84ea-77fc46576678\\\
2026-03-27 11:40:20,196 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (API-Job-Executor-5:[ctx-b89628a6, job-45, ctx-3f103b6b]) (logid:783d37a9) Complete async job-45, jobStatus: SUCCEEDED, resultCode: 0, result: org.apache.cloudstack.api.response.SuccessResponse/null/{"success":"true"}

@abh1sar

Copy link
Copy Markdown
Contributor

@Damans227 Its working based on the changes you made. However I happen to notice this scenario where the backup can be deleted before the async job for the createVMfromBackup is submitted, the the API for the create VM was received first by the backup can be deleted before the async job starts. This may be a corner case.

I think this will be small window and can be ignored. User ideally shouldn't give createVmfromBackup and backupDelete in succession.

@rajujithrajujith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sureshanaparti
sureshanaparti merged commit 7ba5240 into apache:4.22Apr 9, 2026
25 of 26 checks passed
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in Apache CloudStack 4.22.1Apr 9, 2026
dhslove added a commit to dhslove/ablestack-cloud that referenced this pull request Apr 21, 2026
Source Apache commits:\n- 7ba5240 Block backup deletion while create-VM-from-backup or restore jobs are in progress (apache#12792)\n\nChange summary:\n- count pending async jobs for backup-driven create and restore flows before deleteBackup\n- reject backup deletion while related jobs are still in progress\n- add unit coverage for the pending-job rejection path\n\nFunctional impact:\n- prevents deleteBackup from racing with active create-from-backup or restore operations\n- reduces the risk of mid-flight backup removal during restore workflows\n\nConflict handling:\n- BackupManagerImpl applied cleanly on local/main\n- BackupManagerTest required a small mock-field merge to keep existing mocks and add AsyncJobManager\n\nVerification:\n- resolved test file inspected and pending-job coverage preserved\n- mvn/mvnw not available in this workspace, so tests could not be executed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Disallow or block Backup deletion if create Instance from Backup is in progress

8 participants

@Damans227@blueorangutan@rajujith@abh1sar@sureshanaparti@weizhouapache@DaanHoogland