From 85e74bd8064c9bc31742d3b4a62f66b8ae93e695 Mon Sep 17 00:00:00 2001 From: Aidan Oldershaw Date: Mon, 26 Jul 2021 11:56:22 -0400 Subject: [PATCH] speed up TSA tests The HTTP client used by the TSA uses http.DefaultTransport, which has an IdleConnTimeout of 90s. Some of these tests rely on this timeout being hit, meaning that there's a minimum time of 90s for these tests. This resulted in the tsa suite being the slowest to complete, typically taking ~5m. This commit compiles the tsa binary with an overridden IdleConnTimeout of 5s in tests. This does not affect the behaviour of Concourse, as the default behaviour is left unchanged. Signed-off-by: Aidan Oldershaw --- atc/worker/gclient/factory.go | 17 ++++++++++++++++- tsa/cmd/tsa/register_test.go | 13 ++++++++++--- tsa/cmd/tsa/suite_test.go | 2 +- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/atc/worker/gclient/factory.go b/atc/worker/gclient/factory.go index 40cf90ef1..1fc6a3b9b 100644 --- a/atc/worker/gclient/factory.go +++ b/atc/worker/gclient/factory.go @@ -1,6 +1,7 @@ package gclient import ( + "fmt" "net/http" "time" @@ -12,6 +13,10 @@ import ( "github.com/tedsuo/rata" ) +// Allows tests to override http.DefaultTransport's IdleConnTimeout. Only +// applies to BasicGardenClientWithRequestTimeout. +var idleConnTimeoutOverride string + type gardenClientFactory struct { db transport.TransportDB logger lager.Logger @@ -73,8 +78,18 @@ func (gcf *gardenClientFactory) NewClient() Client { // Do not try any client method that requires hijack functionality (streaming logs)! func BasicGardenClientWithRequestTimeout(logger lager.Logger, requestTimeout time.Duration, address string) Client { + httpTransport := http.DefaultTransport.(*http.Transport).Clone() + if idleConnTimeoutOverride != "" { + timeout, err := time.ParseDuration(idleConnTimeoutOverride) + if err != nil { + panic(fmt.Sprintf("invalid idleConnTimeoutOverride: %v", err)) + } + httpTransport.IdleConnTimeout = timeout + } + streamClient := &http.Client{ - Timeout: requestTimeout, + Transport: httpTransport, + Timeout: requestTimeout, } streamer := &transport.WorkerHijackStreamer{ diff --git a/tsa/cmd/tsa/register_test.go b/tsa/cmd/tsa/register_test.go index 86084514f..887af5d78 100644 --- a/tsa/cmd/tsa/register_test.go +++ b/tsa/cmd/tsa/register_test.go @@ -433,13 +433,20 @@ var _ = Describe("Register", func() { Expect(res.StatusCode).To(Equal(http.StatusTeapot)) By("exiting successfully") - // https://golang.org/src/net/http/transport.go -> IdleConnTimeout is 90s in the DefaultTransport used by gclient.BasicGardenClientWithRequestTimeout - Eventually(registerErr, time.Second*100).Should(Receive(BeNil())) + // If this starts failing, it may be because we are no longer + // properly setting atc/worker/gclient.idleConnTimeoutOverride + // in the test binary. It defaults to 90s, which is incredibly + // slow for these tests. + Eventually(registerErr, time.Second*10).Should(Receive(BeNil())) }) Context("with a drain timeout", func() { BeforeEach(func() { - opts.ConnectionDrainTimeout = 5 * time.Second + // Note: this must not be "too close" to the idleConnTimeoutOverride + // value of 5s - otherwise, these timeouts seem to compound + // (e.g. if the drain timeout was also 5s, the effective + // timeout would be ~10s). Not too sure why this is + opts.ConnectionDrainTimeout = 3 * time.Second }) It("breaks connections after the configured drain timeout", func() { diff --git a/tsa/cmd/tsa/suite_test.go b/tsa/cmd/tsa/suite_test.go index b518db033..6028e7523 100644 --- a/tsa/cmd/tsa/suite_test.go +++ b/tsa/cmd/tsa/suite_test.go @@ -39,7 +39,7 @@ var tsaPath string var _ = BeforeSuite(func() { var err error - tsaPath, err = gexec.Build("github.com/concourse/concourse/tsa/cmd/tsa") + tsaPath, err = gexec.Build("github.com/concourse/concourse/tsa/cmd/tsa", "-ldflags", "-X 'github.com/concourse/concourse/atc/worker/gclient.idleConnTimeoutOverride=5s'") Expect(err).NotTo(HaveOccurred()) })