-
Notifications
You must be signed in to change notification settings - Fork 142
Add all load balancing methods #4325
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
base: feat/session-persistence
Are you sure you want to change the base?
Conversation
ec269d8 to
912649a
Compare
912649a to
80fe344
Compare
|
In your |
|
And your |
internal/controller/nginx/config/policies/clientsettings/validator.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Saylor Berman <[email protected]>
4255861 to
f3bddb7
Compare
internal/controller/nginx/config/policies/upstreamsettings/validator.go
Outdated
Show resolved
Hide resolved
72e70cd to
49b16b1
Compare
| } | ||
|
|
||
| if a.LoadBalancingMethod != nil && b.LoadBalancingMethod != nil { | ||
| if !checkConflictForLoadBalancingFields(a, b) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reasoning behind inverting this function? Can we make the function return what we want without needing to invert it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah agree, feels counter intuitive. Just checks if anything of the load balancing fields conflicts, good catch.
Proposed changes
Write a clear and concise description that helps reviewers understand the purpose and impact of your changes. Use the
following format:
Problem: Users want to specify all load balancing methods.
Solution: Specify a way for user to be able to specify all combinations of load balancing methods.
Testing:
Manual testing
Errors
hash key format wrong
plus features with OSS
Spec does not allow any other features so do not need to validate for plus(everything allowed)
USP is set for tea svc for all , verified traffic passing for all
Only mentioning special cases -- round_robin, hash and hash consistent
OSS
Plus verified on tea svc
Verified all load balancing methods with working traffic for plus
Closes #4190
Checklist
Before creating a PR, run through this checklist and mark each as complete.
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.