Uh oh!
There was an error while loading. Please reload this page.
CLOUDSTACK-9491: incorrect parsing of device list to find ethernet index of plugged NIC - #1681
Merged
Conversation
serg38
commented
Sep 21, 2016
@murali-reddy This issue might be related to PR1660. Do you mind testing against latest master to see if you it is addressed already? |
murali-reddy
commented
Sep 23, 2016
ContributorAuthor
@serg38 Though same code path, i ran into different issue. When i was adding new tier to VPC, eth3 was plugged into VR from vCenter. As commented in the function there is delay for ethernet device to appear in the guest instance. I could see code loops through eth0, eth1, eth2 only, because it read the '/proc/sys/net/ipv4/conf' too early. |
serg38
commented
Sep 23, 2016
@murali-reddy Thanks for the explanation. LGTM for code review |
abhinandanprateek
commented
Sep 23, 2016
Contributor
The fix looks like the correct thing to do. |
sateesh-chodapuneedi
commented
Sep 23, 2016
Member
@murali-reddy Makes sense to get fresh listing of virtual NICs in next iteration as the first iteration might have missed the hot-plugged virtual NIC (for VPC tier being configured). Though one side effect is we will loop through all the virtual NICs, except the newly hot-plugged NIC, at least one more time. |
jburwell
commented
Sep 26, 2016
Contributor
@murali-reddy have you had a chance to address @sateesh-chodapuneedi comments? |
jburwell
commented
Sep 26, 2016
Contributor
@blueorangutan package |
blueorangutan
commented
Sep 26, 2016
@jburwell a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Sep 26, 2016
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-25 |
jburwell
commented
Sep 26, 2016
Contributor
@blueorangutan test centos7 vmware55u3 |
blueorangutan
commented
Sep 26, 2016
@jburwell a Trillian-Jenkins test job (centos7 mgmt + vmware55u3) has been kicked to run smoke tests |
jburwell
commented
Sep 26, 2016
Contributor
@murali-reddy is there a way to create or update a Marvin test case to verify this fix? |
murali-reddy
commented
Sep 26, 2016
ContributorAuthor
@sateesh-chodapuneedi added optimization for now, but its better we eliminate the whole logic of finding device index from the VR. |
murali-reddy
commented
Sep 27, 2016
ContributorAuthor
@jburwell function 'findRouterEthDeviceIndex' which is modified, is invoked when ever a new tier to VPC is added (virtual NIC is hot-plugged). Many VPC tests run againest the VMware environment will cover this scenario. |
murali-reddy
commented
Sep 27, 2016
ContributorAuthor
@blueorangutan test centos7 vmware55u3 |
blueorangutan
commented
Sep 27, 2016
@murali-reddy a Trillian-Jenkins test job (centos7 mgmt + vmware55u3) has been kicked to run smoke tests |
blueorangutan
commented
Sep 27, 2016
Trillian test result (tid-58)
|
sateesh-chodapuneedi
commented
Sep 28, 2016
Member
Code LGTM, 👍 +1 |
yadvr
commented
Sep 28, 2016
Member
LGTM, some test failures have been addressed here: #1692 |
jburwell
commented
Oct 17, 2016
Contributor
@murali-reddy could you please squash your commits? |
yadvr
commented
Oct 21, 2016
Member
@murali-reddy can you squash your commits |
blueorangutan
commented
Oct 21, 2016
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Oct 21, 2016
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-86 |
…dex of plugged NIC In VmwareResource, findRouterEthDeviceIndex() method find ethernet interface index given the mac address. This method is used, once a nic is plugged to determine ethernet interface. "/proc/sys/net/ipv4/conf" from the VR and looped through the devices to find the right ethernet interface. However current logic read it once, and loops through the device list. Its observerd device may not show up '/proc/sys/net/ipv4/conf' immediatly once NIC is plugged in the VM from vCenter.Fix ensured, while waiting for 15 sec in the loop, read the latest content from /proc/sys/net/ipv4/conf, so that right device list is processed.
murali-reddy
commented
Oct 28, 2016
ContributorAuthor
@blueorangutan package |
blueorangutan
commented
Oct 28, 2016
@murali-reddy a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Oct 28, 2016
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-114 |
murali-reddy
commented
Oct 28, 2016
ContributorAuthor
@blueorangutan test centos7 vmware55u3 |
blueorangutan
commented
Oct 28, 2016
@murali-reddy a Trillian-Jenkins test job (centos7 mgmt + vmware55u3) has been kicked to run smoke tests |
murali-reddy
commented
Oct 31, 2016
ContributorAuthor
@blueorangutan test centos7 vmware55u3 |
blueorangutan
commented
Oct 31, 2016
@murali-reddy a Trillian-Jenkins test job (centos7 mgmt + vmware55u3) has been kicked to run smoke tests |
jburwell
commented
Nov 1, 2016
Contributor
@rhtyd could you check into the status of the test run for this PR? |
yadvr
commented
Nov 1, 2016
Member
Invalid params were provided, the hypervisor to be passed was vmware-55u3. I'll fix this in blueorangutan as it currently does not check/validate the hypervisor name properly. Kicked a new job manually from Jenkins. |
blueorangutan
commented
Nov 1, 2016
Trillian test result (tid-232)
|
jburwell
commented
Nov 1, 2016
Contributor
@murali-reddy could you please investigate the following test case failures:
|
jburwell
commented
Nov 17, 2016
Contributor
@murali-reddy have you had a chance to investigate the test failures? |
yadvr
commented
Nov 21, 2016
Member
LGTM on tests, the failures wrt VPC are known intermittent issues. I'm not sure about the internal lb failures, I'll kick another test round since there have been several changes on the base branch. @blueorangutan package |
blueorangutan
commented
Nov 21, 2016
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Nov 21, 2016
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-226 |
yadvr
commented
Nov 21, 2016
Member
@blueorangutan test centos7 vmware55u3 |
blueorangutan
commented
Nov 21, 2016
@rhtyd unsupported parameters provided. Supported mgmt server os are: |
yadvr
commented
Nov 21, 2016
Member
@blueorangutan test centos7 vmware-55u3 |
blueorangutan
commented
Nov 21, 2016
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
blueorangutan
commented
Nov 21, 2016
Trillian test result (tid-378)
|
yadvr
commented
Nov 23, 2016
Member
asfgit pushed a commit
that referenced
this pull request
Nov 23, 2016
CLOUDSTACK-9491: incorrect parsing of device list to find ethernet index of plugged NICIn VmwareResource, findRouterEthDeviceIndex() method find ethernet interface index given the mac address. This method is used, once a nic is plugged to determine ethernet interface. "/proc/sys/net/ipv4/conf" from the VR and looped through the devices to find the right ethernet interface. Howver current logic read it once, and loops through the device list. Its observerd device may not show up '/proc/sys/net/ipv4/conf' immediatly once NIC is plugged in the VM from vCenter. Fix ensured, while waiting for 15 sec in the loop, read the latest content from /proc/sys/net/ipv4/conf , so that right device list is processed. Manual tested VPC scenarios of adding new tiers which uses findRouterEthDeviceIndex, to find the guest/public network ethernet index. * pr/1681: CLOUDSTACK-9491: incorrect parsing of device list to find ethernet index of plugged NIC Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In VmwareResource, findRouterEthDeviceIndex() method find ethernet interface index given
the mac address. This method is used, once a nic is plugged to determine ethernet interface.
"/proc/sys/net/ipv4/conf" from the VR and looped through the devices to find the right
ethernet interface. Howver current logic read it once, and loops through the device list.
Its observerd device may not show up '/proc/sys/net/ipv4/conf' immediatly once NIC is plugged
in the VM from vCenter.
Fix ensured, while waiting for 15 sec in the loop, read the latest content from /proc/sys/net/ipv4/conf
, so that right device list is processed.
Manual tested VPC scenarios of adding new tiers which uses findRouterEthDeviceIndex, to find the guest/public network ethernet index.