engine-storage: Set SecretConsumerDetail for VM live migration with storage on shared NFS #9222

abh1sar · 2024-06-11T14:06:30Z

Description

This PR Fixes the issue where live migration of a VM with encrypted volume on shared NFS due to wrongly setup secret key on the destination.
Before fix uuid of the secret key was corresponding to the source volume, while actually it should be of the destination volume.

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?

Reproduced the original issue by

Create a VM with encrypted data volume
Open the Live VM migrate UI wizard
Select Migrate with Storage option and made sure that the data volume is moved to a different pool.
Start live migrate.

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

…tion with storage on shared NFS

codecov · 2024-06-11T14:14:47Z

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 15.08%. Comparing base (3f5a77e) to head (487b788).
Report is 42 commits behind head on 4.19.

Files with missing lines	Patch %	Lines
...torage/motion/StorageSystemDataMotionStrategy.java	0.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #9222      +/-   ##
============================================
- Coverage     15.08%   15.08%   -0.01%     
+ Complexity    11190    11188       -2     
============================================
  Files          5406     5406              
  Lines        473210   473211       +1     
  Branches      61188    59224    -1964     
============================================
  Hits          71376    71376              
- Misses       393887   393888       +1     
  Partials       7947     7947

Flag	Coverage Δ
uitests	`4.30% <ø> (ø)`
unittests	`15.80% <0.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.

weizhouapache

code lgtm

weizhouapache · 2024-06-11T14:24:38Z

@abh1sar
just a suggestion, you can create a new branch (instread of 4.19)

abh1sar · 2024-06-12T06:32:05Z

Removed a condition which was not allowing live migrate for powerflex as @harikrishna-patnala suggested on issue #8255

harikrishna-patnala

code LGTM. This needs testing with powerflex storage as well.

abh1sar · 2024-06-12T07:01:27Z

@blueorangutan package

blueorangutan · 2024-06-12T07:02:04Z

@abh1sar 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 · 2024-06-12T08:04:02Z

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

rohityadavcloud

LGTM didn't test it though

abh1sar · 2024-06-12T22:18:40Z

@blueorangutan test

blueorangutan · 2024-06-12T22:20:04Z

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

blueorangutan · 2024-06-13T10:56:13Z

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

Test	Result	Time (s)	Test File

...tion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java

abh1sar · 2024-06-26T05:03:30Z

@blueorangutan package

blueorangutan · 2024-06-26T05:04:04Z

@abh1sar 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 · 2024-06-26T06:05:34Z

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

abh1sar · 2024-06-28T13:38:03Z

@borisstoyanov I have merged with the latest changes and built packages. Can you please try once more.

abh1sar · 2024-08-21T10:53:06Z

@blueorangutan package

blueorangutan · 2024-08-21T10:54:03Z

@abh1sar 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 · 2024-08-21T11:49:57Z

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

vladimirpetrov

LGTM based on manual testing. Tested with Oracle Linux 8 KVM using the provided steps.

DaanHoogland · 2024-08-22T19:45:00Z

@borisstoyanov @sureshanaparti are your concerns met?

abh1sar · 2024-08-23T00:51:14Z

@blueorangutan test

blueorangutan · 2024-08-23T00:52:03Z

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

borisstoyanov

LGMT, based on code review, marvin tests

blueorangutan · 2024-08-23T14:26:46Z

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

Test	Result	Time (s)	Test File
test_03_secured_to_nonsecured_vm_migration	`Error`	397.15	test_vm_life_cycle.py

sureshanaparti · 2024-08-26T08:06:45Z

@borisstoyanov @sureshanaparti are your concerns met?

not yet, will confirm the powerflex check with the author.

sureshanaparti · 2024-08-29T04:54:32Z

@vladimirpetrov this needs testing with powerflex as well.

rohityadavcloud · 2024-09-19T11:58:10Z

cc @kiranchavala @vladimirpetrov

engine-storage-datamotion: Set SecretConsumerDetail for VM live migra…

c1d3315

…tion with storage on shared NFS

boring-cyborg bot added the component:storage label Jun 11, 2024

abh1sar requested a review from mlsorensen June 11, 2024 14:06

abh1sar added the component:migration label Jun 11, 2024

abh1sar added this to the 4.19.1.0 milestone Jun 11, 2024

abh1sar self-assigned this Jun 11, 2024

weizhouapache approved these changes Jun 11, 2024

View reviewed changes

DaanHoogland added the status:needs-testing label Jun 11, 2024

vishesh92 linked an issue Jun 12, 2024 that may be closed by this pull request

live migration of encrypted volume fails with nfs #8255

Closed

VM live migration - powerflex encrypted volume

8e95d94

harikrishna-patnala approved these changes Jun 12, 2024

View reviewed changes

rohityadavcloud approved these changes Jun 12, 2024

View reviewed changes

DaanHoogland unassigned abh1sar Jun 18, 2024

rohityadavcloud assigned kiranchavala Jun 18, 2024

sureshanaparti reviewed Jun 24, 2024

View reviewed changes

...tion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java Outdated Show resolved Hide resolved

sureshanaparti assigned harikrishna-patnala Jun 26, 2024

abh1sar added 2 commits June 26, 2024 09:58

Merge remote-tracking branch 'upstream/4.19' into secret

396ac29

rename isPowerFlex

51ae90e

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

abh1sar deleted the branch apache:4.19 August 20, 2024 11:39

abh1sar closed this Aug 20, 2024

abh1sar deleted the 4.19 branch August 20, 2024 11:39

abh1sar restored the 4.19 branch August 20, 2024 12:51

abh1sar reopened this Aug 20, 2024

Merge branch '4.19' into 4.19

e46ac53

vladimirpetrov self-assigned this Aug 22, 2024

vladimirpetrov approved these changes Aug 22, 2024

View reviewed changes

vladimirpetrov removed the status:needs-testing label Aug 22, 2024

borisstoyanov approved these changes Aug 23, 2024

View reviewed changes

Merge branch '4.19' into 4.19

3338e05

sureshanaparti added the status:needs-testing label Aug 29, 2024

Merge branch '4.19' into 4.19

487b788

rohityadavcloud removed the status:needs-testing label Nov 7, 2024

rohityadavcloud merged commit 22c6f08 into apache:4.19 Nov 7, 2024
25 of 26 checks passed

rohityadavcloud mentioned this pull request Nov 7, 2024

live migration of encrypted volume fails with nfs #8255

Closed

abh1sar deleted the 4.19 branch November 8, 2024 01:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

engine-storage: Set SecretConsumerDetail for VM live migration with storage on shared NFS #9222

engine-storage: Set SecretConsumerDetail for VM live migration with storage on shared NFS #9222

abh1sar commented Jun 11, 2024 •

edited

Loading

codecov bot commented Jun 11, 2024 •

edited

Loading

weizhouapache left a comment

weizhouapache commented Jun 11, 2024

abh1sar commented Jun 12, 2024

harikrishna-patnala left a comment

abh1sar commented Jun 12, 2024

blueorangutan commented Jun 12, 2024

blueorangutan commented Jun 12, 2024

rohityadavcloud left a comment

abh1sar commented Jun 12, 2024

blueorangutan commented Jun 12, 2024

blueorangutan commented Jun 13, 2024

abh1sar commented Jun 26, 2024

blueorangutan commented Jun 26, 2024

blueorangutan commented Jun 26, 2024

abh1sar commented Jun 28, 2024

abh1sar commented Aug 21, 2024

blueorangutan commented Aug 21, 2024

blueorangutan commented Aug 21, 2024

vladimirpetrov left a comment

DaanHoogland commented Aug 22, 2024

abh1sar commented Aug 23, 2024

blueorangutan commented Aug 23, 2024

borisstoyanov left a comment

blueorangutan commented Aug 23, 2024

sureshanaparti commented Aug 26, 2024

sureshanaparti commented Aug 29, 2024 •

edited

Loading

rohityadavcloud commented Sep 19, 2024 •

edited

Loading

engine-storage: Set SecretConsumerDetail for VM live migration with storage on shared NFS #9222

engine-storage: Set SecretConsumerDetail for VM live migration with storage on shared NFS #9222

Conversation

abh1sar commented Jun 11, 2024 • edited Loading

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?

codecov bot commented Jun 11, 2024 • edited Loading

Codecov Report

weizhouapache left a comment

Choose a reason for hiding this comment

weizhouapache commented Jun 11, 2024

abh1sar commented Jun 12, 2024

harikrishna-patnala left a comment

Choose a reason for hiding this comment

abh1sar commented Jun 12, 2024

blueorangutan commented Jun 12, 2024

blueorangutan commented Jun 12, 2024

rohityadavcloud left a comment

Choose a reason for hiding this comment

abh1sar commented Jun 12, 2024

blueorangutan commented Jun 12, 2024

blueorangutan commented Jun 13, 2024

abh1sar commented Jun 26, 2024

blueorangutan commented Jun 26, 2024

blueorangutan commented Jun 26, 2024

abh1sar commented Jun 28, 2024

abh1sar commented Aug 21, 2024

blueorangutan commented Aug 21, 2024

blueorangutan commented Aug 21, 2024

vladimirpetrov left a comment

Choose a reason for hiding this comment

DaanHoogland commented Aug 22, 2024

abh1sar commented Aug 23, 2024

blueorangutan commented Aug 23, 2024

borisstoyanov left a comment

Choose a reason for hiding this comment

blueorangutan commented Aug 23, 2024

sureshanaparti commented Aug 26, 2024

sureshanaparti commented Aug 29, 2024 • edited Loading

rohityadavcloud commented Sep 19, 2024 • edited Loading

abh1sar commented Jun 11, 2024 •

edited

Loading

codecov bot commented Jun 11, 2024 •

edited

Loading

sureshanaparti commented Aug 29, 2024 •

edited

Loading

rohityadavcloud commented Sep 19, 2024 •

edited

Loading