New feature: Add support to destroy/recover volumes by ustcweizhou · Pull Request #3688 · apache/cloudstack · GitHub

Description

cloudstack provides options to destroy and recover a vm. For volumes, we do not have similar operations. When we delete a volume, it will be removed from primary storage permanently.

With this change,
(1) we have option to destroy a volume (expunge=false) or delete a volume permanently (expunge=true)
(2) we are be able to recover a Destroy volume to Ready state.
(3) Destroy volume will be removed in storage clean up.
(4) when reinstall a vm, the old root volume will be Destroy state. It can be recovered to Ready state and attached to other vm so we can restore some data.

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)

Screenshots (if appropriate):

How Has This Been Tested?

destroy vm (expunge=false)
recover a Destroy vm
delete vm (same as destroy vm with expunge=True)

More test cases can be found in the integration tests.

…unged but not removed Steps to reproduce the issue (1) create a vm and stop it. check resource count of primary storage (2) download volume. resource count of primary storage is not changed. (3) expunge the vm, the volume will be Expunged state as there is a volume snapshot on secondary storage. The resource count of primary storage decreased. (4) update resource count of the account (or domain), the resource count of primary storage is reset to the value in step (2).

hihi

You have your own to translate @DaanHoogland as well ;)

@K0zka @cl-k-takahashi @svenvogel @nvazquez @pdion891 @div8cn @GabrielBrascher
If you guys want to donate 5min of your life to translate those 2-3 lines to your languages - you are most welcome.

@andrijapanicsb
"label.action.destroy.volume":"ボリュームの破棄",
"label.action.recover.volume":"ボリュームの復元",
"message.action.destroy.volume":"このボリュームを破棄してもよろしいですか?",
"message.action.recover.volume":"このボリュームを復元してもよろしいですか?",

@andrijapanicsb here we go. 😄 (german)

"label.action.recover.volume":"Volume wiederherstellen",
"message.action.destroy.volume":"Bitte bestätigen, dass Sie das Volume zerstören wollen.",
"message.action.recover.volume":"Bitte bestätigen, dass die das Volume wiederherstellen wollen.",

do we speak about this lines?

@svenvogel one line is missing its translation...please add ASAP?

"label.action.destroy.volume":"vernietig schijf",
"label.action.recover.volume":"herstel schijf",
"message.action.destroy.volume":"Bevestig alstublieft dat U deze schijf wilt vernietigen?",
"message.action.recover.volume":"Bevestig alstublieft dat U deze schijf wilt herstellen?",

there's Dutch for you @andrijapanicsb

I took the liberty to capitalise those first 2, I'm sure it would be painful for you to see it lowercase @DaanHoogland

ping @svenvogel

We, the Dutch, are not getting high on capitalisation usually, @andrijapanicsb ! I did it exactly according to convention; start of sentence, 'U', personal names, geographical names. That's it.

seems like a useful feature @ustcweizhou . I had a global look and saw some style issues but no real ones. I'll do a thourough review when I have time. @PaulAngus @andrijapanicsb any thoughts?

@blueorangutan package

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

Packaging result: ✖centos6 ✖centos7 ✖debian. JID-438

@blueorangutan package

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

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-532

@blueorangutan test

@andrijapanicsb a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

Trillian test result (tid-695)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39191 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3688-t695-kvm-centos7.zip
Smoke tests completed. 77 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

code looks good, two minor remarks @weizhouapache

@weizhouapache I've been committing the translation messages directly to your branch - hope that is ok? (wouldn't dare to touch the code)

LGTM based on code

@blueorangutan package

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

Packaging result: ✖centos6 ✖centos7 ✖debian. JID-666

@blueorangutan package

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

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-690

@blueorangutan test

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

Trillian test result (tid-841)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32003 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3688-t841-kvm-centos7.zip
Smoke tests completed. 78 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

@andrijapanicsb Thanks a lot for your translations !

@DaanHoogland Thanks for review ! I added a commit per your comments.

@blueorangutan package

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

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-739

@rhtyd this passed all tests last week, do we need a retest?

For what tis worth, the Dutch capitalisation convention; start of sentence, 'U', personal names, geographical names. That's it, not even month or weekday names not designations of conditions like nationality get capatalised. This means the translation in general is not exactly according to national standards. Having gotten that of my chest; I am not investing time in this now, so ... lgtm

2 approving changes,
The last commit just edits the version in the comment/message - i.e. no need to retest again, previous Trillian tests are all fine.
Jenkins happy.

Merging.

@rhtyd are you referring to this with #3601 (comment) ?

Yes @DaanHoogland maybe Wei can comment?

@rhtyd @DaanHoogland
I have run the integration test component/test_volumes.py on kvm, all passed.

ok
Detach a Volume attached to a VM ... === TestName: test_02_detach_volume | Status : SUCCESS ===
ok
Delete a Volume unattached to an VM ... === TestName: test_03_delete_detached_volume | Status : SUCCESS ===
ok
Create a volume under a non-root domain as non-root-domain user ... === TestName: test_create_volume_under_domain | Status : SUCCESS ===
ok

----------------------------------------------------------------------
Ran 4 tests in 277.694s

OK

and smoke/test_volumes.py on kvm

Test destroy Virtual Machine and it's volumes ... === TestName: test_11_destroy_vm_and_volumes | Status : SUCCESS ===
ok

----------------------------------------------------------------------
Ran 1 test in 678.315s

OK

and smoke/test_vm_life_cycle.py

