Fix OVN-K Interconnect: Remove deprecated nexthop field

[yboaron]

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: #4124

Summary by CodeRabbit

• Bug Fixes
  • Improved logical router policy reconciliation for reliable next-hop handling.
  • Policy creation and updates now preserve next-hop information correctly.
  • Legacy policies with missing or outdated next-hop entries are detected and migrated automatically.
  • Improved route matching for different IP address families during gateway and non-gateway route reconciliation.
  • Enhanced migration handling to preserve routing behavior while removing outdated next-hop configurations.

[submariner-bot]

🤖 Created branch: z_pr4125/yboaron/fix-ovn-nexthop-deprecated-field
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Logical router policy reconciliation now uses the Nexthops slice. Legacy policies with deprecated Nexthop values are eligible for migration. Tests now validate IP-family-specific matches and next-hop migration.

Changes

Logical route policy reconciliation

Layer / File(s)	Summary
Use Nexthops for policy reconciliation pkg/routeagent_driver/handlers/ovn/ovn_logical_routes.go	Stale-policy validation detects legacy fields. Policy matching compares Nexthops. New policies set the next hop through Nexthops.
Validate legacy policy migration and IP-family matches pkg/routeagent_driver/handlers/ovn/handler_test.go	Route policy tests use IP-family-specific matches and Nexthops. Migration coverage verifies that reconciliation clears deprecated Nexthop values while retaining the next hop.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• submariner-io/submariner#3881: Modifies OVN logical router policy reconciliation and stale-entry handling.

Suggested labels: release-note-needed

Suggested reviewers: tpantelis, vthapar, oats87

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary change: removing the deprecated OVN-K nexthop field.
Linked Issues check	✅ Passed	The changes remove deprecated Nexthop usage, retain Nexthops, and add migration coverage for issue #4124.
Out of Scope Changes check	✅ Passed	All production and test changes directly support the OVN nexthop migration objective in issue #4124.
Actionable Comments Resolved	✅ Passed	The target commit explicitly addresses the CodeRabbit actionable comment; stale policies with deprecated Nexthop are migrated, and tests verify removal with Nexthops retained.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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/ovn_logical_routes.go`:
- Around line 116-118: The stale-policy predicate in the OVN logical route
reconciliation must also match policies whose legacy Nexthop field is non-nil,
even when Nexthops equals []string{nextHop}; update the condition around the
existing remoteSubnets and Nexthops checks to include that legacy-field state,
and add a regression test covering a managed policy with both Nexthop and
Nexthops populated.

🪄 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: 1e395212-0ec3-439c-abb8-3d5f19bcc0a2

📥 Commits

Reviewing files that changed from the base of the PR and between 7cb77d4 and 074ff3b.

📒 Files selected for processing (1)

• pkg/routeagent_driver/handlers/ovn/ovn_logical_routes.go

[coderabbitai]

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/handler_test.go`:
- Around line 638-651: Update the reconciliation test around the existing policy
assertions to save the original oldPolicy.Nexthop pointer before reconciliation,
then call AwaitNoModel with the matching policy and deprecated Nexthop value to
verify the legacy policy is removed. Keep the existing assertions confirming the
replacement policy has Nexthops populated and 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: 5e5fb236-fb34-423b-8ca4-912b4f196773

📥 Commits

Reviewing files that changed from the base of the PR and between 074ff3b and d843a6d.

📒 Files selected for processing (2)

• pkg/routeagent_driver/handlers/ovn/handler_test.go
• pkg/routeagent_driver/handlers/ovn/ovn_logical_routes.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/routeagent_driver/handlers/ovn/ovn_logical_routes.go

[coderabbitai]

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/handler_test.go`:
- Around line 691-695: The fake OVN client’s logical router policy matching
ignores the expected Nexthops field, allowing replacement policies to match
incorrectly. Update hasModel and GetModel to reuse one shared matcher that
compares Nexthops when the expected model specifies it, while treating
unspecified fields as wildcards to preserve existing match-only lookups.

🪄 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: ce1f7be3-c300-49e8-96e6-fe063aa83540

📥 Commits

Reviewing files that changed from the base of the PR and between d756ecc and 7b3c173.

📒 Files selected for processing (1)

• pkg/routeagent_driver/handlers/ovn/handler_test.go

[submariner-bot]

🤖 Closed branches: [z_pr4125/yboaron/fix-ovn-nexthop-deprecated-field]