Merge branch 'pks-testhelper-gitaly-health-checks' into 'master'

testserver: Fix Praefect resource exhaustion and assert Gitaly's healthiness

See merge request gitlab-org/gitaly!3945

4 files changed, 77 insertions, 172 deletions

diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go
index 81209bf3d..53926aa7e 100644
--- a/cmd/gitaly-ssh/auth_test.go
+++ b/cmd/gitaly-ssh/auth_test.go
@@ -25,11 +25,11 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testserver"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/x509"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/protobuf/encoding/protojson"
 )
 
-//go:generate openssl req -newkey rsa:4096 -new -nodes -x509 -days 3650 -out testdata/certs/gitalycert.pem -keyout testdata/gitalykey.pem -subj "/C=US/ST=California/L=San Francisco/O=GitLab/OU=GitLab-Shell/CN=localhost" -addext "subjectAltName = IP:127.0.0.1, DNS:localhost"
 func TestConnectivity(t *testing.T) {
 	cfg, repo, _ := testcfg.BuildWithRepo(t)
 
@@ -39,8 +39,6 @@ func TestConnectivity(t *testing.T) {
 	cwd, err := os.Getwd()
 	require.NoError(t, err)
 
-	certPoolPath := filepath.Join(cwd, "testdata", "certs")
-
 	tempDir := testhelper.TempDir(t)
 
 	relativeSocketPath, err := filepath.Rel(cwd, filepath.Join(tempDir, "gitaly.socket"))
@@ -56,54 +54,58 @@ func TestConnectivity(t *testing.T) {
 
 	testCases := []struct {
 		name  string
-		addr  func(t *testing.T, cfg config.Cfg) string
+		addr  func(t *testing.T, cfg config.Cfg) (string, string)
 		proxy bool
 	}{
 		{
 			name: "tcp",
-			addr: func(t *testing.T, cfg config.Cfg) string {
+			addr: func(t *testing.T, cfg config.Cfg) (string, string) {
 				cfg.ListenAddr = "localhost:0"
-				return runGitaly(t, cfg)
+				return runGitaly(t, cfg), ""
 			},
 		},
 		{
 			name: "unix absolute",
-			addr: func(t *testing.T, cfg config.Cfg) string {
-				return runGitaly(t, cfg)
+			addr: func(t *testing.T, cfg config.Cfg) (string, string) {
+				return runGitaly(t, cfg), ""
 			},
 		},
 		{
 			name: "unix abs with proxy",
-			addr: func(t *testing.T, cfg config.Cfg) string {
-				return runGitaly(t, cfg)
+			addr: func(t *testing.T, cfg config.Cfg) (string, string) {
+				return runGitaly(t, cfg), ""
 			},
 			proxy: true,
 		},
 		{
 			name: "unix relative",
-			addr: func(t *testing.T, cfg config.Cfg) string {
+			addr: func(t *testing.T, cfg config.Cfg) (string, string) {
 				cfg.SocketPath = fmt.Sprintf("unix:%s", relativeSocketPath)
-				return runGitaly(t, cfg)
+				return runGitaly(t, cfg), ""
 			},
 		},
 		{
 			name: "unix relative with proxy",
-			addr: func(t *testing.T, cfg config.Cfg) string {
+			addr: func(t *testing.T, cfg config.Cfg) (string, string) {
 				cfg.SocketPath = fmt.Sprintf("unix:%s", relativeSocketPath)
-				return runGitaly(t, cfg)
+				return runGitaly(t, cfg), ""
 			},
 			proxy: true,
 		},
 		{
 			name: "tls",
-			addr: func(t *testing.T, cfg config.Cfg) string {
+			addr: func(t *testing.T, cfg config.Cfg) (string, string) {
+				certFile, keyFile := testhelper.GenerateCerts(t)
+
+				revertEnv := testhelper.ModifyEnvironment(t, x509.SSLCertFile, certFile)
+				t.Cleanup(revertEnv)
+
 				cfg.TLSListenAddr = "localhost:0"
 				cfg.TLS = config.TLS{
-					// regenerate the test cert and key via `go generate`
-					CertPath: "testdata/certs/gitalycert.pem",
-					KeyPath:  "testdata/gitalykey.pem",
+					CertPath: certFile,
+					KeyPath:  keyFile,
 				}
-				return runGitaly(t, cfg)
+				return runGitaly(t, cfg), certFile
 			},
 		},
 	}
@@ -115,7 +117,7 @@ func TestConnectivity(t *testing.T) {
 	require.NoError(t, err)
 	for _, testcase := range testCases {
 		t.Run(testcase.name, func(t *testing.T) {
-			addr := testcase.addr(t, cfg)
+			addr, certFile := testcase.addr(t, cfg)
 
 			cmd := exec.Command(cfg.Git.BinPath, "ls-remote", "git@localhost:test/test.git", "refs/heads/master")
 			cmd.Stderr = os.Stderr
@@ -125,7 +127,7 @@ func TestConnectivity(t *testing.T) {
 				fmt.Sprintf("GITALY_WD=%s", cwd),
 				fmt.Sprintf("PATH=.:%s", os.Getenv("PATH")),
 				fmt.Sprintf("GIT_SSH_COMMAND=%s upload-pack", filepath.Join(cfg.BinDir, "gitaly-ssh")),
-				fmt.Sprintf("SSL_CERT_DIR=%s", certPoolPath),
+				fmt.Sprintf("SSL_CERT_FILE=%s", certFile),
 			}
 
 			if testcase.proxy {
diff --git a/cmd/gitaly-ssh/testdata/certs/gitalycert.pem b/cmd/gitaly-ssh/testdata/certs/gitalycert.pem
deleted file mode 100755
index edda651a7..000000000
--- a/cmd/gitaly-ssh/testdata/certs/gitalycert.pem
+++ /dev/null
@@ -1,34 +0,0 @@
------BEGIN CERTIFICATE-----
-MIIF6TCCA9GgAwIBAgIUKo53wOmaepsmtwtSQ5SBvsMPh7AwDQYJKoZIhvcNAQEL
-BQAwdjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM
-DVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkdpdExhYjEVMBMGA1UECwwMR2l0TGFi
-LVNoZWxsMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMjAxMjAyMjMxMzQzWhcNMzAx
-MTMwMjMxMzQzWjB2MQswCQYDVQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEW
-MBQGA1UEBwwNU2FuIEZyYW5jaXNjbzEPMA0GA1UECgwGR2l0TGFiMRUwEwYDVQQL
-DAxHaXRMYWItU2hlbGwxEjAQBgNVBAMMCWxvY2FsaG9zdDCCAiIwDQYJKoZIhvcN
-AQEBBQADggIPADCCAgoCggIBAOUTmd3OYpVKeP3ENOLL+Tm3fxdz2YK2k/eqmJzL
-LSOXyLr3wNTD6FRn8tPgy7jv4jfBb7yzKTkU8T1uqaaaZHHZ2TiJjSkZ5vybuxNQ
-pTn7Y6rXK1X6ZKLBoNq6tx2BxGJUUEemyq1JN4yCOf/hDiqdETFfvZmTnUAUD+N3
-QzvfNp5ycDRGvjnDXHtRQBVA4VIJ9wDZYbtd7YM6q3HND8i2LNGtTiHjsAGFE/EJ
-1sjpUuLihy5Q8eXny0sQwJyjoOz5KYOId9pmq1OqRPZ9tXk3GITPdAqVSUZb397J
-VZlBLEeKy/RRO+XRjogdzgzCJqpKeK4QGNXjoRw9gYNZYoCkvNskYqn4uqf7Lr79
-4olYKSlqTaUq3T6/OZ/AMUAaedB7h0u+v5PO7XYKit8iHdmvMIxzHMuSaQeaquLX
-UoWCYU5md2Hn5SsSx+ssGPnpDpxMYdAxvNQzxjG1XN0xaVSgsIw3q/BgFg4Fs7RF
-MYv49F+JTXBjJZixnW/CjIWJS1+pHkkHf0kmXyXPNtwNOecsEF+hqVjZaqVkocjl
-v+ZDqVBScdKtSqlRtSH/7ouHZnMMBYS41AuBIwlxncj5xFB0gRJIyzxFKikSebWC
-S6n2gE49I/ZnXBf4lwb/rwEKl74Q516v0OG7fOUHcM5E9CK70b77GY1k8UpPCmhl
-KUvTAgMBAAGjbzBtMB0GA1UdDgQWBBS4m6vDJZwMQLD3wB2NwPCGXgP4jzAfBgNV
-HSMEGDAWgBS4m6vDJZwMQLD3wB2NwPCGXgP4jzAPBgNVHRMBAf8EBTADAQH/MBoG
-A1UdEQQTMBGHBH8AAAGCCWxvY2FsaG9zdDANBgkqhkiG9w0BAQsFAAOCAgEAAUs0
-CaLxTc2l8dg+RLm5lzToXJns+mRyGoV0fkEJ/KdnqsxbAvBFqOMdOmA8nmdVLOnf
-OlItx93gLovsrZrpFHJ/gUQOp3FirekAebLvdrw33ofXGoPHftutb8Dn/JnI54AN
-iG17HEkjyGf7Gp3uDIVnUFmVmrnI04BKMyJWMNyKd+oUSgx5bN1coTMSD+Ozilqm
-j/pmZHfl0OLedHgULyTJ7CEFToXnoKBDCF8tc2vDQdWcJKQfEDkvJYPpelgCF7oG
-bVKqMv26Q5zmJRXI708t7effFomjX3F6SQUsTq2D1B5LrFmZPUzdgLnkDrxgjwms
-swsLYAVkO0gY2nRtCMlYOJ+QMpOYE15CualyIrlfujOxAuk7jOaREubX/TUneDt6
-hbF3UnLuSe5KbVmGV7XGXj1LRIbtgKI1jg+/KXjyMR1GVOvnqnXflaQ/3vO3JyLY
-RhgmhPuuQ7RCsbzjSWwy2jStEyXvMkPJCFM1IMYZE2MzcwLwiGqjjPy73GL4hzt1
-gTgP0JyT1iLCZRmdVtYc9F8jrhzcVjc8CYjwxCPfftFD80vnaSATSWVNEYU08QOu
-rATSU2EwdpdZI9WskoDUlcwCcinRx3BZK1gOXyty4ovf6PdekIq1v0A4G6CQ3ixa
-Q3DXa7PBFZL8d9OCqxROMIhCOB+Sbmjipc743a4=
------END CERTIFICATE-----
diff --git a/cmd/gitaly-ssh/testdata/gitalykey.pem b/cmd/gitaly-ssh/testdata/gitalykey.pem
deleted file mode 100644
index b38c0308f..000000000
--- a/cmd/gitaly-ssh/testdata/gitalykey.pem
+++ /dev/null
@@ -1,52 +0,0 @@
------BEGIN PRIVATE KEY-----
-MIIJQwIBADANBgkqhkiG9w0BAQEFAASCCS0wggkpAgEAAoICAQDlE5ndzmKVSnj9
-xDTiy/k5t38Xc9mCtpP3qpicyy0jl8i698DUw+hUZ/LT4Mu47+I3wW+8syk5FPE9
-bqmmmmRx2dk4iY0pGeb8m7sTUKU5+2Oq1ytV+mSiwaDaurcdgcRiVFBHpsqtSTeM
-gjn/4Q4qnRExX72Zk51AFA/jd0M73zaecnA0Rr45w1x7UUAVQOFSCfcA2WG7Xe2D
-OqtxzQ/ItizRrU4h47ABhRPxCdbI6VLi4ocuUPHl58tLEMCco6Ds+SmDiHfaZqtT
-qkT2fbV5NxiEz3QKlUlGW9/eyVWZQSxHisv0UTvl0Y6IHc4MwiaqSniuEBjV46Ec
-PYGDWWKApLzbJGKp+Lqn+y6+/eKJWCkpak2lKt0+vzmfwDFAGnnQe4dLvr+Tzu12
-CorfIh3ZrzCMcxzLkmkHmqri11KFgmFOZndh5+UrEsfrLBj56Q6cTGHQMbzUM8Yx
-tVzdMWlUoLCMN6vwYBYOBbO0RTGL+PRfiU1wYyWYsZ1vwoyFiUtfqR5JB39JJl8l
-zzbcDTnnLBBfoalY2WqlZKHI5b/mQ6lQUnHSrUqpUbUh/+6Lh2ZzDAWEuNQLgSMJ
-cZ3I+cRQdIESSMs8RSopEnm1gkup9oBOPSP2Z1wX+JcG/68BCpe+EOder9Dhu3zl
-B3DORPQiu9G++xmNZPFKTwpoZSlL0wIDAQABAoICAQCs+rAlncMzmJjkh1SoLf50
-ZuvBeem1mskBLAEx5Pqg9ezNwmgKQiUeF6B1/jnX3Myl7ZvCkkd+oyHHkdjpxYwz
-rJ0q5DOD5dtWxQHvA+bAceRBDVncAsgbXrtob7X2whbZaXF1qa1CTRd1MqE5Z2ib
-JR/meLVjUuRbjsqwqU5L17IW07OGxiS+2ZbsR95sLiH3IS8zpPL32WWQwqzTslVI
-6sfNTWDClnxBqYv1Z8iYzheY3BXYE6eWRAJvHKdVoPnT+BIxEMvwT7bv5pNMzoBK
-mCAbqcumcjRyvM3AHahVobEeFOEODIl+cU9/2YHOgI1R6gpW5jDa0pjGCmvSE50c
-uV+oaWY2dICd61JL3EEvTzCI+8l10r8cLI4jIdc+cLDhM21Yp59aVxNOzyIcz/SI
-CLwppTEvx6yYfx6aGpBQj7Gvy/nmalxmbPZvsl5ViilwDt8i87bo0QfU1lIRz6Zh
-LbgsR7vJEdquYA0F5LpGCBU0Oe14Z/J2K4vi7IJwibt9hMoTu79R7Wvlwiiqgi07
-1s1X63XXRQ9/YHZNahQAPycrEakyjMaH/n1CZZ75r+efL8nFBA5CYOweExUkcdXJ
-90n78llnfsJXcPwFY51GeR4jsREUtenER2ubp6QhUsaBccLu+EY5OP9Id8wscVBN
-asr+9cofYerBWjUQlD1HAQKCAQEA+e6EV0eTW7RYtI+WlM40WxTZiOIhX+lUKifg
-MtCrFnfJ5LqeGXf/ZLCmykHSl/cOMOIuq3hY92eNE11nZqSK4FMwPkyMUQKuWlq9
-DvAdACqCo9UTy+BiRLD4ZEBO5HVFtO7wqsop4qctLBH6K5SlGsrIUxekkpPjf7+I
-CIWzQhS6naP1Rtf9whxZm9tYx8TyN/YHwC1cdnrtl65UIPTyRrHxo09ZXlMkq8YA
-SUbGsXPNvx0x6OS8I/K1UASyPuatCuUxuC0depgj9Vmx/IVm7pdUvV4PuGoSjFJc
-oYgZv6mGCufKNqwPDMDVibMfEriZbPNuFNsELaCgKE3M4jLZKwKCAQEA6qN0zIiX
-Lb+dGS5kLRaW889oO7Tr2eiAAAjPm/k0OCes2MXUMIk0SeM5gxLwouRfvoMTsiZI
-wxWB6sClmqvvYGirChKSMc+cs/koRk4mcjUo2Dpfb2lAJkkWusUcfICX4Ud5SPiI
-QtB1xrOoIWRmEcBUVw9hQ87h+3pX/7BtZ4NqXHfMz7OOXhCKk3Vvrp1GYDuKKG18
-yQlaWzLzJx0BTCLve0zUPTUnwcyyIMGYj5ayF0+Cf2KSaPMV+hnrPasWzjvZoaPc
-8uj6AXZ5DenMuRdsd6Mb3eHpW8xOOw9Es5dKhUWuJ+wJZVvLVjyguZMHOQ6aD9RR
-iLS2tNC/OBGz+QKCAQEAqQTEOsIMxVeRJYp1KuzIO1A+THOgXlxMrrWIyNSU3vzj
-a+tIP7EG5fEQ2nbpFBvfXO+Y4BzjDWzZ2t6vrppX0JxxY7AgtCg8AJ6cfB2lfxdA
-gcr3OEjmK84CLMnQkG8+VL3Bw74z1TN5OPRvWkmL3jKEhqzWfsnEb//23syMDYQ3
-L6RRmzc2RK8nal9MCs0FgRLA6xXCEd3QD2O1QRRlQ6nS374pkcBobEHar7NNa3QF
-D9mOhxEoqqbYNuT2JdOZ8mRM22CnLuD0cFfYJg2RLojopaeWugAByyNnwVVpFFRw
-ca+O8KorA1TlE9ouiVqNZ0C4kSIMEU8vABKdG9uHQQKCAQB547HgMrC+TkUQ+L2H
-gq2mOD+AjMrcp2AfJHSdJQcQiOPMtHA3Sn9ERurwgV1KcXKfaD5KFINF/J1B0xP0
-Vo8CuTzo/Z6+i4hvIebfJufn+l+efCxplvaBLQTGv3C96Jt9pwCFfL5kXyBfuaKE
-vkAbxprJoy625/kTB4pBRGiBFo6hPIbD+xXhHZyvX5lFz2Pox+VMfRwO33H14HDa
-B8JNx/Q6TD4REdT2YxzB0XAsX29ilqZHAHDoXiczhlVi8sblGchDpPk5oveE3QOj
-y6nTGSz6tVvblukADBEzswnQda57ryf6iy6SXe4yfUpNPOjKpiA4SuaUKrSjF7HO
-MbwZAoIBAFVlNl3KXXHesDR0ldO9Oxi5mKljsxxZZTYEpCBQOu3WMrErKdaPY8zP
-RyyRFblcNPhFyHz8jbVWOQXH/zRyfT7E4GWt0zpSYUMc+MC9qaJTcgYZluW8fI/x
-diOjTYPs5oU0X3lBOQKWIGQv+G+QCZ9t2xWTfTVaNgBH6FvhAMhT8Jk5++u0er0L
-b/wVSHelw+GhAbOJ2AeJjywHa+6YBvNK+q/Nwn/KHOAfDGt3oGkNO0Cl9v4uvhpb
-CUiq3c/jsb1FsbawBvuVRLxytoCs67kHitLh+7+85Q6+ejnG1FVfei8BE4yQ+fmw
-cYxCTSbHEtUru9v5t6sVsllFqDMQFvQ=
------END PRIVATE KEY-----
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index cf7bf7b04..6ac90f335 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -39,26 +39,47 @@ import (
 	healthpb "google.golang.org/grpc/health/grpc_health_v1"
 )
 
+// praefectSpawnTokens limits the number of concurrent Praefect instances we spawn. With parallel
+// tests, it can happen that we otherwise would spawn so many Praefect executables, with two
+// consequences: first, they eat up all the hosts' memory. Second, they start to saturate Postgres
+// such that new connections start to fail becaue of too many clients. The limit of concurrent
+// instances is not scientifically chosen, but is picked such that tests do not fail on my machine
+// anymore.
+//
+// Note that this only limits concurrency for a single package. If you test multiple packages at
+// once, then these would also run concurrently, leading to `16 * len(packages)` concurrent Praefect
+// instances. To limit this, you can run `go test -p $n` to test at most `$n` concurrent packages.
+var praefectSpawnTokens = make(chan struct{}, 16)
+
 // RunGitalyServer starts gitaly server based on the provided cfg and returns a connection address.
 // It accepts addition Registrar to register all required service instead of
 // calling service.RegisterAll explicitly because it creates a circular dependency
 // when the function is used in on of internal/gitaly/service/... packages.
 func RunGitalyServer(t testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, registrar func(srv *grpc.Server, deps *service.Dependencies), opts ...GitalyServerOpt) string {
-	_, gitalyAddr, disablePraefect := runGitaly(t, cfg, rubyServer, registrar, opts...)
+	return StartGitalyServer(t, cfg, rubyServer, registrar, opts...).Address()
+}
+
+// StartGitalyServer creates and runs gitaly (and praefect as a proxy) server.
+func StartGitalyServer(t testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, registrar func(srv *grpc.Server, deps *service.Dependencies), opts ...GitalyServerOpt) GitalyServer {
+	gitalySrv, gitalyAddr, disablePraefect := runGitaly(t, cfg, rubyServer, registrar, opts...)
 
 	if !isPraefectEnabled() || disablePraefect {
-		return gitalyAddr
+		return GitalyServer{
+			shutdown: gitalySrv.Stop,
+			address:  gitalyAddr,
+		}
 	}
 
 	testhelper.BuildPraefect(t, cfg)
 
-	praefectAddr, _ := runPraefectProxy(t, cfg, gitalyAddr, filepath.Join(cfg.BinDir, "praefect"))
-
-	// 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
+	praefectAddr, shutdownPraefect := runPraefectProxy(t, cfg, gitalyAddr, filepath.Join(cfg.BinDir, "praefect"))
+	return GitalyServer{
+		shutdown: func() {
+			shutdownPraefect()
+			gitalySrv.Stop()
+		},
+		address: praefectAddr,
+	}
 }
 
 // createDatabase create a new database with randomly generated name and returns it back to the caller.
@@ -68,6 +89,11 @@ func createDatabase(t testing.TB) string {
 }
 
 func runPraefectProxy(t testing.TB, cfg config.Cfg, gitalyAddr, praefectBinPath string) (string, func()) {
+	praefectSpawnTokens <- struct{}{}
+	t.Cleanup(func() {
+		<-praefectSpawnTokens
+	})
+
 	tempDir := testhelper.TempDir(t)
 
 	praefectServerSocketPath := "unix://" + testhelper.GetTemporaryGitalySocketFileName(t)
@@ -117,12 +143,14 @@ func runPraefectProxy(t testing.TB, cfg config.Cfg, gitalyAddr, praefectBinPath
 	cmd.Stdout = os.Stdout
 
 	require.NoError(t, cmd.Start())
+	shutdown := func() { _ = cmd.Process.Kill() }
+	t.Cleanup(func() {
+		shutdown()
+		_ = cmd.Wait()
+	})
 
-	waitHealthy(t, cfg, praefectServerSocketPath, 3, time.Second)
+	waitHealthy(t, cfg, praefectServerSocketPath)
 
-	t.Cleanup(func() { _ = cmd.Wait() })
-	shutdown := func() { _ = cmd.Process.Kill() }
-	t.Cleanup(shutdown)
 	return praefectServerSocketPath, shutdown
 }
 
@@ -143,70 +171,29 @@ func (gs GitalyServer) Address() string {
 	return gs.address
 }
 
-// StartGitalyServer creates and runs gitaly (and praefect as a proxy) server.
-func StartGitalyServer(t testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, registrar func(srv *grpc.Server, deps *service.Dependencies), opts ...GitalyServerOpt) GitalyServer {
-	gitalySrv, gitalyAddr, disablePraefect := runGitaly(t, cfg, rubyServer, registrar, opts...)
-
-	if !isPraefectEnabled() || disablePraefect {
-		return GitalyServer{
-			shutdown: gitalySrv.Stop,
-			address:  gitalyAddr,
-		}
-	}
-
-	testhelper.BuildPraefect(t, cfg)
-
-	praefectAddr, shutdownPraefect := runPraefectProxy(t, cfg, gitalyAddr, filepath.Join(cfg.BinDir, "praefect"))
-	return GitalyServer{
-		shutdown: func() {
-			shutdownPraefect()
-			gitalySrv.Stop()
-		},
-		address: praefectAddr,
-	}
-}
-
-// 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, cfg config.Cfg, addr string, retries int, timeout time.Duration) {
-	grpcOpts := []grpc.DialOption{grpc.WithInsecure()}
+// waitHealthy waits until the server hosted at address becomes healthy. Times out after a fixed
+// amount of time.
+func waitHealthy(t testing.TB, cfg config.Cfg, addr string) {
+	var grpcOpts []grpc.DialOption
 	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
-		}
-	}
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
 
-	require.FailNow(t, "server not yet ready to serve")
-}
+	conn, err := client.DialContext(ctx, addr, grpcOpts)
+	require.NoError(t, err)
+	defer testhelper.MustClose(t, conn)
 
-// IsHealthy creates a health client to passed in connection and send `Check` request.
-// It waits for `timeout` duration to get response back.
-// It returns `true` only if remote responds with `SERVING` status.
-func IsHealthy(conn *grpc.ClientConn, timeout time.Duration) bool {
 	healthClient := healthpb.NewHealthClient(conn)
 
-	ctx, cancel := context.WithTimeout(context.Background(), timeout)
-	defer cancel()
-
 	resp, err := healthClient.Check(ctx, &healthpb.HealthCheckRequest{}, grpc.WaitForReady(true))
-	if err != nil {
-		return false
-	}
+	require.NoError(t, err)
 
 	if resp.Status != healthpb.HealthCheckResponse_SERVING {
-		return false
+		require.FailNow(t, "server not yet ready to serve")
 	}
-
-	return true
 }
 
 func runGitaly(t testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, registrar func(srv *grpc.Server, deps *service.Dependencies), opts ...GitalyServerOpt) (*grpc.Server, string, bool) {
@@ -278,6 +265,8 @@ func runGitaly(t testing.TB, cfg config.Cfg, rubyServer *rubyserver.Server, regi
 
 	go srv.Serve(listener)
 
+	waitHealthy(t, cfg, addr)
+
 	return srv, addr, gsd.disablePraefect
 }