Added hot plugging of vifs in case of VMware isolated networks #5912

harikrishna-patnala · 2022-01-31T11:03:25Z

Description

This PR fixes #5716 where isolated network VR failed to deploy when Vmxnet3 adapter type is used and when router.extra.public.nics is not 0. This is because of the extra nics that are reserved in the VR based on the global setting router.extra.public.nics.

To address the issue we have now implemented nic hot plugging for isolated networks because systemvm templates had open-vm-tools installed and support hot plugging which is what is already used in the case of VPC VR.

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?

Steps to reproduce:

Set the global setting vmware.systemvm.nic.device.type to Vmxnet3 and router.extra.public.nics to some value (not 0)
Create an isolated network and VM in it.
VR will stuck in starting state

After the fix, following are the test cases that are performed

Set the global setting vmware.systemvm.nic.device.type to Vmxnet3. router.extra.public.nics is not used in VMware anymore.
Create an isolated network N1 and VM in it
Add a new IP range with different vlan under "Physical Network Guest Public".
Aquire an IP from the above range in the network N1
Enable staticnat on that IP
Observed VR interfaces a new interface is created dynamically

Followed the same steps with vmware.systemvm.nic.device.type value E1000 and VR came up and subsequent operations of acquiring IP, enabling static NAT worked fine.

…g router.extra.public.nics in case of VMware. Added hot plugging of vifs whenever required in case of isolated networks. VPC networks already uses hotplugging of vifs.

harikrishna-patnala · 2022-01-31T11:05:59Z

@blueorangutan package

blueorangutan · 2022-01-31T11:07:03Z

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

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

blueorangutan · 2022-01-31T12:03:14Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2402

sureshanaparti · 2022-01-31T13:34:34Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2022-01-31T13:35:03Z

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

borisstoyanov

LGTM, manually tested

harikrishna-patnala · 2022-02-03T05:47:56Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2022-02-03T05:49:03Z

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

weizhouapache · 2022-02-03T08:45:27Z

@blueorangutan package

blueorangutan · 2022-02-03T08:46:03Z

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

blueorangutan · 2022-02-03T09:34:36Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2444

DaanHoogland · 2022-02-03T12:53:34Z

@blueorangutan test

blueorangutan · 2022-02-03T12:54:02Z

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

blueorangutan · 2022-02-03T22:25:46Z

Trillian test result (tid-3143)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32926 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5912-t3143-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2022-02-03T23:51:43Z

Trillian test result (tid-3144)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36943 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5912-t3144-vmware-67u3.zip
Smoke tests completed. 91 look OK, 1 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_08_reboot_cpvm	`Failure`	26.75	test_ssvm.py

yadvr

LGTM - manual testing to confirm change required

yadvr · 2022-02-04T07:32:14Z

@harikrishna-patnala can you check/address any outstanding review comments
@blueorangutan package

blueorangutan · 2022-02-04T07:34:03Z

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

blueorangutan · 2022-02-04T08:27:11Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2450

yadvr · 2022-02-04T08:44:09Z

@blueorangutan test centos7 vmware-67u3

weizhouapache

@harikrishna-patnala
I have tested many scenarios with isolated networks and vpcs.

all looks ok, except an issue: #5934
I think it is not related to this PR.

harikrishna-patnala · 2022-02-04T10:35:31Z

Thanks for all the testing @weizhouapache @rohityadavcloud @borisstoyanov
@rohityadavcloud no outstanding review comments to address.

DaanHoogland · 2022-02-04T11:08:55Z

@harikrishna-patnala I have tested many scenarios with isolated networks and vpcs.

all looks ok, except an issue: #5934 I think it is not related to this PR.

did you test that issue in a generic 4.16, @weizhouapache ?

weizhouapache · 2022-02-04T11:39:17Z

@harikrishna-patnala I have tested many scenarios with isolated networks and vpcs.
all looks ok, except an issue: #5934 I think it is not related to this PR.

did you test that issue in a generic 4.16, @weizhouapache ?

@DaanHoogland
yes, 4.16 does not support both hotplug and hot-unplug.
with this pr, hotplug is supported.
we could merge this at first, then support hot-unplug afterwards.

DaanHoogland · 2022-02-04T11:41:34Z

good merging...

Remove code that reserves extra interfaces based on the global settin…

31742cc

…g router.extra.public.nics in case of VMware. Added hot plugging of vifs whenever required in case of isolated networks. VPC networks already uses hotplugging of vifs.

harikrishna-patnala added this to the 4.16.1.0 milestone Jan 31, 2022

harikrishna-patnala requested review from weizhouapache and yadvr January 31, 2022 11:04

harikrishna-patnala added component:advanced-networking component:virtual-router component:vmware labels Jan 31, 2022

sureshanaparti added the status:needs-testing label Jan 31, 2022

borisstoyanov self-requested a review January 31, 2022 11:24

sureshanaparti assigned borisstoyanov Jan 31, 2022

sureshanaparti reviewed Jan 31, 2022

View reviewed changes

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java Show resolved Hide resolved

weizhouapache self-assigned this Feb 1, 2022

sureshanaparti linked an issue Feb 1, 2022 that may be closed by this pull request

Isolated Network VR with Vmxnet3 fails deployment when router.extra.public.nics is not 0 #5716

Closed

borisstoyanov approved these changes Feb 1, 2022

View reviewed changes

borisstoyanov removed their assignment Feb 1, 2022

yadvr approved these changes Feb 4, 2022

View reviewed changes

apache deleted a comment from blueorangutan Feb 4, 2022

weizhouapache approved these changes Feb 4, 2022

View reviewed changes

DaanHoogland merged commit 56a8852 into 4.16 Feb 4, 2022

DaanHoogland mentioned this pull request Feb 4, 2022

Isolated Network VR with Vmxnet3 fails deployment when router.extra.public.nics is not 0 #5716

Closed

weizhouapache mentioned this pull request Feb 21, 2022

Synchronization of network devices on newly added hosts for Persistent Networks #5977

Merged

7 tasks

Added hot plugging of vifs in case of VMware isolated networks #5912

Added hot plugging of vifs in case of VMware isolated networks #5912

Uh oh!

Conversation

harikrishna-patnala commented Jan 31, 2022 • 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!

harikrishna-patnala commented Jan 31, 2022

Uh oh!

blueorangutan commented Jan 31, 2022

Uh oh!

Uh oh!

blueorangutan commented Jan 31, 2022

Uh oh!

sureshanaparti commented Jan 31, 2022

Uh oh!

blueorangutan commented Jan 31, 2022

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnala commented Feb 3, 2022

Uh oh!

blueorangutan commented Feb 3, 2022

Uh oh!

weizhouapache commented Feb 3, 2022

Uh oh!

blueorangutan commented Feb 3, 2022

Uh oh!

blueorangutan commented Feb 3, 2022

Uh oh!

DaanHoogland commented Feb 3, 2022

Uh oh!

blueorangutan commented Feb 3, 2022

Uh oh!

blueorangutan commented Feb 3, 2022

Uh oh!

blueorangutan commented Feb 3, 2022

Uh oh!

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

yadvr commented Feb 4, 2022

Uh oh!

blueorangutan commented Feb 4, 2022

Uh oh!

blueorangutan commented Feb 4, 2022

Uh oh!

yadvr commented Feb 4, 2022

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnala commented Feb 4, 2022

Uh oh!

DaanHoogland commented Feb 4, 2022

Uh oh!

weizhouapache commented Feb 4, 2022

Uh oh!

DaanHoogland commented Feb 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

harikrishna-patnala commented Jan 31, 2022 •

edited

Loading

yadvr left a comment •

edited

Loading