Automated backport of #4126: Fix OVN-K Interconnect: Remove deprecated nexthop field - #4128
Merged
tpantelis merged 4 commits intoAug 17, 2026
Conversation
OVN 26.03.3+ (OCP 4.21.27+, 4.22.6+) ignores Logical Router Policies that have the deprecated 'nexthop' (singular) field set, even when 'nexthops' (array) is correctly populated. This causes RouteAgent health checks to fail because ovn-northd does not generate southbound flows for these policies, logging: 'policy uses deprecated column nexthop, this column is ignored' Why not keep both fields? Manual testing showed that when BOTH nexthop and nexthops are set, ovn-northd ignores the entire policy. Only when nexthop field is cleared (leaving only nexthops populated) does the southbound flow appear and RouteAgent health checks succeed. Fixes: submariner-io#4124 Signed-off-by: Yossi Boaron <yboaron@redhat.com>
Enhance stale policy detection to also match policies with the deprecated Nexthop field set, even when Nexthops is correctly populated. This ensures complete migration from old Submariner versions. Add regression test that verifies: - Old policies with both Nexthop (deprecated) and Nexthops (correct) set - Are detected as stale and replaced during reconciliation - New policies have only Nexthops field (Nexthop is nil) Addresses: CodeRabbit actionable comment on PR submariner-io#4125 Signed-off-by: Yossi Boaron <yboaron@redhat.com>
yboaron
requested review from
Oats87,
skitt,
sridhargaddam,
tpantelis and
vthapar
as code owners
August 11, 2026 20:31
Contributor
|
🤖 Created branch: z_pr4128/yboaron/automated-backport-of-#4126-origin-release-0.23 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughOVN logical router policy reconciliation now uses ChangesOVN Nexthops reconciliation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OVNHandler
participant ovn_logical_routes
participant fake_ovsdb_client
participant LogicalRouterPolicy
OVNHandler->>ovn_logical_routes: reconcile GatewayRoute
ovn_logical_routes->>fake_ovsdb_client: find stale policies
fake_ovsdb_client->>LogicalRouterPolicy: evaluate Nexthops and legacy Nexthop
ovn_logical_routes->>fake_ovsdb_client: delete stale policies
ovn_logical_routes->>fake_ovsdb_client: create policy with Nexthops
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/routeagent_driver/handlers/ovn/fake/ovsdb_client.go`:
- Around line 151-173: The fake OVSDB client must preserve distinct
LogicalRouterPolicy rows during migration instead of deduplicating by Match
alone. Update Create at
pkg/routeagent_driver/handlers/ovn/fake/ovsdb_client.go:151-173, and adjust the
related hasModel logic at
pkg/routeagent_driver/handlers/ovn/fake/ovsdb_client.go:186-195 and GetModel
logic at pkg/routeagent_driver/handlers/ovn/fake/ovsdb_client.go:243-252 so an
omitted Nexthops probe treats that field as unconstrained. Update the migration
assertions in pkg/routeagent_driver/handlers/ovn/handler_test.go:650-669 to
verify the legacy row is absent and the migrated row has Nexthop == nil.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6dc3d98c-2a09-42e5-9b70-d3a1c45582ab
📒 Files selected for processing (3)
pkg/routeagent_driver/handlers/ovn/fake/ovsdb_client.gopkg/routeagent_driver/handlers/ovn/handler_test.gopkg/routeagent_driver/handlers/ovn/ovn_logical_routes.go
yboaron
force-pushed
the
automated-backport-of-#4126-origin-release-0.23
branch
from
August 12, 2026 05:00
34107fc to
5477d0c
Compare
- Fix Go 1.26 new(expr) syntax to Go 1.25 compatible pointer creation - Remove invalid context parameters from helper method calls (release-0.24 helper signatures don't accept context) - Update fake OVSDB client to match on Nexthop field when set - Remove unused k8s.io/utils/ptr import This addresses CodeRabbit comments #2, #3, and #4 for the backport. Comment #1 (Priority check) is a false positive for Submariner's design. Signed-off-by: Yossi Boaron <yboaron@redhat.com>
yboaron
force-pushed
the
automated-backport-of-#4126-origin-release-0.23
branch
from
August 12, 2026 06:57
5477d0c to
2ceb4be
Compare
Contributor
Author
|
It seems the lint vulnerability scanning failure is pre-existing on release-0.23 and unrelated to this PR — PR #4130 was merged on 0.23 repo with the identical failure, should be safe to merge. |
tpantelis
approved these changes
Aug 17, 2026
…4126-origin-release-0.23
tpantelis
enabled auto-merge (rebase)
August 17, 2026 19:11
Contributor
|
🤖 Closed branches: [z_pr4128/yboaron/automated-backport-of-#4126-origin-release-0.23] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #4126 on release-0.23.
#4126: Fix OVN-K Interconnect: Remove deprecated nexthop field
For details on the backport process, see the backport requests page.
Summary by CodeRabbit