Cleanup allocated snapshots / vm snapshots, and update pending ones to Error on MS start #8452

sureshanaparti · 2024-01-05T07:39:15Z

Description

Some latest active snapshots / vm snapshots are stuck in allocated / creating state when MS is stopped, these are listed / shown in UI as well (not allowed to delete). Remove allocated ones on MS start itself, and move creating ones to Error state (these will be removed by the storage garbage collector).

This PR removes snapshots / vm snapshots in allocated state on MS start, and moves creating one to Error state.

Fixes #8424

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

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Manually tested: Take VM Snapshot -> Stop MS -> Start MS

VM Snapshot record after MS stopped =>

                 id: 1
               uuid: 07542708-27b7-47c4-959a-f7f92828f43a
               name: i-2-3-VM_VS_20240108075812
       display_name: testvm01-snap
        description: NULL
              vm_id: 3
         account_id: 2
          domain_id: 1
service_offering_id: 1
   vm_snapshot_type: DiskAndMemory
              state: Allocated
             parent: NULL
            current: NULL
       update_count: 0
            updated: NULL
            created: 2024-01-08 07:58:12
            removed: NULL

VM Snapshot record after MS started (Not listed / shown in the UI) =>

                 id: 1
               uuid: 07542708-27b7-47c4-959a-f7f92828f43a
               name: i-2-3-VM_VS_20240108075812
       display_name: testvm01-snap
        description: NULL
              vm_id: 3
         account_id: 2
          domain_id: 1
service_offering_id: 1
   vm_snapshot_type: DiskAndMemory
              state: Allocated
             parent: NULL
            current: NULL
       update_count: 0
            updated: NULL
            created: 2024-01-08 07:58:12
            removed: 2024-01-08 07:59:26

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

sureshanaparti · 2024-01-05T07:40:10Z

@blueorangutan package

blueorangutan · 2024-01-05T07:48:03Z

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

codecov · 2024-01-05T07:48:16Z

Codecov Report

❌ Patch coverage is 1.44928% with 68 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.17%. Comparing base (253ac03) to head (56f5195).
⚠️ Report is 13 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...stack/framework/jobs/impl/AsyncJobManagerImpl.java	2.70%	36 Missing ⚠️
...tack/storage/snapshot/SnapshotDataFactoryImpl.java	0.00%	11 Missing and 1 partial ⚠️
.../storage/vmsnapshot/DefaultVMSnapshotStrategy.java	0.00%	8 Missing ⚠️
.../storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java	0.00%	8 Missing ⚠️
...a/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java	0.00%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20    #8452      +/-   ##
============================================
- Coverage     16.17%   16.17%   -0.01%     
- Complexity    13297    13298       +1     
============================================
  Files          5656     5656              
  Lines        498136   498218      +82     
  Branches      60432    60451      +19     
============================================
+ Hits          80584    80585       +1     
- Misses       408584   408659      +75     
- Partials       8968     8974       +6

