Support the random load balancing method #376
Conversation
eduardborges
requested review from
isaachawley,
peterkellydev and
pleshakov
There was a problem hiding this comment.
random algorithm is a bit more involved. take a look at http://nginx.org/en/docs/http/ngx_http_upstream_module.html#random it takes two additional parameters. It is important to consider those cases and validate them.
eduardborges
force-pushed
the
add-support-lb-random-algortihm
branch
from
b5a70ab to
3b7fad6
Compare
| @@ -10,6 +10,9 @@ func TestParseLBMethod(t *testing.T) { | |||
| {"least_conn", "least_conn"}, | |||
| {"round_robin", ""}, | |||
| {"ip_hash", "ip_hash"}, | |||
| {"random", "random"}, | |||
| {"random two", "random two"}, | |||
| {"random two least_conn", "random two least_conn"}, | |||
| {"hash $request_id", "hash $request_id"}, | |||
There was a problem hiding this comment.
please also test for some of invalid input:
random one, random two ip_hash, random two least_time
| {"random two", "random two"}, | ||
| {"random two least_conn", "random two least_conn"}, | ||
| {"random two least_time=header", "random two least_time=header"}, | ||
| {"random two least_time=last_byte", "random two least_time=last_byte"}, |
There was a problem hiding this comment.
please also test on some of invalid input:
random one
random two ip_hash"
internal/nginx/extensions.go
Outdated
| func validateRandomLBMethodForPlus(method string) (string, error) { | ||
| keyWords := strings.Split(method, " ") | ||
| if keyWords[0] == "random" { | ||
| if len(keyWords) == 1 || len(keyWords) == 2 && keyWords[1] == "two" || |
There was a problem hiding this comment.
since the total number of combinations for the random algorithms is small, I suggest to putting them in the nginxPlusLBValidInput map. This will simplify the code.
internal/nginx/extensions.go
Outdated
| @@ -17,6 +17,10 @@ func ParseLBMethod(method string) (string, error) { | |||
| method, err := validateHashLBMethod(method) | |||
| return method, err | |||
| } | |||
| if strings.HasPrefix(method, "random") { | |||
There was a problem hiding this comment.
I suggest creating a map with valid input for NGINX OSS, similar to nginxPlusLBValidInput. It will include least_conn, ip_hash and combinations for the random method.
pleshakov
changed the title
Brings support for the random load balancing method - http://nginx.org/en/docs/http/ngx_http_upstream_module.html#random - added in NGINX OSS 1.15.1 and NGINX Plus R16.
pleshakov
force-pushed
the
add-support-lb-random-algortihm
branch
from
312d362 to
0ea4885
Compare
Proposed changes
Add support for the new load balancing random algorithm
Checklist
Before creating a PR, run through this checklist and mark each as complete.