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.

Sign up for GitHub

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

Add retries support for vs/vsr #628

Merged
merged 15 commits into from
Jul 26, 2019
Merged

Add retries support for vs/vsr #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
90f19e0
add support for retires
LorcanMcVeigh Jul 17, 2019
56d6827
add retries support for vs/vsr
LorcanMcVeigh Jul 18, 2019
875d454
Add tests for retries
LorcanMcVeigh Jul 22, 2019
ba13b9a
Resolve PR feedback
LorcanMcVeigh Jul 23, 2019
a749e72
Regen the automatic code
LorcanMcVeigh Jul 23, 2019
3d3dea2
Add unit test
LorcanMcVeigh Jul 23, 2019
084e853
Remove generateTime and replaced with generateString
LorcanMcVeigh Jul 23, 2019
3f2fa62
Add validation for upstream fields
LorcanMcVeigh Jul 23, 2019
4d47357
Add test for validation
LorcanMcVeigh Jul 23, 2019
9ebcc9b
Address feedback from PR
LorcanMcVeigh Jul 25, 2019
bfa535c
Address feedback from PR
LorcanMcVeigh Jul 25, 2019
e6d5cf4
Fix errors for tests
LorcanMcVeigh Jul 25, 2019
ba63652
Resolve feedback from review
LorcanMcVeigh Jul 25, 2019
d9f8874
Update tests
LorcanMcVeigh Jul 25, 2019
02ed166
Add final changes from feedback
LorcanMcVeigh Jul 26, 2019
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
14 changes: 10 additions & 4 deletions docs/virtualserver-and-virtualserverroute.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ keepalive: 32
connect-timeout: 30s
read-timeout: 30s
send-timeout: 30s
next-upstream: "error timeout non_idempotent"
next-upstream-timeout: 5s
next-upstreeam-tries: 10
tls:
enable: True
```
Expand All @@ -196,10 +199,13 @@ tls:
| `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
| `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 |
| `next-upstream` | Specifies in which cases a request should be passed to the next upstream server. See the [proxy_next_upstream](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream) directive. The default is `error timeout`. | `string` | No |
| `next-upstream-timeout` | The time during which a request can be passed to the next upstream server. See the [proxy_next_upstream_timeout](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream_timeout) directive. The `0` value turns off the time limit. The default is `0`. | `string` | No |
| `next-upstream-tries` | The number of possible tries for passing a request to the next upstream server. See the [proxy_next_upstream_tries](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_next_upstream_tries) directive. The `0` value turns off this limit. The default is `0`. | `int` | No |
| `tls` | The TLS configuration for the Upstream. | [`tls`](#UpstreamTLS) | No |

### Upstream.TLS
Expand Down
27 changes: 15 additions & 12 deletions internal/configs/version2/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,21 @@ type SSL struct {

// Location defines a location.
type Location struct {
Path string
Snippets []string
ProxyConnectTimeout string
ProxyReadTimeout string
ProxySendTimeout string
ClientMaxBodySize string
ProxyMaxTempFileSize string
ProxyBuffering bool
ProxyBuffers string
ProxyBufferSize string
ProxyPass string
HasKeepalive bool
Path string
Snippets []string
ProxyConnectTimeout string
ProxyReadTimeout string
ProxySendTimeout string
ClientMaxBodySize string
ProxyMaxTempFileSize string
ProxyBuffering bool
ProxyBuffers string
ProxyBufferSize string
ProxyPass string
ProxyNextUpstream string
ProxyNextUpstreamTimeout string
ProxyNextUpstreamTries int
HasKeepalive bool
}

// SplitClient defines a split_clients.
Expand Down
3 changes: 3 additions & 0 deletions internal/configs/version2/nginx-plus.virtualserver.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ server {
proxy_set_header X-Forwarded-Proto $scheme;

proxy_pass {{ $l.ProxyPass }};
proxy_next_upstream {{ $l.ProxyNextUpstream }};
proxy_next_upstream_timeout {{ $l.ProxyNextUpstreamTimeout }};
proxy_next_upstream_tries {{ $l.ProxyNextUpstreamTries }};
}
{{ end }}
}
3 changes: 3 additions & 0 deletions internal/configs/version2/nginx.virtualserver.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ server {
proxy_set_header X-Forwarded-Proto $scheme;

proxy_pass {{ $l.ProxyPass }};
proxy_next_upstream {{ $l.ProxyNextUpstream }};
proxy_next_upstream_timeout {{ $l.ProxyNextUpstreamTimeout }};
proxy_next_upstream_tries {{ $l.ProxyNextUpstreamTries }};
}
{{ end }}
}
45 changes: 24 additions & 21 deletions internal/configs/virtualserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, endp
s := version2.UpstreamServer{
Address: e,
MaxFails: generateIntFromPointer(upstream.MaxFails, cfgParams.MaxFails),
FailTimeout: generateTime(upstream.FailTimeout, cfgParams.FailTimeout),
FailTimeout: generateString(upstream.FailTimeout, cfgParams.FailTimeout),
}
upsServers = append(upsServers, s)
}
Expand All @@ -214,7 +214,7 @@ func generateUpstream(upstreamName string, upstream conf_v1alpha1.Upstream, endp
s := version2.UpstreamServer{
Address: nginx502Server,
MaxFails: generateIntFromPointer(upstream.MaxFails, cfgParams.MaxFails),
FailTimeout: generateTime(upstream.FailTimeout, cfgParams.FailTimeout),
FailTimeout: generateString(upstream.FailTimeout, cfgParams.FailTimeout),
}
upsServers = append(upsServers, s)
}
Expand All @@ -236,13 +236,6 @@ func generateLBMethod(method string, defaultMethod string) string {
return method
}

func generateTime(time string, defaultTime string) string {
if time == "" {
return defaultTime
}
return time
}

func generateIntFromPointer(n *int, defaultN int) int {
if n == nil {
return defaultN
Expand All @@ -264,20 +257,30 @@ func generateProxyPassProtocol(upstream conf_v1alpha1.Upstream) string {
return "http"
}

func generateString(s string, defaultS string) string {
if s == "" {
return defaultS
}
return s
}

func generateLocation(path string, upstreamName string, upstream conf_v1alpha1.Upstream, cfgParams *ConfigParams) version2.Location {
return version2.Location{
Path: path,
Snippets: cfgParams.LocationSnippets,
ProxyConnectTimeout: generateTime(upstream.ProxyConnectTimeout, cfgParams.ProxyConnectTimeout),
ProxyReadTimeout: generateTime(upstream.ProxyReadTimeout, cfgParams.ProxyReadTimeout),
ProxySendTimeout: generateTime(upstream.ProxySendTimeout, cfgParams.ProxySendTimeout),
ClientMaxBodySize: cfgParams.ClientMaxBodySize,
ProxyMaxTempFileSize: cfgParams.ProxyMaxTempFileSize,
ProxyBuffering: cfgParams.ProxyBuffering,
ProxyBuffers: cfgParams.ProxyBuffers,
ProxyBufferSize: cfgParams.ProxyBufferSize,
ProxyPass: fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream), upstreamName),
HasKeepalive: upstreamHasKeepalive(upstream, cfgParams),
Path: path,
Snippets: cfgParams.LocationSnippets,
ProxyConnectTimeout: generateString(upstream.ProxyConnectTimeout, cfgParams.ProxyConnectTimeout),
ProxyReadTimeout: generateString(upstream.ProxyReadTimeout, cfgParams.ProxyReadTimeout),
ProxySendTimeout: generateString(upstream.ProxySendTimeout, cfgParams.ProxySendTimeout),
ClientMaxBodySize: cfgParams.ClientMaxBodySize,
ProxyMaxTempFileSize: cfgParams.ProxyMaxTempFileSize,
ProxyBuffering: cfgParams.ProxyBuffering,
ProxyBuffers: cfgParams.ProxyBuffers,
ProxyBufferSize: cfgParams.ProxyBufferSize,
ProxyPass: fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream), upstreamName),
ProxyNextUpstream: generateString(upstream.ProxyNextUpstream, "error timeout"),
ProxyNextUpstreamTimeout: generateString(upstream.ProxyNextUpstreamTimeout, "0s"),
ProxyNextUpstreamTries: upstream.ProxyNextUpstreamTries,
HasKeepalive: upstreamHasKeepalive(upstream, cfgParams),
}
}

Expand Down
157 changes: 114 additions & 43 deletions internal/configs/virtualserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,20 @@ func TestGenerateVirtualServerConfig(t *testing.T) {
Snippets: []string{"# server snippet"},
Locations: []version2.Location{
{
Path: "/tea",
ProxyPass: "http://vs_default_cafe_tea",
HasKeepalive: true,
Path: "/tea",
ProxyPass: "http://vs_default_cafe_tea",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
HasKeepalive: true,
},
{
Path: "/coffee",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee",
HasKeepalive: true,
Path: "/coffee",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
HasKeepalive: true,
},
},
},
Expand Down Expand Up @@ -460,20 +466,32 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) {
},
Locations: []version2.Location{
{
Path: "@splits_0_split_0",
ProxyPass: "http://vs_default_cafe_tea-v1",
Path: "@splits_0_split_0",
ProxyPass: "http://vs_default_cafe_tea-v1",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@splits_0_split_1",
ProxyPass: "http://vs_default_cafe_tea-v2",
Path: "@splits_0_split_1",
ProxyPass: "http://vs_default_cafe_tea-v2",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@splits_1_split_0",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1",
Path: "@splits_1_split_0",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@splits_1_split_1",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2",
Path: "@splits_1_split_1",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
},
},
Expand Down Expand Up @@ -704,20 +722,32 @@ func TestGenerateVirtualServerConfigForVirtualServerWithRules(t *testing.T) {
},
Locations: []version2.Location{
{
Path: "@rules_0_match_0",
ProxyPass: "http://vs_default_cafe_tea-v2",
Path: "@rules_0_match_0",
ProxyPass: "http://vs_default_cafe_tea-v2",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@rules_0_default",
ProxyPass: "http://vs_default_cafe_tea-v1",
Path: "@rules_0_default",
ProxyPass: "http://vs_default_cafe_tea-v1",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@rules_1_match_0",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2",
Path: "@rules_1_match_0",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@rules_1_default",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1",
Path: "@rules_1_default",
ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
},
},
Expand Down Expand Up @@ -895,6 +925,29 @@ func TestGenerateProxyPassProtocol(t *testing.T) {
}
}

func TestGenerateString(t *testing.T) {
tests := []struct {
inputS string
expected string
}{
{
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
inputS: "http_404",
expected: "http_404",
},
{
inputS: "",
expected: "error timeout",
},
}

for _, test := range tests {
result := generateString(test.inputS, "error timeout")
if result != test.expected {
t.Errorf("generateString() return %v but expected %v", result, test.expected)
}
}
}

func TestGenerateLocation(t *testing.T) {
cfgParams := ConfigParams{
ProxyConnectTimeout: "30s",
Expand All @@ -911,17 +964,20 @@ func TestGenerateLocation(t *testing.T) {
upstreamName := "test-upstream"

expected := version2.Location{
Path: "/",
Snippets: []string{"# location snippet"},
ProxyConnectTimeout: "30s",
ProxyReadTimeout: "31s",
ProxySendTimeout: "32s",
ClientMaxBodySize: "1m",
ProxyMaxTempFileSize: "1024m",
ProxyBuffering: true,
ProxyBuffers: "8 4k",
ProxyBufferSize: "4k",
ProxyPass: "http://test-upstream",
Path: "/",
Snippets: []string{"# location snippet"},
ProxyConnectTimeout: "30s",
ProxyReadTimeout: "31s",
ProxySendTimeout: "32s",
ClientMaxBodySize: "1m",
ProxyMaxTempFileSize: "1024m",
ProxyBuffering: true,
ProxyBuffers: "8 4k",
ProxyBufferSize: "4k",
ProxyPass: "http://test-upstream",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
}

result := generateLocation(path, upstreamName, conf_v1alpha1.Upstream{}, &cfgParams)
Expand Down Expand Up @@ -1156,12 +1212,18 @@ func TestGenerateSplitRouteConfig(t *testing.T) {
},
Locations: []version2.Location{
{
Path: "@splits_1_split_0",
ProxyPass: "http://vs_default_cafe_coffee-v1",
Path: "@splits_1_split_0",
ProxyPass: "http://vs_default_cafe_coffee-v1",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@splits_1_split_1",
ProxyPass: "http://vs_default_cafe_coffee-v2",
Path: "@splits_1_split_1",
ProxyPass: "http://vs_default_cafe_coffee-v2",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
},
InternalRedirectLocation: version2.InternalRedirectLocation{
Expand Down Expand Up @@ -1364,16 +1426,25 @@ func TestGenerateRulesRouteConfig(t *testing.T) {
},
Locations: []version2.Location{
{
Path: "@rules_1_match_0",
ProxyPass: "http://vs_default_cafe_coffee-v1",
Path: "@rules_1_match_0",
ProxyPass: "http://vs_default_cafe_coffee-v1",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@rules_1_match_1",
ProxyPass: "http://vs_default_cafe_coffee-v2",
Path: "@rules_1_match_1",
ProxyPass: "http://vs_default_cafe_coffee-v2",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
{
Path: "@rules_1_default",
ProxyPass: "http://vs_default_cafe_tea",
Path: "@rules_1_default",
ProxyPass: "http://vs_default_cafe_tea",
ProxyNextUpstream: "error timeout",
ProxyNextUpstreamTimeout: "0s",
ProxyNextUpstreamTries: 0,
},
},
InternalRedirectLocation: version2.InternalRedirectLocation{
Expand Down
Loading