Fix flaky tests

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)

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)
 		})
 	})

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()
 

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"))
 			})
 		})
 	})

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)

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)

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)
 }