Skip to content

Commit

Permalink
Update default lb-method to be least_conn
Browse files Browse the repository at this point in the history
  • Loading branch information
Dean-Coakley committed Apr 20, 2018
1 parent ae9c98c commit 547055c
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 15 deletions.
4 changes: 2 additions & 2 deletions examples/customization/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#choosing-a-load-balancing-method). To use the round-robin method, specify `"round_robin"`. | `"least_conn"` | |
| `nginx.org/listen-ports` | N/A | Configures HTTP ports that NGINX will listen on. | `[80]` | |
| `nginx.org/listen-ports-ssl` | N/A | Configures HTTPS ports that NGINX will listen on. | `[443]` | |
| N/A | `worker-processes` | Sets the value of the [worker_processes](http://nginx.org/en/docs/ngx_core_module.html#worker_processes) directive. | `auto` | |
Expand All @@ -64,7 +64,7 @@ The table below summarizes all of the options. For some of them, there are examp

1. Make sure that you specify the configmaps resource to use when you start an Ingress controller.
For example, `-nginx-configmaps=default/nginx-config`, where we specify
the config map to use with the following format: `<namespace>/<name>`.
the config map to use with the following format: `<namespace>/<name>`.

1. Create a configmaps file with the name *nginx-config.yaml* and set the values
that make sense for your setup:
Expand Down
1 change: 1 addition & 0 deletions examples/customization/nginx-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ data:
if ($new_uri) {
rewrite ^ $new_uri permanent;
}
lb-method: "round_robin" # default is least_conn. Sets the load balancing method for upstreams. See https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#choosing-a-load-balancing-method
location-snippets: | # No default. Pipe is used for multiple line snippets. Make sure the snippet is not a default value, in order to avoid duplication.
proxy_temp_path /var/nginx/proxy_temp;
charset koi8-r;
Expand Down
14 changes: 11 additions & 3 deletions nginx-controller/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,10 +450,18 @@ func (lbc *LoadBalancerController) syncCfgm(task Task) {
}

if lbMethod, exists := cfgm.Data["lb-method"]; exists {
if parsedMethod, err := nginx.ParseLBMethod(lbMethod); err != nil {
glog.Errorf("Configmap %s/%s: Invalid value for the lb-method key: got %q", cfgm.GetNamespace(), cfgm.GetName(), lbMethod)
if lbc.nginxPlus {
if parsedMethod, err := nginx.ParseLBMethodForPlus(lbMethod); err != nil {
glog.Errorf("Configmap %s/%s: Invalid value for the lb-method key: got %q: %v", cfgm.GetNamespace(), cfgm.GetName(), lbMethod, err)
} else {
cfg.LBMethod = parsedMethod
}
} else {
cfg.LBMethod = parsedMethod
if parsedMethod, err := nginx.ParseLBMethod(lbMethod); err != nil {
glog.Errorf("Configmap %s/%s: Invalid value for the lb-method key: got %q: %v", cfgm.GetNamespace(), cfgm.GetName(), lbMethod, err)
} else {
cfg.LBMethod = parsedMethod
}
}
}

Expand Down
14 changes: 11 additions & 3 deletions nginx-controller/nginx/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,18 @@ func (cnf *Configurator) createConfig(ingEx *IngressEx) Config {

//Override from annotation
if lbMethod, exists := ingEx.Ingress.Annotations["nginx.org/lb-method"]; exists {
if parsedMethod, err := ParseLBMethod(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)
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: %v", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), lbMethod, err)
} else {
ingCfg.LBMethod = parsedMethod
}
} else {
ingCfg.LBMethod = parsedMethod
if parsedMethod, err := ParseLBMethod(lbMethod); err != nil {
glog.Errorf("Ingress %s/%s: Invalid value for the nginx.org/lb-method: got %q: %v", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), lbMethod, err)
} else {
ingCfg.LBMethod = parsedMethod
}
}
}

Expand Down
50 changes: 48 additions & 2 deletions nginx-controller/nginx/extensions.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,61 @@
package nginx

import (
"fmt"
"strings"
)

// ParseLBMethod parses method and matches it to a corresponding load balancing method in NGINX. An error is returned if method is not valid
func ParseLBMethod(method string) (string, error) {
method = strings.TrimSpace(method)
if method == "round_robin" {
return "", nil
}
return method, nil
if strings.HasPrefix(method, "hash") {
method, err := validateHashLBMethod(method)
return method, err
}

if method == "least_conn" || method == "ip_hash" {
return method, nil
}
return "", fmt.Errorf("Invalid load balancing method: %q", method)
}

var nginxPlusLBValidInput = map[string]bool{
"least_time": true,
"last_byte": true,
"least_conn": true,
"ip_hash": true,
"least_time header": true,
"least_time last_byte": true,
"least_time header inflight": true,
"least_time last_byte inflight": true,
}

// ParseLBMethodForPlus parses method and matches it to a corresponding load balancing method in NGINX Plus. An error is returned if method is not valid
func ParseLBMethodForPlus(method string) (string, error) {
method = strings.TrimSpace(method)
if method == "round_robin" {
return "", nil
}
return method, nil
if strings.HasPrefix(method, "hash") {
method, err := validateHashLBMethod(method)
return method, err
}

if _, exists := nginxPlusLBValidInput[method]; exists {
return method, nil
}
return "", fmt.Errorf("Invalid load balancing method: %q", method)
}

func validateHashLBMethod(method string) (string, error) {
keyWords := strings.Split(method, " ")
if keyWords[0] == "hash" {
if len(keyWords) == 2 || len(keyWords) == 3 && keyWords[2] == "consistent" {
return method, nil
}
}
return "", fmt.Errorf("Invalid load balancing method: %q", method)
}
12 changes: 7 additions & 5 deletions nginx-controller/nginx/extensions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@ func TestParseLBMethod(t *testing.T) {
input string
expected string
}{
{"", "least_conn"},
{"least_conn", "least_conn"},
{"round_robin", ""},
{"ip_hash", "ip_hash"},
{"hash $request_id", "hash $request_id"},
{"hash $request_id consistent", "hash $request_id consistent"},
}

var invalidInput = []string{
"",
"blabla",
"least_time header",
"hash123",
"hash $request_id conwrongspelling",
}

for _, test := range testsWithValidInput {
Expand All @@ -44,7 +46,6 @@ func TestParseLBMethodForPlus(t *testing.T) {
input string
expected string
}{
{"", "least_conn"},
{"least_conn", "least_conn"},
{"round_robin", ""},
{"ip_hash", "ip_hash"},
Expand All @@ -56,13 +57,14 @@ func TestParseLBMethodForPlus(t *testing.T) {
}

var invalidInput = []string{
"",
"blabla",
"hash123",
"least_time header inflight",
"least_time inflight header",
}

for _, test := range testsWithValidInput {
result, err := ParseLBMethod(test.input)
result, err := ParseLBMethodForPlus(test.input)
if err != nil {
t.Errorf("TestParseLBMethod(%q) returned an error for valid input", test.input)
}
Expand All @@ -73,7 +75,7 @@ func TestParseLBMethodForPlus(t *testing.T) {
}

for _, input := range invalidInput {
_, err := ParseLBMethod(input)
_, err := ParseLBMethodForPlus(input)
if err == nil {
t.Errorf("TestParseLBMethod(%q) does not return an error for invalid input", input)
}
Expand Down

0 comments on commit 547055c

Please sign in to comment.