Uh oh!
There was an error while loading. Please reload this page.
CLOUDSTACK-9339 Virtual Routers don't handle Multiple Public Interfaces correctly - #1659
Merged
Conversation
| "-A PREROUTING -s %s/32 -m state --state NEW -j MARK --set-xmark 0x%s/0xffffffff" % \ | ||
| (rule["internal_ip"], device[len("eth"):])]) | ||
| self.fw.append(["mangle", "", | ||
| "-A PREROUTING -s %s/32 -m state --state NEW -j CONNMARK --save-mark --nfmask 0xffffffff --ctmask 0xffffffff" % \ |
Contributor
There was a problem hiding this comment.
Is there value in extracting the 0xffffffff to a constant?
QuiteClose
commented
Aug 28, 2016
Contributor
@murali-reddy - thank you for picking this up. Let me know when you think it's appropriate to close PR #1519 and I shall do so. |
jburwell
commented
Aug 29, 2016
Contributor
@dsclose what is your opinion of the changes in this PR relative to your work? Do you see any gaps or missing pieces? |
jburwell
commented
Aug 29, 2016
Contributor
@murali-reddy Looking through the Marvin tests, there is a lot of boilerplate code and duplicated setup in test cases. It feels like there is an opportunity to introduce one or more base classes that would not only reduce the duplication/boilerplate, but make building future VR test cases easier. Do you agree? |
jburwell
commented
Sep 18, 2016
Contributor
@murali-reddy what is the status of addressing the review feedback. Also, have addressed the VPC-related issues as well? |
murali-reddy
commented
Sep 22, 2016
ContributorAuthor
@blueorangutan package |
blueorangutan
commented
Sep 22, 2016
@murali-reddy a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Sep 22, 2016
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-20 |
murali-reddy
commented
Sep 22, 2016
ContributorAuthor
@blueorangutan test |
blueorangutan
commented
Sep 22, 2016
@murali-reddy a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
blueorangutan
commented
Sep 23, 2016
Trillian test result (#21)
|
jburwell
commented
Sep 26, 2016
Contributor
@murali-reddy 8 errors occurred in the VMware test run. Could you please investigate these failures? |
murali-reddy
commented
Sep 26, 2016
ContributorAuthor
@jburwell i am investigating them. Just so you know, blueorangutan is running full smoke test suite. Similar failures are seen in other PR as well. So some of the tests may have been failing for some time and need to be fixed. From the above list of failures test_router_dhcphosts are due to test errors that is being addressed by #1683 I have addressed below two test failures. support for internal LB was not added in the logic creating ips.json databag, I made a fix for that as well. test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 rest of the failures i am investigating if they are regressions, and test failures. |
ustcweizhou
commented
Sep 26, 2016
Contributor
@murali-reddy FYI |
murali-reddy
commented
Sep 26, 2016
ContributorAuthor
@ustcweizhou thanks, i have fixed it earlier today and tested, am trying to fix other test failures and re-run the CI |
jburwell
commented
Sep 26, 2016
Contributor
| firewall_rule.delete(self.apiclient) | ||
Contributor
There was a problem hiding this comment.
I think creating and validating a firewall rule can be moved to net utility class, as this has possibilities to be reused.
blueorangutan
commented
Dec 1, 2016
@jburwell a Trillian-Jenkins test job (centos7 mgmt + xenserver-65sp1) has been kicked to run smoke tests |
blueorangutan
commented
Dec 2, 2016
Trillian test result (tid-533)
|
blueorangutan
commented
Dec 2, 2016
Trillian test result (tid-532)
|
yadvr
commented
Dec 2, 2016
Member
Test lgtm, @murali-reddy are we good on this PR? I'm seeing some failures though not sure if they related to your changes. /cc @jburwell@abhinandanprateek |
yadvr
commented
Dec 5, 2016
Member
@blueorangutan package |
blueorangutan
commented
Dec 5, 2016
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
yadvr
commented
Dec 5, 2016
Member
@murali-reddy@abhinandanprateek let me know any help needed from my end? |
blueorangutan
commented
Dec 5, 2016
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-323 |
yadvr
commented
Dec 5, 2016
Member
@blueorangutan test matrix |
blueorangutan
commented
Dec 5, 2016
@rhtyd a Trillian-Jenkins matrix job (centos6 mgmt + xs65sp1, centos7 mgmt + vmware55u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests |
blueorangutan
commented
Dec 5, 2016
Trillian test result (tid-581)
|
blueorangutan
commented
Dec 6, 2016
Trillian test result (tid-591)
|
abhinandanprateek
commented
Dec 6, 2016
Contributor
LGTM on code review and testing @murali-reddy @rhtyd |
murali-reddy
commented
Dec 6, 2016
ContributorAuthor
Can this merged now? test failures are specific to redundent VR/VPC VR which have been failing in 4.8/4.9/master. |
yadvr
commented
Dec 6, 2016
Member
@blueorangutan help |
blueorangutan
commented
Dec 6, 2016
@rhtyd I understand these words: "help", "hello", "thanks", "package", "test" Blessed contributors for kicking Trillian test jobs: ['rhtyd', 'jburwell', 'murali-reddy', 'abhinandanprateek', 'PaulAngus', 'borisstoyanov', 'karuturi'] |
borisstoyanov
commented
Dec 6, 2016
Contributor
Yes I think test failures does not seem to be caused by this PR, LGTM based on code review. |
borisstoyanov
approved these changes
Dec 6, 2016
yadvr
commented
Dec 6, 2016
Member
Thanks @murali-reddy@borisstoyanov I'll proceed with merging this now. @murali-reddy do you want to run the component test on this PR or on 1753/1754? |
murali-reddy
commented
Dec 6, 2016
ContributorAuthor
@rhtyd please proceed with merge. I have not added the test suite(test_multiple_public_interfaces.py) part of this PR as smoke tests, because it takes around an hour for execution so they are best fit for component testing. Also tests need routable additional public IP range for the test case to succeed. I will run component test against 1753/1754 and report back results. |
blueorangutan
commented
Dec 7, 2016
Trillian test result (tid-597)
|
murali-reddy
commented
Dec 7, 2016
ContributorAuthor
component test results. If any one wish to run component test test_multiple_public_interfaces.py, add a new public ip range in the test_daya.py, in the 'publiciprange' dict object. Test iptable rules in case we have IP associated with a network which is in ... === TestName: test_iptable_rules | Status : SUCCESS === Ran 6 tests in 1539.183s OK |
…es correctly -when processing static nat rule, add a mangle table rule, to mark the traffic from the guest vm when it has associated static nat rule so that traffic gets routed using the route tabe of the device which has public ip associated -fix the case where nic_device_id is empty when ip is getting disassociated resulting in empty deviceid in ips.json -add utility methods in CsRule, and CsRoute to add 'ip rule' and 'ip route' rules respectivley -ensure traffic from all public interfaces are connection marked with device number, and restored for the reverse traffic. use the connection marked number to do device specific routing table lookup fill the device specific routing table with default route -component tests for testing multiple public interfaces of VR
yadvr
commented
Dec 7, 2016
Member
Travis failure is to do with a test_volumes failures in one of the component tests, not related to this PR. Will merge, and handle the Travis failure separately. |
asfgit pushed a commit
that referenced
this pull request
Dec 7, 2016
…non_vpc CLOUDSTACK-9339 Virtual Routers don't handle Multiple Public Interfaces correctlyAs pointed out in CLOUDSTACK-9339, in case of multiple public IP's from different public IP ranges are associated with VR, VR functionality is broken from 4.6. Below are the brief list of problems specific to non-VPC networks addressed in the PR. This PR handles both VPC and non-VPC scenarios. - reverse traffic for the connections accepted on the eth3 and above public interfaces are getting blocked. Need a rule for e.g "-A FORWARD -i eth3 -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT" in the FORWARD chain of filter table to permit reverse path traffic for established connections. - outbound public traffic from eth0 to eth3 (or for interfaces above like eth4 eth5 etc) needs rule to run through FW_OUTBOUND chain in the filter table - network stats on public interfaces eth3 are getting gathered - default gateway is missing in the device specific routing table, resulting in traffic to be looked up in main routing table - creating a device specific route table is generating "from all lookup Table_eth3" in the ip rules, resulting in rest of the traffic getting blocked. Picked few commits from #1519 from dsclose (#1519) submitted for 4.7 Marvin tests are added to test below - Static NAT works on the public interfaces above eth2, in case non-vpc networks - Portforwarding works on the public interfaces above eth2, in case non-vpc networks - Route tables are configured as expected for the device specific table for the public interfaces above eth2, in case non-vpc networks - IP tables rules are as expected for the traffic from and to the public interfaces above eth2, in case non-vpc networks * pr/1659: CLOUDSTACK-9339 Virtual Routers don't handle Multiple Public Interfaces correctly Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
milamberspace
commented
Feb 2, 2017
Contributor
@murali-reddy @rhtyd Possible regression introduce by the PR1659 see https://issues.apache.org/jira/browse/CLOUDSTACK-9770 |
yadvr
commented
Feb 3, 2017
Member
@murali-reddy@abhinandanprateek@DaanHoogland can you comment on this? |
winterhazel pushed a commit
that referenced
this pull request
Jan 28, 2026
…20.0.0-scclouds' Adição de _tooltip_ no consumo de Quota Closes#1659 See merge request scclouds/scclouds!1212
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
10 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.
As pointed out in CLOUDSTACK-9339, in case of multiple public IP's from different public IP ranges are associated with VR, VR functionality is broken from 4.6. Below are the brief list of problems specific to non-VPC networks addressed in the PR. This PR handles both VPC and non-VPC scenarios.
ip rules, resulting in rest of the traffic getting blocked.
Picked few commits from #1519 from dsclose (#1519) submitted for 4.7
Marvin tests are added to test below