[release-4.17] OCPBUGS-64856: Fix stale EIP assignments during failover and controller restart#2850
Conversation
pperiyasamy
force-pushed
the
eip_failover_ovnkubenode_restart_4.17
branch
2 times, most recently
from
bff3110 to
4224af7
Compare
pperiyasamy
changed the title
openshift-ci-robot
added
jira/severity-critical
|
@pperiyasamy: This pull request references Jira Issue OCPBUGS-64856, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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. |
|
@pperiyasamy: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/payload 4.17 ci blocking |
openshift-merge-robot
added
the
needs-rebase
|
@pperiyasamy: trigger 4 job(s) of type blocking for the ci release of OCP 4.17
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/55f0b830-bf0b-11f0-91e2-218f3052739c-0 trigger 8 job(s) of type blocking for the nightly release of OCP 4.17
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/55f0b830-bf0b-11f0-91e2-218f3052739c-1 |
|
/test e2e-azure-ovn-upgrade |
Scenario: - Nodes: node-1, node-2, node-3 - Egress IPs: EIP-1 - Pods: pod1 on node-1, pod2 on node-3 (pods are created via deployment replicas) - Egress-assignable nodes: node-1, node-2 - EIP-1 assigned to node-1 During a simultaneous reboot of node-1 and node-2, EIP-1 failed over to node-2 and ovnkube-controller restarted at nearly the same time: 1) EIP-1 was reassigned to node-2 by the cluster manager. 2) The sync EIP happened for EIP1 with stale status, though it cleaned SNATs/LRPs referring to node-1 due to outdated pod IPs (this is because pods will be recreated due to node reboots). 3) pod1/pod2 Add events arrived while the informer cache still had the old EIP status, so new SNATs/LRPs were created pointing to node-1. 4) The EIP-1 Add event arrived with the new status; entries for node-2 were added/updated. 5) Result: stale SNATs and LRPs with stale nexthops for node-1 remained. Fix: - Populate pod EIP status during EgressIP sync so podAssignment has accurate egressStatuses. - Reconcile stale assignments using podAssignment (egressStatuses) when the informer cache is not up to date, ensuring SNAT/LRP for the previously assigned node are corrected. - Remove stale EIP SNAT entries for remote-zone pods accordingly. - Add coverage for simultaneous EIP failover and controller restart. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 1667a51) (cherry picked from commit 7060af6) (cherry picked from commit 0a3e1b9) (cherry picked from commit 75f0b7e) Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
During an ovnkube-controller restart, pod add/remove events for EgressIP-served pods may occur before the factory.egressIPPod handler is registered in the watch factory. As a result, the EIP controller never able to handle pod delete, leading to stale logical router policy (LRP) entry. Scenario: ovnkube-controller starts. The EIP controller processes the namespace add event (oc.WatchEgressIPNamespaces) and creates an LRP entry for the served pod. The pod is deleted. The factory.egressIPPod handler registration happens afterward via oc.WatchEgressIPPods. The pod delete event is never processed by the EIP controller. Fix: 1. Start oc.WatchEgressIPPods followed by oc.WatchEgressIPNamespaces. 2. Sync EgressIPs before registering factory.egressIPPod event handler. 3. Removal of Sync EgressIPs for factory.EgressIPNamespaceType which is no longer needed. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 8975b00) (cherry picked from commit b8303a2) (cherry picked from commit 3f391e7) (cherry picked from commit 6cd0f09) Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
When the EIP controller cleans up a stale EIP assignment for a pod, it also removes the pod object from the podAssignment cache. This is incorrect, as it prevents the EIP controller from processing the subsequent pod delete event. Scenario: 1. pod-1 is served by eip-1, both hosted on node1. 2. node1’s ovnkube-controller restarts. 3. Pod add event is received by the EIP controller — no changes. 4. eip-1 moves from node1 to node0. 5. The EIP controller receives the eip-1 add event. 6. eip-1 cleans up pod-1’s stale assignment (SNAT and LRP) for node1, but removes the pod object from the podAssignment cache when no other assignments found. 7. The EIP controller programs the LRP entry with node0’s transit IP as the next hop, but the pod assignment cache is not updated with new podAssignmentState. 8. The pod delete event is received by the EIP controller but ignored, since the pod object is missing from the assignment cache. So this commit fixes the issue by adding podAssignmentState back into podAssignment cache at step 7. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 16dedd1) (cherry picked from commit f4e2c17) (cherry picked from commit be0f3b8) (cherry picked from commit 3a524b5)
pperiyasamy
force-pushed
the
eip_failover_ovnkubenode_restart_4.17
branch
from
4224af7 to
7d8eb17
Compare
openshift-merge-robot
removed
the
needs-rebase
|
/payload 4.17 ci blocking |
|
@pperiyasamy: trigger 4 job(s) of type blocking for the ci release of OCP 4.17
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4b1ea9f0-bf13-11f0-9e7a-97f5f5553708-0 trigger 8 job(s) of type blocking for the nightly release of OCP 4.17
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4b1ea9f0-bf13-11f0-9e7a-97f5f5553708-1 |
|
/verified by @qiowang721 |
openshift-ci-robot
added
the
verified
|
@qiowang721: This PR has been marked as verified by DetailsIn 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. |
|
/retest-required |
|
/payload-job periodic-ci-openshift-release-master-ci-4.17-e2e-gcp-ovn-upgrade |
|
@pperiyasamy: trigger 4 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c7d809b0-bfb7-11f0-8ca8-47fb15bd2018-0 |
|
Why is Are you actually ryunning the unti tests? Please note they might not be running as part of the ci/prow/unit job. |
The
yes, validated EIP unit tests locally with command |
|
/lgtm waiting for CI |
openshift-ci
bot
added
the
backport-risk-assessed
openshift-ci
bot
assigned
anuragthehatter,
asood-rh,
barbora137,
huiran0826,
jechen0648,
Meina-rh,
mffiedler,
qiowang721,
rbbratta and
weliang1
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcaamano, pperiyasamy The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@pperiyasamy: This pull request references Jira Issue OCPBUGS-64856, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn 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. |
openshift-merge-bot
bot
merged commit 7aa4415
into
openshift:release-4.17
|
@pperiyasamy: Jira Issue Verification Checks: Jira Issue OCPBUGS-64856 Jira Issue OCPBUGS-64856 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn 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. |
15 participants
Manual cherry-pick of 4.18 commits 7b12cc6, 75f0b7e, 6cd0f09 and 3a524b5 from PR #2849.
The commits 87005cd, b7d8695 and 947941b aren't needed for 4.16 because it already handles stale LRP nexthops and its logging in the sync function. So adding only unit test commit 7b12cc6 to validate it.
The other commits had major conflict with 4.17 due to much code difference in EIP controller and test files. so manually resolved the conflicts.