Add keepalive support to vs/vsr

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 specified in the `proxy-send-timeout` ConfigMap key. | `string` | No

internal/configs/annotations.go

@@ -275,7 +275,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 {

internal/configs/config_params.go

@@ -39,7 +39,7 @@ type ConfigParams struct {
 	MainWorkerShutdownTimeout     string
 	MainWorkerConnections         string
 	MainWorkerRlimitNofile        string
-	Keepalive                     int64
+	Keepalive                     int
 	MaxFails                      int
 	MaxConns                      int
 	FailTimeout                   string

internal/configs/configmaps.go

@@ -291,7 +291,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 {

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{

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.

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 }}
 

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 }}
 

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) {

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),
 	}
 }
 

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)
+		}
+	}
+}