refactor: migrate vm with storage #5030

shwstppr · 2021-05-19T11:01:52Z

Description

Refactors:

UserVmManagerImpl::migrateVirtualMachineWithVolume
Boolean HypervisorCapabilitiesDao::isStorageMotionSupported to boolean HypervisorCapabilitiesDao::isStorageMotionSupported for simplifying callers.
StorageManager::isStoragePoolComplaintWithStoragePolicy to StorageManager::isStoragePoolCompliantWithStoragePolicy

Update log messages.

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?

Refactors Boolean HypervisorCapabilitiesDao::isStorageMotionSupported to boolean HypervisorCapabilitiesDao::isStorageMotionSupported for simplifying callers. Refactors log messages. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

sureshanaparti

code LGTM

server/src/main/java/com/cloud/storage/StorageManagerImpl.java

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

GutoVeronezi · 2021-05-24T13:47:10Z

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

+        }
+
+        // Check that Vm does not have VM Snapshots
+        if (_vmSnapshotDao.findByVm(vmId).size() > 0) {


We could assign the result of the method to a var and use CollectionUtils to validate it.

VMSnapshotDao::findByVm will always return a non-null list, I don't see an issue with directly checking the list size here.

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2021-05-25T08:15:46Z

@blueorangutan package

blueorangutan · 2021-05-25T08:17:02Z

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

rohityadavcloud · 2021-06-18T01:13:39Z

@blueorangutan package

blueorangutan · 2021-06-18T01:14:03Z

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

blueorangutan · 2021-06-18T02:26:17Z

Packaging result: ✖️ centos7 ✔️ centos8 ✖️ debian. SL-JID 270

shwstppr · 2021-06-18T12:38:58Z

@blueorangutan package

blueorangutan · 2021-06-18T12:40:03Z

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

blueorangutan · 2021-06-18T13:22:06Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 294

blueorangutan · 2021-06-19T01:54:56Z

Trillian test result (tid-1008)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36476 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5030-t1008-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 88 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2021-06-21T08:29:24Z

@GutoVeronezi I resolved some of your comments on outdated code. Are all your questions/comments addressed (i.e. lgty?)

GutoVeronezi

LGTM, I've did not test it though

GutoVeronezi · 2021-06-21T13:24:14Z

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

    private static final ConfigKey<Boolean> VmDestroyForcestop = new ConfigKey<Boolean>("Advanced", Boolean.class, "vm.destroy.forcestop", "false",
            "On destroy, force-stop takes this value ", true);

+    public static final List<HypervisorType> VM_STORAGEMIGRATION_SUPPORTING_HYPERVISORS = new ArrayList<>(Arrays.asList(


I think it would be better as VM_STORAGE_MIGRATION_SUPPORTING_HYPERVISORS.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

blueorangutan · 2021-06-24T08:37:03Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 349

blueorangutan · 2021-06-28T06:58:31Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 385

rohityadavcloud · 2021-06-29T07:27:21Z

@blueorangutan test matrix

blueorangutan · 2021-06-29T07:28:05Z

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

blueorangutan · 2021-06-29T21:21:43Z

Trillian test result (tid-1153)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47203 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5030-t1153-vmware-65u2.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Smoke tests completed. 86 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_invalid_upgrade_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_02_deploy_and_upgrade_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_04_basic_lifecycle_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_06_deploy_invalid_kubernetes_ha_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_01_migrate_VM_and_root_volume	`Error`	113.60	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	105.41	test_vm_life_cycle.py

blueorangutan · 2021-06-29T22:08:59Z

Trillian test result (tid-1151)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 50758 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5030-t1151-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_domain_vpc_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_pvlan.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Smoke tests completed. 86 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_invalid_upgrade_kubernetes_cluster	`Failure`	3611.93	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	135.96	test_kubernetes_clusters.py
test_create_pvlan_network	`Error`	0.03	test_pvlan.py

blueorangutan · 2021-06-29T22:35:21Z

Trillian test result (tid-1152)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 51740 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5030-t1152-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_accounts.py
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_projects.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 87 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_invalid_upgrade_kubernetes_cluster	`Failure`	3037.69	test_kubernetes_clusters.py
test_02_deploy_and_upgrade_kubernetes_cluster	`Failure`	3607.36	test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	0.05	test_kubernetes_clusters.py
test_04_basic_lifecycle_kubernetes_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster	`Failure`	0.05	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster	`Failure`	0.04	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	0.04	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	82.67	test_kubernetes_clusters.py

nvazquez · 2021-07-13T02:04:51Z

@blueorangutan package

blueorangutan · 2021-07-13T02:05:03Z

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

blueorangutan · 2021-07-13T03:00:17Z

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

nvazquez · 2021-07-13T03:02:36Z

@blueorangutan test matrix

blueorangutan · 2021-07-13T03:03: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-07-13T13:36:55Z

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

Test	Result	Time (s)	Test File

blueorangutan · 2021-07-13T18:14:08Z

Trillian test result (tid-1243)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 52567 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5030-t1243-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 87 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	672.49	test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	`Error`	313.98	test_vpc_redundant.py

shwstppr added 3 commits May 19, 2021 16:28

simplify

c4976c8

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

refactor

f2cddde

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

apache deleted a comment from blueorangutan May 19, 2021

shwstppr marked this pull request as draft May 19, 2021 12:15

changes

b266b58

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

sureshanaparti approved these changes May 20, 2021

View reviewed changes

shwstppr closed this May 24, 2021

shwstppr reopened this May 24, 2021

shwstppr marked this pull request as ready for review May 24, 2021 08:39

GutoVeronezi reviewed May 24, 2021

View reviewed changes

review comments addressed

0d26afa

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

apache deleted a comment from blueorangutan May 25, 2021

shwstppr changed the title ~~refactor: migrate with storage host capability check~~ refactor: migrate vm with storage May 25, 2021

svenvogel added the type:cleanup label May 27, 2021

GutoVeronezi approved these changes Jun 21, 2021

View reviewed changes

var rename

2c6e126

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

DaanHoogland added this to the 4.16.0.0 milestone Jun 29, 2021

shwstppr closed this Jul 8, 2021

shwstppr reopened this Jul 8, 2021

rohityadavcloud merged commit 50a1697 into apache:main Jul 15, 2021

shwstppr mentioned this pull request Aug 6, 2021

Fix migration issue in UserVmManagerImpl.migrateVirtualMachineWithVolume #5288

Merged

10 tasks

refactor: migrate vm with storage #5030

refactor: migrate vm with storage #5030

Uh oh!

Conversation

shwstppr commented May 19, 2021 • 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?

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GutoVeronezi May 24, 2021

Choose a reason for hiding this comment

Uh oh!

shwstppr May 25, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shwstppr commented May 25, 2021

Uh oh!

blueorangutan commented May 25, 2021

Uh oh!

rohityadavcloud commented Jun 18, 2021

Uh oh!

blueorangutan commented Jun 18, 2021

Uh oh!

blueorangutan commented Jun 18, 2021

Uh oh!

shwstppr commented Jun 18, 2021

Uh oh!

blueorangutan commented Jun 18, 2021

Uh oh!

blueorangutan commented Jun 18, 2021

Uh oh!

blueorangutan commented Jun 19, 2021

Uh oh!

DaanHoogland commented Jun 21, 2021

Uh oh!

GutoVeronezi left a comment • 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!

GutoVeronezi Jun 21, 2021

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jun 24, 2021

Uh oh!

blueorangutan commented Jun 28, 2021

Uh oh!

rohityadavcloud commented Jun 29, 2021

Uh oh!

blueorangutan commented Jun 29, 2021

Uh oh!

blueorangutan commented Jun 29, 2021

Uh oh!

blueorangutan commented Jun 29, 2021

Uh oh!

blueorangutan commented Jun 29, 2021

Uh oh!

nvazquez commented Jul 13, 2021

Uh oh!

blueorangutan commented Jul 13, 2021

Uh oh!

blueorangutan commented Jul 13, 2021

Uh oh!

nvazquez commented Jul 13, 2021

Uh oh!

blueorangutan commented Jul 13, 2021

Uh oh!

blueorangutan commented Jul 13, 2021

Uh oh!

shwstppr commented May 19, 2021 •

edited

Loading

GutoVeronezi left a comment •

edited

Loading