Merge branch 'smh-fix-praefect-start' into 'master'

Fix Praefect not starting with unhealthy Gitalys

Closes #2847

See merge request gitlab-org/gitaly!2422

3 files changed, 23 insertions, 22 deletions

diff --git a/changelogs/unreleased/smh-fix-praefect-start.yml b/changelogs/unreleased/smh-fix-praefect-start.yml
new file mode 100644
index 000000000..9353c9327
--- /dev/null
+++ b/changelogs/unreleased/smh-fix-praefect-start.yml
@@ -0,0 +1,5 @@
+---
+title: Fix Praefect not starting with unhealthy Gitalys
+merge_request: 2422
+author:
+type: fixed
diff --git a/internal/praefect/nodes/manager.go b/internal/praefect/nodes/manager.go
index 96c5a61ec..1cf93b18f 100644
--- a/internal/praefect/nodes/manager.go
+++ b/internal/praefect/nodes/manager.go
@@ -141,7 +141,6 @@ func NewManager(log *logrus.Entry, c config.Config, db *sql.DB, queue datastore.
 			conn, err := client.DialContext(ctx, node.Address,
 				append(
 					[]grpc.DialOption{
-						grpc.WithBlock(),
 						grpc.WithDefaultCallOptions(grpc.ForceCodec(proxy.NewCodec())),
 						grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(node.Token)),
 						grpc.WithStreamInterceptor(grpc_middleware.ChainStreamClient(
diff --git a/internal/praefect/nodes/manager_test.go b/internal/praefect/nodes/manager_test.go
index d958dc06d..c268ad878 100644
--- a/internal/praefect/nodes/manager_test.go
+++ b/internal/praefect/nodes/manager_test.go
@@ -141,39 +141,34 @@ func TestManagerFailoverDisabledElectionStrategySQL(t *testing.T) {
 	require.Equal(t, primaryStorage, shard.Primary.GetStorage())
 }
 
-func TestBlockingDial(t *testing.T) {
-	storageName := "default"
-	praefectSocket := testhelper.GetTemporaryGitalySocketFileName()
-	socketName := "unix://" + praefectSocket
-
-	gitalySocket := testhelper.GetTemporaryGitalySocketFileName()
-
-	lis, err := net.Listen("unix", gitalySocket)
+func TestDialWithUnhealthyNode(t *testing.T) {
+	primaryLn, err := net.Listen("unix", testhelper.GetTemporaryGitalySocketFileName())
 	require.NoError(t, err)
 
+	primaryAddress := "unix://" + primaryLn.Addr().String()
+	const secondaryAddress = "unix://does-not-exist"
+	const storageName = "default"
+
 	conf := config.Config{
-		SocketPath: socketName,
 		VirtualStorages: []*config.VirtualStorage{
 			{
 				Name: storageName,
 				Nodes: []*config.Node{
 					{
-						Storage: "internal-storage",
-						Address: "unix://" + gitalySocket,
+						Storage: "starts",
+						Address: primaryAddress,
+					},
+					{
+						Storage: "never-starts",
+						Address: secondaryAddress,
 					},
 				},
 			},
 		},
-		Failover: config.Failover{Enabled: true},
 	}
 
-	// simulate gitaly node starting up later
-	go func() {
-		time.Sleep(healthcheckTimeout + 10*time.Millisecond)
-
-		_, healthSrv0 := testhelper.NewHealthServerWithListener(t, lis)
-		healthSrv0.SetServingStatus("", grpc_health_v1.HealthCheckResponse_SERVING)
-	}()
+	srv, _ := testhelper.NewHealthServerWithListener(t, primaryLn)
+	defer srv.Stop()
 
 	mgr, err := NewManager(testhelper.DiscardTestEntry(t), conf, nil, nil, promtest.NewMockHistogramVec())
 	require.NoError(t, err)
@@ -182,8 +177,10 @@ func TestBlockingDial(t *testing.T) {
 
 	shard, err := mgr.GetShard(storageName)
 	require.NoError(t, err)
-	require.Equal(t, "internal-storage", shard.Primary.GetStorage())
-	require.Empty(t, shard.Secondaries)
+	assertShard(t, shardAssertion{
+		Primary:     &nodeAssertion{Storage: "starts", Address: primaryAddress},
+		Secondaries: []nodeAssertion{{Storage: "never-starts", Address: secondaryAddress}},
+	}, shard)
 }
 
 func TestNodeManager(t *testing.T) {