Skip to content

Commit

Permalink
Support latest backend config for nginx upstream module (#228)
Browse files Browse the repository at this point in the history 
* Support latest backend config for nginx upstream module

- Support setting keepalive_requests and keepalive_timeout for nginx upstream module
- Support annotations on ingress to be able to set the values in nginx config for each upstream

* Handle error when parsing annotations

* Build upstream id including ingress name

- Needed to be able to configure an upstream for ingresses which has same hostname and backend service but with a different path. This is so that the configuration won't be overwritten in this scenario.
  • Loading branch information
@supreethrao
supreethrao authored Jul 7, 2020
1 parent 0e2cd58 commit 9bc56a3
Show file tree
Hide file tree
Showing 8 changed files with 299 additions and 98 deletions.
29 changes: 26 additions & 3 deletions controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"strconv"
"strings"
"sync"
"time"

log "github.com/sirupsen/logrus"
"github.com/sky-uk/feed/k8s"
Expand All @@ -31,9 +32,13 @@ const (
stripPathAnnotation = "sky.uk/strip-path"
exactPathAnnotation = "sky.uk/exact-path"

backendTimeoutSeconds = "sky.uk/backend-timeout-seconds"
proxyBufferSizeAnnotation = "sky.uk/proxy-buffer-size-in-kb"
proxyBufferBlocksAnnotation = "sky.uk/proxy-buffer-blocks"
backendTimeoutSeconds = "sky.uk/backend-timeout-seconds"
// sets keepalive_timeout on nginx upstream (http://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive)
backendConnectionKeepalive = "sky.uk/backend-connection-keepalive"
// sets keepalive_requests on nginx upstream (http://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive)
backendMaxRequestsPerConnection = "sky.uk/backend-max-requests-per-connection"
proxyBufferSizeAnnotation = "sky.uk/proxy-buffer-size-in-kb"
proxyBufferBlocksAnnotation = "sky.uk/proxy-buffer-blocks"

maxAllowedProxyBufferSize = 32
maxAllowedProxyBufferBlocks = 8
Expand Down Expand Up @@ -304,6 +309,24 @@ func (c *controller) updateIngresses() (err error) {
entry.BackendMaxConnections = tmp
}

if maxRequestsPerConnection, ok := ingress.Annotations[backendMaxRequestsPerConnection]; ok {
intVal, err := strconv.ParseUint(maxRequestsPerConnection, 10, 64)
if err != nil {
log.Warnf("invalid value %v set for annotation for %q. Will continue with defaults", maxRequestsPerConnection, backendMaxRequestsPerConnection)
} else {
entry.BackendMaxRequestsPerConnection = intVal
}
}

if connectionKeepalive, ok := ingress.Annotations[backendConnectionKeepalive]; ok {
keepaliveTimeout, err := time.ParseDuration(connectionKeepalive)
if err != nil {
log.Warnf("invalid value %v set for annotation for %q. Will continue with defaults", connectionKeepalive, backendConnectionKeepalive)
} else {
entry.BackendKeepaliveTimeout = keepaliveTimeout
}
}

if proxyBufferSizeString, ok := ingress.Annotations[proxyBufferSizeAnnotation]; ok {
tmp, _ := strconv.Atoi(proxyBufferSizeString)
entry.ProxyBufferSize = tmp
Expand Down
97 changes: 97 additions & 0 deletions controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,99 @@ func TestUpdaterIsUpdatedForIngressWithOverriddenBackendMaxConnections(t *testin
})
}

func TestUpdaterIsUpdatedForIngressWithOverriddenBackendMaxRequestsPerConnection(t *testing.T) {
runAndAssertUpdates(t, expectGetAllIngresses, testSpec{
"ingress with overridden backend max requests per connection",
createIngressesFixture(ingressNamespace, ingressHost, ingressSvcName, ingressSvcPort, map[string]string{
ingressAllowAnnotation: "",
stripPathAnnotation: "false",
frontendSchemeAnnotation: "internal",
ingressClassAnnotation: defaultIngressClass,
backendMaxRequestsPerConnection: "100",
}, ingressPath),
createDefaultServices(),
createDefaultNamespaces(),
[]IngressEntry{{
Namespace: ingressNamespace,
Name: ingressName,
Host: ingressHost,
Path: ingressPath,
ServiceAddress: serviceIP,
ServicePort: ingressSvcPort,
LbScheme: "internal",
IngressClass: defaultIngressClass,
Allow: []string{},
StripPaths: false,
BackendTimeoutSeconds: 10,
BackendMaxConnections: defaultMaxConnections,
BackendMaxRequestsPerConnection: 100,
}},
defaultConfig(),
})
}

func TestUpdaterIsUpdatedForIngressWithOverriddenBackendConnectionKeepAlive(t *testing.T) {
runAndAssertUpdates(t, expectGetAllIngresses, testSpec{
"ingress with overridden backend max requests per connection",
createIngressesFixture(ingressNamespace, ingressHost, ingressSvcName, ingressSvcPort, map[string]string{
ingressAllowAnnotation: "",
stripPathAnnotation: "false",
frontendSchemeAnnotation: "internal",
ingressClassAnnotation: defaultIngressClass,
backendConnectionKeepalive: "5m",
}, ingressPath),
createDefaultServices(),
createDefaultNamespaces(),
[]IngressEntry{{
Namespace: ingressNamespace,
Name: ingressName,
Host: ingressHost,
Path: ingressPath,
ServiceAddress: serviceIP,
ServicePort: ingressSvcPort,
LbScheme: "internal",
IngressClass: defaultIngressClass,
Allow: []string{},
StripPaths: false,
BackendTimeoutSeconds: 10,
BackendMaxConnections: defaultMaxConnections,
BackendKeepaliveTimeout: 5 * time.Minute,
}},
defaultConfig(),
})
}

func TestUpdaterSkipsEntriesForIngressWithInvalidBackendConnectionKeepAliveAndBackendMaxRequestsPerConnection(t *testing.T) {
runAndAssertUpdates(t, expectGetAllIngresses, testSpec{
"ingress with overridden backend max requests per connection",
createIngressesFixture(ingressNamespace, ingressHost, ingressSvcName, ingressSvcPort, map[string]string{
ingressAllowAnnotation: "",
stripPathAnnotation: "false",
frontendSchemeAnnotation: "internal",
ingressClassAnnotation: defaultIngressClass,
backendConnectionKeepalive: "50a",
backendMaxRequestsPerConnection: "-1",
}, ingressPath),
createDefaultServices(),
createDefaultNamespaces(),
[]IngressEntry{{
Namespace: ingressNamespace,
Name: ingressName,
Host: ingressHost,
Path: ingressPath,
ServiceAddress: serviceIP,
ServicePort: ingressSvcPort,
LbScheme: "internal",
IngressClass: defaultIngressClass,
Allow: []string{},
StripPaths: false,
BackendTimeoutSeconds: 10,
BackendMaxConnections: defaultMaxConnections,
}},
defaultConfig(),
})
}

func TestUpdaterIsUpdatedForIngressWithDefaultBackendMaxConnections(t *testing.T) {
runAndAssertUpdates(t, expectGetAllIngresses, testSpec{
"ingress with default backend max connections",
Expand Down Expand Up @@ -1329,6 +1422,10 @@ func createIngressesFixture(namespace string, host string, serviceName string, s
annotations[proxyBufferBlocksAnnotation] = annotationVal
case ingressClassAnnotation:
annotations[ingressClassAnnotation] = annotationVal
case backendConnectionKeepalive:
annotations[backendConnectionKeepalive] = annotationVal
case backendMaxRequestsPerConnection:
annotations[backendMaxRequestsPerConnection] = annotationVal
}
}

Expand Down
4 changes: 4 additions & 0 deletions controller/ingress_entry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ type IngressEntry struct {
BackendTimeoutSeconds int
// BackendMaxConnections maximum backend connections
BackendMaxConnections int
// BackendKeepaliveTimeout timeout for idle connections to upstream
BackendKeepaliveTimeout time.Duration
// BackendMaxRequestsPerConnection max requests per connection to upstream, after which it will be closed
BackendMaxRequestsPerConnection uint64
// Ingress creation time
CreationTimestamp time.Time
// Ingress resource
Expand Down
3 changes: 2 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ require (
golang.org/x/image v0.0.0-20190828090100-23ea20f87cfc // indirect
golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
golang.org/x/mobile v0.0.0-20190826170111-cafc553e1ac5 // indirect
golang.org/x/mod v0.3.0 // indirect
golang.org/x/sys v0.0.0-20191104094858-e8c54fb511f6 // indirect
golang.org/x/tools v0.0.0-20200312194400-c312e98713c2 // indirect
golang.org/x/tools v0.0.0-20200626171337-aa94e735be7f // indirect
google.golang.org/api v0.9.0 // indirect
google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 // indirect
google.golang.org/grpc v1.23.0
Expand Down
9 changes: 9 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ github.com/vishvananda/netns v0.0.0-20190625233234-7109fa855b0f h1:nBX3nTcmxEtHS
github.com/vishvananda/netns v0.0.0-20190625233234-7109fa855b0f/go.mod h1:ZjcWmFBXmLKZu9Nxj3WKYEafiSqer2rnvPr0en9UNpI=
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
go.etcd.io/bbolt v1.3.3/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
Expand Down Expand Up @@ -330,6 +331,8 @@ golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY=
golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
golang.org/x/mod v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
Expand Down Expand Up @@ -429,6 +432,12 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn
golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
golang.org/x/tools v0.0.0-20200312194400-c312e98713c2 h1:6TB4+MaZlkcSsJDu+BS5yxSEuZIYhjWz+jhbSLEZylI=
golang.org/x/tools v0.0.0-20200312194400-c312e98713c2/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw=
golang.org/x/tools v0.0.0-20200624225443-88f3c62a19ff h1:foic6oVZ4MKltJC6MXzuFZFswE7NCjjtc0Hxbyblawc=
golang.org/x/tools v0.0.0-20200624225443-88f3c62a19ff/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200626032829-bcbc01e07a20 h1:q+ysxVHVQNTVHgzwjuk4ApAILRbfOLARfnEaqCIBR6A=
golang.org/x/tools v0.0.0-20200626032829-bcbc01e07a20/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200626171337-aa94e735be7f h1:JcoF/bowzCDI+MXu1yLqQGNO3ibqWsWq+Sk7pOT218w=
golang.org/x/tools v0.0.0-20200626171337-aa94e735be7f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
Expand Down
31 changes: 22 additions & 9 deletions nginx/nginx.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import (
)

const (
nginxStartDelay = time.Millisecond * 100
metricsUpdateInterval = time.Second * 10
nginxStartDelay = time.Millisecond * 100
metricsUpdateInterval = time.Second * 10
defaultMaxRequestsPerUpstreamConnection = uint64(1024)
)

// Port configuration
Expand Down Expand Up @@ -130,9 +131,11 @@ type server struct {
}

type upstream struct {
ID string
Server string
MaxConnections int
ID string
Server string
MaxConnections int
KeepaliveTimeout string
KeepaliveRequests uint64
}

type location struct {
Expand Down Expand Up @@ -422,10 +425,20 @@ func createUpstreamEntries(entries controller.IngressEntries) []*upstream {
idToUpstream := make(map[string]*upstream)

for _, ingressEntry := range entries {
maxRequestsPerConnection := defaultMaxRequestsPerUpstreamConnection
if ingressEntry.BackendMaxRequestsPerConnection != 0 {
maxRequestsPerConnection = ingressEntry.BackendMaxRequestsPerConnection
}
keepaliveTimeout := ""
if ingressEntry.BackendKeepaliveTimeout != 0 {
keepaliveTimeout = fmt.Sprintf("%ds", uint64(ingressEntry.BackendKeepaliveTimeout.Seconds()))
}
upstream := &upstream{
ID: upstreamID(ingressEntry),
Server: fmt.Sprintf("%s:%d", ingressEntry.ServiceAddress, ingressEntry.ServicePort),
MaxConnections: ingressEntry.BackendMaxConnections,
ID: upstreamID(ingressEntry),
Server: fmt.Sprintf("%s:%d", ingressEntry.ServiceAddress, ingressEntry.ServicePort),
MaxConnections: ingressEntry.BackendMaxConnections,
KeepaliveRequests: maxRequestsPerConnection,
KeepaliveTimeout: keepaliveTimeout,
}
idToUpstream[upstream.ID] = upstream
}
Expand All @@ -440,7 +453,7 @@ func createUpstreamEntries(entries controller.IngressEntries) []*upstream {
}

func upstreamID(e controller.IngressEntry) string {
return fmt.Sprintf("%s.%s.%d", e.Namespace, e.ServiceAddress, e.ServicePort)
return fmt.Sprintf("%s.%s.%s.%d", e.Namespace, e.Name, e.ServiceAddress, e.ServicePort)
}

func (s server) HasRootLocation() bool {
Expand Down
4 changes: 4 additions & 0 deletions nginx/nginx.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ http {
upstream {{ $upstream.ID }} {
server {{ $upstream.Server }} max_conns={{ $upstream.MaxConnections }};
keepalive {{ $keepalive }};
keepalive_requests {{ $upstream.KeepaliveRequests }};
{{- if ne $upstream.KeepaliveTimeout "" }}
keepalive_timeout {{ $upstream.KeepaliveTimeout}};
{{- end }}
}
{{ end }}

Expand Down
Loading

0 comments on commit 9bc56a3

Please sign in to comment.