Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Gateway API: handle Route conflicts with GRPCRoute.Matches #6566

Merged
merged 5 commits into from
Jul 26, 2024
Merged

Gateway API: handle Route conflicts with GRPCRoute.Matches #6566

merged 5 commits into from
Jul 26, 2024

Conversation

lubronzhan
Copy link
Contributor

Fix #6267
Handle the match conflict of grpcroutes

It's possible that multiple GRPCRoutes will define the same Match conditions. In this case the following logic is applied to resolve the conflict:

  • The oldest Route based on creation timestamp. For example, a Route with a creation timestamp of “2020-09-08 01:02:03” is given precedence over a Route with a creation timestamp of “2020-09-08 01:02:04”.
  • The Route appearing first in alphabetical order (namespace/name) for example, foo/bar is given precedence over foo/baz.

@lubronzhan lubronzhan requested a review from a team as a code owner July 17, 2024 19:21
@lubronzhan lubronzhan requested review from tsaarni and sunjayBhatia and removed request for a team July 17, 2024 19:21
@sunjayBhatia sunjayBhatia requested review from a team, izturn and clayton-gonsalves and removed request for a team July 17, 2024 19:21
@lubronzhan lubronzhan force-pushed the topic/lubron/fix-6267 branch 2 times, most recently from f689ea5 to dcf568b Compare July 17, 2024 20:13
@codecov Codecov
Copy link

codecov bot commented Jul 17, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.70%. Comparing base (cd05c4f) to head (b5fd797).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6566      +/-   ##
==========================================
+ Coverage   81.64%   81.70%   +0.06%     
==========================================
  Files         133      133              
  Lines       15873    15893      +20     
==========================================
+ Hits        12959    12986      +27     
+ Misses       2620     2614       -6     
+ Partials      294      293       -1
Files Coverage Δ
internal/dag/gatewayapi_processor.go 93.87% <100.00%> (+0.59%) ⬆️
internal/status/routeconditions.go 53.01% <ø> (ø)

@lubronzhan lubronzhan added the release-note/minor A minor change that needs about a paragraph of explanation in the release notes. label Jul 17, 2024
sunjayBhatia
sunjayBhatia reviewed Jul 22, 2024
View reviewed changes
Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm overall, just some nits

changelogs/unreleased/6566-lubronzhan-minor.md Outdated Show resolved Hide resolved
internal/dag/gatewayapi_processor_test.go Outdated Show resolved Hide resolved
internal/dag/status_test.go Show resolved Hide resolved
@lubronzhan
Gateway API: handle Route conflicts with GRPCRoute.Matches
201b5f6 
Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
@lubronzhan
Release note
adfef18 
Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
@lubronzhan
Fix test
ffacce9 
Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
@lubronzhan
More test cases
039e83e 
Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
@lubronzhan
Resolve comments
b5fd797 
Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
@lubronzhan lubronzhan force-pushed the topic/lubron/fix-6267 branch from 0f02e9c to b5fd797 Compare July 24, 2024 22:28
skriss
skriss approved these changes Jul 26, 2024
View reviewed changes
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @lubronzhan

@sunjayBhatia sunjayBhatia merged commit 7fa5725 into projectcontour:main Jul 26, 2024
26 checks passed
@lubronzhan lubronzhan deleted the topic/lubron/fix-6267 branch July 27, 2024 05:06
geomacy pushed a commit to chaosbox/contour that referenced this pull request Aug 22, 2024
@lubronzhan
Gateway API: handle Route conflicts with GRPCRoute.Matches (projectco…
a01e8a7 
…ntour#6566)

Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
Signed-off-by: Geoff Macartney <geoff.macartney@sky.uk>
SamMHD pushed a commit to SamMHD/contour that referenced this pull request Sep 8, 2024
@lubronzhan @SamMHD
Gateway API: handle Route conflicts with GRPCRoute.Matches (projectco…
3a8271c 
…ntour#6566)

Signed-off-by: Lubron Zhan <lubronzhan@gmail.com>
Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sunjayBhatia sunjayBhatia sunjayBhatia approved these changes

@skriss skriss skriss approved these changes

@tsaarni tsaarni Awaiting requested review from tsaarni tsaarni is a code owner automatically assigned from projectcontour/maintainers

@izturn izturn Awaiting requested review from izturn izturn was automatically assigned from projectcontour/contour-reviewers

@clayton-gonsalves clayton-gonsalves Awaiting requested review from clayton-gonsalves clayton-gonsalves was automatically assigned from projectcontour/contour-reviewers

Assignees
No one assigned
Labels
release-note/minor A minor change that needs about a paragraph of explanation in the release notes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Gateway API: handle Route conflicts with GRPCRoute.Matches
3 participants
@lubronzhan @skriss @sunjayBhatia