12-03-2020 merge with Ingress ACL fix #370
Merged
Conversation
This file contains 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
SetIPs allows for the address set to be set to an explicit set of values will be used by egressfirewallDNS code Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
in order to make the testing easier and prepare for egressfirewall dns refactor the code that generates the "match" section of the ovn ACLs Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
add rules to egressfirewall to support allowing or denying traffic by the dns name. Similar to allow/deny based on cidr this adds a rule to the ovn database based on the resolved dns name. Updates to these new rules are maintained by Sync() that runs when a ttl expires or if non set every 30 minutes, and every time a new rule is added and checks that the dns names still resolve the same and update as needed. This assumes that the nodes and masters are located in a similar location as the dns names get resolved by the master Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
implement dns names in egressfirewall
GW load balancers exist on the GR and node switches. Moving Ingress LB there allows us to handle Ingress services on the GR directly, allowing in a future patch to remove the DNAT setup in the shared gw bridge by ovn-k8s. Signed-off-by: Tim Rozet <trozet@redhat.com>
Upon endpoint deletion we should clear all the backing endpoints for the OVN load balancer and create reject ACLs (if applicable). Instead of this we were deleting the entire service (all ACLs, OVN LB entry) for External IP and node port. This fixes the behavior and refactors the function to be more clear. Signed-off-by: Tim Rozet <trozet@redhat.com>
Ingress LB handling was previously done by doing a DNAT to the corresponding node port in the shared gw bridge, using CT and table 2 in our shared gw bridge pipeline. This was incorrect, because it always assumed that an Ingress LB service was tied to a Node Port type service. This patch fixes that and removes using ovn-k8s programmed flows to do DNAT on shared gw bridge. Instead, we just leverage the OVN gateway load-balancer which is already configured to handle Ingress LB traffic. Thus our flows now simply steer traffic towards that LB Ingress VIP towards OVN. Signed-off-by: Tim Rozet <trozet@redhat.com>
Update was accidentally trying to update from "new" to "old" and was not comparing the services correctly to determine if an update was necessary. Also, removed an unnecessary check for syncServices. Signed-off-by: Tim Rozet <trozet@redhat.com>
Fix lb ingress shared gw and fixes endpoint deletion
The ACL was accidentally being configured on the cluster load balancer instead of the gateway load balancer. Signed-off-by: Tim Rozet <trozet@redhat.com>
openshift-ci-robot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
approved
|
/retest |
1 similar comment
|
/retest |
|
@trozet: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
dcbw
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
/lgtm |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dcbw, trozet The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
- What this PR does and why is it needed
- Special notes for reviewers
- How to verify it
- Description for the changelog