vmware: fix inter-cluster stopped vm and volume migration by shwstppr · Pull Request #4895 · apache/cloudstack

shwstppr · 2021-04-05T11:26:07Z

Description

For inter-cluster migration without shared storage, VMware needs a host to be specified. Fix is to specify an appropriate host in the target cluster during a stopped VM migration. Also, find target datastore using the host in the target cluster.

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?

Fixes apache#4838 For inter-cluster migration without shared storage, VMware needs a host to be specified. Fix is to specify an appropriate host in the target cluster. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

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

shwstppr · 2021-04-06T07:40:28Z

@blueorangutan package

blueorangutan · 2021-04-06T07:41:02Z

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

blueorangutan · 2021-04-06T08:19:12Z

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

shwstppr · 2021-04-06T08:32:52Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-04-06T08:34:02Z

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

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

DaanHoogland

mostly structural change requests so far.

DaanHoogland · 2021-04-06T10:17:41Z

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java

+            List<VolumeVO> volumes = _volumeDao.findByInstanceAndType(vmId, Volume.Type.ROOT);
+            if (CollectionUtils.isNotEmpty(volumes)) {
+                VolumeVO rootVolume = volumes.get(0);
+                if (rootVolume.getPoolId() != null) {
+                    StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId());
+                    if (pool != null && pool.getClusterId() != null) {
+                        clusterId = pool.getClusterId();
+                    }
+                }
+            }


please extract into a separate method. i.e. Long getPoolFromRootVolume(...)

@DaanHoogland done

DaanHoogland · 2021-04-06T10:19:54Z

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java

+        final boolean isInterClusterMigration = srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId);
+        Host hostInTargetCluster = null;
+        if (isInterClusterMigration) {
+            // Without host vMotion might fail between non-shared storages with error similar to,
+            // https://kb.vmware.com/s/article/1003795
+            // As this is offline migration VM won't be started on this host
+            List<HostVO> hosts = _hostDao.findHypervisorHostInCluster(destClusterId);
+            if (CollectionUtils.isNotEmpty(hosts)) {
+                hostInTargetCluster = hosts.get(0);
+            }
+            if (hostInTargetCluster == null) {
+                throw new CloudRuntimeException("Migration failed, unable to find suitable target host for VM placement while migrating between storage pools of different clusters without shared storages");
+            }
+        }
+        MigrateVmToPoolCommand migrateVmToPoolCommand = new MigrateVmToPoolCommand(vm.getInstanceName(),
+                vols, destination.getUuid(), hostInTargetCluster == null ? null : hostInTargetCluster.getGuid(), true);
+        commands.add(migrateVmToPoolCommand);


please restructure this in smaller methods

@DaanHoogland done

DaanHoogland · 2021-04-06T10:22:14Z

...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

