Refactor lb-method key and annotation

nginx · Apr 18, 2018 · ae9c98c · ae9c98c
1 parent ef6d825
commit ae9c98c
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 3 deletions.
diff --git 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,11 @@ func (lbc *LoadBalancerController) syncCfgm(task Task) {
 		}
 
 		if lbMethod, exists := cfgm.Data["lb-method"]; exists {
-			cfg.LBMethod = lbMethod
+			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)
+			} 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
@@ -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
@@ -280,7 +280,11 @@ 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 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)
+		} 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
@@ -0,0 +1,15 @@
+package nginx
+
+func ParseLBMethod(method string) (string, error) {
+	if method == "round_robin" {
+		return "", nil
+	}
+	return method, nil
+}
+
+func ParseLBMethodForPlus(method string) (string, error) {
+	if method == "round_robin" {
+		return "", nil
+	}
+	return method, nil
+}
diff --git a/nginx-controller/nginx/extensions_test.go b/nginx-controller/nginx/extensions_test.go
@@ -0,0 +1,81 @@
+package nginx
+
+import "testing"
+
+func TestParseLBMethod(t *testing.T) {
+	var testsWithValidInput = []struct {
+		input    string
+		expected string
+	}{
+		{"", "least_conn"},
+		{"least_conn", "least_conn"},
+		{"round_robin", ""},
+		{"ip_hash", "ip_hash"},
+		{"hash $request_id", "hash $request_id"},
+	}
+
+	var invalidInput = []string{
+		"blabla",
+		"least_time header",
+		"hash123",
+	}
+
+	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", "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 header inflight",
+	}
+
+	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)
+		}
+	}
+}