[release-4.10] Bug 2105657: Fix egressIP object deletion if the node is deleted first #1228
Conversation
tssurya
changed the title
openshift-ci
bot
added
bugzilla/severity-high
|
@tssurya: This pull request references Bugzilla bug 2105657, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 8 validation(s) were run on this bug
Requesting review from QA contact: 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/test-infra repository. |
openshift-ci
bot
requested review from
anuragthehatter,
JacobTanenbaum and
jcaamano
openshift-merge-robot
added
the
needs-rebase
|
damn what's with the rebase... yikes.. |
Currently gatewayCleanup doesn't delete rtoj-* ports which is fine since it deletes the router and probably on the OVN side the ports on the router are also removed as they are dependent on the router. However from a testing perspective its nice to have the gatewayCleanup remove these ports explictly just like multiJoinSwitchGatewayCleanup does. Corresponding gatewayCleanup unit tests are also updated. We also add a unit test to egressip testing where we test reassignment upon egress node deletion, specifically where the node has already gotten deleted by the node watcher. This test uncovers a bug we have in egressIP code where the reassignment will not be successful and will fail with the error E0608 09:42:53.781497 971577 obj_retry.go:1513] Failed to delete resource object node1 of type *factory.egressNode, error: Re-assignment for EgressIP: egressip failed, unable to update object, err: unable to retrieve gateway IP for node: node1, protocol is IPv6: false, err: attempt at finding node gateway router network information failed, err: unable to find router port rtoj-GR_node1: object not found Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com> (cherry picked from commit 9d0aebe) (cherry picked from commit 2100d49) Conflicts: go-controller/pkg/ovn/gateway_cleanup.go Conflict because libovsdbops.DeleteLogicalRouterPorts(oc.nbClient, &logicalRouter, &logicalRouterPort) doesn't exist in 4.10 since we are missing openshift@8b14898#diff-5d62dde13a2856ea3d4b324968dbaeed261fcb74cb3b81909edf40a6634fa1f4R161 {libovsdb-refactor-cleanup-4.11}
…dicate Partial pick of openshift@4e3c26a#diff-5d62dde13a2856ea3d4b324968dbaeed261fcb74cb3b81909edf40a6634fa1f4 from 4e3c26a. Had to bring in the code required to do DeleteNextHopFromLogicalRouterPoliciesWithPredicate Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
openshift-merge-robot
removed
the
needs-rebase
|
/assign @huiran0826 |
|
/lgtm |
There are a few instances in egressIP code where we shouldn't fail hard if objects are not found, example if a node gets deleted before egressIP watcher takes action. In this case, we can log a warning saying objects couldn't be deleted and move on since its technically not an error if the object was deleted by some other code path. In oder to ensure we don't leave behind stale reroute policy entries, we also add the logic of removing policies that have nexthop == joinIP of the node that's getting deleted into the gateway cleanup code. Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com> (cherry picked from commit 95ab759) (cherry picked from commit 95486d9) Conflicts: go-controller/pkg/ovn/egressip.go go-controller/pkg/util/net.go Conflicts because EIP retry code is absent in 4.10 Note: Brought in the chnages done in retry code for getGatewayRouterJoinIP since they made more sense that way.
|
/retest |
|
@tssurya: The following test 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. |
|
/lgtm |
openshift-ci
bot
added
backport-risk-assessed
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flavio-fernandes, jcaamano, tssurya 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 |
openshift-ci
bot
added
the
approved
|
/label cherry-pick-approved |
openshift-ci
bot
added
the
cherry-pick-approved
|
@tssurya: All pull requests linked via external trackers have merged: Bugzilla bug 2105657 has been moved to the MODIFIED state. 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/test-infra repository. |
Brings in #1143 from 4.11 into 4.10. Conflicts are outlined in each PR. Extra commit (partially, needed parts) picked up from the libovsdb cleanup code to facilitate the backport: 4e3c26a
/assign @jcaamano