-    private Answer migrateAndAnswer(VirtualMachineMO vmMo, String poolUuid, VmwareHypervisorHost hyperHost, Command cmd) throws Exception {
-        ManagedObjectReference morDs = getTargetDatastoreMOReference(poolUuid, hyperHost);
-
+    private Answer migrateAndAnswer(VirtualMachineMO vmMo, String poolUuid, VmwareHypervisorHost sourceHyperHost,


please restructure this method (at least extract the new pieces)

@DaanHoogland done

DaanHoogland · 2021-04-06T10:23:01Z

...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

            morDcOfTargetHost = tgtHyperHost.getHyperHostDatacenter();
            if (!morDc.getValue().equalsIgnoreCase(morDcOfTargetHost.getValue())) {
-                String msg = "Source host & target host are in different datacentesr";
+                String msg = "Source host & target host are in different datacenter";


DaanHoogland · 2021-04-06T10:23:27Z

...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

+        VmwareHypervisorHost hyperHostInTargetCluster = null;
+        if (cmd.getHostGuidInTargetCluster() != null) {
+            hyperHostInTargetCluster = VmwareHelper.getHostMOFromHostName(getServiceContext(), cmd.getHostGuidInTargetCluster());
+        }
+        VmwareHypervisorHost targetDSHost = hyperHostInTargetCluster != null ? hyperHostInTargetCluster : hyperHost;


this merrits a call to a specific method

@DaanHoogland which specific method? I didn't get you

something like

Suggested change

VmwareHypervisorHost hyperHostInTargetCluster = null;

if (cmd.getHostGuidInTargetCluster() != null) {

hyperHostInTargetCluster = VmwareHelper.getHostMOFromHostName(getServiceContext(), cmd.getHostGuidInTargetCluster());

}

VmwareHypervisorHost targetDSHost = hyperHostInTargetCluster != null ? hyperHostInTargetCluster : hyperHost;

VmwareHypervisorHost targetDSHost = getTargetHostFromTargetCluster(cmd);

and

Suggested change

VmwareHypervisorHost hyperHostInTargetCluster = null;

if (cmd.getHostGuidInTargetCluster() != null) {

hyperHostInTargetCluster = VmwareHelper.getHostMOFromHostName(getServiceContext(), cmd.getHostGuidInTargetCluster());

}

VmwareHypervisorHost targetDSHost = hyperHostInTargetCluster != null ? hyperHostInTargetCluster : hyperHost;

private VmwareHypervisorHost getTargetHostFromTargetCluster(MigrateVolumeCmd cmd) {

VmwareHypervisorHost hyperHostInTargetCluster = null;

if (cmd.getHostGuidInTargetCluster() != null) {

hyperHostInTargetCluster = VmwareHelper.getHostMOFromHostName(getServiceContext(), cmd.getHostGuidInTargetCluster());

}

return hyperHostInTargetCluster != null ? hyperHostInTargetCluster : hyperHost;

}

@DaanHoogland both hyperHostInTargetCluster and targetDSHost are used in the method later multiple times. I used two different variables to prevent using if..else multiple times.

DaanHoogland · 2021-04-06T10:24:42Z

...ns/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

+            String hardwareVersion = null;
+            if (hyperHostInTargetCluster != null) {
+                Integer sourceHardwareVersion = HypervisorHostHelper.getHostHardwareVersion(hyperHost);
+                Integer destinationHardwareVersion = HypervisorHostHelper.getHostHardwareVersion(hyperHostInTargetCluster);
+                if (sourceHardwareVersion != null && destinationHardwareVersion != null && !sourceHardwareVersion.equals(destinationHardwareVersion)) {
+                    hardwareVersion = String.valueOf(Math.min(sourceHardwareVersion, destinationHardwareVersion));
+                }
+            }


please extract

@DaanHoogland done

DaanHoogland · 2021-04-06T10:27:44Z

...s/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java

+        ScopeType targetScopeType = destData.getDataStore().getScope().getScopeType();
+        Long hostId = null;
+        String hostGuidInTargetCluster = null;
+        if (ScopeType.CLUSTER.equals(sourceScopeType)) {
+            // Find Volume source cluster and select any Vmware hypervisor host to attach worker VM
+            hostId = findSuitableHostIdForWorkerVmPlacement(sourcePool.getClusterId());
+            if (hostId == null) {
+                throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + sourcePool.getName());
+            }
+            if (ScopeType.CLUSTER.equals(targetScopeType) && !sourcePool.getClusterId().equals(targetPool.getClusterId())) {
+                // Without host vMotion might fail between non-shared storages with error similar to,
+                // https://kb.vmware.com/s/article/1003795
+                List<HostVO> hosts = hostDao.findHypervisorHostInCluster(targetPool.getClusterId());
+                if (CollectionUtils.isNotEmpty(hosts)) {
+                    hostGuidInTargetCluster = hosts.get(0).getGuid();
+                }
+                if (hostGuidInTargetCluster == null) {
+                    throw new CloudRuntimeException("Offline Migration failed, unable to find suitable target host for worker VM placement while migrating between storage pools of different cluster without shared storages");
+                }
+            }
+        } else if (ScopeType.CLUSTER.equals(targetScopeType)) {
+            hostId = findSuitableHostIdForWorkerVmPlacement(targetPool.getClusterId());
+            if (hostId == null) {
+                throw new CloudRuntimeException("Offline Migration failed, unable to find suitable host for worker VM placement in the cluster of storage pool: " + targetPool.getName());
+            }
+        }


please extract getTargetScopeType(..) and possibly more methods from that.

@DaanHoogland done

blueorangutan · 2021-04-06T19:13:39Z

Trillian test result (tid-357)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37216 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4895-t357-vmware-67u3.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. 84 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_deploy_kubernetes_cluster	`Failure`	91.50	test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster	`Failure`	82.93	test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	61.53	test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster	`Failure`	65.72	test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster	`Failure`	71.91	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	80.17	test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster	`Failure`	71.87	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	66.15	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	110.82	test_kubernetes_clusters.py
ContextSuite context=TestVAppsVM>:setup	`Error`	43.81	test_vm_life_cycle.py

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

shwstppr · 2021-04-07T06:45:02Z

@blueorangutan package

blueorangutan · 2021-04-07T06:46:03Z

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

blueorangutan · 2021-04-07T07:10:14Z

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

shwstppr · 2021-04-07T11:26:34Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-04-07T11:27:04Z

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

blueorangutan · 2021-04-07T22:34:01Z

Trillian test result (tid-370)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38520 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4895-t370-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 83 look OK, 4 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_10_traceroute_in_vr	`Failure`	61.23	test_diagnostics.py
test_01_deploy_kubernetes_cluster	`Failure`	96.60	test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster	`Failure`	99.39	test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	65.70	test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster	`Failure`	64.58	test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster	`Failure`	58.55	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	54.61	test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster	`Failure`	66.72	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	82.07	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	121.47	test_kubernetes_clusters.py
ContextSuite context=TestVAppsVM>:setup	`Error`	44.65	test_vm_life_cycle.py
test_01_redundant_vpc_site2site_vpn	`Failure`	373.50	test_vpc_vpn.py

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java

vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java

VM can have multiple ROOT volumes and some can be on zone-wide store therefore iterate over all of them till a cluster ID is found. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2021-04-08T05:05:14Z

@blueorangutan package

blueorangutan · 2021-04-08T05:06:02Z

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

blueorangutan · 2021-04-08T05:27:39Z

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

borisstoyanov · 2021-04-08T07:46:48Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-04-08T07:48:02Z

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

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

shwstppr · 2021-04-08T10:13:57Z

@blueorangutan package

blueorangutan · 2021-04-08T10:15:02Z

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

blueorangutan · 2021-04-08T10:44:20Z

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

borisstoyanov · 2021-04-08T11:51:06Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-04-08T11:52:02Z

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

blueorangutan · 2021-04-08T21:53:48Z

Trillian test result (tid-385)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35028 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4895-t385-vmware-67u3.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. 85 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_deploy_kubernetes_cluster	`Failure`	89.55	test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster	`Failure`	56.36	test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	55.27	test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster	`Failure`	56.29	test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster	`Failure`	53.22	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	100.16	test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster	`Failure`	59.30	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	57.29	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	101.95	test_kubernetes_clusters.py
ContextSuite context=TestVAppsVM>:setup	`Error`	45.65	test_vm_life_cycle.py

borisstoyanov

LGTM, tested on VMware with single and successive migration of VM

yadvr · 2021-04-09T07:06:50Z

@nvazquez are you lgtm on the PR? Will hold merging until your confirmation.

nvazquez

Thanks @shwstppr for addressing the comments, LGTM

yadvr · 2021-04-10T07:52:44Z

Merged based on Bobby, Nicolas's review and testing, and smoketests.

vmware: fix inter-cluster stopped vm migration

54ef6e7

Fixes apache#4838 For inter-cluster migration without shared storage, VMware needs a host to be specified. Fix is to specify an appropriate host in the target cluster. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr added this to the 4.15.1.0 milestone Apr 5, 2021

yadvr requested review from DaanHoogland, harikrishna-patnala and nvazquez April 5, 2021 11:49

fix detached volume inter-cluster migration

7cc6b6e

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

shwstppr changed the title ~~vmware: fix inter-cluster stopped vm migration~~ vmware: fix inter-cluster stopped vm and volume migration Apr 6, 2021

shwstppr marked this pull request as ready for review April 6, 2021 07:40

cleanup unused method

ee6aacd

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

DaanHoogland reviewed Apr 6, 2021

View reviewed changes

yadvr assigned nvazquez Apr 6, 2021

shwstppr added 2 commits April 7, 2021 11:54

review changes

db2363d

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

changes

e172623

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

shwstppr mentioned this pull request Apr 7, 2021

vmware: fix stopped VM volume migration #4758

Merged

12 tasks

nvazquez reviewed Apr 8, 2021

View reviewed changes

find vm clusterid with multiple ROOT volumes

9973e57

VM can have multiple ROOT volumes and some can be on zone-wide store therefore iterate over all of them till a cluster ID is found. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

fix successive storage migration

2356894

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

borisstoyanov approved these changes Apr 9, 2021

View reviewed changes

yadvr approved these changes Apr 9, 2021

View reviewed changes

yadvr requested a review from nvazquez April 9, 2021 07:38

nvazquez approved these changes Apr 9, 2021

View reviewed changes

yadvr merged commit fdefee7 into apache:4.15 Apr 10, 2021

yadvr mentioned this pull request Apr 10, 2021

VMware: VM operations fail after inter-cluster storage migration #4838

Closed

weizhouapache mentioned this pull request Jul 22, 2021

Migration not working in ACS 4.14 with Basic Network - VMware Local storage #4187

Closed

Conversation

shwstppr commented Apr 5, 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!

shwstppr commented Apr 6, 2021

Uh oh!

blueorangutan commented Apr 6, 2021

Uh oh!

blueorangutan commented Apr 6, 2021

Uh oh!

shwstppr commented Apr 6, 2021

Uh oh!

blueorangutan commented Apr 6, 2021

Uh oh!

DaanHoogland left a comment

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!

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!

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!

blueorangutan commented Apr 6, 2021

Uh oh!

shwstppr commented Apr 7, 2021

Uh oh!

blueorangutan commented Apr 7, 2021

Uh oh!

blueorangutan commented Apr 7, 2021

Uh oh!

shwstppr commented Apr 7, 2021

Uh oh!

blueorangutan commented Apr 7, 2021

Uh oh!

blueorangutan commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shwstppr commented Apr 8, 2021

Uh oh!

blueorangutan commented Apr 8, 2021

Uh oh!

blueorangutan commented Apr 8, 2021

Uh oh!

borisstoyanov commented Apr 8, 2021

Uh oh!

blueorangutan commented Apr 8, 2021

shwstppr commented Apr 5, 2021 •

edited

Loading

yadvr commented Apr 9, 2021 •

edited

Loading