Uh oh!
There was an error while loading. Please reload this page.
VPC: fix some issues related to multiple public IP ranges and private gateway - #4484
Conversation
ustcweizhou
commented
Nov 20, 2020
@div8cn@richardlawley@ravening@DennisKonrad |
weizhouapache
commented
Nov 20, 2020
@DaanHoogland @rhtyd could you kick-off Trillian test for this pr ? |
DaanHoogland
commented
Nov 20, 2020
these are a lot @weizhouapache can you add the highest severity to the labels? |
blueorangutan
commented
Nov 20, 2020
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Nov 20, 2020
Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2414 |
DaanHoogland
commented
Nov 20, 2020
@blueorangutan test |
blueorangutan
commented
Nov 20, 2020
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
yadvr
commented
Nov 20, 2020
@blueorangutan test centos7 vmware-67u3 |
blueorangutan
commented
Nov 20, 2020
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests |
blueorangutan
commented
Nov 20, 2020
Trillian test result (tid-3223)
|
blueorangutan
commented
Nov 21, 2020
Trillian test result (tid-3224)
|
DaanHoogland
commented
Nov 21, 2020
hm, this exact set of failures I have seen before. I hope nothing slipped through. |
yadvr
commented
Nov 22, 2020
@blueorangutan test centos7 vmware-67u3 |
blueorangutan
commented
Nov 22, 2020
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests |
blueorangutan
commented
Nov 22, 2020
Trillian test result (tid-3230)
|
DaanHoogland
left a comment
There was a problem hiding this comment.
I see nothing strange or wrong with this code. two remarks thought:
- it is a lot in one go and will require verification in a lot of different kinds of environments.
- I recognise some changes that already went into master over the last few weeks so merging forward might give some conflicts (nothing that should stop us now.
DaanHoogland
commented
Nov 23, 2020
@PaulAngus as RM, do we merge this before release? cc @rhtyd |
yadvr
left a comment
There was a problem hiding this comment.
Lgtm, I see some changes are backported from master and many changes are in tests. The scope of changes are in vpc and private gw. Some manual upgrade tests can be done around that.
yadvr
commented
Nov 23, 2020
@DaanHoogland can you ping and discuss with @PaulAngus thnx |
yadvr
commented
Nov 23, 2020
@weizhouapache can you share what tests were done, did you perform any upgrade tests? |
This reverts commit f4f9b3a.
This reverts commit 65cb222.
…pache#3604)" This reverts commit 82d94a8.
…ernet via the VPC VR if it is gateway
… multiple private gateways
ab3fe1e to
788ed28CompareDaanHoogland
commented
Nov 23, 2020
@blueorangutan package |
blueorangutan
commented
Nov 23, 2020
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
@DaanHoogland @rhtyd rebased with latest 4.14 I have added some test cases in this PR which verify the ips on nics and UP/DOWN state of public interfaces. However, for some changes on iptable rules , they require manual test. setup expected results By the way, if you merge his pr, could you use "Rebase and Merge" option so it would be better to track why a line of change is made if there are issues in the future. |
blueorangutan
commented
Nov 23, 2020
Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2423 |
DaanHoogland
commented
Nov 23, 2020
@DennisKonrad any conclusions from your testing yet? |
@DaanHoogland I'm looking into it right now. So far all I tested looked good. Giving my approval today if nothing else turns up |
yadvr
commented
Nov 24, 2020
@blueorangutan test |
blueorangutan
commented
Nov 24, 2020
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
DaanHoogland
commented
Nov 24, 2020
i think we are good to go here. restarted the failed test env to be more than 100% sure. |
blueorangutan
commented
Nov 24, 2020
Trillian test result (tid-3244)
|
Description
This PR contains 13 commits below
Here are description of bugs
(1) ips on wrong interfaces after rebooting vpc vrs
When add new vpc tier, create private gateway, associated IP in new range to a VPC, nics will be plugged to VPC VRs.
However, when reboot(or start) a VPC VR, the nics will be added by order: Public IP (source nat), other Public IP range, private gateway, VPC tiers. so the device_id of nics are different before stopping VR.
I have created a PR for this issue #4467
(2) remove first public ip will remove all ips on the nic
When use more public IPs in new public IP range (not same as source nat), a nic will be plugged to VPC VR. All public IPs used by the VPC will be attached to the nic.
However, when we release the first public IP of the nic, it will remove the nic, all other IPs on the nic will be gone as well.
(3) public ip is not marked as "add: false" in /etc/cloudstack/ips.json when release it.
When use a public IP in new range to VPC VR, a nic will be plugged to VPC VR.
When remove the public from VPC, the nic will be unplugged. however, the ip is still marked as "add: true" in /etc/cloudstack/ips.json
so when we add a new nic to the VPC, the (old) public ip will be added back to the nic.
(4) When a VPC VR is stopped, we cannot add/remove new nic to VPC VR.
(5) Static NAT with multiple public interfaces uses wrong outgoing IP #4234
This is a regression of the fix for #3604
we need to revert the commit "Fix Policy Based Routing for private gateway static routes (#3604)"
(6) There is no ACL rule for private gateway
This is a regression of the fix for #3402
private gateway is changed to 'public' in commit "vpc: set traffic type of private gateway IP to Public to fix ke… (#3851)"
so we need to add ACL rules.
(7) servers in private gateway cannot reach internet via the VPC VR if it is gateway
When add private gateway and use VPC as gateway (private gateway IP = gateway IP), the servers in private gateway network cannot reach internet via VPC VR.
need to add rule to accept packet if VPC VR is used as gateway.
(8) INBOUND rules for traffic between vm and private gateway servers does not work.
Even rules are added to fix bug (6), the incoming traffic between vm and private gateway network is always accepted.
for example, if the ACL of vm disallow traffic from private gateway network, vm still accepts traffic from private gateway network.
need to add rules to check the INBOUND ACL rules.
(9) vm with static nat cannot connect to private gateway network.
As the fix is reverted to fix (5), the issue described in #3604 is back.
this PR introduced another way to fix the issue.
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?