Fixes for e2e race conditions

test/e2e/bootstrap/bootstrap_test.go

@@ -76,17 +76,15 @@ var _ = Describe("Bootstrap", func() {
 		var err error
 		contourCmd, contourConfigFile, err = f.Deployment.StartLocalContour(contourConfig, contourConfiguration)
 		require.NoError(f.T(), err)
+		DeferCleanup(f.Deployment.StopLocalContour, contourCmd, contourConfigFile)
 
 		// Wait for Envoy to be healthy.
 		require.NoError(f.T(), f.Deployment.WaitForEnvoyUpdated())
+		require.NoError(f.T(), f.WaitForReachable())
 
 		kubectlCmd, err = f.Kubectl.StartKubectlPortForward(19001, 9001, "projectcontour", f.Deployment.EnvoyResourceAndName())
 		require.NoError(f.T(), err)
-	})
-
-	AfterEach(func() {
-		f.Kubectl.StopKubectlPortForward(kubectlCmd)
-		require.NoError(f.T(), f.Deployment.StopLocalContour(contourCmd, contourConfigFile))
+		DeferCleanup(f.Kubectl.StopKubectlPortForward, kubectlCmd)
 	})
 
 	f.Test(func() {

test/e2e/deployment.go

@@ -326,7 +326,7 @@ func (d *Deployment) EnsureCertgenJob() error {
 			return err
 		}
 	}
-	if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*50, time.Minute, true, jobDeleted); err != nil {
+	if err := wait.PollUntilContextTimeout(context.Background(), time.Millisecond*200, time.Minute, true, jobDeleted); err != nil {
 		return err
 	}
 	return d.client.Create(context.TODO(), d.CertgenJob)
@@ -416,6 +416,9 @@ func (d *Deployment) EnsureRateLimitResources(namespace, configContents string)
 	if err := d.ensureResource(deployment, new(apps_v1.Deployment)); err != nil {
 		return err
 	}
+	if err := WaitForDeployment(deployment, d.client); err != nil {
+		return err
+	}
 
 	service := d.RateLimitService.DeepCopy()
 	service.Namespace = setNamespace
@@ -439,6 +442,9 @@ func (d *Deployment) EnsureGlobalExternalAuthResources(namespace string) error {
 	if err := d.ensureResource(deployment, new(apps_v1.Deployment)); err != nil {
 		return err
 	}
+	if err := WaitForDeployment(deployment, d.client); err != nil {
+		return err
+	}
 
 	service := d.GlobalExtAuthService.DeepCopy()
 	service.Namespace = setNamespace

test/e2e/fixtures.go

@@ -159,6 +159,10 @@ func (e *Echo) DeployN(ns, name string, replicas int32) (func(), *apps_v1.Deploy
 	}
 	require.NoError(e.t, e.client.Create(context.TODO(), deployment))
 
+	if err := WaitForDeployment(deployment, e.client); err != nil {
+		require.NoError(e.t, err)
+	}
+
 	service := &core_v1.Service{
 		ObjectMeta: meta_v1.ObjectMeta{
 			Namespace: ns,
@@ -297,6 +301,7 @@ func (e *EchoSecure) Deploy(ns, name string, preApplyHook func(deployment *apps_
 			Name:      name,
 		},
 		Spec: apps_v1.DeploymentSpec{
+			Replicas: ptr.To(int32(1)),
 			Selector: &meta_v1.LabelSelector{
 				MatchLabels: map[string]string{"app.kubernetes.io/name": name},
 			},
@@ -421,6 +426,10 @@ func (e *EchoSecure) Deploy(ns, name string, preApplyHook func(deployment *apps_
 	require.NoError(e.t, e.client.Create(context.TODO(), deployment))
 	require.NoError(e.t, e.client.Create(context.TODO(), service))
 
+	if err := WaitForDeployment(deployment, e.client); err != nil {
+		require.NoError(e.t, err)
+	}
+
 	return func() {
 		require.NoError(e.t, e.client.Delete(context.TODO(), service))
 		require.NoError(e.t, e.client.Delete(context.TODO(), deployment))
@@ -514,6 +523,10 @@ func (g *GRPC) Deploy(ns, name string) func() {
 	}
 	require.NoError(g.t, g.client.Create(context.TODO(), deployment))
 
+	if err := WaitForDeployment(deployment, g.client); err != nil {
+		require.NoError(g.t, err)
+	}
+
 	service := &core_v1.Service{
 		ObjectMeta: meta_v1.ObjectMeta{
 			Namespace: ns,

test/e2e/framework.go

@@ -572,3 +572,16 @@ func VerifyTLSServerCert(caCert []byte) func(*tls.Config) {
 		c.InsecureSkipVerify = false
 	}
 }
+
+func (f *Framework) WaitForReachable() error {
+	return wait.PollUntilContextTimeout(context.Background(), f.RetryInterval, f.RetryTimeout, true, func(context.Context) (bool, error) {
+		res, err := f.HTTP.Request(&HTTPRequestOpts{
+			OverrideURL: f.HTTP.HTTPURLMetricsBase,
+			Path:        "/ready",
+		})
+		if err != nil {
+			return false, nil
+		}
+		return res.StatusCode == http.StatusOK, nil
+	})
+}

test/e2e/gateway/gateway_test.go

@@ -125,6 +125,7 @@ var _ = Describe("Gateway API", func() {
 		var err error
 		contourCmd, contourConfigFile, err = f.Deployment.StartLocalContour(contourConfig, contourConfiguration, additionalContourArgs...)
 		require.NoError(f.T(), err)
+		DeferCleanup(f.Deployment.StopLocalContour, contourCmd, contourConfigFile)
 
 		// Wait for Envoy to be healthy.
 		require.NoError(f.T(), f.Deployment.WaitForEnvoyUpdated())
@@ -135,12 +136,11 @@ var _ = Describe("Gateway API", func() {
 		gatewayClassCond := func(*gatewayapi_v1.GatewayClass) bool { return true }
 
 		require.True(f.T(), f.CreateGatewayClassAndWaitFor(contourGatewayClass, gatewayClassCond))
+		DeferCleanup(f.DeleteGatewayClass, contourGatewayClass, false)
+
 		require.True(f.T(), f.CreateGatewayAndWaitFor(contourGateway, e2e.GatewayProgrammed))
-	})
 
-	AfterEach(func() {
-		require.NoError(f.T(), f.DeleteGatewayClass(contourGatewayClass, false))
-		require.NoError(f.T(), f.Deployment.StopLocalContour(contourCmd, contourConfigFile))
+		require.NoError(f.T(), f.WaitForReachable())
 	})
 
 	Describe("Gateway with one HTTP listener", func() {

test/e2e/httpproxy/cookie_rewrite_test.go

@@ -878,6 +878,10 @@ func deployEchoServer(t require.TestingT, c client.Client, ns, name string) {
 	}
 	require.NoError(t, c.Create(context.TODO(), deployment))
 
+	if err := e2e.WaitForDeployment(deployment, c); err != nil {
+		require.NoError(t, err)
+	}
+
 	service := &core_v1.Service{
 		ObjectMeta: meta_v1.ObjectMeta{
 			Namespace: ns,

test/e2e/httpproxy/httpproxy_test.go

@@ -99,13 +99,11 @@ var _ = Describe("HTTPProxy", func() {
 		var err error
 		contourCmd, contourConfigFile, err = f.Deployment.StartLocalContour(contourConfig, contourConfiguration, additionalContourArgs...)
 		require.NoError(f.T(), err)
+		DeferCleanup(f.Deployment.StopLocalContour, contourCmd, contourConfigFile)
 
 		// Wait for Envoy to be healthy.
 		require.NoError(f.T(), f.Deployment.WaitForEnvoyUpdated())
-	})
-
-	AfterEach(func() {
-		require.NoError(f.T(), f.Deployment.StopLocalContour(contourCmd, contourConfigFile))
+		require.NoError(f.T(), f.WaitForReachable())
 	})
 	f.NamespacedTest("httpproxy-direct-response-policy", testDirectResponseRule)
 

test/e2e/incluster/incluster_test.go

@@ -79,25 +79,26 @@ var _ = Describe("Incluster", func() {
 		require.NoError(f.T(), f.Deployment.EnsureContourDeployment())
 		require.NoError(f.T(), f.Deployment.WaitForContourDeploymentUpdated())
 		require.NoError(f.T(), f.Deployment.WaitForEnvoyUpdated())
-	})
-
-	AfterEach(func() {
-		require.NoError(f.T(), f.Deployment.DumpContourLogs())
-
-		// Clean out contour after each test.
-		require.NoError(f.T(), f.Deployment.EnsureDeleted(f.Deployment.ContourDeployment))
-		require.NoError(f.T(), f.Deployment.EnsureDeleted(contourConfig))
-		require.Eventually(f.T(), func() bool {
-			pods := new(core_v1.PodList)
-			podListOptions := &client.ListOptions{
-				LabelSelector: labels.SelectorFromSet(f.Deployment.ContourDeployment.Spec.Selector.MatchLabels),
-				Namespace:     f.Deployment.ContourDeployment.Namespace,
-			}
-			if err := f.Client.List(context.TODO(), pods, podListOptions); err != nil {
-				return false
-			}
-			return len(pods.Items) == 0
-		}, time.Minute, time.Millisecond*50)
+		require.NoError(f.T(), f.WaitForReachable())
+
+		DeferCleanup(func() {
+			require.NoError(f.T(), f.Deployment.DumpContourLogs())
+
+			// Clean out contour after each test.
+			require.NoError(f.T(), f.Deployment.EnsureDeleted(f.Deployment.ContourDeployment))
+			require.NoError(f.T(), f.Deployment.EnsureDeleted(contourConfig))
+			require.Eventually(f.T(), func() bool {
+				pods := new(core_v1.PodList)
+				podListOptions := &client.ListOptions{
+					LabelSelector: labels.SelectorFromSet(f.Deployment.ContourDeployment.Spec.Selector.MatchLabels),
+					Namespace:     f.Deployment.ContourDeployment.Namespace,
+				}
+				if err := f.Client.List(context.TODO(), pods, podListOptions); err != nil {
+					return false
+				}
+				return len(pods.Items) == 0
+			}, time.Minute, time.Millisecond*50)
+		})
 	})
 
 	f.NamespacedTest("smoke-test", testSimpleSmoke)

test/e2e/infra/infra_test.go

@@ -110,17 +110,15 @@ var _ = Describe("Infra", func() {
 		var err error
 		contourCmd, contourConfigFile, err = f.Deployment.StartLocalContour(contourConfig, contourConfiguration, additionalContourArgs...)
 		require.NoError(f.T(), err)
+		DeferCleanup(f.Deployment.StopLocalContour, contourCmd, contourConfigFile)
 
 		// Wait for Envoy to be healthy.
 		require.NoError(f.T(), f.Deployment.WaitForEnvoyUpdated())
+		require.NoError(f.T(), f.WaitForReachable())
 
 		kubectlCmd, err = f.Kubectl.StartKubectlPortForward(19001, 9001, "projectcontour", f.Deployment.EnvoyResourceAndName(), additionalContourArgs...)
 		require.NoError(f.T(), err)
-	})
-
-	AfterEach(func() {
-		f.Kubectl.StopKubectlPortForward(kubectlCmd)
-		require.NoError(f.T(), f.Deployment.StopLocalContour(contourCmd, contourConfigFile))
+		DeferCleanup(f.Kubectl.StopKubectlPortForward, kubectlCmd)
 	})
 
 	f.Test(testMetrics)

test/e2e/infra/metrics_test.go

@@ -18,77 +18,40 @@ package infra
 import (
 	"crypto/tls"
 	"net/http"
-	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
+	"github.com/onsi/gomega/gexec"
 	"github.com/stretchr/testify/require"
 
 	"github.com/projectcontour/contour/test/e2e"
 )
 
 func testMetrics() {
 	Specify("requests to default metrics listener are served", func() {
-		t := f.T()
-
 		res, ok := f.HTTP.MetricsRequestUntil(&e2e.HTTPRequestOpts{
 			Path:      "/stats",
 			Condition: e2e.HasStatusCode(200),
 		})
-		require.NotNil(t, res, "request never succeeded")
-		require.Truef(t, ok, "expected 200 response code, got %d", res.StatusCode)
+		require.NotNil(f.T(), res, "request never succeeded")
+		require.Truef(f.T(), ok, "expected 200 response code, got %d", res.StatusCode)
 	})
 }
 
 func testReady() {
 	Specify("requests to default ready listener are served", func() {
-		t := f.T()
-
 		res, ok := f.HTTP.MetricsRequestUntil(&e2e.HTTPRequestOpts{
 			Path:      "/ready",
 			Condition: e2e.HasStatusCode(200),
 		})
-		require.NotNil(t, res, "request never succeeded")
-		require.Truef(t, ok, "expected 200 response code, got %d", res.StatusCode)
+		require.NotNil(f.T(), res, "request never succeeded")
+		require.Truef(f.T(), ok, "expected 200 response code, got %d", res.StatusCode)
 	})
 }
 
 func testEnvoyMetricsOverHTTPS() {
 	// Flake tracking issue: https://github.com/projectcontour/contour/issues/5932
 	Specify("requests to metrics listener are served", FlakeAttempts(3), func() {
-		t := f.T()
-
-		// Port-forward seems to be flaky. Following sequence happens:
-		//
-		// 1. Envoy becomes ready.
-		// 2. Port-forward is started.
-		// 3. HTTPS request is sent but the connection times out with errors
-		//     "error creating error stream for port 18003 -> 8003: Timeout occurred",
-		//     "error creating forwarding stream for port 18003 -> 8003: Timeout occurred"
-		// 4. Meanwhile the metrics listener gets added.
-		// 5. Sometimes (one out of ~1-50 runs) port-forward gets stuck and packets are not forwarded
-		//    even after listener is up and connection attempts are still regularly retried.
-		//
-		// When the problem occurs, Wireshark does not show any traffic on the container side.
-		// The problem could be e.g. undiscovered race condition with Kubernetes port-forward.
-		//
-		// Following workarounds seem to work:
-		//
-		// a) Add a fixed delay before port-forwarding.
-		// b) Wait for Envoy to have listener by observing Envoy logs before port-forwarding.
-		// c) Restart port-forwarding when connection attempts fail.
-		//
-		// Executing port-forward started in BeforeEach(), JustBeforeEach() or combining metrics
-		// port with the admin port-forward command (127.0.0.1:19001 -> 9001) did not help.
-		//
-		// The simplest workaround (a) is taken here.
-		time.Sleep(5 * time.Second)
-
-		// Port-forward for metrics over HTTPS
-		kubectlCmd, err := f.Kubectl.StartKubectlPortForward(18003, 8003, "projectcontour", f.Deployment.EnvoyResourceAndName())
-		require.NoError(t, err)
-		defer f.Kubectl.StopKubectlPortForward(kubectlCmd)
-
 		clientCert, caBundle := f.Certs.GetTLSCertificate("projectcontour", "metrics-client")
 		client := http.Client{
 			Transport: &http.Transport{
@@ -100,13 +63,32 @@ func testEnvoyMetricsOverHTTPS() {
 			},
 		}
 
+		var kubectlCmd *gexec.Session
+		defer func() {
+			if kubectlCmd != nil {
+				f.Kubectl.StopKubectlPortForward(kubectlCmd)
+			}
+		}()
+
 		gomega.Eventually(func() int {
+			var err error
+			if kubectlCmd == nil {
+				kubectlCmd, err = f.Kubectl.StartKubectlPortForward(18003, 8003, "projectcontour", f.Deployment.EnvoyResourceAndName())
+				if err != nil {
+					GinkgoWriter.Println("failed to start port-forward:", err)
+					return 0
+				}
+			}
+
 			resp, err := client.Get("https://localhost:18003/stats")
 			if err != nil {
-				GinkgoWriter.Println(err)
+				GinkgoWriter.Println("request failed, restarting port-forward:", err)
+				f.Kubectl.StopKubectlPortForward(kubectlCmd)
+				kubectlCmd = nil
 				return 0
 			}
+			defer resp.Body.Close()
 			return resp.StatusCode
-		}, "10s", "1s").Should(gomega.Equal(http.StatusOK))
+		}, "30s", "1s").Should(gomega.Equal(http.StatusOK))
 	})
 }

test/e2e/ingress/ingress_test.go

@@ -81,15 +81,12 @@ var _ = Describe("Ingress", func() {
 		var err error
 		contourCmd, contourConfigFile, err = f.Deployment.StartLocalContour(contourConfig, contourConfiguration, additionalContourArgs...)
 		require.NoError(f.T(), err)
+		DeferCleanup(f.Deployment.StopLocalContour, contourCmd, contourConfigFile)
 
 		// Wait for Envoy to be healthy.
 		require.NoError(f.T(), f.Deployment.WaitForEnvoyUpdated())
 	})
 
-	AfterEach(func() {
-		require.NoError(f.T(), f.Deployment.StopLocalContour(contourCmd, contourConfigFile))
-	})
-
 	f.NamespacedTest("ingress-tls-wildcard-host", testTLSWildcardHost)
 
 	f.NamespacedTest("backend-tls", func(namespace string) {