Bug 2062842: [4.8z] After reboot egress node, lr-policy-list was not correct, some duplicate records or missed internal IPs by flavio-fernandes · Pull Request #1009 · openshift/ovn-kubernetes

flavio-fernandes · 2022-03-22T21:45:22Z

Use pod IPs from annotation in sync

We cannot rely on pod.Status.PodIPs for obtaining the IPs, since that is unset in between the restart and the moment it retrieves that information. Use annotation instead.

In cases where OVN database for logical router policies or NATs used by EgressIPs have stale nexthops or wrong external_ips, the sync function should remove them, so the proper row/column gets set.

Backport from 4.9: Egress IP with ECMP routing (#981)

Signed-off-by: Flavio Fernandes flaviof@redhat.com

THe assignment set sequence is slightly incorrect for egress nodes as it couldn't correctly set all fields in the case a node is reachable but not ready. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> (cherry picked from commit 11b6a83)

Returning an error in this case is slightly incorrect since this case shouldn't really be considered an error. ovnkube-node will set the annotation once it has parsed it, but it is not expected to always be readily available for ovnkube-master to read. This also happens to spam the logs Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> (cherry picked from commit 1468b31)

Egress IP has until now been flawed. For EgressIP objects with multiple statuses (essentially inidcating HA mode request) we created several individual logical router policies for every node used for egress IP, however only one was ever used at any given moment. OVN has now implemented ECMP routing for logical router policies allowing us to specify multiple nexthops and OVN to load balance between them. Now all egress nodes assigned to an EgressIP should be used equally during traffic distribution. Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> (cherry picked from commit 19e2f18)

Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> (cherry picked from commit ef615e6)

The old egress IP implementation used to create an LRP per egress node and set nexthop for each. We thus need to remove all of that since we'll be creating one LRP now with multiple nexthops. Conflicts: go-controller/pkg/ovn/egressip.go go-controller/pkg/ovn/egressip_test.go Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> (cherry picked from commit 904ebce)

openshift-ci · 2022-03-22T21:45:26Z

@flavio-fernandes: This pull request references Bugzilla bug 2062842, which is invalid:

expected dependent Bugzilla bug 2059700 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE), but it is POST instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 2062842: [4.8z] After reboot egress node, lr-policy-list was not correct, some duplicate records or missed internal IPs

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.

flavio-fernandes · 2022-03-22T21:48:45Z

/assign @trozet

Conflicts: go-controller/pkg/ovn/egressip.go Signed-off-by: Flavio Fernandes <flaviof@redhat.com> (cherry picked from commit fd9fae4)

Instead of relying on kapi.Pod.Status.PodIPs use the same approach as addPodEgressIPAssignments Conflicts: go-controller/pkg/ovn/egressip.go Signed-off-by: Patryk Diak <pdiak@redhat.com> (cherry picked from commit 8081e22)

Signed-off-by: Flavio Fernandes <flaviof@redhat.com> (cherry picked from commit bf6bcfa)

In cases where OVN database for logical router policies or NATs used by EgressIPs have stale nexthops or wrong external_ips, the sync function should remove them, so the proper row/column gets set. This commit backports the changes in a way that libovsdb is unused. Because of that, all libovsdb calls needed to be converted to legacy util.RunOVNNbctl() implementations. Conflicts: go-controller/pkg/ovn/egressip.go Signed-off-by: Flavio Fernandes <flaviof@redhat.com> (cherry picked from commit 39fcdcf)

Do not use nexhops as part of findReroutePolicyIDs lookup. Ensure nexthops are correct when updating egress ip policies. Signed-off-by: Flavio Fernandes <flaviof@redhat.com> (cherry picked from commit 5852d49)

jechen0648 · 2022-03-24T15:22:05Z

/bugzilla cc-qa

jechen0648 · 2022-03-24T15:24:29Z

/label qe-approved

openshift-ci · 2022-03-24T15:25:23Z

@jechen0648: This pull request references Bugzilla bug 2062842, which is invalid:

expected dependent Bugzilla bug 2059700 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE), but it is POST instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/bugzilla cc-qa

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.

flavio-fernandes · 2022-03-24T18:44:46Z

/retest-required

palonsoro · 2022-03-25T12:14:12Z

/retest-required

flavio-fernandes · 2022-03-27T12:06:29Z

/retest-required

vpickard · 2022-04-06T14:30:43Z

/bugzilla refresh

openshift-ci · 2022-04-06T14:30:53Z

@vpickard: This pull request references Bugzilla bug 2062842, which is valid.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.8.z) matches configured target release for branch (4.8.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 2059700 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE))
dependent Bugzilla bug 2059700 targets the "4.9.z" release, which is one of the valid target releases: 4.9.0, 4.9.z
bug has dependents

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (jechen@redhat.com), skipping review request.

Details

In response to this:

