From d607cf6dd73e15a428923bd2f54afa2d5d52f073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lourens=20Naud=C3=A9?= Date: Sat, 30 Sep 2017 22:29:16 +0100 Subject: [PATCH] Introduce an upstream-hash-by annotation to support consistent hashing by nginx variable or text --- configuration.md | 10 ++++ .../annotations/upstreamhashby/main.go | 42 ++++++++++++++ .../annotations/upstreamhashby/main_test.go | 58 +++++++++++++++++++ pkg/ingress/controller/annotations.go | 8 +++ pkg/ingress/controller/annotations_test.go | 25 ++++++++ pkg/ingress/controller/controller.go | 5 ++ pkg/ingress/defaults/main.go | 6 ++ pkg/ingress/types.go | 2 + pkg/ingress/types_equals.go | 3 + rootfs/etc/nginx/template/nginx.tmpl | 4 ++ tests/data/config.json | 1 + tests/manifests/configuration-a.json | 1 + tests/manifests/configuration-b.json | 1 + 13 files changed, 166 insertions(+) create mode 100644 pkg/ingress/annotations/upstreamhashby/main.go create mode 100644 pkg/ingress/annotations/upstreamhashby/main_test.go diff --git a/configuration.md b/configuration.md index f1f8d8f9a..4e8b04723 100644 --- a/configuration.md +++ b/configuration.md @@ -5,6 +5,7 @@ * [Custom NGINX template](#custom-nginx-template) * [Annotations](#annotations) * [Custom NGINX upstream checks](#custom-nginx-upstream-checks) +* [Custom NGINX upstream hashing](#custom-nginx-upstream-hashing) * [Authentication](#authentication) * [Rewrite](#rewrite) * [Rate limiting](#rate-limiting) @@ -74,6 +75,7 @@ The following annotations are supported: |[ingress.kubernetes.io/ssl-passthrough](#ssl-passthrough)|true or false| |[ingress.kubernetes.io/upstream-max-fails](#custom-nginx-upstream-checks)|number| |[ingress.kubernetes.io/upstream-fail-timeout](#custom-nginx-upstream-checks)|number| +|[ingress.kubernetes.io/upstream-hash-by](#custom-nginx-upstream-hashing)|string| |[ingress.kubernetes.io/whitelist-source-range](#whitelist-source-range)|CIDR| #### Custom NGINX template @@ -115,6 +117,14 @@ In NGINX, backend server pools are called "[upstreams](http://nginx.org/en/docs/ Please check the [custom upstream check](../../examples/customization/custom-upstream-check/README.md) example. +### Custom NGINX upstream hashing + +NGINX supports load balancing by client-server mapping based on [consistent hashing](http://nginx.org/en/docs/http/ngx_http_upstream_module.html#hash) for a given key. The key can contain text, variables or any combination thereof. This feature allows for request stickiness other than client IP or cookies. The [ketama](http://www.last.fm/user/RJ/journal/2007/04/10/392555/) consistent hashing method will be used which ensures only a few keys would be remapped to different servers on upstream group changes. + +To enable consistent hashing for a backend: + +`ingress.kubernetes.io/upstream-hash-by`: the nginx variable, text value or any combination thereof to use for consistent hashing. For example `ingress.kubernetes.io/upstream-hash-by: "$request_uri"` to consistently hash upstream requests by the current request URI. + ### Authentication Is possible to add authentication adding additional annotations in the Ingress rule. The source of the authentication is a secret that contains usernames and passwords inside the key `auth`. diff --git a/pkg/ingress/annotations/upstreamhashby/main.go b/pkg/ingress/annotations/upstreamhashby/main.go new file mode 100644 index 000000000..b4d898000 --- /dev/null +++ b/pkg/ingress/annotations/upstreamhashby/main.go @@ -0,0 +1,42 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package upstreamhashby + +import ( + extensions "k8s.io/api/extensions/v1beta1" + + "k8s.io/ingress-nginx/pkg/ingress/annotations/parser" +) + +const ( + annotation = "ingress.kubernetes.io/upstream-hash-by" +) + +type upstreamhashby struct { +} + +// NewParser creates a new CORS annotation parser +func NewParser() parser.IngressAnnotation { + return upstreamhashby{} +} + +// Parse parses the annotations contained in the ingress rule +// used to indicate if the location/s contains a fragment of +// configuration to be included inside the paths of the rules +func (a upstreamhashby) Parse(ing *extensions.Ingress) (interface{}, error) { + return parser.GetStringAnnotation(annotation, ing) +} diff --git a/pkg/ingress/annotations/upstreamhashby/main_test.go b/pkg/ingress/annotations/upstreamhashby/main_test.go new file mode 100644 index 000000000..ad5afafc8 --- /dev/null +++ b/pkg/ingress/annotations/upstreamhashby/main_test.go @@ -0,0 +1,58 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package upstreamhashby + +import ( + "testing" + + api "k8s.io/api/core/v1" + extensions "k8s.io/api/extensions/v1beta1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestParse(t *testing.T) { + ap := NewParser() + if ap == nil { + t.Fatalf("expected a parser.IngressAnnotation but returned nil") + } + + testCases := []struct { + annotations map[string]string + expected string + }{ + {map[string]string{annotation: "$request_uri"}, "$request_uri"}, + {map[string]string{annotation: "false"}, "false"}, + {map[string]string{}, ""}, + {nil, ""}, + } + + ing := &extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.IngressSpec{}, + } + + for _, testCase := range testCases { + ing.SetAnnotations(testCase.annotations) + result, _ := ap.Parse(ing) + if result != testCase.expected { + t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations) + } + } +} diff --git a/pkg/ingress/controller/annotations.go b/pkg/ingress/controller/annotations.go index caeed0c41..463d1e8ff 100644 --- a/pkg/ingress/controller/annotations.go +++ b/pkg/ingress/controller/annotations.go @@ -41,6 +41,7 @@ import ( "k8s.io/ingress-nginx/pkg/ingress/annotations/sessionaffinity" "k8s.io/ingress-nginx/pkg/ingress/annotations/snippet" "k8s.io/ingress-nginx/pkg/ingress/annotations/sslpassthrough" + "k8s.io/ingress-nginx/pkg/ingress/annotations/upstreamhashby" "k8s.io/ingress-nginx/pkg/ingress/annotations/upstreamvhost" "k8s.io/ingress-nginx/pkg/ingress/annotations/vtsfilterkey" "k8s.io/ingress-nginx/pkg/ingress/errors" @@ -82,6 +83,7 @@ func newAnnotationExtractor(cfg extractorConfig) annotationExtractor { "Alias": alias.NewParser(), "ClientBodyBufferSize": clientbodybuffersize.NewParser(), "DefaultBackend": defaultbackend.NewParser(cfg), + "UpstreamHashBy": upstreamhashby.NewParser(), "UpstreamVhost": upstreamvhost.NewParser(), "VtsFilterKey": vtsfilterkey.NewParser(), "ServerSnippet": serversnippet.NewParser(), @@ -131,6 +133,7 @@ const ( clientBodyBufferSize = "ClientBodyBufferSize" certificateAuth = "CertificateAuth" serverSnippet = "ServerSnippet" + upstreamHashBy = "UpstreamHashBy" ) func (e *annotationExtractor) ServiceUpstream(ing *extensions.Ingress) bool { @@ -189,3 +192,8 @@ func (e *annotationExtractor) ServerSnippet(ing *extensions.Ingress) string { val, _ := e.annotations[serverSnippet].Parse(ing) return val.(string) } + +func (e *annotationExtractor) UpstreamHashBy(ing *extensions.Ingress) string { + val, _ := e.annotations[upstreamHashBy].Parse(ing) + return val.(string) +} diff --git a/pkg/ingress/controller/annotations_test.go b/pkg/ingress/controller/annotations_test.go index edc4694ab..04581d99b 100644 --- a/pkg/ingress/controller/annotations_test.go +++ b/pkg/ingress/controller/annotations_test.go @@ -37,6 +37,7 @@ const ( annotationAffinityType = "ingress.kubernetes.io/affinity" annotationAffinityCookieName = "ingress.kubernetes.io/session-cookie-name" annotationAffinityCookieHash = "ingress.kubernetes.io/session-cookie-hash" + annotationUpstreamHashBy = "ingress.kubernetes.io/upstream-hash-by" ) type mockCfg struct { @@ -233,6 +234,30 @@ func TestSSLPassthrough(t *testing.T) { } } +func TestUpstreamHashBy(t *testing.T) { + ec := newAnnotationExtractor(mockCfg{}) + ing := buildIngress() + + fooAnns := []struct { + annotations map[string]string + er string + }{ + {map[string]string{annotationUpstreamHashBy: "$request_uri"}, "$request_uri"}, + {map[string]string{annotationUpstreamHashBy: "false"}, "false"}, + {map[string]string{annotationUpstreamHashBy + "_no": "true"}, ""}, + {map[string]string{}, ""}, + {nil, ""}, + } + + for _, foo := range fooAnns { + ing.SetAnnotations(foo.annotations) + r := ec.UpstreamHashBy(ing) + if r != foo.er { + t.Errorf("Returned %v but expected %v", r, foo.er) + } + } +} + func TestAffinitySession(t *testing.T) { ec := newAnnotationExtractor(mockCfg{}) ing := buildIngress() diff --git a/pkg/ingress/controller/controller.go b/pkg/ingress/controller/controller.go index e127f3cb9..b484d1fb6 100644 --- a/pkg/ingress/controller/controller.go +++ b/pkg/ingress/controller/controller.go @@ -707,6 +707,7 @@ func (ic *GenericController) createUpstreams(data []*extensions.Ingress, du *ing secUpstream := ic.annotations.SecureUpstream(ing) hz := ic.annotations.HealthCheck(ing) serviceUpstream := ic.annotations.ServiceUpstream(ing) + upstreamHashBy := ic.annotations.UpstreamHashBy(ing) var defBackend string if ing.Spec.Backend != nil { @@ -767,6 +768,10 @@ func (ic *GenericController) createUpstreams(data []*extensions.Ingress, du *ing upstreams[name].SecureCACert = secUpstream.CACert } + if upstreams[name].UpstreamHashBy == "" { + upstreams[name].UpstreamHashBy = upstreamHashBy + } + svcKey := fmt.Sprintf("%v/%v", ing.GetNamespace(), path.Backend.ServiceName) // Add the service cluster endpoint as the upstream instead of individual endpoints diff --git a/pkg/ingress/defaults/main.go b/pkg/ingress/defaults/main.go index 20b0a89fb..40598028c 100644 --- a/pkg/ingress/defaults/main.go +++ b/pkg/ingress/defaults/main.go @@ -92,6 +92,12 @@ type Backend struct { // Default: 0, ie use platform liveness probe UpstreamFailTimeout int `json:"upstream-fail-timeout"` + // Enable stickiness by client-server mapping based on a NGINX variable, text or a combination of both. + // A consistent hashing method will be used which ensures only a few keys would be remapped to different + // servers on upstream group changes + // http://nginx.org/en/docs/http/ngx_http_upstream_module.html#hash + UpstreamHashBy string `json:"upstream-hash-by"` + // WhitelistSourceRange allows limiting access to certain client addresses // http://nginx.org/en/docs/http/ngx_http_access_module.html WhitelistSourceRange []string `json:"whitelist-source-range,-"` diff --git a/pkg/ingress/types.go b/pkg/ingress/types.go index 2f3bbeb1c..fac9764a1 100644 --- a/pkg/ingress/types.go +++ b/pkg/ingress/types.go @@ -170,6 +170,8 @@ type Backend struct { Endpoints []Endpoint `json:"endpoints,omitempty"` // StickySessionAffinitySession contains the StickyConfig object with stickness configuration SessionAffinity SessionAffinityConfig `json:"sessionAffinityConfig"` + // Consistent hashing by NGINX variable + UpstreamHashBy string `json:"upstream-hash-by,omitempty"` } // SessionAffinityConfig describes different affinity configurations for new sessions. diff --git a/pkg/ingress/types_equals.go b/pkg/ingress/types_equals.go index c52c6461d..3791080da 100644 --- a/pkg/ingress/types_equals.go +++ b/pkg/ingress/types_equals.go @@ -173,6 +173,9 @@ func (b1 *Backend) Equal(b2 *Backend) bool { if !(&b1.SessionAffinity).Equal(&b2.SessionAffinity) { return false } + if b1.UpstreamHashBy != b2.UpstreamHashBy { + return false + } if len(b1.Endpoints) != len(b2.Endpoints) { return false diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index f3ac79fb2..f8f1c0d08 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -271,6 +271,10 @@ http { {{ $cfg.LoadBalanceAlgorithm }}; {{ end }} + {{ if $upstream.UpstreamHashBy }} + hash {{ $upstream.UpstreamHashBy }} consistent; + {{ end }} + {{ if (gt $cfg.UpstreamKeepaliveConnections 0) }} keepalive {{ $cfg.UpstreamKeepaliveConnections }}; {{ end }} diff --git a/tests/data/config.json b/tests/data/config.json index 1ac608b78..53ab7e6d9 100644 --- a/tests/data/config.json +++ b/tests/data/config.json @@ -11,6 +11,7 @@ "ssl-redirect": true, "upstream-fail-timeout": 0, "upstream-max-fails": 0, + "upstream-hash-by": "$request_uri", "whitelist-source-range": null }, "bodySize": "1m", diff --git a/tests/manifests/configuration-a.json b/tests/manifests/configuration-a.json index c9ec72a2d..d2fb585b4 100644 --- a/tests/manifests/configuration-a.json +++ b/tests/manifests/configuration-a.json @@ -31,6 +31,7 @@ } }, "port": 0, + "upstream-hash-by": "$request_uri", "secure": false, "secureCert": { "secret": "", diff --git a/tests/manifests/configuration-b.json b/tests/manifests/configuration-b.json index f9235a17a..9fb62a406 100644 --- a/tests/manifests/configuration-b.json +++ b/tests/manifests/configuration-b.json @@ -31,6 +31,7 @@ } }, "port": 0, + "upstream-hash-by": "$request_uri", "secure": false, "secureCert": { "secret": "",