diff --git a/examples/customization/README.md b/examples/customization/README.md index c83b71c179..b029ac689e 100644 --- a/examples/customization/README.md +++ b/examples/customization/README.md @@ -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` | | @@ -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: `/`. +the config map to use with the following format: `/`. 1. Create a configmaps file with the name *nginx-config.yaml* and set the values that make sense for your setup: diff --git a/examples/customization/nginx-config.yaml b/examples/customization/nginx-config.yaml index 23c374b2ab..0ffb1cadcd 100644 --- a/examples/customization/nginx-config.yaml +++ b/examples/customization/nginx-config.yaml @@ -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; diff --git a/nginx-controller/controller/controller.go b/nginx-controller/controller/controller.go index 947ab0e997..8724f64440 100644 --- a/nginx-controller/controller/controller.go +++ b/nginx-controller/controller/controller.go @@ -34,10 +34,11 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" + "sort" + api_v1 "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sort" ) const ( @@ -449,7 +450,19 @@ func (lbc *LoadBalancerController) syncCfgm(task Task) { } if lbMethod, exists := cfgm.Data["lb-method"]; exists { - cfg.LBMethod = 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 { + 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 + } + } } if proxyConnectTimeout, exists := cfgm.Data["proxy-connect-timeout"]; exists { diff --git a/nginx-controller/nginx/config.go b/nginx-controller/nginx/config.go index 324509c857..45e264a74b 100644 --- a/nginx-controller/nginx/config.go +++ b/nginx-controller/nginx/config.go @@ -73,5 +73,6 @@ func NewDefaultConfig() *Config { SSLPorts: []int{443}, MaxFails: 1, FailTimeout: "10s", + LBMethod: "least_conn", } } diff --git a/nginx-controller/nginx/configurator.go b/nginx-controller/nginx/configurator.go index 6f2f0909e4..3e12aa57f7 100644 --- a/nginx-controller/nginx/configurator.go +++ b/nginx-controller/nginx/configurator.go @@ -280,7 +280,19 @@ func (cnf *Configurator) createConfig(ingEx *IngressEx) Config { //Override from annotation if lbMethod, exists := ingEx.Ingress.Annotations["nginx.org/lb-method"]; exists { - 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: %v", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), lbMethod, err) + } else { + ingCfg.LBMethod = parsedMethod + } + } else { + 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 + } + } } if serverTokens, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.org/server-tokens", ingEx.Ingress); exists { diff --git a/nginx-controller/nginx/extensions.go b/nginx-controller/nginx/extensions.go new file mode 100644 index 0000000000..80ca0745c6 --- /dev/null +++ b/nginx-controller/nginx/extensions.go @@ -0,0 +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 + } + 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 + } + 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) +} diff --git a/nginx-controller/nginx/extensions_test.go b/nginx-controller/nginx/extensions_test.go new file mode 100644 index 0000000000..39011e0e14 --- /dev/null +++ b/nginx-controller/nginx/extensions_test.go @@ -0,0 +1,83 @@ +package nginx + +import "testing" + +func TestParseLBMethod(t *testing.T) { + var testsWithValidInput = []struct { + input string + expected string + }{ + {"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 { + result, err := ParseLBMethod(test.input) + if err != nil { + t.Errorf("TestParseLBMethod(%q) returned an error for valid input", test.input) + } + + if result != test.expected { + t.Errorf("TestParseLBMethod(%q) returned %q expected %q", test.input, result, test.expected) + } + } + + for _, input := range invalidInput { + _, err := ParseLBMethod(input) + if err == nil { + t.Errorf("TestParseLBMethod(%q) does not return an error for invalid input", input) + } + } +} + +func TestParseLBMethodForPlus(t *testing.T) { + var testsWithValidInput = []struct { + input string + expected string + }{ + {"least_conn", "least_conn"}, + {"round_robin", ""}, + {"ip_hash", "ip_hash"}, + {"hash $request_id", "hash $request_id"}, + {"least_time header", "least_time header"}, + {"least_time last_byte", "least_time last_byte"}, + {"least_time header inflight", "least_time header inflight"}, + {"least_time last_byte inflight", "least_time last_byte inflight"}, + } + + var invalidInput = []string{ + "", + "blabla", + "hash123", + "least_time inflight header", + } + + for _, test := range testsWithValidInput { + result, err := ParseLBMethodForPlus(test.input) + if err != nil { + t.Errorf("TestParseLBMethod(%q) returned an error for valid input", test.input) + } + + if result != test.expected { + t.Errorf("TestParseLBMethod(%q) returned %q expected %q", test.input, result, test.expected) + } + } + + for _, input := range invalidInput { + _, err := ParseLBMethodForPlus(input) + if err == nil { + t.Errorf("TestParseLBMethod(%q) does not return an error for invalid input", input) + } + } +}