/bugzilla refresh

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-bot · 2022-04-08T00:10:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T00:46:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T01:22:57Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T01:58:55Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T02:10:54Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T02:58:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T03:34:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T04:10:54Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T04:46:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T05:58:55Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T07:11:57Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T07:35:57Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T09:01:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T09:14:55Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T09:27:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T10:17:55Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T10:54:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T11:06:57Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T11:54:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T12:32:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2022-04-08T13:09:11Z

@flavio-fernandes: The following test 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/4.8-upgrade-from-stable-4.7-e2e-aws-ovn-upgrade	`fba695a`	link	false	`/test 4.8-upgrade-from-stable-4.7-e2e-aws-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.

openshift-bot · 2022-04-08T13:11:59Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

flavio-fernandes · 2022-04-08T14:39:40Z

@trozet can we overwrite e2e-metal-ipi-ovn-ipv6 ?
Job info : https://prow.ci.openshift.org/job-history/gs/origin-ci-test/pr-logs/directory/pull-ci-openshift-ovn-kubernetes-release-4.8-e2e-metal-ipi-ovn-ipv6

openshift-bot · 2022-04-08T14:55:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-04-08T15:21:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

abhat · 2022-04-08T15:32:37Z

/override ci/prow/e2e-metal-ipi-ovn-ipv6

openshift-ci · 2022-04-08T15:33:18Z

@abhat: Overrode contexts on behalf of abhat: ci/prow/e2e-metal-ipi-ovn-ipv6

Details

In response to this:

/override ci/prow/e2e-metal-ipi-ovn-ipv6

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 · 2022-04-08T15:34:27Z

@flavio-fernandes: All pull requests linked via external trackers have merged:

openshift/ovn-kubernetes#1009

Bugzilla bug 2062842 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2062842: [4.8z] After reboot egress node, lr-policy-list was not correct, some duplicate records or missed internal IPs

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.

alexanderConstantinescu added 5 commits March 22, 2022 19:23

Update e2e to verify egress IP with ECMP

dc423eb

Signed-off-by: Alexander Constantinescu <aconstan@redhat.com> (cherry picked from commit ef615e6)

openshift-ci bot added the bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. label Mar 22, 2022

openshift-ci bot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Mar 22, 2022

openshift-ci bot requested review from abhat and dcbw March 22, 2022 21:45

openshift-ci bot assigned trozet Mar 22, 2022

flavio-fernandes force-pushed the bz2062842_egressip-4.8 branch from a344cc4 to 04ee445 Compare March 22, 2022 21:56

flavio-fernandes added 5 commits March 23, 2022 22:45

Enable info logging for successful assignment of egress IP

57bb0de

Conflicts: go-controller/pkg/ovn/egressip.go Signed-off-by: Flavio Fernandes <flaviof@redhat.com> (cherry picked from commit fd9fae4)

Fix cache building used for removing stale egress IPs

4a73da1

Instead of relying on kapi.Pod.Status.PodIPs use the same approach as addPodEgressIPAssignments Conflicts: go-controller/pkg/ovn/egressip.go Signed-off-by: Patryk Diak <pdiak@redhat.com> (cherry picked from commit 8081e22)

Egress IP: Remove legacy logical router policies in bulk

9732a40

Signed-off-by: Flavio Fernandes <flaviof@redhat.com> (cherry picked from commit bf6bcfa)

CARRY: fix idempotently updating egress IP route policies

fba695a

Do not use nexhops as part of findReroutePolicyIDs lookup. Ensure nexthops are correct when updating egress ip policies. Signed-off-by: Flavio Fernandes <flaviof@redhat.com> (cherry picked from commit 5852d49)

flavio-fernandes force-pushed the bz2062842_egressip-4.8 branch from 04ee445 to fba695a Compare March 23, 2022 22:59

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Mar 24, 2022

openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Apr 6, 2022

openshift-merge-robot merged commit 2661138 into openshift:release-4.8 Apr 8, 2022

flavio-fernandes deleted the bz2062842_egressip-4.8 branch April 8, 2022 18:48

Conversation

flavio-fernandes commented Mar 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Mar 22, 2022

Uh oh!

flavio-fernandes commented Mar 22, 2022

Uh oh!

jechen0648 commented Mar 24, 2022

Uh oh!

jechen0648 commented Mar 24, 2022

Uh oh!

openshift-ci bot commented Mar 24, 2022

Uh oh!

flavio-fernandes commented Mar 24, 2022

Uh oh!

palonsoro commented Mar 25, 2022

Uh oh!

flavio-fernandes commented Mar 27, 2022

Uh oh!

vpickard commented Apr 6, 2022

Uh oh!

openshift-ci bot commented Apr 6, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-ci bot commented Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

flavio-fernandes commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

openshift-bot commented Apr 8, 2022

Uh oh!

abhat commented Apr 8, 2022

Uh oh!

openshift-ci bot commented Apr 8, 2022

Uh oh!

openshift-ci bot commented Apr 8, 2022

flavio-fernandes commented Mar 22, 2022 •

edited

Loading

openshift-ci bot commented Apr 8, 2022 •

edited

Loading