Fix storage cleanup corner case preventing VM deletion #5575

nvazquez · 2021-10-13T15:13:26Z

Description

This PR prevents a corner case for the storage cleanup task observed in Vmware in which:

There are storage pool migrations for the ROOT disk of a VM
After the migrations, the volumes table creates new volume records, marking the ones to be deleted with state = 'Destroy'
The storage cleanup task will attempt to expunge the volumes in 'Destroy' state, but if there are underlying storage issues then this task is not completed
Once the storage issues are solved and the current volume is on the same storage pool as one of the volumes to be removed then the VM can be completely removed

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

nvazquez · 2021-10-13T15:29:02Z

@blueorangutan package

blueorangutan · 2021-10-13T15:30:03Z

@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

rohityadavcloud · 2021-10-13T15:36:35Z

engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java

            destVol.setUuid(uuid);
            destVol.setInstanceId(instanceId);
+            // Prevent storage cleanup corner case of ROOT volume deletion
+            srcVol.setVolumeType(Type.DATADISK);


Could that cause any regression, does this save the VO in DB?

Currently testing for regressions. This saves the removed volume after storage migrations in DB as datadisk. @weizhouapache and I are considering the approach taken on restore VM as well: https://github.com/apache/cloudstack/blob/main/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L7407-L7409

nvazquez · 2021-10-13T16:00:54Z

@blueorangutan package

blueorangutan · 2021-10-13T16:02:03Z

@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-10-13T16:56:46Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1563

nvazquez · 2021-10-13T18:20:13Z

@blueorangutan test matrix

blueorangutan · 2021-10-13T18:22:03Z

@nvazquez a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2021-10-14T03:43:05Z

Trillian test result (tid-2408)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32285 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5575-t2408-xenserver-71.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2021-10-14T03:43:40Z

Trillian test result (tid-2409)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32260 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5575-t2409-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2021-10-14T04:19:10Z

Trillian test result (tid-2410)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34629 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5575-t2410-vmware-65u2.zip
Smoke tests completed. 90 look OK, 1 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_create_pvlan_network	`Error`	0.03	test_pvlan.py

weizhouapache

code lgtm

engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java

nvazquez · 2021-10-14T13:11:52Z

@blueorangutan package

blueorangutan · 2021-10-14T13:13:03Z

@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

DaanHoogland

I think the vmware storage processor addresses this in a harmfull way;
"to destroy a volume; get the vm and destroy that"
I don't think we can solve it there now, but it should be addressed there as well
it is formally right to detach the volume when it should no longer be associated with the VM so this CLGTM

blueorangutan · 2021-10-14T13:53:18Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1568

nvazquez · 2021-10-14T13:59:44Z

@blueorangutan test matrix

blueorangutan · 2021-10-14T14:00:03Z

@nvazquez a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

weizhouapache

approved again

vladimirpetrov · 2021-10-15T10:35:46Z

@blueorangutan package

blueorangutan · 2021-10-15T10:38:04Z

@vladimirpetrov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-10-15T11:22:10Z

Packaging result: ✔️ el7 ✔️ el8 ✖️ debian ✔️ suse15. SL-JID 1572

nvazquez · 2021-10-15T12:07:50Z

@blueorangutan package

blueorangutan · 2021-10-15T12:09:03Z

@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-10-15T12:49:15Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1575

rohityadavcloud · 2021-10-15T14:28:47Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-10-15T14:31:17Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

vladimirpetrov

LGTM based on manual testing. Steps used:

(1) Create a VMWare 6.7u3 env with 2 pools (pool1, pool2).
(2) Create L2 network, and a VM. The created volume is on pool2.
(3) Put pool2 in maintenance mode, the VM is stopped.
(4) Start the VM. Its volume is copied to pool1.
(5) There are two records in the database with the same name ‘ROOT-xx’. Update the 'Expunged' volume like this:

MariaDB [cloud]> update volumes set removed=null,state='Destroy' where id=x;

(6) Change the following global settings, and restart the management server:

storage.cleanup.interval -> to 60 (seconds)
storage.cleanup.delay -> to 60 (seconds)

systemctl restart cloudstack-management

Result: Only the inactive volume is removed (expunged) and our VM with the active volume is still running fine.

2021-10-15 15:13:10,160 INFO [c.c.s.r.VmwareStorageProcessor] (DirectAgent-1:ctx-5a08de4b 10.0.35.14, cmd: DeleteCommand) (logid:5cbc532f) Executing resource DeleteCommand: {"data":{"org.apache.cloudstack.storage.to.VolumeObjectTO":{"volumeType":"DATADISK","dataStore":{"org.apache.cloudstack.storage.to.PrimaryDataStoreTO":{"uuid":"43c3140e-8caf-38b8-9540-238fa5409707","name":"ref-trl-2009-v-M7-vladimir-petrov-esxi-pri2","id":2,"poolType":"NetworkFilesystem","host":"10.0.32.4","path":"/acs/primary/ref-trl-2009-v-M7-vladimir-petrov/ref-trl-2009-v-M7-vladimir-petrov-esxi-pri2","port":2049,"url":"NetworkFilesystem://10.0.32.4/acs/primary/ref-trl-2009-v-M7-vladimir-petrov/ref-trl-2009-v-M7-vladimir-petrov-esxi-pri2/?ROLE=Primary&STOREUUID=43c3140e-8caf-38b8-9540-238fa5409707","isManaged":false}},"name":"ROOT-6","size":2147483648,"path":"ROOT-6","volumeId":9,"accountId":2,"chainInfo":"{"diskDeviceBusName":"ide0:1","diskChain":["[43c3140e8caf38b89540238fa5409707] i-2-6-VM/ROOT-6.vmdk","[43c3140e8caf38b89540238fa5409707] e765b12f796432fc80cd4091e8c59f32/e765b12f796432fc80cd4091e8c59f32.vmdk"]}","format":"OVA","provisioningType":"THIN","poolId":2,"id":9,"hypervisorType":"VMware","directDownload":false,"deployAsIs":false}},"wait":0,"bypassHostMaintenance":false}

