Refactor consul-container fortio helpers

Refactor fortio test helpers to separate HTTP retries from waiting on
fortio result changes due to e.g. service startup and failovers.

sdk/testutil/retry/timer.go

@@ -2,6 +2,12 @@ package retry
 
 import "time"
 
+// ThirtySeconds repeats an operation for thirty seconds and waits 500ms in between.
+// Best for known slower operations like waiting on eventually consistent state.
+func ThirtySeconds() *Timer {
+	return &Timer{Timeout: 30 * time.Second, Wait: 500 * time.Millisecond}
+}
+
 // TwoSeconds repeats an operation for two seconds and waits 25ms in between.
 func TwoSeconds() *Timer {
 	return &Timer{Timeout: 2 * time.Second, Wait: 25 * time.Millisecond}

test/integration/consul-container/libs/assert/service.go

@@ -216,7 +216,54 @@ func AssertFortioName(t *testing.T, urlbase string, name string, reqHost string)
 // client must be a custom http.Client
 func AssertFortioNameWithClient(t *testing.T, urlbase string, name string, reqHost string, client *http.Client) {
 	t.Helper()
-	var fortioNameRE = regexp.MustCompile(("\nFORTIO_NAME=(.+)\n"))
+	foundName, err := FortioNameWithClient(t, urlbase, name, reqHost, client)
+	require.NoError(t, err)
+	t.Logf("got response from server name %q expect %q", foundName, name)
+	assert.Equal(t, name, foundName)
+}
+
+// WaitForFortioName is a convenience function for [WaitForFortioNameWithClient], using a [cleanhttp.DefaultClient()]
+func WaitForFortioName(t *testing.T, r retry.Retryer, urlbase string, name string, reqHost string) {
+	t.Helper()
+	client := cleanhttp.DefaultClient()
+	WaitForFortioNameWithClient(t, r, urlbase, name, reqHost, client)
+}
+
+// WaitForFortioNameWithClient enables waiting for FortioNameWithClient to return a specific
+// value. It uses the provided Retryer to wait for the expected name and only fails when
+// retries are exhausted.
+//
+// This is useful when performing failovers in tests and in other eventual consistency
+// scenarios that may take multiple seconds to resolve.
+//
+// Note that the underlying FortioNameWithClient has its own retry for successfully making
+// an HTTP request, which will be counted against the timeout of the provided Retryer if it
+// is a Timer, or incorporated into each attempt if it is a Counter.
+func WaitForFortioNameWithClient(t *testing.T, r retry.Retryer, urlbase string, name string, reqHost string, client *http.Client) {
+	t.Helper()
+	retry.RunWith(r, t, func(r *retry.R) {
+		actual, err := FortioNameWithClient(r, urlbase, name, reqHost, client)
+		require.NoError(r, err)
+		if name != actual {
+			r.Errorf("name %s did not match expected %s", name, actual)
+		}
+	})
+}
+
+// FortioNameWithClient returns the `FORTIO_NAME` returned by the fortio service at
+// urlbase/debug. This can be used to validate that the client is sending traffic to
+// the right envoy proxy.
+//
+// If reqHost is set, the Host field of the HTTP request will be set to its value.
+//
+// It retries with timeout defaultHTTPTimeout and wait defaultHTTPWait.
+//
+// client must be a custom http.Client
+func FortioNameWithClient(t retry.Failer, urlbase string, name string, reqHost string, client *http.Client) (string, error) {
+	t.Helper()
+	var fortioNameRE = regexp.MustCompile("\nFORTIO_NAME=(.+)\n")
+	var body []byte
+
 	retry.RunWith(&retry.Timer{Timeout: defaultHTTPTimeout, Wait: defaultHTTPWait}, t, func(r *retry.R) {
 		fullurl := fmt.Sprintf("%s/debug?env=dump", urlbase)
 		req, err := http.NewRequest("GET", fullurl, nil)
@@ -236,16 +283,17 @@ func AssertFortioNameWithClient(t *testing.T, urlbase string, name string, reqHo
 			r.Fatalf("could not make request to %q: status %d", fullurl, resp.StatusCode)
 		}
 
-		body, err := io.ReadAll(resp.Body)
+		body, err = io.ReadAll(resp.Body)
 		if err != nil {
 			r.Fatalf("failed to read response body from %q: %v", fullurl, err)
 		}
-
-		m := fortioNameRE.FindStringSubmatch(string(body))
-		require.GreaterOrEqual(r, len(m), 2)
-		t.Logf("got response from server name %q expect %q", m[1], name)
-		assert.Equal(r, name, m[1])
 	})
+
+	m := fortioNameRE.FindStringSubmatch(string(body))
+	if len(m) < 2 {
+		return "", fmt.Errorf("fortio name not found %s", name)
+	}
+	return m[1], nil
 }
 
 // AssertContainerState validates service container status

test/integration/consul-container/test/upgrade/peering/peering_http_test.go

@@ -7,10 +7,12 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/sdk/testutil/retry"
 	libassert "github.com/hashicorp/consul/test/integration/consul-container/libs/assert"
 	libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
 	libtopology "github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
@@ -83,13 +85,13 @@ func TestPeering_HTTPRouter(t *testing.T) {
 	}
 	require.NoError(t, acceptingCluster.ConfigEntryWrite(routerConfigEntry))
 	_, appPort := dialing.Container.GetAddr()
-	libassert.AssertFortioName(t, fmt.Sprintf("http://localhost:%d/static-server-2", appPort), "static-server-2", "")
+	libassert.WaitForFortioName(t, retry.ThirtySeconds(), fmt.Sprintf("http://localhost:%d/static-server-2", appPort), "static-server-2", "")
 
 	peeringUpgrade(t, accepting, dialing, utils.TargetVersion)
 
 	peeringPostUpgradeValidation(t, dialing)
 	// TODO: restart static-server-2's sidecar
-	libassert.AssertFortioName(t, fmt.Sprintf("http://localhost:%d/static-server-2", appPort), "static-server-2", "")
+	libassert.WaitForFortioName(t, retry.ThirtySeconds(), fmt.Sprintf("http://localhost:%d/static-server-2", appPort), "static-server-2", "")
 }
 
 // Verify resolver and failover can direct traffic to server in peered cluster
