Change the default load balancing method to least_conn #274
Conversation
examples/customization/README.md
Outdated
| @@ -39,7 +39,7 @@ The table below summarizes all of the options. For some of them, there are examp | |||
| | N/A | `http-snippets` | Sets a custom snippet in http context. | N/A | | | |||
| | `nginx.org/location-snippets` | `location-snippets` | Sets a custom snippet in location context. | N/A | | | |||
| | `nginx.org/server-snippets` | `server-snippets` | Sets a custom snippet in server context. | N/A | | | |||
| | `nginx.org/lb-method` | `lb-method` | Sets the [load balancing method](https://www.nginx.com/resources/admin-guide/load-balancer/#method). The default `""` specifies the round-robin method. | `""` | | | |||
| | `nginx.org/lb-method` | `lb-method` | Sets the [load balancing method](http://nginx.org/en/docs/http/load_balancing.html#nginx_load_balancing_methods). The default `""` specifies the `least_conn` method. | `""` | | | |||
There was a problem hiding this comment.
Better to use this link -- https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#choosing-a-load-balancing-method
Please mention: To use the round-robin method, specify round_robin. Remove "The default "" specifies the least_conn method"
Also, add least_conn to the default column.
| @@ -45,6 +45,7 @@ data: | |||
| if ($new_uri) { | |||
| rewrite ^ $new_uri permanent; | |||
| } | |||
| lb-method: "" # default is least_conn. Sets the load balancing method for upstreams. See http://nginx.org/en/docs/http/load_balancing.html#nginx_load_balancing_methods | |||
There was a problem hiding this comment.
Change "" to "round_robin". Replace the link with https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#choosing-a-load-balancing-method
| ingCfg.LBMethod = lbMethod | ||
| if cnf.isPlus() { | ||
| if parsedMethod, err := ParseLBMethodForPlus(lbMethod); err != nil { | ||
| glog.Errorf("Ingress %s/%s: Invalid value for the nginx.org/lb-method: got %q", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), lbMethod) |
There was a problem hiding this comment.
could you print the returned err in the logs?
glog.Errorf("Ingress %s/%s: Invalid value for the nginx.org/lb-method: got %q", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), lbMethod)to
glog.Errorf("Ingress %s/%s: Invalid value for the nginx.org/lb-method, got %q: %v", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), lbMethod, err)Please do that here in Configurator and also in Controller
nginx-controller/nginx/extensions.go
Outdated
| if method == "round_robin" { | ||
| return "", nil | ||
| } | ||
| if method == "" { |
There was a problem hiding this comment.
we should treat "" as an invalid value.
nginx-controller/nginx/extensions.go
Outdated
| if method == "least_conn" || method == "ip_hash" { | ||
| return method, nil | ||
| } | ||
| return "", fmt.Errorf("Invalid load balancing method: %v", method) |
There was a problem hiding this comment.
change %v to %q here and in the other places in this file
| input string | ||
| expected string | ||
| }{ | ||
| {"", "least_conn"}, |
There was a problem hiding this comment.
we should treat "" as an invalid value
least_conn""lb-method annotationTested on both NGINX and NGINX Plus