-
Notifications
You must be signed in to change notification settings - Fork 168
[release-4.19] OCPBUGS-62670: Fix EgressIP stale GARP post reboot + pod restart #2774
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
[release-4.19] OCPBUGS-62670: Fix EgressIP stale GARP post reboot + pod restart #2774
Conversation
Currently, we are force exiting with the trap before the background processes can end, container is removed and the orphaned processes end early causing our config to go into an unknown state because we dont end in an orderly manner. Wait until the pid file for ovnkube controller with node is removed which shows the process has completed. Signed-off-by: Martin Kennelly <[email protected]> (cherry picked from commit 8b29419) (cherry picked from commit d65ec5c)
Prevent ovn-controller from sending stale GARP by adding drop flows on external bridge patch ports until ovnkube-controller synchronizes the southbound database - henceforth known as "drop flows". This addresses race conditions where ovn-controller processes outdated SB DB state before ovnkube-controller updates it, particularly affecting EIP SNAT configurations attached to logical router ports. Fixes: https://issues.redhat.com/browse/FDP-1537 ovnkube-controller controls the lifecycle of the drop flows. ovs / ovn-controller running is required to configure external bridge. Downstream, the external bridge maybe precreated and ovn-controller will use this. This fix considers three primary scenarios: node, container and pod restart. On Node restart means the ovs flows installed priotior to reboot on the node are cleared but the external bridge exists. Add the flows before ovnkube controller with node starts. The reason to add it here is that our gateway code depends on ovn-controller started and running... There is now a race here between ovn-controller starting (and garping) before we set this flow but I think the risk is low however it needs serious testing. The reason I did not naturally at the drop flows before ovn-controller started is because I have no way to detect if its a node reboot or pod reboot and i dont want to inject drop flows for simple ovn-controller container restart which could disrupt traffic. ovnkube-controller starts, we create a new gateway and apply flows the same flows in-order to ensure we always drop GARP when ovnkube controller hasn't sync. Remove the flows when ovnkube-controller has syncd. There is also a race here between ovnkube-controller removing the flows and ovn-controller GARPing with stale SB DB info. There is no easy way to detect what SB DB data ovn-controller has consumed. On Pod restart, we add the drop flows before exit. ovnkube-controller-with-node will also add it before it starts the go code. Container restart: - ovnkube-controller: adds flows upon start and exit - ovn-controller: no changes While the drop flows are set, OVN may not be able to resolve IPs it doesn't know about in its Logical Router pipelines generation. Following removal of the drop flows, OVN may resolve the IPs using GARP requests. OVN-Controller always sends out GARPs with op code 1 on startup. Signed-off-by: Martin Kennelly <[email protected]> (cherry picked from commit 82fc3bf) (cherry picked from commit 50a94e1)
PR 5373 to drop the GARP flows didnt consider that we set the default network controller and later we set the gateway obj. In-between this period, ovnkube node may receive a stop signal and we do not guard against accessing the gateway if its not yet set. OVNKube controller may have sync'd before the gateway obj is set. There is nothing to reconcile if the gateway is not set. Signed-off-by: Martin Kennelly <[email protected]> (cherry picked from commit e60220a) (cherry picked from commit a7869b2)
|
@martinkennelly: This pull request references Jira Issue OCPBUGS-62670, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
@martinkennelly: This PR was included in a payload test run from openshift/machine-config-operator#5324
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a89d3170-9f8b-11f0-93fd-ee70f1d60e20-0 |
Ensure ovn-controller has processed the SB DB updates before removing the GARP drop flows by utilizing the hv_cfg field in NB_Global [1] OVNKube controller increments the nb_cfg value post sync, which is copied to SB DB by northd. OVN-Controllers copy this nb_cfg value from SB DB and write it to their chassis_private tables nb_cfg field after they have processed the SB DB changes. Northd will then look at all the chassis_private tables nb_cfg value and set the NB DBs Nb_global hv_cfg value to the min integer found. Since IC currently only supports one node per zone, we can be sure ovn-controller is running locally and therefore its ok to block removing the drop GARP flows. [1] https://man7.org/linux/man-pages/man5/ovn-nb.5.html Signed-off-by: Martin Kennelly <[email protected]> (cherry picked from commit 3b5da01) (cherry picked from commit a4776fb)
|
@martinkennelly: This PR was included in a payload test run from openshift/machine-config-operator#5324
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c3464cd0-a43d-11f0-8c73-68fbceb6751c-0 |
|
/test e2e-aws-ovn-windows It failed during the installation phase. Unrelated to this. This fix doesnt run on windows. |
|
/test 4.19-upgrade-from-stable-4.18-e2e-gcp-ovn-rt-upgrade Hit overall job timeout: |
|
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw Probably unrelated nmstate image pull issue: |
|
/test e2e-aws-ovn-windows |
|
/test e2e-aws-ovn-windows Unrelated: |
|
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw Unrelated issues including the CNI add error because of mismatch in UID: |
|
Payload is looking good. |
|
@martinkennelly: This PR was included in a payload test run from openshift/machine-config-operator#5324
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/abaa8680-a533-11f0-9725-55c07ace1ad6-0 |
|
@martinkennelly: This PR was included in a payload test run from openshift/machine-config-operator#5324
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/b7685ab0-a533-11f0-95ec-ed831b73fdca-0 |
|
/verified by 'pre-merge testing' |
|
@jechen0648: This PR has been marked as verified by 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. |
|
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw |
|
/retest |
|
Foregoing process since this is urgent escalation |
|
/retest-required |
|
Still no bug for the k-nmsate-console image pull backoff seen on the bgp job. We are trying to find out whats wrong and therefore whos responsibile. Not clear. Its clear its unrelated to this PR but unsure whos problem. See the slack thread with art: https://redhat-internal.slack.com/archives/CJARLA942/p1760354263237459 That job seems to be using images from a QE source ( |
|
/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw |
|
@jechen0648 thanks jean but that job is borked for 4.19 for k-nmstate operator image - see previous comment. |
|
For latest comments on the supportability of that step we use in our CI job |
|
/override ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw given the timeline of this escalation, going to override CI for BGP w/o a bug open. But this is clearly unrelated to this PR |
|
@tssurya: Overrode contexts on behalf of tssurya: ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw 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 kubernetes-sigs/prow repository. |
|
/tide refresh |
|
/shrug |
|
/test e2e-aws-ovn-upgrade Passes 99.6% of the time. No bug. Unrelated. |
|
/test e2e-metal-ipi-ovn-dualstack Unrelated. Passes 99% of the time. No bug. |
|
/tide refresh |
|
/test e2e-aws-ovn-upgrade Unrelated. Failed to create a release image to test. :))) |
|
Same error as previous comment for |
|
Bug for dualstack-bgp-local-gw https://issues.redhat.com/browse/OCPBUGS-63027 |
|
https://redhat-internal.slack.com/archives/CBN38N3MW/p1760439494041289 Asking test platform folks regarding the payload build errors. |
|
/test e2e-metal-ipi-ovn-dualstack Test platforum team says its either image not present or what we pulled was corrupted. |
|
/override ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw |
|
@tssurya: Overrode contexts on behalf of tssurya: ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw 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 kubernetes-sigs/prow repository. |
|
@martinkennelly: 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-sigs/prow repository. I understand the commands that are listed here. |
1c04cc3
into
openshift:release-4.19
|
@martinkennelly: Jira Issue OCPBUGS-62670: Some pull requests linked via external trackers have merged: The following pull request, linked via external tracker, has not merged: All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-62670 has not been moved to the MODIFIED state. This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the changes from the PRs are available in an accepted nightly payload. 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. |
/hold
Depends on #2767