Skip to content

api: add zoneAware lb config#6154

Merged
rudrakhp merged 39 commits intoenvoyproxy:mainfrom
jukie:api-locality-routing-config
Jun 27, 2025
Merged

api: add zoneAware lb config#6154
rudrakhp merged 39 commits intoenvoyproxy:mainfrom
jukie:api-locality-routing-config

Conversation

@jukie
Copy link
Contributor

@jukie jukie commented May 22, 2025

What type of PR is this?

api: add ZoneAware field in BackendTrafficPolicy.LoadBalancer

What this PR does / why we need it:
Exposes explicit configuration for zone aware and locality weighted lb

Which issue(s) this PR fixes:

xRef #6025

Release Notes: No

Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@codecov
Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.84%. Comparing base (8cc91b5) to head (f716197).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6154      +/-   ##
==========================================
- Coverage   70.85%   70.84%   -0.01%     
==========================================
  Files         220      220              
  Lines       37177    37177              
==========================================
- Hits        26341    26338       -3     
- Misses       9294     9296       +2     
- Partials     1542     1543       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jukie and others added 4 commits May 22, 2025 07:51
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
@jukie jukie force-pushed the api-locality-routing-config branch from f7db49a to d0d1373 Compare May 22, 2025 16:46
@jukie jukie marked this pull request as ready for review May 22, 2025 17:05
@jukie jukie requested a review from a team as a code owner May 22, 2025 17:05
@jukie
Copy link
Contributor Author

jukie commented May 26, 2025

/retest

@jukie
Copy link
Contributor Author

jukie commented May 27, 2025

/retest

jukie and others added 6 commits May 27, 2025 22:21
@jukie
Copy link
Contributor Author

jukie commented May 29, 2025

/retest

@jukie
Copy link
Contributor Author

jukie commented May 31, 2025

Hey @arkodg @guydc could I have a review on this when you get the chance?

@jukie jukie requested review from arkodg and rudrakhp June 26, 2025 18:19
//
// +optional
// +notImplementedHide
MinEndpointsCount *uint64 `json:"minEndpointsCount,omitempty"`
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 MinEndpointsThreshold ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me

Copy link
Contributor

Choose a reason for hiding this comment

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

wdyt @envoyproxy/gateway-maintainers

Copy link
Member

@rudrakhp rudrakhp Jun 26, 2025

Choose a reason for hiding this comment

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

For consistency, IMO we should align the naming of minEndpointsCount and minEndpointsInZoneCount. If we're switching to Threshold, lets apply to both? Fine with both 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's what I had assumed and changed both to use "Threshold"

jukie added 2 commits June 26, 2025 13:02
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
Signed-off-by: jukie <10012479+Jukie@users.noreply.github.com>
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 review from a team June 26, 2025 19:36
@jukie jukie changed the title api: add requestdistribution config api: add zoneAware lb config Jun 26, 2025
@jukie
Copy link
Contributor Author

jukie commented Jun 27, 2025

@rudrakhp could you take another look here?

Copy link
Member

@rudrakhp rudrakhp left a comment

Choose a reason for hiding this comment

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

LGTM!

@rudrakhp rudrakhp enabled auto-merge (squash) June 27, 2025 14:24
@rudrakhp rudrakhp merged commit 7b7052c into envoyproxy:main Jun 27, 2025
31 checks passed
@jukie jukie deleted the api-locality-routing-config branch June 27, 2025 14:40
@rudrakhp
Copy link
Member

@jukie I missed this the first time, but looks like the json name for the attribute has not been updated here:

MinEndpointsThreshold *uint64 `json:"minEndpointsCount,omitempty"`

Can you please raise a quick fix for this, thanks!

@jukie
Copy link
Contributor Author

jukie commented Jun 27, 2025

Good catch, will do!

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