blueorangutan · 2021-10-16T01:13:14Z

Trillian test result (tid-2422)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37214 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5575-t2422-vmware-67u3.zip
Smoke tests completed. 90 look OK, 1 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_04_autoscale_kubernetes_cluster	`Failure`	35.96	test_kubernetes_clusters.py

* Fix storage cleanup corner case * Improve deletion * Refactor

Fix storage cleanup corner case

eaa3fdb

nvazquez force-pushed the fixrootvoldeletion branch from 2d8bccb to eaa3fdb Compare October 13, 2021 15:25

apache deleted a comment from blueorangutan Oct 13, 2021

rohityadavcloud reviewed Oct 13, 2021

View reviewed changes

Improve deletion

8d4dfa5

nvazquez requested review from DaanHoogland, rohityadavcloud, sureshanaparti and weizhouapache October 13, 2021 16:00

rohityadavcloud added this to the 4.16.0.0 milestone Oct 14, 2021

weizhouapache closed this Oct 14, 2021

weizhouapache reopened this Oct 14, 2021

weizhouapache approved these changes Oct 14, 2021

View reviewed changes

sureshanaparti reviewed Oct 14, 2021

View reviewed changes

engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java Outdated Show resolved Hide resolved

Refactor

f883f43

DaanHoogland approved these changes Oct 14, 2021

View reviewed changes

nvazquez added Severity:Critical Critical bug type:bug labels Oct 14, 2021

weizhouapache approved these changes Oct 14, 2021

View reviewed changes

nvazquez marked this pull request as ready for review October 15, 2021 11:02

nvazquez assigned vladimirpetrov Oct 15, 2021

rohityadavcloud approved these changes Oct 15, 2021

View reviewed changes

vladimirpetrov approved these changes Oct 15, 2021

View reviewed changes

nvazquez merged commit a5372a9 into apache:main Oct 16, 2021

nvazquez deleted the fixrootvoldeletion branch February 12, 2022 02:38

shwstppr pushed a commit to shapeblue/cloudstack that referenced this pull request Mar 17, 2023

Fix storage cleanup corner case preventing VM deletion (apache#5575)

f476771

* Fix storage cleanup corner case * Improve deletion * Refactor

shwstppr mentioned this pull request Mar 17, 2023

vmware: vm/volume inter-cluster migration failure during vm start operation #7311

Closed

shwstppr mentioned this pull request Mar 28, 2023

storage: fix vmware volume cleanup failure #7312

Closed

12 tasks

Fix storage cleanup corner case preventing VM deletion #5575

Fix storage cleanup corner case preventing VM deletion #5575

Uh oh!

Conversation

nvazquez commented Oct 13, 2021

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!

nvazquez commented Oct 13, 2021

Uh oh!

blueorangutan commented Oct 13, 2021

Uh oh!

rohityadavcloud Oct 13, 2021

Choose a reason for hiding this comment

Uh oh!

nvazquez Oct 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Oct 13, 2021

Uh oh!

blueorangutan commented Oct 13, 2021

Uh oh!

blueorangutan commented Oct 13, 2021

Uh oh!

nvazquez commented Oct 13, 2021

Uh oh!

blueorangutan commented Oct 13, 2021

Uh oh!

blueorangutan commented Oct 14, 2021

Uh oh!

blueorangutan commented Oct 14, 2021

Uh oh!

blueorangutan commented Oct 14, 2021

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nvazquez commented Oct 14, 2021

Uh oh!

blueorangutan commented Oct 14, 2021

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 14, 2021

Uh oh!

nvazquez commented Oct 14, 2021

Uh oh!

blueorangutan commented Oct 14, 2021

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

vladimirpetrov commented Oct 15, 2021

Uh oh!

blueorangutan commented Oct 15, 2021

Uh oh!

blueorangutan commented Oct 15, 2021

Uh oh!

nvazquez commented Oct 15, 2021

Uh oh!

blueorangutan commented Oct 15, 2021

Uh oh!

blueorangutan commented Oct 15, 2021

Uh oh!

rohityadavcloud commented Oct 15, 2021

Uh oh!

blueorangutan commented Oct 15, 2021

Uh oh!

vladimirpetrov left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 16, 2021

nvazquez Oct 13, 2021 •

edited

Loading