Delete a Volume unattached to an VM ... === TestName: test_09_delete_detached_volume | Status : SUCCESS ===
ok

----------------------------------------------------------------------
Ran 1 test in 364.770s

OK

I have create a pr for travis test: #3880

Yes smoketests indeed pass on kvm but Travis runs tests against simulator. I'll review the PR. Thanks.

* server: fix resource count of primary storage if some volumes are Expunged but not removed Steps to reproduce the issue (1) create a vm and stop it. check resource count of primary storage (2) download volume. resource count of primary storage is not changed. (3) expunge the vm, the volume will be Expunged state as there is a volume snapshot on secondary storage. The resource count of primary storage decreased. (4) update resource count of the account (or domain), the resource count of primary storage is reset to the value in step (2). * New feature: Add support to destroy/recover volumes * Add integration test for volume destroy/recover * marvin: check resource count of more types * messages translate to JP * Update messages for CN * translate message for NL * fix two issues per Daan's comments Co-authored-by: Andrija Panic <45762285+andrijapanicsb@users.noreply.github.com>

ustcweizhou added 4 commits November 12, 2019 09:46

New feature: Add support to destroy/recover volumes

34a76ce

marvin: check resource count of more types

2014f86

Add integration test for volume destroy/recover

02bbc57

svenvogel added type:new-feature component:api labels Nov 15, 2019

DaanHoogland reviewed Nov 22, 2019

View reviewed changes

yadvr added this to the 4.14.0.0 milestone Dec 7, 2019

andrijapanicsb added 2 commits January 9, 2020 09:24

messages translate to JP

6a7d998

Update messages for CN

6b02ce8

DaanHoogland reviewed Jan 9, 2020

View reviewed changes

Comment thread api/src/main/java/org/apache/cloudstack/api/response/CapabilitiesResponse.java Resolved

DaanHoogland reviewed Jan 9, 2020

View reviewed changes

Comment thread server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Resolved

translate message for NL

4a5b166

wido approved these changes Jan 15, 2020

View reviewed changes

fix two issues per Daan's comments

ffa38d7

yadvr closed this Feb 3, 2020

yadvr reopened this Feb 3, 2020

apache deleted a comment from blueorangutan Feb 3, 2020

apache deleted a comment from yadvr Feb 3, 2020

apache deleted a comment from blueorangutan Feb 3, 2020

apache deleted a comment from yadvr Feb 3, 2020

DaanHoogland approved these changes Feb 3, 2020

View reviewed changes

andrijapanicsb merged commit fd5bea8 into apache:master Feb 7, 2020

yadvr mentioned this pull request Jun 8, 2020

[FEATURE] Support to destroy/recover volumes apache/cloudstack-primate#405

Closed

weizhouapache mentioned this pull request Aug 18, 2021

Destroy VM should detach the volumes from VM and recover VM should not try to attach the volumes again #4902

Closed

                   "label.action.destroy.instance.processing": "正在销毁实例...",
                   "label.action.destroy.systemvm": "销毁系统 VM",
                   "label.action.destroy.systemvm.processing": "正在销毁系统 VM...",
+                  "label.action.destroy.volume":"Destroy Volume",

New feature: Add support to destroy/recover volumes by ustcweizhou · Pull Request #3688 · apache/cloudstack · GitHub

Repository navigation

New feature: Add support to destroy/recover volumes - #3688

New feature: Add support to destroy/recover volumes#3688andrijapanicsb merged 8 commits intoapache:masterapache/cloudstack:masterfrom ustcweizhou:4.14.0-volume-destroy-recoverustcweizhou/cloudstack:4.14.0-volume-destroy-recoverCopy head branch name to clipboard

Conversation

ustcweizhou commented Nov 14, 2019

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cl-k-takahashi Jan 4, 2020 • 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!

svenvogel Jan 4, 2020 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Nov 22, 2019

Uh oh!

yadvr commented Dec 7, 2019

Uh oh!

blueorangutan commented Dec 7, 2019

Uh oh!

blueorangutan commented Dec 7, 2019

Uh oh!

andrijapanicsb commented Jan 3, 2020

Uh oh!

andrijapanicsb commented Jan 4, 2020

Uh oh!

blueorangutan commented Jan 4, 2020

Uh oh!

blueorangutan commented Jan 4, 2020

Uh oh!

andrijapanicsb commented Jan 4, 2020

Uh oh!

blueorangutan commented Jan 4, 2020

Uh oh!

blueorangutan commented Jan 5, 2020

Uh oh!

Uh oh!

Uh oh!

DaanHoogland commented Jan 9, 2020

Uh oh!

andrijapanicsb commented Jan 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wido left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Jan 28, 2020

Uh oh!

blueorangutan commented Jan 28, 2020

Uh oh!

blueorangutan commented Jan 28, 2020

Uh oh!

yadvr commented Jan 29, 2020

Uh oh!

blueorangutan commented Jan 29, 2020

Uh oh!

blueorangutan commented Jan 29, 2020

Uh oh!

DaanHoogland commented Jan 29, 2020

New feature: Add support to destroy/recover volumes#3688
andrijapanicsb merged 8 commits into
apache:masterfrom
ustcweizhou:4.14.0-volume-destroy-recover

cl-k-takahashi Jan 4, 2020 •
edited
Loading

svenvogel Jan 4, 2020 •
edited
Loading

andrijapanicsb commented Jan 13, 2020 •
edited
Loading