Add configurable timeouts to TransportServer

[LorcanMcVeigh]

Proposed changes

This PR adds support for proxy_connect_timeout, proxy_timeout, proxy_next_upstream, proxy_next_upstream_timeout and proxy_next_upstream_tries directives for TransportServers.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

• I have read the CONTRIBUTING doc
• I have added tests that prove my fix is effective or that my feature works
• I have checked that all unit tests pass after adding my changes
• I have updated necessary documentation
• I have rebased my branch onto master
• I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

[pleshakov]

Hi @LorcanMcVeigh Please see my feedback

[pleshakov]

valid timeout?

[pleshakov]

for cases when we check if a field is not empty (multiple places in this file), I suggest we use generateString from internal/configs/virtualserver.go:1325

nextUpstreamTimeout = generateString(TransportServer.Spec.UpstreamParameters.NextUpstreamTimeout, "0")

Because the function will be used both in virtualserver.go and here, better to put it in a new file, something like generation_helpers.go (and generation_helpers_test.go for unit tests).

[mikestephen]

Could be called something like stringWithDefaultValue().

[Dean-Coakley]

Re: #1346 (comment)
Makes sense to give the test case a name "Valid timeout", etc

LGTM overall.

[pleshakov]

here we have two default values for connectTimeout :

1. "" - if transportServerEx.TransportServer.Spec.UpstreamParameters is nil
2. "0" - if transportServerEx.TransportServer.Spec.UpstreamParameters isn't nil but TransportServer.Spec.UpstreamParameters.ConnectTimeout is "".

It looks like that because of that, you added addition ifs in the template:

   {{ if $s.ProxyConnectTimeout }}
    proxy_connect_timeout {{ $s.ProxyConnectTimeout }};
    {{ end }}

As you can see, having two default values makes things more complicated. Additionally, there is a bug - because the second default value "0" should be "60s".

Instead, we can have something like this:

	var connectTimeoutFieldVal string 
	if transportServerEx.TransportServer.Spec.UpstreamParameters != nil {
		connectTimeoutFieldVal = transportServerEx.TransportServer.Spec.UpstreamParameters.ConnectTimeout	
	}
	
	connectTimeout = generateString(connectTimeoutFieldVal, "60s") 

and in the template:

  proxy_connect_timeout {{ $s.ProxyConnectTimeout }};

That will be similar to what we have in VS:

		ProxyConnectTimeout:      generateString(upstream.ProxyConnectTimeout, cfgParams.ProxyConnectTimeout),

template:

        proxy_connect_timeout {{ $l.ProxyConnectTimeout }};

Could you also apply this approach to proxyTimeout ?

[mikestephen]

Looks good Lorcan.