grpc-proxy: Synchronize shutdown of the gRPC servers

We're regularly observing a flaky test in the grpc-proxy test suite that
is quite hard to decipher because the failure happens in a Goroutine
that fails when the subtest is done already. The resulting panic looks
like the following when executed locally, but in CI it is completely
lacking any further information:

    === FAIL: internal/praefect/grpc-proxy/proxy TestProxyErrorPropagation/director_error_is_propagated (unknown)
    panic: Log in goroutine after TestProxyErrorPropagation/director_error_is_propagated has completed:
            Error Trace:	/home/pks/Development/gitlab/gdk/gitaly/internal/praefect/grpc-proxy/proxy/testhelper.go:123
                                                    /home/pks/Development/gitlab/gdk/gitaly/internal/praefect/grpc-proxy/proxy/asm_amd64.s:1594
            Error:      	Received unexpected error:
                            grpc: the server has been stopped
            Test:       	TestProxyErrorPropagation/director_error_is_propagated

    goroutine 160225 [running]:
    testing.(*common).logDepth(0xc000d229c0, {0xc0003e8600, 0x177}, 0x3)
            /usr/lib/go/src/testing/testing.go:894 +0x4c7
    testing.(*common).log(...)
            /usr/lib/go/src/testing/testing.go:876
    testing.(*common).Errorf(0xc000d229c0, {0xca598d?, 0xc000d229c0?}, {0xc0006e4030?, 0x47a765?, 0x13199a0?})
            /usr/lib/go/src/testing/testing.go:940 +0x5a
    github.com/stretchr/testify/assert.Fail({0x7ea60ee3caf8, 0xc000d229c0}, {0xc0004d2000, 0x3c}, {0x0, 0x0, 0x0})
            /home/pks/Development/gitlab/.go/pkg/mod/github.com/stretchr/testify@v1.8.1/assert/assertions.go:264 +0x384
    github.com/stretchr/testify/assert.NoError({0x7ea60ee3caf8, 0xc000d229c0}, {0xdcc1a0, 0xc0001196f0}, {0x0, 0x0, 0x0})
            /home/pks/Development/gitlab/.go/pkg/mod/github.com/stretchr/testify@v1.8.1/assert/assertions.go:1392 +0x105
    github.com/stretchr/testify/require.NoError({0xdd4278, 0xc000d229c0}, {0xdcc1a0, 0xc0001196f0}, {0x0, 0x0, 0x0})
            /home/pks/Development/gitlab/.go/pkg/mod/github.com/stretchr/testify@v1.8.1/require/require.go:1261 +0x96
    gitlab.com/gitlab-org/gitaly/v15/internal/testhelper.MustServe({0xddf1d8, 0xc000d229c0}, {0xdd0160, 0xc0005f6000}, {0xdd5d30, 0xc00067d230})
            /home/pks/Development/gitlab/gdk/gitaly/internal/testhelper/testhelper.go:123 +0xb6
    created by gitlab.com/gitlab-org/gitaly/v15/internal/praefect/grpc-proxy/proxy_test.TestProxyErrorPropagation.func4
            /home/pks/Development/gitlab/gdk/gitaly/internal/praefect/grpc-proxy/proxy/handler_ext_test.go:347 +0x2c5

Synchronize shutdown of the gRPC servers so that the failure happens
while the testcase is still executing. This gives us the following, much
more reasonable test failure:

    === FAIL: internal/praefect/grpc-proxy/proxy TestProxyErrorPropagation/director_error_is_propagated (0.01s)
        handler_ext_test.go:358:
                    Error Trace:	/home/pks/Development/gitlab/gdk/gitaly/internal/praefect/grpc-proxy/proxy/testhelper.go:123
                                                            /home/pks/Development/gitlab/gdk/gitaly/internal/praefect/grpc-proxy/proxy/handler_ext_test.go:358
                                                            /home/pks/Development/gitlab/gdk/gitaly/internal/praefect/grpc-proxy/proxy/asm_amd64.s:1594
                    Error:      	Received unexpected error:
                                    grpc: the server has been stopped
                    Test:       	TestProxyErrorPropagation/director_error_is_propagated
        --- FAIL: TestProxyErrorPropagation/director_error_is_propagated (0.01s)

The failure itself will be fixed in the next commit.

1 files changed, 17 insertions, 2 deletions

diff --git a/internal/praefect/grpc-proxy/proxy/handler_ext_test.go b/internal/praefect/grpc-proxy/proxy/handler_ext_test.go
index fa06ca360..36ab8bc53 100644
--- a/internal/praefect/grpc-proxy/proxy/handler_ext_test.go
+++ b/internal/praefect/grpc-proxy/proxy/handler_ext_test.go
@@ -15,6 +15,7 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"path/filepath"
+	"sync"
 	"testing"
 
 	"github.com/getsentry/sentry-go"
@@ -342,10 +343,20 @@ func TestProxyErrorPropagation(t *testing.T) {
 			backendListener, err := net.Listen("unix", filepath.Join(tmpDir, "backend"))
 			require.NoError(t, err)
 
+			// We use a wait group to synchronize shutdown of the gRPC servers. This is
+			// done so that any errors returned by the servers are propagated while the
+			// test is still execution.
+			var wg sync.WaitGroup
+			defer wg.Wait()
+
 			backendServer := grpc.NewServer(grpc.UnknownServiceHandler(func(interface{}, grpc.ServerStream) error {
 				return tc.backendError
 			}))
-			go testhelper.MustServe(t, backendServer, backendListener)
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
+				testhelper.MustServe(t, backendServer, backendListener)
+			}()
 			defer backendServer.Stop()
 
 			backendClientConn, err := grpc.DialContext(ctx, "unix://"+backendListener.Addr().String(),
@@ -373,7 +384,11 @@ func TestProxyErrorPropagation(t *testing.T) {
 					), tc.directorError
 				})),
 			)
-			go testhelper.MustServe(t, proxyServer, proxyListener)
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
+				testhelper.MustServe(t, proxyServer, proxyListener)
+			}()
 			defer proxyServer.Stop()
 
 			proxyClientConn, err := grpc.DialContext(ctx, "unix://"+proxyListener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))