Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nginx/proxy: allow specifying next upstream behaviour #907

Merged
merged 1 commit into from
Jun 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions controllers/nginx/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand Down
1 change: 1 addition & 0 deletions controllers/nginx/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down
19 changes: 19 additions & 0 deletions controllers/nginx/pkg/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ var (
"toUpper": strings.ToUpper,
"toLower": strings.ToLower,
"formatIP": formatIP,
"buildNextUpstream": buildNextUpstream,
}
)

Expand Down Expand Up @@ -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)
Copy link
Member

@aledbf aledbf Jun 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return the default values here (do not continue)

Edit: forget this commenr

}

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, " ")
}
6 changes: 3 additions & 3 deletions controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down Expand Up @@ -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 "";
Expand Down
9 changes: 8 additions & 1 deletion core/pkg/ingress/annotations/proxy/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
8 changes: 8 additions & 0 deletions core/pkg/ingress/annotations/proxy/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func (m mockBackend) GetDefaultBackend() defaults.Backend {
ProxyReadTimeout: 20,
ProxyBufferSize: "10k",
ProxyBodySize: "3k",
ProxyNextUpstream: "error",
}
}

Expand All @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
}
1 change: 1 addition & 0 deletions core/pkg/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions core/pkg/ingress/defaults/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down