From ffeb1fe34810ab738a9b9c9c7c8b38f640a95f26 Mon Sep 17 00:00:00 2001 From: Alex Kursell Date: Mon, 15 Apr 2019 11:08:57 -0400 Subject: [PATCH] Support proxy_next_upstream_timeout --- .../nginx-configuration/annotations.md | 2 + .../nginx-configuration/configmap.md | 5 +++ internal/ingress/annotations/proxy/main.go | 37 ++++++++++------- .../ingress/annotations/proxy/main_test.go | 28 ++++++++----- internal/ingress/controller/config/config.go | 41 ++++++++++--------- internal/ingress/controller/controller.go | 30 +++++++------- internal/ingress/defaults/main.go | 4 ++ rootfs/etc/nginx/template/nginx.tmpl | 1 + test/e2e/annotations/proxy.go | 25 ++++++++++- 9 files changed, 113 insertions(+), 60 deletions(-) diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 965b351420..0be636124f 100644 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -58,6 +58,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[nginx.ingress.kubernetes.io/proxy-send-timeout](#custom-timeouts)|number| |[nginx.ingress.kubernetes.io/proxy-read-timeout](#custom-timeouts)|number| |[nginx.ingress.kubernetes.io/proxy-next-upstream](#custom-timeouts)|string| +|[nginx.ingress.kubernetes.io/proxy-next-upstream-timeout](#custom-timeouts)|number| |[nginx.ingress.kubernetes.io/proxy-next-upstream-tries](#custom-timeouts)|number| |[nginx.ingress.kubernetes.io/proxy-request-buffering](#custom-timeouts)|string| |[nginx.ingress.kubernetes.io/proxy-redirect-from](#proxy-redirect)|string| @@ -489,6 +490,7 @@ In some scenarios is required to have different values. To allow this we provide - `nginx.ingress.kubernetes.io/proxy-send-timeout` - `nginx.ingress.kubernetes.io/proxy-read-timeout` - `nginx.ingress.kubernetes.io/proxy-next-upstream` +- `nginx.ingress.kubernetes.io/proxy-next-upstream-timeout` - `nginx.ingress.kubernetes.io/proxy-next-upstream-tries` - `nginx.ingress.kubernetes.io/proxy-request-buffering` diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 1849d4bd44..d13d49149c 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -138,6 +138,7 @@ The following table shows a configuration option's name, type, and the default v |[proxy-cookie-path](#proxy-cookie-path)|string|"off"| |[proxy-cookie-domain](#proxy-cookie-domain)|string|"off"| |[proxy-next-upstream](#proxy-next-upstream)|string|"error timeout"| +|[proxy-next-upstream-timeout](#proxy-next-upstream-timeout)|int|0| |[proxy-next-upstream-tries](#proxy-next-upstream-tries)|int|3| |[proxy-redirect-from](#proxy-redirect-from)|string|"off"| |[proxy-request-buffering](#proxy-request-buffering)|string|"on"| @@ -789,6 +790,10 @@ Sets a text that [should be changed in the domain attribute](http://nginx.org/en 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. +## proxy-next-upstream-timeout + +[Limits the time](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream_timeout) in seconds during which a request can be passed to the next server. + ## proxy-next-upstream-tries Limit the number of [possible tries](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream_tries) a request should be passed to the next server. diff --git a/internal/ingress/annotations/proxy/main.go b/internal/ingress/annotations/proxy/main.go index 1e3fcd49ee..9183968337 100644 --- a/internal/ingress/annotations/proxy/main.go +++ b/internal/ingress/annotations/proxy/main.go @@ -25,20 +25,21 @@ import ( // Config returns the proxy timeout to use in the upstream server/s type Config struct { - BodySize string `json:"bodySize"` - ConnectTimeout int `json:"connectTimeout"` - SendTimeout int `json:"sendTimeout"` - ReadTimeout int `json:"readTimeout"` - BuffersNumber int `json:"buffersNumber"` - BufferSize string `json:"bufferSize"` - CookieDomain string `json:"cookieDomain"` - CookiePath string `json:"cookiePath"` - NextUpstream string `json:"nextUpstream"` - NextUpstreamTries int `json:"nextUpstreamTries"` - ProxyRedirectFrom string `json:"proxyRedirectFrom"` - ProxyRedirectTo string `json:"proxyRedirectTo"` - RequestBuffering string `json:"requestBuffering"` - ProxyBuffering string `json:"proxyBuffering"` + BodySize string `json:"bodySize"` + ConnectTimeout int `json:"connectTimeout"` + SendTimeout int `json:"sendTimeout"` + ReadTimeout int `json:"readTimeout"` + BuffersNumber int `json:"buffersNumber"` + BufferSize string `json:"bufferSize"` + CookieDomain string `json:"cookieDomain"` + CookiePath string `json:"cookiePath"` + NextUpstream string `json:"nextUpstream"` + NextUpstreamTimeout int `json:"nextUpstreamTimeout"` + NextUpstreamTries int `json:"nextUpstreamTries"` + ProxyRedirectFrom string `json:"proxyRedirectFrom"` + ProxyRedirectTo string `json:"proxyRedirectTo"` + RequestBuffering string `json:"requestBuffering"` + ProxyBuffering string `json:"proxyBuffering"` } // Equal tests for equality between two Configuration types @@ -76,6 +77,9 @@ func (l1 *Config) Equal(l2 *Config) bool { if l1.NextUpstream != l2.NextUpstream { return false } + if l1.NextUpstreamTimeout != l2.NextUpstreamTimeout { + return false + } if l1.NextUpstreamTries != l2.NextUpstreamTries { return false } @@ -157,6 +161,11 @@ func (a proxy) Parse(ing *extensions.Ingress) (interface{}, error) { config.NextUpstream = defBackend.ProxyNextUpstream } + config.NextUpstreamTimeout, err = parser.GetIntAnnotation("proxy-next-upstream-timeout", ing) + if err != nil { + config.NextUpstreamTimeout = defBackend.ProxyNextUpstreamTimeout + } + config.NextUpstreamTries, err = parser.GetIntAnnotation("proxy-next-upstream-tries", ing) if err != nil { config.NextUpstreamTries = defBackend.ProxyNextUpstreamTries diff --git a/internal/ingress/annotations/proxy/main_test.go b/internal/ingress/annotations/proxy/main_test.go index 33bb6b6143..3ccd0e5044 100644 --- a/internal/ingress/annotations/proxy/main_test.go +++ b/internal/ingress/annotations/proxy/main_test.go @@ -70,16 +70,17 @@ type mockBackend struct { func (m mockBackend) GetDefaultBackend() defaults.Backend { return defaults.Backend{ - ProxyConnectTimeout: 10, - ProxySendTimeout: 15, - ProxyReadTimeout: 20, - ProxyBuffersNumber: 4, - ProxyBufferSize: "10k", - ProxyBodySize: "3k", - ProxyNextUpstream: "error", - ProxyNextUpstreamTries: 3, - ProxyRequestBuffering: "on", - ProxyBuffering: "off", + ProxyConnectTimeout: 10, + ProxySendTimeout: 15, + ProxyReadTimeout: 20, + ProxyBuffersNumber: 4, + ProxyBufferSize: "10k", + ProxyBodySize: "3k", + ProxyNextUpstream: "error", + ProxyNextUpstreamTimeout: 0, + ProxyNextUpstreamTries: 3, + ProxyRequestBuffering: "on", + ProxyBuffering: "off", } } @@ -94,6 +95,7 @@ func TestProxy(t *testing.T) { data[parser.GetAnnotationWithPrefix("proxy-buffer-size")] = "1k" data[parser.GetAnnotationWithPrefix("proxy-body-size")] = "2k" data[parser.GetAnnotationWithPrefix("proxy-next-upstream")] = "off" + data[parser.GetAnnotationWithPrefix("proxy-next-upstream-timeout")] = "5" data[parser.GetAnnotationWithPrefix("proxy-next-upstream-tries")] = "3" data[parser.GetAnnotationWithPrefix("proxy-request-buffering")] = "off" data[parser.GetAnnotationWithPrefix("proxy-buffering")] = "on" @@ -128,6 +130,9 @@ func TestProxy(t *testing.T) { if p.NextUpstream != "off" { t.Errorf("expected off as next-upstream but returned %v", p.NextUpstream) } + if p.NextUpstreamTimeout != 5 { + t.Errorf("expected 5 as next-upstream-timeout but returned %v", p.NextUpstreamTimeout) + } if p.NextUpstreamTries != 3 { t.Errorf("expected 3 as next-upstream-tries but returned %v", p.NextUpstreamTries) } @@ -174,6 +179,9 @@ func TestProxyWithNoAnnotation(t *testing.T) { if p.NextUpstream != "error" { t.Errorf("expected error as next-upstream but returned %v", p.NextUpstream) } + if p.NextUpstreamTimeout != 0 { + t.Errorf("expected 0 as next-upstream-timeout but returned %v", p.NextUpstreamTimeout) + } if p.NextUpstreamTries != 3 { t.Errorf("expected 3 as next-upstream-tries but returned %v", p.NextUpstreamTries) } diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 4378819fd8..961c862bba 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -672,26 +672,27 @@ func NewDefault() Configuration { UseHTTP2: true, ProxyStreamTimeout: "600s", Backend: defaults.Backend{ - ProxyBodySize: bodySize, - ProxyConnectTimeout: 5, - ProxyReadTimeout: 60, - ProxySendTimeout: 60, - ProxyBuffersNumber: 4, - ProxyBufferSize: "4k", - ProxyCookieDomain: "off", - ProxyCookiePath: "off", - ProxyNextUpstream: "error timeout", - ProxyNextUpstreamTries: 3, - ProxyRequestBuffering: "on", - ProxyRedirectFrom: "off", - ProxyRedirectTo: "off", - SSLRedirect: true, - CustomHTTPErrors: []int{}, - WhitelistSourceRange: []string{}, - SkipAccessLogURLs: []string{}, - LimitRate: 0, - LimitRateAfter: 0, - ProxyBuffering: "off", + ProxyBodySize: bodySize, + ProxyConnectTimeout: 5, + ProxyReadTimeout: 60, + ProxySendTimeout: 60, + ProxyBuffersNumber: 4, + ProxyBufferSize: "4k", + ProxyCookieDomain: "off", + ProxyCookiePath: "off", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: 0, + ProxyNextUpstreamTries: 3, + ProxyRequestBuffering: "on", + ProxyRedirectFrom: "off", + ProxyRedirectTo: "off", + SSLRedirect: true, + CustomHTTPErrors: []int{}, + WhitelistSourceRange: []string{}, + SkipAccessLogURLs: []string{}, + LimitRate: 0, + LimitRateAfter: 0, + ProxyBuffering: "off", }, UpstreamKeepaliveConnections: 32, UpstreamKeepaliveTimeout: 60, diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 90f7897703..b95604e7ba 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -18,12 +18,13 @@ package controller import ( "fmt" - "k8s.io/ingress-nginx/internal/ingress/annotations/log" "sort" "strconv" "strings" "time" + "k8s.io/ingress-nginx/internal/ingress/annotations/log" + "github.com/mitchellh/hashstructure" "k8s.io/klog" @@ -841,19 +842,20 @@ func (n *NGINXController) createServers(data []*ingress.Ingress, bdef := n.store.GetDefaultBackend() ngxProxy := proxy.Config{ - BodySize: bdef.ProxyBodySize, - ConnectTimeout: bdef.ProxyConnectTimeout, - SendTimeout: bdef.ProxySendTimeout, - ReadTimeout: bdef.ProxyReadTimeout, - BuffersNumber: bdef.ProxyBuffersNumber, - BufferSize: bdef.ProxyBufferSize, - CookieDomain: bdef.ProxyCookieDomain, - CookiePath: bdef.ProxyCookiePath, - NextUpstream: bdef.ProxyNextUpstream, - NextUpstreamTries: bdef.ProxyNextUpstreamTries, - RequestBuffering: bdef.ProxyRequestBuffering, - ProxyRedirectFrom: bdef.ProxyRedirectFrom, - ProxyBuffering: bdef.ProxyBuffering, + BodySize: bdef.ProxyBodySize, + ConnectTimeout: bdef.ProxyConnectTimeout, + SendTimeout: bdef.ProxySendTimeout, + ReadTimeout: bdef.ProxyReadTimeout, + BuffersNumber: bdef.ProxyBuffersNumber, + BufferSize: bdef.ProxyBufferSize, + CookieDomain: bdef.ProxyCookieDomain, + CookiePath: bdef.ProxyCookiePath, + NextUpstream: bdef.ProxyNextUpstream, + NextUpstreamTimeout: bdef.ProxyNextUpstreamTimeout, + NextUpstreamTries: bdef.ProxyNextUpstreamTries, + RequestBuffering: bdef.ProxyRequestBuffering, + ProxyRedirectFrom: bdef.ProxyRedirectFrom, + ProxyBuffering: bdef.ProxyBuffering, } // generated on Start() with createDefaultSSLCertificate() diff --git a/internal/ingress/defaults/main.go b/internal/ingress/defaults/main.go index 0242c9f7f2..b492f6ba31 100644 --- a/internal/ingress/defaults/main.go +++ b/internal/ingress/defaults/main.go @@ -73,6 +73,10 @@ type Backend struct { // http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream ProxyNextUpstream string `json:"proxy-next-upstream"` + // Limits the time during which a request can be passed to the next server. + // http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream_timeout + ProxyNextUpstreamTimeout int `json:"proxy-next-upstream-timeout"` + // Limits the number of possible tries for passing a request to the next server. // https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream_tries ProxyNextUpstreamTries int `json:"proxy-next-upstream-tries"` diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index f64463b5b5..eb8503c8e7 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -1237,6 +1237,7 @@ stream { # In case of errors try the next upstream server before returning an error proxy_next_upstream {{ buildNextUpstream $location.Proxy.NextUpstream $all.Cfg.RetryNonIdempotent }}; + proxy_next_upstream_timeout {{ $location.Proxy.NextUpstreamTimeout }}; proxy_next_upstream_tries {{ $location.Proxy.NextUpstreamTries }}; {{/* Add any additional configuration defined */}} diff --git a/test/e2e/annotations/proxy.go b/test/e2e/annotations/proxy.go index 3b5e5205f5..8a12d63380 100644 --- a/test/e2e/annotations/proxy.go +++ b/test/e2e/annotations/proxy.go @@ -180,8 +180,9 @@ var _ = framework.IngressNginxDescribe("Annotations - Proxy", func() { It("should build proxy next upstream", func() { annotations := map[string]string{ - "nginx.ingress.kubernetes.io/proxy-next-upstream": "error timeout http_502", - "nginx.ingress.kubernetes.io/proxy-next-upstream-tries": "5", + "nginx.ingress.kubernetes.io/proxy-next-upstream": "error timeout http_502", + "nginx.ingress.kubernetes.io/proxy-next-upstream-timeout": "10", + "nginx.ingress.kubernetes.io/proxy-next-upstream-tries": "5", } ing := framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, &annotations) @@ -190,10 +191,30 @@ var _ = framework.IngressNginxDescribe("Annotations - Proxy", func() { f.WaitForNginxServer(host, func(server string) bool { return strings.Contains(server, "proxy_next_upstream error timeout http_502;") && + strings.Contains(server, "proxy_next_upstream_timeout 10;") && strings.Contains(server, "proxy_next_upstream_tries 5;") }) }) + It("should build proxy next upstream using configmap values", func() { + annotations := map[string]string{} + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.SetNginxConfigMapData(map[string]string{ + "proxy-next-upstream": "timeout http_502", + "proxy-next-upstream-timeout": "53", + "proxy-next-upstream-tries": "44", + }) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "proxy_next_upstream timeout http_502;") && + strings.Contains(server, "proxy_next_upstream_timeout 53;") && + strings.Contains(server, "proxy_next_upstream_tries 44;") + }) + }) + It("should setup proxy cookies", func() { annotations := map[string]string{ "nginx.ingress.kubernetes.io/proxy-cookie-domain": "localhost example.org",