-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
COS-2822: port gcp-routes to nftables #1562
Conversation
gcp-routes had a rule "so that existing flows (with an entry in conntrack) continue to be balanced, even if the DNAT entry is removed". The only way this iptables rule would actually be needed is if (a) your masters have an iptables-based firewall (which they shouldn't, on OCP), and (b) the firewall is so aggressive that it even drops packets from established connections (which no firewall should do anyway). At any rate, even if the rule *was* necessary in some clusters, it won't work in future nftables-only versions of RHCOS anyway, because nftables doesn't let "accept" rules in one table override "drop"/"reject" rules in another table; if your firewall is broken and dropping packets that it shouldn't, you have to actually fix your firewall rules, not hack around them somewhere else.
@danwinship: This pull request explicitly references no jira issue. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
/assign @jcaamano |
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved https://issues.redhat.com/browse/MCO-1266 to https://issues.redhat.com/browse/COS-2882. Hopefully that's what you meant. If so, you can use that for the Jira link. Also, can you add a commit message to the second commit and link to the OCPSTRAT?
Trivial LGTM for the change itself. The script lives here but we're not really the SMEs for it.
That was my first attempt, yes, though it seems like "Clone" in jira is generally terrible anyway, because that card ended up with a bunch of MCO-tagged qe-related sub-tasks whereas presumably it should have ended up with COS ones? 🤷♂️ |
@danwinship: This pull request references COS-2822 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
IPTables is going away in RHEL 10 so everything needs to be done with nftables. https://issues.redhat.com/browse/OCPSTRAT-940
@jlebon repushed with updated commit message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
/lgtm
@jlebon try again? bot didn't seem to notice... |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, jcaamano, jlebon 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 |
Quay.io infra flake |
@danwinship: all tests passed! 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-sigs/prow repository. I understand the commands that are listed here. |
iptables is going away in RHEL10; this PR ports gcp-routes.sh from iptables to nftables.
This is just a port of openshift/machine-config-operator#4494 which updated MCO's copy of gcp-routes... the version in os is slightly simpler (because it's older and it only needs to support bootstrap and so hasn't needed the fixes that are in the MCO version). (Or, honestly, probably "it has the same problems the MCO version used to have, but our CI doesn't flag the apiserver connection failures-and-retries during bootstrap switchover like it does during upgrade, so nobody ever bothered to fix the bugs in the RHCOS version.) Anyway, see discussion in that PR.
I tried to make a jira issue for this but jira insists (to me anyway) that there is no such project as "COS" and will not let me create a new issue in it. (But if someone else wants to create one and link it to OCPSTRAT-940, go ahead.)