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

api: BackendTrafficPolicy DNSLookupFamily #5249

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

guydc
Copy link
Contributor

@guydc guydc commented Feb 10, 2025

What type of PR is this?

What this PR does / why we need it:

  • Makes it possible to define DNS lookup family for xRoute and non-xRoute BackendRefs.
  • This is particularly useful when using the Backend resource, see Breaking change: Backend cluster DNS lookup is V4_PREFERRED #5247
  • These settings will override any default configuration that Envoy Gateway autogenerates based on Service IpFamilies.

Which issue(s) this PR fixes:

Fixes #

Release Notes: In the implementation PR

@guydc guydc requested a review from a team as a code owner February 10, 2025 22:31
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.03%. Comparing base (338b3c2) to head (0c53c02).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5249      +/-   ##
==========================================
+ Coverage   64.96%   65.03%   +0.06%     
==========================================
  Files         214      214              
  Lines       33532    33532              
==========================================
+ Hits        21785    21806      +21     
+ Misses      10407    10389      -18     
+ Partials     1340     1337       -3     

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

@guydc guydc force-pushed the btp-dns-lookup-family branch from b69dd8d to 7b834cd Compare February 10, 2025 22:53
@arkodg
Copy link
Contributor

arkodg commented Feb 10, 2025

thanks @guydc, this looks good !
reg naming, does any other implementation have a similar setting

@guydc
Copy link
Contributor Author

guydc commented Feb 10, 2025

Some examples:

nginx:

istio:

contour:

F5:

So, there's a variety of styles.

type DNSLookupFamily string

const (
// IPv4Only means the DNS resolver will first perform a lookup for addresses in the IPv4 family.
Copy link
Contributor

Choose a reason for hiding this comment

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

thoughts on just IPv4 ? its less to type, but will it confuse the user who is aware of the envoyproxy api enums?

api/v1alpha1/dns_types.go Outdated Show resolved Hide resolved
@guydc guydc force-pushed the btp-dns-lookup-family branch from bfecbf3 to 7c9ee30 Compare February 18, 2025 19:05
Copy link
Contributor

@arkodg arkodg 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 !

@arkodg arkodg requested a review from zirain February 18, 2025 20:20
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
@zirain zirain force-pushed the btp-dns-lookup-family branch from 7c9ee30 to 0c53c02 Compare February 18, 2025 23:52
@arkodg arkodg merged commit b8eaaed into envoyproxy:main Feb 19, 2025
28 checks passed
tobrien-nydig pushed a commit to tobrien-nydig/gateway that referenced this pull request Feb 19, 2025
* api: BackendTrafficPolicy DNSLookupFamily

Signed-off-by: Guy Daich <[email protected]>

* fix api

Signed-off-by: Guy Daich <[email protected]>

* fix enum

Signed-off-by: Guy Daich <[email protected]>

---------

Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Tim OBrien <[email protected]>
tobrien-nydig pushed a commit to tobrien-nydig/gateway that referenced this pull request Feb 19, 2025
* api: BackendTrafficPolicy DNSLookupFamily

Signed-off-by: Guy Daich <[email protected]>

* fix api

Signed-off-by: Guy Daich <[email protected]>

* fix enum

Signed-off-by: Guy Daich <[email protected]>

---------

Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Tim OBrien <[email protected]>
tobrien-nydig pushed a commit to tobrien-nydig/gateway that referenced this pull request Feb 19, 2025
* api: BackendTrafficPolicy DNSLookupFamily

Signed-off-by: Guy Daich <[email protected]>

* fix api

Signed-off-by: Guy Daich <[email protected]>

* fix enum

Signed-off-by: Guy Daich <[email protected]>

---------

Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Tim OBrien <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants