diff --git a/docs/virtualserver-and-virtualserverroute.md b/docs/virtualserver-and-virtualserverroute.md index cf7a9dc8f3..9d36b06e26 100644 --- a/docs/virtualserver-and-virtualserverroute.md +++ b/docs/virtualserver-and-virtualserverroute.md @@ -179,6 +179,7 @@ port: 80 lb-method: round_robin fail-timeout: 10s max-fails: 1 +keepalive: 32 connect-timeout: 30s read-timeout: 30s send-timeout: 30s @@ -192,6 +193,7 @@ send-timeout: 30s | `lb-method` | The load [balancing method](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-load-balancer/#choosing-a-load-balancing-method). To use the round-robin method, specify `round_robin`. The default is specified in the `lb-method` ConfigMap key. | `string` | No | | `fail-timeout` | The time during which the specified number of unsuccessful attempts to communicate with an upstream server should happen to consider the server unavailable. See the [fail_timeout](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#fail_timeout) parameter of the server directive. The default is set in the `fail-timeout` ConfigMap key. | `string` | No | | `max-fails` | The number of unsuccessful attempts to communicate with an upstream server that should happen in the duration set by the `fail-timeout` to consider the server unavailable. See the [max_fails](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#max_fails) parameter of the server directive. The default is set in the `max-fails` ConfgMap key. | `int` | No | +| `keepalive` | Configures the cache for connections to upstream servers. The value `0` disables the cache. See the [keepalive](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive) directive. The default is set in the `keepalive` ConfigMap key. | `int` | No `connect-timeout` | The timeout for establishing a connection with an upstream server. See the [proxy_connect_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_connect_timeout) directive. The default is specified in the `proxy-connect-timeout` ConfigMap key. | `string` | No `read-timeout` | The timeout for reading a response from an upstream server. See the [proxy_read_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_read_timeout) directive. The default is specified in the `proxy-read-timeout` ConfigMap key. | `string` | No `send-timeout` | The timeout for transmitting a request to an upstream server. See the [proxy_send_timeout](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_send_timeout) directive. The default is `60s`. | `string` | No diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index eb70529f56..cdb2facd2b 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -269,7 +269,7 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool cfgParams.SSLPorts = sslPorts } - if keepalive, exists, err := GetMapKeyAsInt64(ingEx.Ingress.Annotations, "nginx.org/keepalive", ingEx.Ingress); exists { + if keepalive, exists, err := GetMapKeyAsInt(ingEx.Ingress.Annotations, "nginx.org/keepalive", ingEx.Ingress); exists { if err != nil { glog.Error(err) } else { diff --git a/internal/configs/config_params.go b/internal/configs/config_params.go index d4ce98a373..d6fcd3cd89 100644 --- a/internal/configs/config_params.go +++ b/internal/configs/config_params.go @@ -39,7 +39,7 @@ type ConfigParams struct { MainWorkerShutdownTimeout string MainWorkerConnections string MainWorkerRlimitNofile string - Keepalive int64 + Keepalive int MaxFails int FailTimeout string HealthCheckEnabled bool diff --git a/internal/configs/configmaps.go b/internal/configs/configmaps.go index eddf0e3739..02b2f01e4d 100644 --- a/internal/configs/configmaps.go +++ b/internal/configs/configmaps.go @@ -287,7 +287,7 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool) *ConfigParams { cfgParams.MainWorkerRlimitNofile = workerRlimitNofile } - if keepalive, exists, err := GetMapKeyAsInt64(cfgm.Data, "keepalive", cfgm); exists { + if keepalive, exists, err := GetMapKeyAsInt(cfgm.Data, "keepalive", cfgm); exists { if err != nil { glog.Error(err) } else { diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index a87ad5b2fd..d275a96235 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -3,7 +3,6 @@ package configs import ( "fmt" "sort" - "strconv" "strings" "github.com/golang/glog" @@ -214,7 +213,7 @@ func generateNginxCfg(ingEx *IngressEx, pems map[string]string, isMinion bool, b var keepalive string if cfgParams.Keepalive > 0 { - keepalive = strconv.FormatInt(cfgParams.Keepalive, 10) + keepalive = fmt.Sprint(cfgParams.Keepalive) } return version1.IngressNginxConfig{ diff --git a/internal/configs/version2/config.go b/internal/configs/version2/config.go index 5ff90ad681..e0c1f324db 100644 --- a/internal/configs/version2/config.go +++ b/internal/configs/version2/config.go @@ -6,14 +6,14 @@ type VirtualServerConfig struct { Upstreams []Upstream SplitClients []SplitClient Maps []Map - Keepalive string } // Upstream defines an upstream. type Upstream struct { - Name string - Servers []UpstreamServer - LBMethod string + Name string + Servers []UpstreamServer + LBMethod string + Keepalive int } // UpstreamServer defines an upstream server. @@ -60,6 +60,7 @@ type Location struct { ProxyBuffers string ProxyBufferSize string ProxyPass string + HasKeepalive bool } // SplitClient defines a split_clients. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 96cd05f75f..9cb108e25f 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -8,8 +8,8 @@ upstream {{ $u.Name }} { server {{ $s.Address }} max_fails={{ $s.MaxFails }} fail_timeout={{ $s.FailTimeout }}; {{ end }} - {{ if $.Keepalive }} - keepalive {{ $.Keepalive }}; + {{ if $u.Keepalive }} + keepalive {{ $u.Keepalive }}; {{ end }} } {{ end }} @@ -107,7 +107,7 @@ server { proxy_http_version 1.1; - {{ if $.Keepalive }} + {{ if $l.HasKeepalive }} proxy_set_header Connection ""; {{ end }} diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 96cd05f75f..9cb108e25f 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -8,8 +8,8 @@ upstream {{ $u.Name }} { server {{ $s.Address }} max_fails={{ $s.MaxFails }} fail_timeout={{ $s.FailTimeout }}; {{ end }} - {{ if $.Keepalive }} - keepalive {{ $.Keepalive }}; + {{ if $u.Keepalive }} + keepalive {{ $u.Keepalive }}; {{ end }} } {{ end }} @@ -107,7 +107,7 @@ server { proxy_http_version 1.1; - {{ if $.Keepalive }} + {{ if $l.HasKeepalive }} proxy_set_header Connection ""; {{ end }} diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go index 9117baccf0..87eb9c230f 100644 --- a/internal/configs/version2/templates_test.go +++ b/internal/configs/version2/templates_test.go @@ -16,7 +16,8 @@ var virtualServerCfg = VirtualServerConfig{ FailTimeout: "10s", }, }, - LBMethod: "random", + LBMethod: "random", + Keepalive: 32, }, { Name: "coffee-v1", @@ -159,7 +160,6 @@ var virtualServerCfg = VirtualServerConfig{ }, }, }, - Keepalive: "10", } func TestVirtualServerForNginxPlus(t *testing.T) { diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 1209958850..7c266c6df0 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -178,11 +178,6 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam } } - keepalive := "" - if baseCfgParams.Keepalive > 0 { - keepalive = fmt.Sprint(baseCfgParams.Keepalive) - } - return version2.VirtualServerConfig{ Upstreams: upstreams, SplitClients: splitClients, @@ -200,7 +195,6 @@ func generateVirtualServerConfig(virtualServerEx *VirtualServerEx, tlsPemFileNam InternalRedirectLocations: internalRedirectLocations, Locations: locations, }, - Keepalive: keepalive, } } @@ -210,7 +204,7 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, endp for _, e := range endpoints { s := version2.UpstreamServer{ Address: e, - MaxFails: generatePositiveIntFromPointer(upstream.MaxFails, cfgParams.MaxFails), + MaxFails: generateIntFromPointer(upstream.MaxFails, cfgParams.MaxFails), FailTimeout: generateTime(upstream.FailTimeout, cfgParams.FailTimeout), } upsServers = append(upsServers, s) @@ -219,16 +213,17 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, endp if !isPlus && len(upsServers) == 0 { s := version2.UpstreamServer{ Address: nginx502Server, - MaxFails: generatePositiveIntFromPointer(upstream.MaxFails, cfgParams.MaxFails), + MaxFails: generateIntFromPointer(upstream.MaxFails, cfgParams.MaxFails), FailTimeout: generateTime(upstream.FailTimeout, cfgParams.FailTimeout), } upsServers = append(upsServers, s) } return version2.Upstream{ - Name: upstreamName, - Servers: upsServers, - LBMethod: generateLBMethod(upstream.LBMethod, cfgParams.LBMethod), + Name: upstreamName, + Servers: upsServers, + LBMethod: generateLBMethod(upstream.LBMethod, cfgParams.LBMethod), + Keepalive: generateIntFromPointer(upstream.Keepalive, cfgParams.Keepalive), } } @@ -248,13 +243,20 @@ func generateTime(time string, defaultTime string) string { return time } -func generatePositiveIntFromPointer(n *int, defaultN int) int { +func generateIntFromPointer(n *int, defaultN int) int { if n == nil { return defaultN } return *n } +func upstreamHasKeepalive(upstream conf_v1alpha1.Upstream, cfgParams *ConfigParams) bool { + if upstream.Keepalive != nil { + return *upstream.Keepalive != 0 + } + return cfgParams.Keepalive != 0 +} + func generateLocation(path string, upstreamName string, upstream conf_v1alpha1.Upstream, cfgParams *ConfigParams) version2.Location { return version2.Location{ Path: path, @@ -268,6 +270,7 @@ func generateLocation(path string, upstreamName string, upstream conf_v1alpha1.U ProxyBuffers: cfgParams.ProxyBuffers, ProxyBufferSize: cfgParams.ProxyBufferSize, ProxyPass: fmt.Sprintf("http://%v", upstreamName), + HasKeepalive: upstreamHasKeepalive(upstream, cfgParams), } } diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index cdb3e10ef1..3fd189cbce 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -240,6 +240,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { Address: "10.0.0.20:80", }, }, + Keepalive: 16, }, { Name: "vs_default_cafe_vsr_default_coffee_coffee", @@ -248,6 +249,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { Address: "10.0.0.30:80", }, }, + Keepalive: 16, }, }, Server: version2.Server{ @@ -261,16 +263,17 @@ func TestGenerateVirtualServerConfig(t *testing.T) { Snippets: []string{"# server snippet"}, Locations: []version2.Location{ { - Path: "/tea", - ProxyPass: "http://vs_default_cafe_tea", + Path: "/tea", + ProxyPass: "http://vs_default_cafe_tea", + HasKeepalive: true, }, { - Path: "/coffee", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee", + Path: "/coffee", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee", + HasKeepalive: true, }, }, }, - Keepalive: "16", } isPlus := false @@ -739,6 +742,7 @@ func TestGenerateUpstream(t *testing.T) { LBMethod: "random", MaxFails: 1, FailTimeout: "10s", + Keepalive: 21, } expected := version2.Upstream{ @@ -750,7 +754,8 @@ func TestGenerateUpstream(t *testing.T) { FailTimeout: "10s", }, }, - LBMethod: "random", + LBMethod: "random", + Keepalive: 21, } result := generateUpstream(name, upstream, endpoints, isPlus, &cfgParams) @@ -759,6 +764,72 @@ func TestGenerateUpstream(t *testing.T) { } } +func TestGenerateUpstreamWithKeepalive(t *testing.T) { + name := "test-upstream" + noKeepalive := 0 + keepalive := 32 + endpoints := []string{ + "192.168.10.10:8080", + } + isPlus := false + + tests := []struct { + upstream conf_v1alpha1.Upstream + cfgParams *ConfigParams + expected version2.Upstream + msg string + }{ + { + conf_v1alpha1.Upstream{Keepalive: &keepalive}, + &ConfigParams{Keepalive: 21}, + version2.Upstream{ + Name: "test-upstream", + Servers: []version2.UpstreamServer{ + { + Address: "192.168.10.10:8080", + }, + }, + Keepalive: 32, + }, + "upstream keepalive set, configparam set", + }, + { + conf_v1alpha1.Upstream{}, + &ConfigParams{Keepalive: 21}, + version2.Upstream{ + Name: "test-upstream", + Servers: []version2.UpstreamServer{ + { + Address: "192.168.10.10:8080", + }, + }, + Keepalive: 21, + }, + "upstream keepalive not set, configparam set", + }, + { + conf_v1alpha1.Upstream{Keepalive: &noKeepalive}, + &ConfigParams{Keepalive: 21}, + version2.Upstream{ + Name: "test-upstream", + Servers: []version2.UpstreamServer{ + { + Address: "192.168.10.10:8080", + }, + }, + }, + "upstream keepalive set to 0, configparam set", + }, + } + + for _, test := range tests { + result := generateUpstream(name, test.upstream, endpoints, isPlus, test.cfgParams) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("generateUpstream() returned %v but expected %v for the case of %v", result, test.expected, test.msg) + } + } +} + func TestGenerateUpstreamForZeroEndpoints(t *testing.T) { name := "test-upstream" upstream := conf_v1alpha1.Upstream{} @@ -1466,3 +1537,41 @@ func TestGenerateLBMethod(t *testing.T) { } } } + +func TestUpstreamHasKeepalive(t *testing.T) { + noKeepalive := 0 + keepalive := 32 + + tests := []struct { + upstream conf_v1alpha1.Upstream + cfgParams *ConfigParams + expected bool + msg string + }{ + { + conf_v1alpha1.Upstream{}, + &ConfigParams{Keepalive: keepalive}, + true, + "upstream keepalive not set, configparam keepalive set", + }, + { + conf_v1alpha1.Upstream{Keepalive: &noKeepalive}, + &ConfigParams{Keepalive: keepalive}, + false, + "upstream keepalive set to 0, configparam keepive set", + }, + { + conf_v1alpha1.Upstream{Keepalive: &keepalive}, + &ConfigParams{Keepalive: noKeepalive}, + true, + "upstream keepalive set, configparam keepalive set to 0", + }, + } + + for _, test := range tests { + result := upstreamHasKeepalive(test.upstream, test.cfgParams) + if result != test.expected { + t.Errorf("upstreamHasKeepalive() returned %v, but expected %v for the case of %v", result, test.expected, test.msg) + } + } +} diff --git a/pkg/apis/configuration/v1alpha1/types.go b/pkg/apis/configuration/v1alpha1/types.go index 87c40a95f2..d6e8bb09e0 100644 --- a/pkg/apis/configuration/v1alpha1/types.go +++ b/pkg/apis/configuration/v1alpha1/types.go @@ -31,6 +31,7 @@ type Upstream struct { LBMethod string `json:"lb-method"` FailTimeout string `json:"fail-timeout"` MaxFails *int `json:"max-fails"` + Keepalive *int `json:"keepalive"` ProxyConnectTimeout string `json:"connect-timeout"` ProxyReadTimeout string `json:"read-timeout"` ProxySendTimeout string `json:"send-timeout"` diff --git a/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go index 9fcf06ba2e..93b74ea498 100644 --- a/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go @@ -139,6 +139,11 @@ func (in *Upstream) DeepCopyInto(out *Upstream) { *out = new(int) **out = **in } + if in.Keepalive != nil { + in, out := &in.Keepalive, &out.Keepalive + *out = new(int) + **out = **in + } return } diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go index 51dc6daefe..1a004702fb 100644 --- a/pkg/apis/configuration/validation/validation.go +++ b/pkg/apis/configuration/validation/validation.go @@ -146,6 +146,7 @@ func validateUpstreams(upstreams []v1alpha1.Upstream, fieldPath *field.Path, isP allErrs = append(allErrs, validateUpstreamLBMethod(u.LBMethod, idxPath.Child("lb-method"), isPlus)...) allErrs = append(allErrs, validateTime(u.FailTimeout, idxPath.Child("fail-timeout"))...) allErrs = append(allErrs, validatePositiveIntOrZero(u.MaxFails, idxPath.Child("max-fails"))...) + allErrs = append(allErrs, validatePositiveIntOrZero(u.Keepalive, idxPath.Child("keepalive"))...) for _, msg := range validation.IsValidPortNum(int(u.Port)) { allErrs = append(allErrs, field.Invalid(idxPath.Child("port"), u.Port, msg)) diff --git a/pkg/apis/configuration/validation/validation_test.go b/pkg/apis/configuration/validation/validation_test.go index 35ff81c893..8846382f06 100644 --- a/pkg/apis/configuration/validation/validation_test.go +++ b/pkg/apis/configuration/validation/validation_test.go @@ -3,14 +3,14 @@ package validation import ( "testing" - "k8s.io/apimachinery/pkg/util/sets" - "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" ) func TestValidateVirtualServer(t *testing.T) { + var keepalive = 32 virtualServer := v1alpha1.VirtualServer{ ObjectMeta: meta_v1.ObjectMeta{ Name: "cafe", @@ -23,10 +23,11 @@ func TestValidateVirtualServer(t *testing.T) { }, Upstreams: []v1alpha1.Upstream{ { - Name: "first", - Service: "service-1", - LBMethod: "random", - Port: 80, + Name: "first", + Service: "service-1", + LBMethod: "random", + Port: 80, + Keepalive: &keepalive, }, { Name: "second", @@ -1160,7 +1161,7 @@ func TestIsValidMatchValue(t *testing.T) { validValues := []string{ "abc", "123", - `\" + `\" abc\"`, `\"`, } @@ -1477,7 +1478,6 @@ func TestValidatePositiveIntOrZeroFails(t *testing.T) { if len(allErrs) == 0 { t.Error("validatePositiveInt returned no errors for case: invalid (-1)") } - } func TestValidateTime(t *testing.T) {