-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
reverseproxy: Allow 0
as weights for weighted_round_robin
#6388
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks -- I wonder, does the actual selection algorithm need adjusting though to accommodate a 0 weight? Have you tested to ensure this works? (I guess we should consider adding test cases.)
if weightInt < 1 { | ||
return d.Errf("invalid weight value '%s': weight should be non-zero and positive", weight) | ||
if weightInt < 0 { | ||
return d.Errf("invalid weight value '%s': weight should be positive", weight) |
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.
return d.Errf("invalid weight value '%s': weight should be positive", weight) | |
return d.Errf("invalid weight value '%s': weight should be non-negative", weight) |
0 isn't positive but is also allowed now 😉
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.
Good question, I don't have the enough knowledge to write the test case for this. Any guide would be helpful.
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.
Have you at least tested various configs yourself to ensure they work? If so that's probably enough to start with.
But it looks like the existing tests are in selectionpolicies_test.go
at func TestWeightedRoundRobinPolicy(t *testing.T) {
. You could test a zero-weight in there as well.
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.
Thank you for your reply, will work on generating the test case.
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.
@peanutduck this still needs tests. Are you interested in finishing this up?
Change positive to non-negative
0
as weights for weighted_round_robin
Enable 0 weights to be allowed when using weighted_round_robin policy with cookie. Discussed in https://caddy.community/t/invalid-weight-value/24440