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

ustcweizhou · 2019-11-14T15:16:26Z

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

DaanHoogland · 2019-11-22T16:25:00Z

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


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.

DaanHoogland · 2019-11-22T16:26:58Z

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?

yadvr · 2019-12-07T22:29:42Z

@blueorangutan package

blueorangutan · 2019-12-07T22:30:29Z

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

blueorangutan · 2019-12-07T23:11:01Z

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

andrijapanicsb · 2020-01-03T22:59:30Z

@blueorangutan package

andrijapanicsb · 2020-01-04T14:18:52Z

@blueorangutan package

blueorangutan · 2020-01-04T14:19:30Z

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

blueorangutan · 2020-01-04T14:41:39Z

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

andrijapanicsb · 2020-01-04T15:57:53Z

@blueorangutan test

blueorangutan · 2020-01-04T15:58:30Z

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

blueorangutan · 2020-01-05T03:21:27Z

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

DaanHoogland · 2020-01-09T16:35:55Z

code looks good, two minor remarks @weizhouapache

andrijapanicsb · 2020-01-13T22:48:11Z

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

wido

LGTM based on code

yadvr · 2020-01-28T05:22:47Z

@blueorangutan package

blueorangutan · 2020-01-28T05:23:15Z

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

blueorangutan · 2020-01-28T06:29:37Z

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

yadvr · 2020-01-29T10:32:01Z

@blueorangutan package

blueorangutan · 2020-01-29T10:32:20Z

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

blueorangutan · 2020-01-29T11:33:38Z

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

DaanHoogland · 2020-01-29T12:40:03Z

@blueorangutan test

blueorangutan · 2020-01-29T12:40:21Z

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

blueorangutan · 2020-01-29T22:07:43Z

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

weizhouapache · 2020-01-30T08:40:22Z

@andrijapanicsb Thanks a lot for your translations !

weizhouapache · 2020-01-30T08:40:44Z

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

yadvr · 2020-02-03T09:16:37Z

@blueorangutan package

blueorangutan · 2020-02-03T09:16:42Z

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

blueorangutan · 2020-02-03T09:39:58Z

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

DaanHoogland · 2020-02-03T12:23:56Z

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

DaanHoogland

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

andrijapanicsb · 2020-02-07T10:24:52Z

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.

DaanHoogland · 2020-02-11T12:10:50Z

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

yadvr · 2020-02-11T13:02:03Z

Yes @DaanHoogland maybe Wei can comment?

weizhouapache · 2020-02-11T22:23:45Z

@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

yadvr · 2020-02-12T05:18:54Z

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 threadapi/src/main/java/org/apache/cloudstack/api/response/CapabilitiesResponse.java Outdated

DaanHoogland reviewed Jan 9, 2020
View reviewed changes

Comment threadserver/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Outdated

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:masterFeb 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

Uh oh!

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-takahashiJan 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!

svenvogelJan 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

Uh oh!

blueorangutan commented Jan 29, 2020

Uh oh!

cl-k-takahashiJan 4, 2020•
edited
Loading

svenvogelJan 4, 2020•
edited
Loading

andrijapanicsb commented Jan 13, 2020•
edited
Loading