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.

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

fix(parser) support same-name services across NSes #4375

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Jul 20, 2023

What this PR does / why we need it:

Correctly build upstreams for multiple backends where multiple services in the backends share the same name in different namespaces. Previously services were indexed by name alone in the rule builder, resulting in one clobbering the other(s).

Which issue this PR fixes:

Fix #3860

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Patch coverage: 100.0% and no project coverage change.

Comparison is base (7b111df) 66.6% compared to head (782c4d5) 66.7%.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #4375   +/-   ##
=====================================
  Coverage   66.6%   66.7%           
=====================================
  Files        158     158           
  Lines      18563   18571    +8     
=====================================
+ Hits       12379   12396   +17     
+ Misses      5429    5426    -3     
+ Partials     755     749    -6     
Files Changed Coverage Δ
internal/dataplane/parser/ingressrules.go 95.1% <100.0%> (+1.4%) ⬆️
internal/dataplane/parser/parser.go 82.0% <100.0%> (+2.8%) ⬆️

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rainest rainest force-pushed the fix/multi-namespace-backends branch from a3c2cfd to e571956 Compare July 21, 2023 17:36
@rainest rainest marked this pull request as ready for review July 21, 2023 17:36
@rainest rainest requested a review from a team as a code owner July 21, 2023 17:36
@rainest rainest enabled auto-merge (squash) July 21, 2023 17:36
@pmalek pmalek added this to the KIC v2.11.0 milestone Jul 24, 2023
Correctly build upstreams for multiple backends where multiple services
in the backends share the same name in different namespaces. Previously
services were indexed by name alone in the rule builder, resulting in
one clobbering the other(s).
@rainest rainest force-pushed the fix/multi-namespace-backends branch from e571956 to 8ed3d77 Compare July 25, 2023 19:51
@rainest rainest force-pushed the fix/multi-namespace-backends branch from 8ed3d77 to 1a217fa Compare July 25, 2023 19:52
@rainest rainest requested a review from pmalek July 25, 2023 19:57
@rainest
Copy link
Contributor Author

rainest commented Jul 26, 2023

This encountered a flake with TestGatewayWithGatewayClassReconciliation in envtests. Could not replicate, and don't think we have an equivalent of the integration dumps to try and analyze why it happens when it does.

@rainest rainest merged commit 7a32afb into main Jul 28, 2023
31 checks passed
@rainest rainest deleted the fix/multi-namespace-backends branch July 28, 2023 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gateway API: HttpRoute with weight and namespace doesn't work well.
3 participants