internal/envoy: increase default upstream connect timeout

[tsaarni]

This change sets the new default connect timeout to 2 seconds. The default connect timeout was 250 msec which can be too short and may cause failed requests with slow upstream services.

Updates #2264

Signed-off-by: Tero Saarni tero.saarni@est.tech

[codecov]

Codecov Report

Merging #4151 (c5ef018) into main (5c65e8a) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4151   +/-   ##
=======================================
  Coverage   72.56%   72.56%           
=======================================
  Files         115      115           
  Lines       10032    10032           
=======================================
  Hits         7280     7280           
  Misses       2597     2597           
  Partials      155      155           

Impacted Files	Coverage Δ
internal/envoy/v3/cluster.go	100.00% <100.00%> (ø)
internal/featuretests/v3/envoy.go	100.00% <100.00%> (ø)

[sunjayBhatia]

LGTM given the plan outlined in #2264

[tsaarni]

@sunjayBhatia This https://github.com/projectcontour/contour/runs/4082714409?check_suite_focus=true#step:6:831
might be same flake as discussed earlier?

• [FAILED] [61.230 seconds]
HTTPProxy
/home/runner/work/contour/contour/test/e2e/httpproxy/httpproxy_test.go:58
  cookie-rewriting
  /home/runner/work/contour/contour/test/e2e/httpproxy/httpproxy_test.go:336
    with namespace: app-cookie-rewrite
    /home/runner/work/contour/contour/test/e2e/framework.go:225
      [It] cookies from app can be rewritten
      /home/runner/work/contour/contour/test/e2e/httpproxy/cookie_rewrite_test.go:183

Though PR #4147 was included in this already...

[skriss]

LGTM

[sunjayBhatia]

@sunjayBhatia This https://github.com/projectcontour/contour/runs/4082714409?check_suite_focus=true#step:6:831 might be same flake as discussed earlier?

• [FAILED] [61.230 seconds]
HTTPProxy
/home/runner/work/contour/contour/test/e2e/httpproxy/httpproxy_test.go:58
  cookie-rewriting
  /home/runner/work/contour/contour/test/e2e/httpproxy/httpproxy_test.go:336
    with namespace: app-cookie-rewrite
    /home/runner/work/contour/contour/test/e2e/framework.go:225
      [It] cookies from app can be rewritten
      /home/runner/work/contour/contour/test/e2e/httpproxy/cookie_rewrite_test.go:183

Though PR #4147 was included in this already...

yeah, thought that naive change would fix it

having some more flakes generally with e2e tests, might have to do a more detailed assessment soon

[tsaarni]

Seems I bumped into another flake
https://github.com/projectcontour/contour/runs/4083566179?check_suite_focus=true#step:6:3740

  	Error Trace:	root_namespaces_test.go:60
  	            				suite.go:724
  	            				asm_amd64.s:1581
  	Error:      	Should be true
  	Test:       	HTTPProxy using root namespaces configured via CLI flag with namespace: root-ns-cli root HTTPProxies outside of root namespaces are not configured
  	Messages:   	expected 200 response code, got 503

[skriss]

everything's passing now, GH Actions seems happier now - was able to get everything passing on another PR plus main recently.

[tsaarni]

That was really strange one, it took 5 attempts.

By the way, I suppose there is reason why e2e uses --keep-going to ignore errors? The unfortunate side is that it leads into less than ideal troubleshooting experience when one sees test run ending with:

Ran 17 of 17 Specs in 128.420 seconds
SUCCESS! -- 17 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Ginkgo ran 4 suites in 9m19.082561032s

Test Suite Failed
make: *** [Makefile:288: run-e2e] Error 1
Error: Process completed with exit code 2.

one needs to realize that the failure is buried thousands of lines above the final success suite.

[sunjayBhatia]

--keep-going is to ensure all failures are found in a given suite and that we will run all the test suites, we can choose to bail out early but would lose the ability to see if a change affects multiple tests at a time

(usually what I do is search for [FAIL in the output)