Address review comments

internal/configs/virtualserver.go

@@ -14,14 +14,14 @@ import (
 
 const nginx502Server = "unix:/var/run/nginx-502-server.sock"
 
-var incompatibleLBMethodsForSlowStart = []string{
-	"random",
-	"ip-hash",
-	"hash",
-	"random two",
-	"random two least_conn",
-	"random two least_time=header",
-	"random two least_time=last_byte",
+var incompatibleLBMethodsForSlowStart = map[string]bool{
+	"random":                          true,
+	"ip-hash":                         true,
+	"hash":                            true,
+	"random two":                      true,
+	"random two least_conn":           true,
+	"random two least_time=header":    true,
+	"random two least_time=last_byte": true,
 }
 
 // VirtualServerEx holds a VirtualServer along with the resources that are referenced in this VirtualServer.
@@ -371,19 +371,19 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, isEx
 }
 
 func generateSlowStartForPlus(upstream conf_v1alpha1.Upstream, lbMethod string) string {
-
 	slowStart := upstream.SlowStart
 
 	if slowStart == "" {
 		return ""
 	}
+	method := strings.TrimSpace(lbMethod)
+	if strings.HasPrefix(method, "hash") {
+		method = "hash"
+	}
 
-	for _, nLBMethod := range incompatibleLBMethodsForSlowStart {
-		if strings.Contains(lbMethod, nLBMethod) {
-			//TODO trigger warning
-			glog.Warningf("slow-start will be disabled for the Upstream %v", upstream.Name)
-			return ""
-		}
+	if _, exists := incompatibleLBMethodsForSlowStart[method]; exists {
+		glog.Warningf("slow-start will be disabled for the Upstream %v", upstream.Name)
+		return ""
 	}
 
 	return slowStart

internal/configs/virtualserver_test.go

@@ -1962,50 +1962,43 @@ func TestGenerateEndpointsForUpstream(t *testing.T) {
 	}
 }
 
-func TestGenerateSlowStartForPlus(t *testing.T) {
-	name := "test-slowstart"
-	// slow_start is "10s" and the method is "hash $something", expects ""
-	// slow_start is "10s" and the method is "ip_hash", expects ""
-	// slow_start is "10s" and the method is "random", expects ""
-	// slow_start is "10s" and the method is "random two", expects ""
-	// slow_start is "10s" and the method is "random two least_conn", expects ""
-	// slow_start is "10s" and the method is "random two least_time=header", expects ""
-	// slow_start is "10s" and the method is "random two least_time=last_byte", expects ""
-
-	{
-		upstream := conf_v1alpha1.Upstream{Service: name, Port: 80, SlowStart: "10s"}
-		for _, lbMethod := range incompatibleLBMethodsForSlowStart {
-			lbMethod := generateLBMethod(upstream.LBMethod, lbMethod)
-			result := generateSlowStartForPlus(upstream, lbMethod)
-			expected := ""
-
-			if !reflect.DeepEqual(result, expected) {
-				t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, expected)
-			}
-		}
-	}
+func TestGenerateSlowStartForPlusWithInCompatibleLBMethods(t *testing.T) {
+	name := "test-slowstart-with-incompatible-LBMethods"
+	upstream := conf_v1alpha1.Upstream{Service: name, Port: 80, SlowStart: "10s"}
+	expected := ""
 
-	// slow start is "" and the method is "least_conn", expects ""
-	{
-		upstream := conf_v1alpha1.Upstream{Service: name, Port: 80, SlowStart: "", LBMethod: "least_conn"}
-		lbMethod := generateLBMethod(upstream.LBMethod, "least_conn")
-		result := generateSlowStartForPlus(upstream, lbMethod)
-		expected := ""
+	for lbMethod, _ := range incompatibleLBMethodsForSlowStart {
+		nlbMethod := generateLBMethod(upstream.LBMethod, lbMethod)
+		result := generateSlowStartForPlus(upstream, nlbMethod)
 
 		if !reflect.DeepEqual(result, expected) {
 			t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, expected)
 		}
 	}
+}
 
-	// slow_start is "10s" and the method is "least_conn", expects "10s"
-	{
-		upstream := conf_v1alpha1.Upstream{Service: name, Port: 80, SlowStart: "10s", LBMethod: "least_conn"}
-		lbMethod := generateLBMethod(upstream.LBMethod, "least_conn")
-		result := generateSlowStartForPlus(upstream, lbMethod)
-		expected := "10s"
+func TestGenerateSlowStartForPlus(t *testing.T) {
+	name := "test-slowstart"
 
-		if !reflect.DeepEqual(result, expected) {
-			t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, expected)
+	tests := []struct {
+		upstream conf_v1alpha1.Upstream
+		expected string
+	}{
+		{
+			upstream: conf_v1alpha1.Upstream{Service: name, Port: 80, SlowStart: "", LBMethod: "least_conn"},
+			expected: "",
+		},
+		{
+			upstream: conf_v1alpha1.Upstream{Service: name, Port: 80, SlowStart: "10s", LBMethod: "least_conn"},
+			expected: "10s",
+		},
+	}
+
+	for _, test := range tests {
+		lbMethod := generateLBMethod(test.upstream.LBMethod, "least_conn")
+		result := generateSlowStartForPlus(test.upstream, lbMethod)
+		if !reflect.DeepEqual(result, test.expected) {
+			t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, test.expected)
 		}
 	}
 }