Flag	Coverage Δ
uitests	`4.00% <ø> (+<0.01%)`	⬆️
unittests	`17.02% <1.44%> (-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.

weizhouapache · 2024-01-05T08:10:59Z

@sureshanaparti
would it be better to clean the snapshots by storage garbage collector which run periodically ?

blueorangutan · 2024-01-05T08:51:40Z

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

sureshanaparti · 2024-01-08T05:35:56Z

@sureshanaparti would it be better to clean the snapshots by storage garbage collector which run periodically ?

@weizhouapache issue is some active snapshots / vm snapshots are left in allocated when MS is stopped, no point in keeping them and wait until storage garbage collector cleans up (what if storage cleanup is disabled?) as these are listed / shown in UI as well (which are not allowed to delete). So, I think, it's better remove them on start itself. Any other thoughts / suggestions?

weizhouapache · 2024-01-08T08:07:08Z

@sureshanaparti would it be better to clean the snapshots by storage garbage collector which run periodically ?

@weizhouapache issue is some active snapshots / vm snapshots are left in allocated when MS is stopped, no point in keeping them and wait until storage garbage collector cleans up (what if storage cleanup is disabled?) as these are listed / shown in UI as well (which are not allowed to delete). So, I think, it's better remove them on start itself. Any other thoughts / suggestions?

@sureshanaparti
the storage garbage collector cleans up the primary and secondary storage, including the snapshot/volumes/templates

                    // remove snapshots in Error state
                    List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error);

We could add the cleanup of Allocated snapshot and Allocated/Error vm snapshots in the process

IMHO, it is not the right place to cleanup during mgmt server start, as mgmt server might be running for several days.

sureshanaparti · 2024-01-08T08:24:24Z

@sureshanaparti would it be better to clean the snapshots by storage garbage collector which run periodically ?

@weizhouapache issue is some active snapshots / vm snapshots are left in allocated when MS is stopped, no point in keeping them and wait until storage garbage collector cleans up (what if storage cleanup is disabled?) as these are listed / shown in UI as well (which are not allowed to delete). So, I think, it's better remove them on start itself. Any other thoughts / suggestions?

@sureshanaparti the storage garbage collector cleans up the primary and secondary storage, including the snapshot/volumes/templates
                    // remove snapshots in Error state
                    List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error);
We could add the cleanup of Allocated snapshot and Allocated/Error vm snapshots in the process

IMHO, it is not the right place to cleanup during mgmt server start, as mgmt server might be running for several days.

@weizhouapache Agreed for the resources in error state (as there might be things to reset / cleanup) and MS is running. snapshot / vm snapshots are in Allocated, mostly for the very recent active snapshot / vm snapshots before MS is stopped, so no need for any cleanup, better to set them as removed (Any event through state transition would still keep it in Allocated). Otherwise, at least have to not list them on UI, or allow explicit deletion from the UI.

Snapshots in destroying state are deleted here. So, I thought, it's better to remove there on MS start.

cloudstack/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java

Lines 1519 to 1523 in 3f9dd4d

    
           //destroy snapshots in destroying state 
        
           List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Destroying); 
        
           for (SnapshotVO snapshotVO : snapshots) { 
        
               try { 
        
                   if (!deleteSnapshot(snapshotVO.getId(), null)) {

weizhouapache · 2024-01-08T08:54:45Z

@weizhouapache issue is some active snapshots / vm snapshots are left in allocated when MS is stopped, no point in keeping them and wait until storage garbage collector cleans up (what if storage cleanup is disabled?) as these are listed / shown in UI as well (which are not allowed to delete). So, I think, it's better remove them on start itself. Any other thoughts / suggestions?

@sureshanaparti the storage garbage collector cleans up the primary and secondary storage, including the snapshot/volumes/templates
                    // remove snapshots in Error state
                    List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error);
We could add the cleanup of Allocated snapshot and Allocated/Error vm snapshots in the process
IMHO, it is not the right place to cleanup during mgmt server start, as mgmt server might be running for several days.
@weizhouapache Agreed for the resources in error state (as there might be things to reset / cleanup) and MS is running. snapshot / vm snapshots are in Allocated, mostly for the very recent active snapshot / vm snapshots before MS is stopped, so no need for any cleanup, better to set them as removed (Any event through state transition would still keep it in Allocated). Otherwise, at least have to not list them on UI, or allow explicit deletion from the UI.

Snapshots in destroying state are deleted here. So, I thought, it's better to remove there on MS start.

cloudstack/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java

Lines 1519 to 1523 in 3f9dd4d

//destroy snapshots in destroying state

List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Destroying);

for (SnapshotVO snapshotVO : snapshots) {

try {

if (!deleteSnapshot(snapshotVO.getId(), null)) {

@sureshanaparti
the mgmt service is a key service for some users, especially on production.
the service is running for several days without stopping.
IMHO it is not good to cleanup resource during start ...

(OK with cleaning up resource in both start process and garbage collector)

sureshanaparti · 2024-01-08T10:15:37Z

@weizhouapache issue is some active snapshots / vm snapshots are left in allocated when MS is stopped, no point in keeping them and wait until storage garbage collector cleans up (what if storage cleanup is disabled?) as these are listed / shown in UI as well (which are not allowed to delete). So, I think, it's better remove them on start itself. Any other thoughts / suggestions?

@sureshanaparti the storage garbage collector cleans up the primary and secondary storage, including the snapshot/volumes/templates
                    // remove snapshots in Error state
                    List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error);
We could add the cleanup of Allocated snapshot and Allocated/Error vm snapshots in the process
IMHO, it is not the right place to cleanup during mgmt server start, as mgmt server might be running for several days.
@weizhouapache Agreed for the resources in error state (as there might be things to reset / cleanup) and MS is running. snapshot / vm snapshots are in Allocated, mostly for the very recent active snapshot / vm snapshots before MS is stopped, so no need for any cleanup, better to set them as removed (Any event through state transition would still keep it in Allocated). Otherwise, at least have to not list them on UI, or allow explicit deletion from the UI.
Snapshots in destroying state are deleted here. So, I thought, it's better to remove there on MS start.

cloudstack/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java

Lines 1519 to 1523 in 3f9dd4d

//destroy snapshots in destroying state

List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Destroying);

for (SnapshotVO snapshotVO : snapshots) {

try {

if (!deleteSnapshot(snapshotVO.getId(), null)) {
@sureshanaparti the mgmt service is a key service for some users, especially on production. the service is running for several days without stopping. IMHO it is not good to cleanup resource during start ...

(OK with cleaning up resource in both start process and garbage collector)

ok, will check & update. thanks @weizhouapache

github-actions · 2024-02-08T13:19:08Z

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

DaanHoogland · 2024-06-18T08:42:06Z

@sureshanaparti , does this need rebasing?

DaanHoogland · 2024-06-18T08:45:27Z

on another note, is it maybe safe to first mark it as State == Error and then delete? (I think this is the same @weizhouapache suggests)

DaanHoogland · 2025-02-03T14:55:53Z

@sureshanaparti , any progress on this?

DaanHoogland · 2025-02-04T14:13:56Z

@blueorangutan package

blueorangutan · 2025-02-04T14:16:03Z

@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 · 2025-02-04T14:52:47Z

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

github-actions · 2025-09-11T16:03:28Z

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

vishesh92

clgtm

sureshanaparti · 2025-09-12T14:11:28Z

@blueorangutan package

blueorangutan · 2025-09-12T14:12:04Z

@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 · 2025-09-12T15:29:40Z

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

weizhouapache · 2025-09-12T18:12:39Z

@blueorangutan test

blueorangutan · 2025-09-12T18:14:04Z

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

blueorangutan · 2025-09-13T10:13:27Z

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

Test	Result	Time (s)	Test File
ContextSuite context=TestClusterDRS>:setup	`Error`	0.00	test_cluster_drs.py

weizhouapache · 2025-09-15T11:38:00Z

@sureshanaparti

tested mgmt server restart during volume snapshot and vm snapshot

vm snapshot is transit from Creating to Error state
volume snapshot is transit from Creating to Error state too. However, the volume is still snapshotting

… job not finished and snapshot in Creating state

sureshanaparti · 2025-09-17T11:18:32Z

@sureshanaparti

tested mgmt server restart during volume snapshot and vm snapshot

vm snapshot is transit from Creating to Error state

volume snapshot is transit from Creating to Error state too. However, the volume is still snapshotting

updated @weizhouapache, to change volume back to Ready (from Snapshotting).

sureshanaparti · 2025-09-17T11:18:41Z

@blueorangutan package

blueorangutan · 2025-09-17T11:20:07Z

@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 · 2025-09-17T13:51:03Z

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

weizhouapache · 2025-09-22T13:15:48Z

tested ok

@blueorangutan test

blueorangutan · 2025-09-22T13:18:04Z

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

blueorangutan · 2025-09-23T05:07:20Z

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

Test	Result	Time (s)	Test File

…g ones to Error on MS start (apache#8452) * Remove allocated snapshots / vm snapshots on start * Check and Cleanup snapshots / vm snapshots on MS start * rebase fixes * Update volume state (from Snapshotting) on MS start when its snapshot job not finished and snapshot in Creating state

boring-cyborg bot added the component:storage label Jan 5, 2024

sureshanaparti marked this pull request as ready for review January 8, 2024 08:08

sureshanaparti marked this pull request as draft January 8, 2024 10:16

DaanHoogland added this to the 4.19.1.0 milestone Jan 22, 2024

github-actions bot added the status:has-conflicts label Feb 8, 2024

sureshanaparti self-assigned this Mar 12, 2024

sureshanaparti added the status:waiting-for-author label Jun 23, 2024

sureshanaparti changed the title ~~Remove allocated snapshots / vm snapshots on start~~ [WIP] Remove allocated snapshots / vm snapshots on start Jun 25, 2024

sureshanaparti modified the milestones: 4.19.1.0, 4.19.2.0 Jun 25, 2024

DaanHoogland changed the base branch from main to 4.19 February 4, 2025 14:12

github-actions bot removed the status:has-conflicts label Feb 5, 2025

DaanHoogland removed this from the 4.19.2 milestone Feb 7, 2025

weizhouapache changed the base branch from 4.19 to 4.20 September 11, 2025 14:44

github-actions bot added the status:has-conflicts label Sep 11, 2025

vishesh92 approved these changes Sep 12, 2025

View reviewed changes

sureshanaparti added 3 commits September 12, 2025 19:30

Remove allocated snapshots / vm snapshots on start

32a6297

Check and Cleanup snapshots / vm snapshots on MS start

09b046b

rebase fixes

549842b

sureshanaparti force-pushed the remove-allocated-snapshots-on-start branch from 6c7d975 to 549842b Compare September 12, 2025 14:10

github-actions bot removed the status:has-conflicts label Sep 12, 2025

Update volume state (from Snapshotting) on MS start when its snapshot…

56f5195

… job not finished and snapshot in Creating state

weizhouapache added status:ready-for-merge and removed status:needs-testing labels Sep 22, 2025

weizhouapache merged commit 40dec99 into apache:4.20 Sep 23, 2025
25 of 26 checks passed

weizhouapache mentioned this pull request Sep 23, 2025

Vm Snapshot left in allocated state are not cleaned up #8424

Closed

DaanHoogland deleted the remove-allocated-snapshots-on-start branch September 24, 2025 07:06

Cleanup allocated snapshots / vm snapshots, and update pending ones to Error on MS start #8452

Cleanup allocated snapshots / vm snapshots, and update pending ones to Error on MS start #8452

Uh oh!

Conversation

sureshanaparti commented Jan 5, 2024 • 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?

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

Uh oh!

sureshanaparti commented Jan 5, 2024

Uh oh!

blueorangutan commented Jan 5, 2024

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

weizhouapache commented Jan 5, 2024

Uh oh!

blueorangutan commented Jan 5, 2024

Uh oh!

sureshanaparti commented Jan 8, 2024

Uh oh!

weizhouapache commented Jan 8, 2024

Uh oh!

sureshanaparti commented Jan 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weizhouapache commented Jan 8, 2024

Uh oh!

sureshanaparti commented Jan 8, 2024

Uh oh!

github-actions bot commented Feb 8, 2024

Uh oh!

DaanHoogland commented Jun 18, 2024

Uh oh!

DaanHoogland commented Jun 18, 2024

Uh oh!

DaanHoogland commented Feb 3, 2025

Uh oh!

DaanHoogland commented Feb 4, 2025

Uh oh!

blueorangutan commented Feb 4, 2025

Uh oh!

blueorangutan commented Feb 4, 2025

Uh oh!

github-actions bot commented Sep 11, 2025

Uh oh!

vishesh92 left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Sep 12, 2025

Uh oh!

blueorangutan commented Sep 12, 2025

Uh oh!

blueorangutan commented Sep 12, 2025

Uh oh!

weizhouapache commented Sep 12, 2025

Uh oh!

blueorangutan commented Sep 12, 2025

Uh oh!

blueorangutan commented Sep 13, 2025

Uh oh!

weizhouapache commented Sep 15, 2025

Uh oh!

sureshanaparti commented Sep 17, 2025

Uh oh!

sureshanaparti commented Sep 17, 2025

Uh oh!

blueorangutan commented Sep 17, 2025

Uh oh!

blueorangutan commented Sep 17, 2025

Uh oh!

weizhouapache commented Sep 22, 2025

sureshanaparti commented Jan 5, 2024 •

edited

Loading

codecov bot commented Jan 5, 2024 •

edited

Loading

sureshanaparti commented Jan 8, 2024 •

edited

Loading