OCPBUGS-3176: Disable global ipv4 and ipv6 forwarding for OVN deployments by trozet · Pull Request #3676 · openshift/machine-config-operator

trozet · 2023-04-12T20:55:00Z

Enable forwarding globally can cause undesirable effects for customers with their server acting as a router (non-default behavior). OVNK now enables and restricts forwarding on a per managed interface basis. So there is no need to enable this globally anymore.

openshift-ci-robot · 2023-04-12T20:55:07Z

@trozet: This pull request references Jira Issue OCPBUGS-3176, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.14.0) matches configured target version for branch (4.14.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @anuragthehatter

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Enable forwarding globally can cause undesirable effects for customers with their server acting as a router (non-default behavior). OVNK now enables and restricts forwarding on a per managed interface basis. So there is no need to enable this globally anymore.

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.

trozet · 2023-04-12T20:55:16Z

/assign @tssurya

trozet · 2023-04-12T20:55:41Z

/hold

want to launch a cluster bot and ensure this works as expected

trozet · 2023-04-28T17:24:25Z

/test e2e-metal-ipi

trozet · 2023-05-03T17:00:29Z

metal passed without enabling ip forwarding, but that is most likely because the node ip interface == br-ex interface and ovn-kube will enable forwarding on the br-ex interface for us. We need to cover a case where the node ip interface differs from br-ex. For these cases keepalived will use the node ip interface according to @cybertron so we need to make sure node ip enables ip forwarding on those interfaces. Will add one more commit to this PR to take care of that.

trozet · 2023-05-03T20:55:35Z

/test e2e-metal-ipi

trozet · 2023-05-04T12:36:59Z

looks to be working...removing hold:

May 03 21:47:48.477113 master-0 configure-ip-forwarding.sh[2334]: +++ ip -j addr
May 03 21:47:48.477242 master-0 configure-ip-forwarding.sh[2335]: +++ jq -r 'first(.[] | select(any(.addr_info[]; .local=="192.168.111.20"))) | .ifname'
May 03 21:47:48.478206 master-0 resolv-prepender.sh[2236]: Writing manifest to image destination
May 03 21:47:48.478248 master-0 resolv-prepender.sh[2236]: Storing signatures
May 03 21:47:48.487865 master-0 resolv-prepender.sh[2236]: 9d8b54b39e892388bd8b873fb43dbcadaab0b8954797c1014fe45c9a652ed920
May 03 21:47:48.487958 master-0 podman[2236]: 2023-05-03 21:47:48.190102867 +0000 UTC m=+0.021496222 image pull  registry.build05.ci.openshift.org/ci-op-2yj26jvi/stable@sha256:2e4b1c3bccf9cdaafdfd5349977c9be2b8471ef7785ed3d816554debd5ad2f1f
May 03 21:47:48.489413 master-0 systemd[1]: var-lib-containers-storage-overlay.mount: Deactivated successfully.
May 03 21:47:48.490282 master-0 resolv-prepender.sh[2233]: NM resolv-prepender: Download of baremetal runtime cfg image completed
May 03 21:47:48.501301 master-0 configure-ip-forwarding.sh[2330]: ++ iface=enp2s0
May 03 21:47:48.501301 master-0 configure-ip-forwarding.sh[2330]: ++ [[ -n enp2s0 ]]
May 03 21:47:48.501365 master-0 configure-ip-forwarding.sh[2330]: ++ echo enp2s0
May 03 21:47:48.501586 master-0 configure-ip-forwarding.sh[2329]: + iface=enp2s0
May 03 21:47:48.501642 master-0 configure-ip-forwarding.sh[2329]: + [[ -z enp2s0 ]]
May 03 21:47:48.501670 master-0 configure-ip-forwarding.sh[2329]: + echo 'Node IP interface determined as: enp2s0. Enabling IP forwarding...'
May 03 21:47:48.501670 master-0 configure-ip-forwarding.sh[2329]: Node IP interface determined as: enp2s0. Enabling IP forwarding...
May 03 21:47:48.501670 master-0 configure-ip-forwarding.sh[2329]: + sysctl -w net.ipv4.conf.enp2s0.forwarding=1
May 03 21:47:48.502666 master-0 configure-ip-forwarding.sh[2342]: net.ipv4.conf.enp2s0.forwarding = 1
May 03 21:47:48.502793 master-0 configure-ip-forwarding.sh[2329]: + sysctl -w net.ipv6.conf.enp2s0.forwarding=1
May 03 21:47:48.503625 master-0 configure-ip-forwarding.sh[2343]: net.ipv6.conf.enp2s0.forwarding = 1
May 03 21:47:48.504533 master-0 systemd[1]: nodeip-configuration.service: Deactivated successfully.

trozet · 2023-05-04T12:37:09Z

/hold cancel

trozet · 2023-05-04T12:37:20Z

/assign @cybertron

cybertron · 2023-05-05T15:48:56Z

/lgtm

This should be fine from an on-prem perspective.

tssurya

thanks @trozet some suggestions, questions

Enable forwarding globally can cause undesirable effects for customers with their server acting as a router (non-default behavior). OVNK now enables and restricts forwarding on a per managed interface basis. So there is no need to enable this globally anymore. Signed-off-by: Tim Rozet <trozet@redhat.com>

A user or IDE may unsuspectingly add a newline to the end of conditionalized rendered templates that will fail the check to see if there is any data actually to be rendered. Signed-off-by: Tim Rozet <trozet@redhat.com>

Service will configure ip forwarding for the selected node ip interface. Signed-off-by: Tim Rozet <trozet@redhat.com>

trozet · 2023-05-15T15:12:08Z

/assign @sinnykumari

sinnykumari

LGTM

openshift-ci · 2023-05-15T17:00:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cybertron, jcaamano, sinnykumari, trozet, tssurya

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [sinnykumari]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2023-05-15T18:26:21Z

/retest-required

Remaining retests: 0 against base HEAD 49f32d4 and 2 for PR HEAD cf172ff in total

trozet · 2023-05-17T14:32:51Z

/retest-required

openshift-ci-robot · 2023-05-18T03:23:40Z

/retest-required

Remaining retests: 0 against base HEAD 655ba33 and 1 for PR HEAD cf172ff in total

trozet · 2023-05-22T15:48:01Z

/retest-required

trozet · 2023-05-23T16:09:56Z

/retest-required

openshift-ci · 2023-05-23T19:00:49Z

@trozet: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-alibabacloud-ovn	`cf172ff`	link	false	`/test e2e-alibabacloud-ovn`
ci/prow/okd-scos-e2e-gcp-ovn-upgrade	`cf172ff`	link	false	`/test okd-scos-e2e-gcp-ovn-upgrade`

Full PR test history. Your PR dashboard.

Details

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.

trozet · 2023-05-23T20:04:20Z

/retest-required

openshift-ci-robot · 2023-05-23T20:13:23Z

@trozet: Jira Issue OCPBUGS-3176: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-3176 has been moved to the MODIFIED state.

Details

In response to this:

Enable forwarding globally can cause undesirable effects for customers with their server acting as a router (non-default behavior). OVNK now enables and restricts forwarding on a per managed interface basis. So there is no need to enable this globally anymore.

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.

With ovn-kubernetes/ovn-kubernetes#3374 and openshift/machine-config-operator#3676 we stopped enabling IP forwarding globally in the OCP host. It is now only enabled on interfaces that OVNK manages. Additionally iptables rules are put in place to limit what traffic is forwarded. This was to resolve an issue where our node by default acts as a router: https://issues.redhat.com/browse/OCPBUGS-3176 With these changes, by default in OCP we will no longer forward IP traffic. Most users should welcome this change as it means the OCP node will not be forwarding traffic unintentionally on their network. However, in case there is some corner case where a user expects traffic to be forwarded we will expose this new config knob to allow them to go back to the old behavior. Signed-off-by: Tim Rozet <trozet@redhat.com>

With changes to disable IP forwarding by default with OVNK: openshift/machine-config-operator#3676 OVN to SDN migration is now broken, because pods roll out first that require IP forwarding. IP forwarding for SDN is enabled via MCO node rollout, which happens later. This change enables IP forwarding in the SDN pod itself if needed. Signed-off-by: Tim Rozet <trozet@redhat.com>

With openshift/machine-config-operator#3676 we removed globally setting ip forwarding in MCO. We should not be setting it in the default profile for OpenShift. We now set it on a per interface basis as needed. However, by setting proc/sys/net/ipv4/forward rather than (forwarding) it will reset all the values: "This variable is special, its change resets all configuration parameters to their default state (RFC1122 for hosts, RFC1812 for routers)" We suspect this causes upgrade to fail. NTO sets this to 1, which then resets all the per interface config that OVNK wrote. Then during upgrade when there is a tuned profile change, the config change is rolled back so forward is 0, and now there is no connectivity to kapi and upgrade fails. Signed-off-by: Tim Rozet <trozet@redhat.com>

openshift-merge-robot · 2023-09-16T01:17:16Z

Fix included in accepted release 4.14.0-0.nightly-2023-09-11-201102

openshift-merge-robot · 2023-09-17T02:24:56Z

Fix included in accepted release 4.14.0-0.nightly-2023-09-12-024050

openshift-merge-robot · 2023-09-17T12:09:15Z

Fix included in accepted release 4.14.0-0.nightly-2023-09-15-101929

openshift-ci Bot requested a review from anuragthehatter April 12, 2023 20:55

openshift-ci Bot assigned tssurya Apr 12, 2023

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 12, 2023

openshift-ci Bot requested review from cgwalters and jkyros April 12, 2023 20:57

trozet force-pushed the OCPBUGS-3176 branch 2 times, most recently from 94a6dc3 to f0978e1 Compare April 28, 2023 17:21

trozet force-pushed the OCPBUGS-3176 branch from 0550a35 to c086c83 Compare May 3, 2023 19:00

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 4, 2023

openshift-ci Bot assigned cybertron May 4, 2023

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 5, 2023

tssurya reviewed May 8, 2023

View reviewed changes

trozet added 3 commits May 8, 2023 10:20

Trim data when checking if there is anything to render

d9b70fb

A user or IDE may unsuspectingly add a newline to the end of conditionalized rendered templates that will fail the check to see if there is any data actually to be rendered. Signed-off-by: Tim Rozet <trozet@redhat.com>

Add ip forwarding service for on-prem deployments

cf172ff

Service will configure ip forwarding for the selected node ip interface. Signed-off-by: Tim Rozet <trozet@redhat.com>

trozet force-pushed the OCPBUGS-3176 branch from c086c83 to cf172ff Compare May 8, 2023 14:21

openshift-ci Bot assigned sinnykumari May 15, 2023

sinnykumari approved these changes May 15, 2023

View reviewed changes

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 15, 2023

openshift-merge-robot merged commit 0dcaf88 into openshift:master May 23, 2023

trozet mentioned this pull request May 24, 2023

OCPBUGS-3176: Add config knob for enabling ip forwarding openshift/api#1468

Merged

trozet mentioned this pull request May 25, 2023

OCPBUGS-3176: Enable IP Forwarding if disabled openshift/sdn#549

Merged

openshift-ci-robot mentioned this pull request May 25, 2023

OCPBUGS-3176: Rework IP Forwarding config openshift/api#1473

Merged

trozet mentioned this pull request May 30, 2023

OCPBUGS-14247: Remove setting ip forwarding via openshift profile openshift/cluster-node-tuning-operator#668

Closed

trozet mentioned this pull request May 30, 2023

OCPBUGS-14247: Remove setting ip forwarding via openshift profile redhat-performance/tuned#536

Merged

openshift-ci-robot mentioned this pull request Aug 25, 2023

OCPBUGS-16051, OCPBUGS-3176: Enables IP Forwarding config in CNO openshift/cluster-network-operator#1952

Merged

tssurya mentioned this pull request Apr 17, 2024

Implementation required to enable Forwarding if it is already disabled ovn-kubernetes/ovn-kubernetes#4172

Merged

Conversation

trozet commented Apr 12, 2023

Uh oh!

openshift-ci-robot commented Apr 12, 2023

Uh oh!

trozet commented Apr 12, 2023

Uh oh!

trozet commented Apr 12, 2023

Uh oh!

trozet commented Apr 28, 2023

Uh oh!

trozet commented May 3, 2023

Uh oh!

trozet commented May 3, 2023

Uh oh!

trozet commented May 4, 2023

Uh oh!

trozet commented May 4, 2023

Uh oh!

trozet commented May 4, 2023

Uh oh!

cybertron commented May 5, 2023

Uh oh!

tssurya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trozet commented May 15, 2023

Uh oh!

sinnykumari left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented May 15, 2023

Uh oh!

openshift-ci-robot commented May 15, 2023

Uh oh!

trozet commented May 17, 2023

Uh oh!

openshift-ci-robot commented May 18, 2023

Uh oh!

trozet commented May 22, 2023

Uh oh!

trozet commented May 23, 2023

Uh oh!

openshift-ci Bot commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trozet commented May 23, 2023

Uh oh!

openshift-ci-robot commented May 23, 2023

Uh oh!

openshift-merge-robot commented Sep 16, 2023

Uh oh!

openshift-merge-robot commented Sep 17, 2023

Uh oh!

openshift-merge-robot commented Sep 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

openshift-ci Bot commented May 23, 2023 •

edited

Loading