diff --git a/src/code.cloudfoundry.org/gorouter/integration/init_test.go b/src/code.cloudfoundry.org/gorouter/integration/init_test.go index 8521e035c..294eb315a 100644 --- a/src/code.cloudfoundry.org/gorouter/integration/init_test.go +++ b/src/code.cloudfoundry.org/gorouter/integration/init_test.go @@ -48,6 +48,7 @@ var _ = SynchronizedBeforeSuite(func() []byte { json.Unmarshal(data, &res) gorouterPath = res.Gorouter testAssets = res.Test + SetDefaultEventuallyTimeout(10 * time.Second) SetDefaultEventuallyPollingInterval(100 * time.Millisecond) SetDefaultConsistentlyDuration(1 * time.Second) SetDefaultConsistentlyPollingInterval(10 * time.Millisecond) diff --git a/src/code.cloudfoundry.org/gorouter/integration/main_test.go b/src/code.cloudfoundry.org/gorouter/integration/main_test.go index cdc0c79ea..846974a43 100644 --- a/src/code.cloudfoundry.org/gorouter/integration/main_test.go +++ b/src/code.cloudfoundry.org/gorouter/integration/main_test.go @@ -635,7 +635,7 @@ var _ = Describe("Router Integration", func() { BeforeEach(func() { testState = NewTestState() - testState.cfg.DebugAddr = "127.0.0.1:17017" + testState.cfg.DebugAddr = fmt.Sprintf("127.0.0.1:%d", test_util.NextAvailPort()) testState.StartGorouterOrFail() gorouterSession = testState.gorouterSession @@ -1288,7 +1288,7 @@ var _ = Describe("Router Integration", func() { It("doesn't start the route fetcher", func() { gorouterSession = startGorouterSession(cfgFile) - Eventually(gorouterSession).ShouldNot(Say("setting-up-routing-api")) + Consistently(gorouterSession).ShouldNot(Say("setting-up-routing-api")) stopGorouter(gorouterSession) }) }) diff --git a/src/code.cloudfoundry.org/gorouter/registry/registry_test.go b/src/code.cloudfoundry.org/gorouter/registry/registry_test.go index 5e828186b..1db3933f9 100644 --- a/src/code.cloudfoundry.org/gorouter/registry/registry_test.go +++ b/src/code.cloudfoundry.org/gorouter/registry/registry_test.go @@ -824,10 +824,11 @@ var _ = Describe("RouteRegistry", func() { r.Register("hb-partial-prune.example.com", staleEndpoint) doneChan := make(chan struct{}) - defer close(doneChan) + stoppedChan := make(chan struct{}) // Keep the fresh endpoint alive during pruning go func() { + defer close(stoppedChan) for { select { case <-doneChan: @@ -838,6 +839,10 @@ var _ = Describe("RouteRegistry", func() { } } }() + defer func() { + close(doneChan) + <-stoppedChan + }() captureCountBefore := reporter.CaptureEndpointsPerPoolCallCount() diff --git a/src/code.cloudfoundry.org/gorouter/router/health_listener_test.go b/src/code.cloudfoundry.org/gorouter/router/health_listener_test.go index c4a2f15e2..0eccc0dfa 100644 --- a/src/code.cloudfoundry.org/gorouter/router/health_listener_test.go +++ b/src/code.cloudfoundry.org/gorouter/router/health_listener_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -129,7 +130,7 @@ var _ = Describe("HealthListener", func() { tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 10 * time.Second} resp, err := client.Do(req) Expect(err).ToNot(HaveOccurred()) Expect(resp).ToNot(BeNil()) @@ -154,7 +155,7 @@ var _ = Describe("HealthListener", func() { tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 10 * time.Second} resp, err := client.Do(req) Expect(err).ToNot(HaveOccurred()) Expect(resp).ToNot(BeNil()) @@ -182,7 +183,7 @@ var _ = Describe("HealthListener", func() { router.stopping = true }) It("does not log an error message", func() { - Eventually(logger).ShouldNot(gbytes.Say("health-listener-failed")) + Consistently(logger).ShouldNot(gbytes.Say("health-listener-failed")) }) }) }) diff --git a/src/code.cloudfoundry.org/gorouter/router/router_suite_test.go b/src/code.cloudfoundry.org/gorouter/router/router_suite_test.go index 2fbfc4ff4..946afbeb1 100644 --- a/src/code.cloudfoundry.org/gorouter/router/router_suite_test.go +++ b/src/code.cloudfoundry.org/gorouter/router/router_suite_test.go @@ -26,6 +26,8 @@ var _ = SynchronizedBeforeSuite(func() []byte { dropsonde.InitializeWithEmitter(fakeEmitter) return nil }, func([]byte) { + http.DefaultClient.Timeout = 10 * time.Second + SetDefaultEventuallyTimeout(10 * time.Second) SetDefaultEventuallyPollingInterval(100 * time.Millisecond) SetDefaultConsistentlyDuration(1 * time.Second) SetDefaultConsistentlyPollingInterval(10 * time.Millisecond) diff --git a/src/code.cloudfoundry.org/gorouter/router/router_test.go b/src/code.cloudfoundry.org/gorouter/router/router_test.go index 80e46605b..b9217227d 100644 --- a/src/code.cloudfoundry.org/gorouter/router/router_test.go +++ b/src/code.cloudfoundry.org/gorouter/router/router_test.go @@ -130,13 +130,13 @@ var _ = Describe("Router", func() { }) AfterEach(func() { - if natsRunner != nil { - natsRunner.Stop() - } - if router != nil { router.Stop() } + + if natsRunner != nil { + natsRunner.Stop() + } }) Describe("Route Services Server", func() { @@ -229,7 +229,7 @@ var _ = Describe("Router", func() { tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 500 * time.Millisecond} resp, err := client.Do(req) if err != nil { return 0, err @@ -514,7 +514,7 @@ var _ = Describe("Router", func() { req, err = http.NewRequest("GET", app.Endpoint(), nil) Expect(err).ToNot(HaveOccurred()) - client := http.Client{} + client := http.Client{Timeout: 10 * time.Second} _, err = client.Do(req) Expect(err).To(HaveOccurred()) }) @@ -610,7 +610,7 @@ var _ = Describe("Router", func() { tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 10 * time.Second} resp, err := client.Do(req) Expect(err).ToNot(HaveOccurred()) Expect(resp).ToNot(BeNil()) @@ -652,7 +652,7 @@ var _ = Describe("Router", func() { r, err := http.NewRequest("PUT", url, buf) Expect(err).ToNot(HaveOccurred()) - client := http.Client{} + client := http.Client{Timeout: 10 * time.Second} resp, err := client.Do(r) Expect(err).ToNot(HaveOccurred()) Expect(resp.StatusCode).To(Equal(http.StatusOK)) @@ -685,6 +685,7 @@ var _ = Describe("Router", func() { conn, err := net.DialTimeout("tcp", host, 10*time.Second) Expect(err).ToNot(HaveOccurred()) defer conn.Close() + conn.SetDeadline(time.Now().Add(10 * time.Second)) fmt.Fprintf(conn, "POST / HTTP/1.1\r\n"+ "Host: %s\r\n"+ @@ -823,7 +824,7 @@ var _ = Describe("Router", func() { tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 10 * time.Second} resp, err := client.Do(req) Expect(err).ToNot(HaveOccurred()) @@ -838,6 +839,11 @@ var _ = Describe("Router", func() { }) Context("HTTP keep-alive", func() { + BeforeEach(func() { + config.EndpointTimeout = 2 * time.Second + backendIdleTimeout = config.EndpointTimeout + requestTimeout = config.EndpointTimeout + }) It("reuses the same connection on subsequent calls", func() { app := test.NewGreetApp([]route.Uri{"keepalive." + test_util.LocalhostDNS}, config.Port, mbusClient, nil) app.RegisterAndListen() @@ -1031,7 +1037,7 @@ var _ = Describe("Router", func() { var client http.Client BeforeEach(func() { - client = http.Client{} + client = http.Client{Timeout: 10 * time.Second} }) JustBeforeEach(func() { @@ -1088,7 +1094,7 @@ var _ = Describe("Router", func() { requestTimeout = 1 * time.Second backendIdleTimeout = 3 * time.Second appResponseTime = 2 * time.Second - client = http.Client{} + client = http.Client{Timeout: 10 * time.Second} }) JustBeforeEach(func() { app := newSlowApp( @@ -1150,7 +1156,7 @@ var _ = Describe("Router", func() { requestTimeout = 3 * time.Second backendIdleTimeout = 1 * time.Second appResponseTime = 2 * time.Second - client = http.Client{} + client = http.Client{Timeout: 10 * time.Second} }) JustBeforeEach(func() { app := newSlowApp( @@ -1338,9 +1344,10 @@ var _ = Describe("Router", func() { RootCAs: rootCAs, } - httpClient = &http.Client{Transport: &http.Transport{ - TLSClientConfig: tlsClientConfig, - }} + httpClient = &http.Client{ + Transport: &http.Transport{TLSClientConfig: tlsClientConfig}, + Timeout: 10 * time.Second, + } }) JustBeforeEach(func() { @@ -1643,9 +1650,10 @@ var _ = Describe("Router", func() { tlsClientConfig := &tls.Config{ RootCAs: rootCAs, } - client := &http.Client{Transport: &http.Transport{ - TLSClientConfig: tlsClientConfig, - }} + client := &http.Client{ + Transport: &http.Transport{TLSClientConfig: tlsClientConfig}, + Timeout: 10 * time.Second, + } app := test.NewGreetApp([]route.Uri{"test." + test_util.LocalhostDNS}, config.Port, mbusClient, nil) app.RegisterAndListen() @@ -1688,9 +1696,10 @@ var _ = Describe("Router", func() { tlsClientConfig = &tls.Config{ RootCAs: rootCAs, } - client = &http.Client{Transport: &http.Transport{ - TLSClientConfig: tlsClientConfig, - }} + client = &http.Client{ + Transport: &http.Transport{TLSClientConfig: tlsClientConfig}, + Timeout: 10 * time.Second, + } }) It("serves ssl traffic", func() { @@ -1729,9 +1738,10 @@ var _ = Describe("Router", func() { RootCAs: rootCAs, } tlsClientConfig.CipherSuites = []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256} - client = &http.Client{Transport: &http.Transport{ - TLSClientConfig: tlsClientConfig, - }} + client = &http.Client{ + Transport: &http.Transport{TLSClientConfig: tlsClientConfig}, + Timeout: 10 * time.Second, + } }) It("serves ssl traffic", func() { @@ -1830,6 +1840,7 @@ var _ = Describe("Router", func() { It("refuses connections to the SSL port", func() { _, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", config.SSLPort)) + Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("connection refused")) }) }) @@ -2021,7 +2032,7 @@ var _ = Describe("Router", func() { }, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 10 * time.Second} resp, err := client.Do(req) Expect(err).To(MatchError(ContainSubstring("remote error: tls: handshake failure"))) Expect(resp).To(BeNil()) @@ -2167,7 +2178,9 @@ var _ = Describe("Router", func() { tlsClientConfig.Certificates = []tls.Certificate{*clientCert} resp, err := client.Do(req) - println("Error", err.Error()) + if err != nil { + println("Error", err.Error()) + } Expect(err).To(HaveOccurred()) Expect(err).To(MatchError(ContainSubstring("remote error: tls: bad certificate"))) Expect(resp).To(BeNil()) @@ -2197,7 +2210,9 @@ var _ = Describe("Router", func() { tlsClientConfig.Certificates = []tls.Certificate{*clientCert} resp, err := client.Do(req) - println("Error", err.Error()) + if err != nil { + println("Error", err.Error()) + } Expect(err).To(HaveOccurred()) Expect(err).To(MatchError(ContainSubstring("remote error: tls: bad certificate"))) Expect(resp).To(BeNil()) @@ -2253,7 +2268,7 @@ var _ = Describe("Router", func() { }, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 10 * time.Second} resp, err := client.Do(req) Expect(err).ToNot(HaveOccurred()) defer resp.Body.Close() @@ -2285,7 +2300,7 @@ var _ = Describe("Router", func() { }, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 10 * time.Second} resp, err := client.Do(req) Expect(err).To(HaveOccurred()) Expect(resp).To(BeNil()) @@ -2345,7 +2360,7 @@ var _ = Describe("Router", func() { RootCAs: certPool, }, } - client := http.Client{Transport: tr} + client := http.Client{Transport: tr, Timeout: 10 * time.Second} req, err := http.NewRequest("GET", fmt.Sprintf("https://myapp.%s:%d/", test_util.LocalhostDNS, config.SSLPort), nil) Expect(err).NotTo(HaveOccurred()) _, err = client.Do(req) diff --git a/src/code.cloudfoundry.org/gorouter/test_util/ports.go b/src/code.cloudfoundry.org/gorouter/test_util/ports.go index 4e0622c0b..aaeec5d70 100644 --- a/src/code.cloudfoundry.org/gorouter/test_util/ports.go +++ b/src/code.cloudfoundry.org/gorouter/test_util/ports.go @@ -1,31 +1,18 @@ package test_util import ( - "sync" - - . "github.com/onsi/ginkgo/v2" -) - -var ( - lastPortUsed uint16 - portLock sync.Mutex - once sync.Once + "net" ) +// NextAvailPort asks the OS for a free port by binding to :0, then closing +// the listener and returning the assigned port. This avoids cross-suite port +// collisions that occur when multiple suites reuse the same static port range. func NextAvailPort() uint16 { - portLock.Lock() - defer portLock.Unlock() - - if lastPortUsed == 0 { - once.Do(func() { - const portRangeStart = 25000 - // #nosec G115 - if we have negative or > 65k parallel ginkgo threads there's something worse happening - lastPortUsed = portRangeStart + uint16(GinkgoParallelProcess()) - }) + l, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + panic("NextAvailPort: " + err.Error()) } - - suiteCfg, _ := GinkgoConfiguration() - // #nosec G115 - if we have negative or > 65k parallel ginkgo threads there's something worse happening - lastPortUsed += uint16(suiteCfg.ParallelTotal) - return lastPortUsed + defer l.Close() + // #nosec G115 - ephemeral ports are always in uint16 range + return uint16(l.Addr().(*net.TCPAddr).Port) }