From 5503e8d0e91c31108aa025c4f69d562c962c92b2 Mon Sep 17 00:00:00 2001 From: Gorka Lerchundi Osa Date: Mon, 26 Jun 2017 21:39:24 +0200 Subject: [PATCH] nginx/proxy: allow specifying next upstream behaviour --- controllers/nginx/configuration.md | 3 +++ controllers/nginx/pkg/config/config.go | 1 + controllers/nginx/pkg/template/template.go | 19 +++++++++++++++++++ .../rootfs/etc/nginx/template/nginx.tmpl | 6 +++--- core/pkg/ingress/annotations/proxy/main.go | 9 ++++++++- .../ingress/annotations/proxy/main_test.go | 8 ++++++++ core/pkg/ingress/controller/controller.go | 1 + core/pkg/ingress/defaults/main.go | 4 ++++ 8 files changed, 47 insertions(+), 4 deletions(-) diff --git a/controllers/nginx/configuration.md b/controllers/nginx/configuration.md index 53cf62c273..e328fa3436 100644 --- a/controllers/nginx/configuration.md +++ b/controllers/nginx/configuration.md @@ -349,6 +349,9 @@ log-format-upstream: '{ "time": "$time_iso8601", "remote_addr": "$proxy_protocol **proxy-send-timeout:** Sets the timeout in seconds for [transmitting a request to the proxied server](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_send_timeout). The timeout is set only between two successive write operations, not for the transmission of the whole request. +**proxy-next-upstream:** Specifies in [which cases](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream) a request should be passed to the next server. + + **retry-non-idempotent:** Since 1.9.13 NGINX will not retry non-idempotent requests (POST, LOCK, PATCH) in case of an error in the upstream server. The previous behavior can be restored using the value "true". diff --git a/controllers/nginx/pkg/config/config.go b/controllers/nginx/pkg/config/config.go index 1bfd2dd78d..27fab96d36 100644 --- a/controllers/nginx/pkg/config/config.go +++ b/controllers/nginx/pkg/config/config.go @@ -354,6 +354,7 @@ func NewDefault() Configuration { ProxyBufferSize: "4k", ProxyCookieDomain: "off", ProxyCookiePath: "off", + ProxyNextUpstream: "error timeout invalid_header http_502 http_503 http_504", SSLRedirect: true, CustomHTTPErrors: []int{}, WhitelistSourceRange: []string{}, diff --git a/controllers/nginx/pkg/template/template.go b/controllers/nginx/pkg/template/template.go index 482497310d..d364edd719 100644 --- a/controllers/nginx/pkg/template/template.go +++ b/controllers/nginx/pkg/template/template.go @@ -146,6 +146,7 @@ var ( "toUpper": strings.ToUpper, "toLower": strings.ToLower, "formatIP": formatIP, + "buildNextUpstream": buildNextUpstream, } ) @@ -450,3 +451,21 @@ func isSticky(host string, loc *ingress.Location, stickyLocations map[string][]s return false } + +func buildNextUpstream(input interface{}) string { + nextUpstream, ok := input.(string) + if !ok { + glog.Errorf("expected an string type but %T was returned", input) + } + + parts := strings.Split(nextUpstream, " ") + + nextUpstreamCodes := make([]string, 0, len(parts)) + for _, v := range parts { + if v != "" && v != "non_idempotent" { + nextUpstreamCodes = append(nextUpstreamCodes, v) + } + } + + return strings.Join(nextUpstreamCodes, " ") +} \ No newline at end of file diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 04bd1ae6b9..2144c02f1a 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -216,9 +216,6 @@ http { {{ range $errCode := $cfg.CustomHTTPErrors }} error_page {{ $errCode }} = @custom_{{ $errCode }};{{ end }} - # In case of errors try the next upstream server before returning an error - proxy_next_upstream error timeout invalid_header http_502 http_503 http_504{{ if $cfg.RetryNonIdempotent }} non_idempotent{{ end }}; - proxy_ssl_session_reuse on; {{ if $cfg.AllowBackendServerHeader }} @@ -439,6 +436,9 @@ http { proxy_cookie_domain {{ $location.Proxy.CookieDomain }}; proxy_cookie_path {{ $location.Proxy.CookiePath }}; + # In case of errors try the next upstream server before returning an error + proxy_next_upstream {{ buildNextUpstream $location.Proxy.NextUpstream }}{{ if $cfg.RetryNonIdempotent }} non_idempotent{{ end }}; + {{/* rewrite only works if the content is not compressed */}} {{ if $location.Redirect.AddBaseURL }} proxy_set_header Accept-Encoding ""; diff --git a/core/pkg/ingress/annotations/proxy/main.go b/core/pkg/ingress/annotations/proxy/main.go index f2ffc1496e..035a812ebe 100644 --- a/core/pkg/ingress/annotations/proxy/main.go +++ b/core/pkg/ingress/annotations/proxy/main.go @@ -31,6 +31,7 @@ const ( bufferSize = "ingress.kubernetes.io/proxy-buffer-size" cookiePath = "ingress.kubernetes.io/proxy-cookie-path" cookieDomain = "ingress.kubernetes.io/proxy-cookie-domain" + nextUpstream = "ingress.kubernetes.io/proxy-next-upstream" ) // Configuration returns the proxy timeout to use in the upstream server/s @@ -42,6 +43,7 @@ type Configuration struct { BufferSize string `json:"bufferSize"` CookieDomain string `json:"cookieDomain"` CookiePath string `json:"cookiePath"` + NextUpstream string `json:"nextUpstream"` } func (l1 *Configuration) Equal(l2 *Configuration) bool { @@ -124,5 +126,10 @@ func (a proxy) Parse(ing *extensions.Ingress) (interface{}, error) { bs = defBackend.ProxyBodySize } - return &Configuration{bs, ct, st, rt, bufs, cd, cp}, nil + nu, err := parser.GetStringAnnotation(nextUpstream, ing) + if err != nil || nu == "" { + nu = defBackend.ProxyNextUpstream + } + + return &Configuration{bs, ct, st, rt, bufs, cd, cp, nu}, nil } diff --git a/core/pkg/ingress/annotations/proxy/main_test.go b/core/pkg/ingress/annotations/proxy/main_test.go index 11e85dc30f..e7bdd42139 100644 --- a/core/pkg/ingress/annotations/proxy/main_test.go +++ b/core/pkg/ingress/annotations/proxy/main_test.go @@ -73,6 +73,7 @@ func (m mockBackend) GetDefaultBackend() defaults.Backend { ProxyReadTimeout: 20, ProxyBufferSize: "10k", ProxyBodySize: "3k", + ProxyNextUpstream: "error", } } @@ -85,6 +86,7 @@ func TestProxy(t *testing.T) { data[read] = "3" data[bufferSize] = "1k" data[bodySize] = "2k" + data[nextUpstream] = "off" ing.SetAnnotations(data) i, err := NewParser(mockBackend{}).Parse(ing) @@ -110,6 +112,9 @@ func TestProxy(t *testing.T) { if p.BodySize != "2k" { t.Errorf("expected 2k as body-size but returned %v", p.BodySize) } + if p.NextUpstream != "off" { + t.Errorf("expected off as next-upstream but returned %v", p.NextUpstream) + } } func TestProxyWithNoAnnotation(t *testing.T) { @@ -141,4 +146,7 @@ func TestProxyWithNoAnnotation(t *testing.T) { if p.BodySize != "3k" { t.Errorf("expected 3k as body-size but returned %v", p.BodySize) } + if p.NextUpstream != "error" { + t.Errorf("expected error as next-upstream but returned %v", p.NextUpstream) + } } diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index ebc290f810..68eca20460 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -907,6 +907,7 @@ func (ic *GenericController) createServers(data []interface{}, BufferSize: bdef.ProxyBufferSize, CookieDomain: bdef.ProxyCookieDomain, CookiePath: bdef.ProxyCookiePath, + NextUpstream: bdef.ProxyNextUpstream, } // This adds the Default Certificate to Default Backend (or generates a new self signed one) diff --git a/core/pkg/ingress/defaults/main.go b/core/pkg/ingress/defaults/main.go index 92f5a72a29..401c32d7c6 100644 --- a/core/pkg/ingress/defaults/main.go +++ b/core/pkg/ingress/defaults/main.go @@ -49,6 +49,10 @@ type Backend struct { // http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_cookie_domain ProxyCookieDomain string `json:"proxy-cookie-domain"` + // Specifies in which cases a request should be passed to the next server. + // http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream + ProxyNextUpstream string `json:"proxy-next-upstream"` + // Name server/s used to resolve names of upstream servers into IP addresses. // The file /etc/resolv.conf is used as DNS resolution configuration. Resolver []net.IP