testserver: Ensure Gitaly is healthy when using Praefect proxy

When using the Praefect proxy for our tests, then Praefect will use
Gitaly's health information to inform routing decisions. As a result, we
must make sure that Gitaly is in fact healthy before the tests start,
otherwise routing may fail.

Call `waitHealthy()` for the Gitaly connection to ensure that this is
the case. In order to avoid code duplication, `waitHealthy()` is
refactored to perform the dialling itself.

This change requires us to get rid of the manually configured listening
address in `TestCoordinator_grpcErrorHandling()` as it keeps us from
connecting to the Gitaly node's health service. It doesn't seem to serve
any immediate purpose anyway, so it's not much of an issue in the first
place.

2 files changed, 16 insertions, 12 deletions

diff --git a/internal/praefect/coordinator_test.go b/internal/praefect/coordinator_test.go
index 279c967b0..2a0d7cb29 100644
--- a/internal/praefect/coordinator_test.go
+++ b/internal/praefect/coordinator_test.go
@@ -1430,7 +1430,6 @@ func TestCoordinator_grpcErrorHandling(t *testing.T) {
 				gitaly := gitaly
 
 				cfg := testcfg.Build(t, testcfg.WithStorages(gitaly))
-				cfg.ListenAddr = ":0"
 
 				operationServer := &mockOperationServer{
 					t:  t,
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index 93254b103..c93bc4b7f 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -50,6 +50,11 @@ func RunGitalyServer(t testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server
 	}
 
 	praefectAddr, _ := runPraefectProxy(t, cfg, gitalyAddr, praefectBinPath)
+
+	// In case we're running with a Praefect proxy, it will use Gitaly's health information to
+	// inform routing decisions. The Gitaly node thus must be healthy.
+	waitHealthy(t, cfg, gitalyAddr, 3, time.Second)
+
 	return praefectAddr
 }
 
@@ -104,16 +109,7 @@ func runPraefectProxy(t testing.TB, cfg config.Cfg, gitalyAddr, praefectBinPath
 
 	require.NoError(t, cmd.Start())
 
-	grpcOpts := []grpc.DialOption{grpc.WithInsecure()}
-	if cfg.Auth.Token != "" {
-		grpcOpts = append(grpcOpts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(cfg.Auth.Token)))
-	}
-
-	conn, err := grpc.Dial(praefectServerSocketPath, grpcOpts...)
-	require.NoError(t, err)
-	t.Cleanup(func() { conn.Close() })
-
-	waitHealthy(t, conn, 3, time.Second)
+	waitHealthy(t, cfg, praefectServerSocketPath, 3, time.Second)
 
 	t.Cleanup(func() { _ = cmd.Wait() })
 	shutdown := func() { _ = cmd.Process.Kill() }
@@ -163,7 +159,16 @@ func StartGitalyServer(t testing.TB, cfg config.Cfg, rubyServer *rubyserver.Serv
 // waitHealthy executes health check request `retries` times and awaits each `timeout` period to respond.
 // After `retries` unsuccessful attempts it returns an error.
 // Returns immediately without an error once get a successful health check response.
-func waitHealthy(t testing.TB, conn *grpc.ClientConn, retries int, timeout time.Duration) {
+func waitHealthy(t testing.TB, cfg config.Cfg, addr string, retries int, timeout time.Duration) {
+	grpcOpts := []grpc.DialOption{grpc.WithInsecure()}
+	if cfg.Auth.Token != "" {
+		grpcOpts = append(grpcOpts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(cfg.Auth.Token)))
+	}
+
+	conn, err := grpc.Dial(addr, grpcOpts...)
+	require.NoError(t, err)
+	defer conn.Close()
+
 	for i := 0; i < retries; i++ {
 		if IsHealthy(conn, timeout) {
 			return