Merge pull request #1515 from mynameiswhm/master

Allow usage of non_idempotent option in proxy_next_upstream

pkg/nginx/template/template.go

@@ -44,6 +44,7 @@ import (
 
 const (
 	slash         = "/"
+	nonIdempotent = "non_idempotent"
 	defBufferSize = 65535
 )
 
@@ -548,20 +549,30 @@ func isSticky(host string, loc *ingress.Location, stickyLocations map[string][]s
 	return false
 }
 
-func buildNextUpstream(input interface{}) string {
-	nextUpstream, ok := input.(string)
+func buildNextUpstream(i, r interface{}) string {
+	nextUpstream, ok := i.(string)
 	if !ok {
-		glog.Errorf("expected a 'string' type but %T was returned", input)
+		glog.Errorf("expected a 'string' type but %T was returned", i)
 		return ""
 	}
 
+	retryNonIdempotent := r.(bool)
+
 	parts := strings.Split(nextUpstream, " ")
 
 	nextUpstreamCodes := make([]string, 0, len(parts))
 	for _, v := range parts {
-		if v != "" && v != "non_idempotent" {
+		if v != "" && v != nonIdempotent {
 			nextUpstreamCodes = append(nextUpstreamCodes, v)
 		}
+
+		if v == nonIdempotent {
+			retryNonIdempotent = true
+		}
+	}
+
+	if retryNonIdempotent {
+		nextUpstreamCodes = append(nextUpstreamCodes, nonIdempotent)
 	}
 
 	return strings.Join(nextUpstreamCodes, " ")

pkg/nginx/template/template_test.go

@@ -311,13 +311,40 @@ func TestBuildResolvers(t *testing.T) {
 }
 
 func TestBuildNextUpstream(t *testing.T) {
-	nextUpstream := "timeout http_500 http_502 non_idempotent"
-	validNextUpstream := "timeout http_500 http_502"
+	cases := map[string]struct {
+		NextUpstream  string
+		NonIdempotent bool
+		Output        string
+	}{
+		"default": {
+			"timeout http_500 http_502",
+			false,
+			"timeout http_500 http_502",
+		},
+		"global": {
+			"timeout http_500 http_502",
+			true,
+			"timeout http_500 http_502 non_idempotent",
+		},
+		"local": {
+			"timeout http_500 http_502 non_idempotent",
+			false,
+			"timeout http_500 http_502 non_idempotent",
+		},
+	}
 
-	buildNextUpstream := buildNextUpstream(nextUpstream)
-
-	if buildNextUpstream != validNextUpstream {
-		t.Errorf("Expected '%v' but returned '%v'", validNextUpstream, buildNextUpstream)
+	for k, tc := range cases {
+		nextUpstream := buildNextUpstream(tc.NextUpstream, tc.NonIdempotent)
+		if nextUpstream != tc.Output {
+			t.Errorf(
+				"%s: called buildNextUpstream('%s', %v); expected '%v' but returned '%v'",
+				k,
+				tc.NextUpstream,
+				tc.NonIdempotent,
+				tc.Output,
+				nextUpstream,
+			)
+		}
 	}
 }