@@ -171,11 +173,12 @@ func TestPeering_HTTPResolverAndFailover(t *testing.T) {
 
 	assertionAdditionalResources := func() {
 		// assert traffic can fail-over to static-server in peered cluster and restor to local static-server
-		libassert.AssertFortioName(t, fmt.Sprintf("http://localhost:%d", appPorts[0]), "static-server-dialing", "")
+		// timeouts in this segment of the test reflect previously implicit retries in fortio name assertions for parity
+		libassert.WaitForFortioName(t, &retry.Timer{Timeout: 2 * time.Minute, Wait: 500 * time.Millisecond}, fmt.Sprintf("http://localhost:%d", appPorts[0]), "static-server-dialing", "")
 		require.NoError(t, serverConnectProxy.Stop())
-		libassert.AssertFortioName(t, fmt.Sprintf("http://localhost:%d", appPorts[0]), libservice.StaticServerServiceName, "")
+		libassert.WaitForFortioName(t, &retry.Timer{Timeout: 2 * time.Minute, Wait: 500 * time.Millisecond}, fmt.Sprintf("http://localhost:%d", appPorts[0]), libservice.StaticServerServiceName, "")
 		require.NoError(t, serverConnectProxy.Start())
-		libassert.AssertFortioName(t, fmt.Sprintf("http://localhost:%d", appPorts[0]), "static-server-dialing", "")
+		libassert.WaitForFortioName(t, &retry.Timer{Timeout: 2 * time.Minute, Wait: 500 * time.Millisecond}, fmt.Sprintf("http://localhost:%d", appPorts[0]), "static-server-dialing", "")
 
 		// assert peer-static-server resolves to static-server in peered cluster
 		libassert.AssertFortioName(t, fmt.Sprintf("http://localhost:%d", appPorts[1]), libservice.StaticServerServiceName, "")
@@ -352,7 +355,7 @@ func peeringUpgrade(t *testing.T, accepting, dialing *libtopology.BuiltCluster,
 func peeringPostUpgradeValidation(t *testing.T, dialing *libtopology.BuiltCluster) {
 	t.Helper()
 
-	clientSidecarService, err := libservice.CreateAndRegisterStaticClientSidecar(dialing.Cluster.Servers()[0], libtopology.DialingPeerName, true, false)
+	clientSidecarService, err := libservice.CreateAndRegisterStaticClientSidecar(dialing.Cluster.Servers()[0], libtopology.DialingPeerName, true, false, nil)
 	require.NoError(t, err)
 	_, port := clientSidecarService.GetAddr()
 	_, adminPort := clientSidecarService.GetAdminAddr()