Merge branch 'jc-faile-on-failed-fetch-internal-remote' into 'master'

ReplicateRepository error when result from FetchInternalRemote is false

Closes #2469

See merge request gitlab-org/gitaly!1879

3 files changed, 108 insertions, 2 deletions

diff --git a/changelogs/unreleased/jc-faile-on-failed-fetch-internal-remote.yml b/changelogs/unreleased/jc-faile-on-failed-fetch-internal-remote.yml
new file mode 100644
index 000000000..bb0646360
--- /dev/null
+++ b/changelogs/unreleased/jc-faile-on-failed-fetch-internal-remote.yml
@@ -0,0 +1,5 @@
+---
+title: ReplicateRepository error when result from FetchInternalRemote is false
+merge_request: 1879
+author:
+type: fixed
diff --git a/internal/service/repository/replicate.go b/internal/service/repository/replicate.go
index 0199dff6f..6fcfbb6b7 100644
--- a/internal/service/repository/replicate.go
+++ b/internal/service/repository/replicate.go
@@ -160,13 +160,18 @@ func (s *server) syncRepository(ctx context.Context, in *gitalypb.ReplicateRepos
 		return err
 	}
 
-	if _, err = remoteClient.FetchInternalRemote(ctx, &gitalypb.FetchInternalRemoteRequest{
+	resp, err := remoteClient.FetchInternalRemote(ctx, &gitalypb.FetchInternalRemoteRequest{
 		Repository:       in.GetRepository(),
 		RemoteRepository: in.GetSource(),
-	}); err != nil {
+	})
+	if err != nil {
 		return err
 	}
 
+	if !resp.Result {
+		return errors.New("FetchInternalRemote failed")
+	}
+
 	return nil
 }
 
diff --git a/internal/service/repository/replicate_test.go b/internal/service/repository/replicate_test.go
index 2824f3f68..7c8524cd8 100644
--- a/internal/service/repository/replicate_test.go
+++ b/internal/service/repository/replicate_test.go
@@ -2,8 +2,10 @@ package repository_test
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io/ioutil"
+	"net"
 	"os"
 	"path"
 	"path/filepath"
@@ -16,8 +18,10 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/service/repository"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
+	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/reflection"
 )
 
 func TestReplicateRepository(t *testing.T) {
@@ -248,3 +252,95 @@ func TestReplicateRepository_BadRepository(t *testing.T) {
 
 	testhelper.MustRunCommand(t, nil, "git", "-C", targetRepoPath, "fsck")
 }
+
+func TestReplicateRepository_FailedFetchInternalRemote(t *testing.T) {
+	tmpPath, cleanup := testhelper.TempDir(t, t.Name())
+	defer cleanup()
+
+	replicaPath := filepath.Join(tmpPath, "replica")
+	require.NoError(t, os.MkdirAll(replicaPath, 0755))
+
+	defer func(storages []config.Storage) {
+		config.Config.Storages = storages
+	}(config.Config.Storages)
+
+	config.Config.Storages = []config.Storage{
+		config.Storage{
+			Name: "default",
+			Path: testhelper.GitlabTestStoragePath(),
+		},
+		config.Storage{
+			Name: "replica",
+			Path: replicaPath,
+		},
+	}
+
+	server, serverSocketPath := runServerWithBadFetchInternalRemote(t)
+	defer server.Stop()
+
+	testRepo, _, cleanupRepo := testhelper.NewTestRepo(t)
+	defer cleanupRepo()
+
+	config.Config.SocketPath = serverSocketPath
+
+	repoClient, conn := repository.NewRepositoryClient(t, serverSocketPath)
+	defer conn.Close()
+
+	targetRepo := *testRepo
+	targetRepo.StorageName = "replica"
+
+	targetRepoPath, err := helper.GetPath(&targetRepo)
+	require.NoError(t, err)
+
+	require.NoError(t, os.MkdirAll(targetRepoPath, 0755))
+	testhelper.MustRunCommand(t, nil, "touch", filepath.Join(targetRepoPath, "invalid_git_repo"))
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
+	injectedCtx := metadata.NewOutgoingContext(ctx, md)
+
+	// first ReplicateRepository call will replicate via snapshot
+	_, err = repoClient.ReplicateRepository(injectedCtx, &gitalypb.ReplicateRepositoryRequest{
+		Repository: &targetRepo,
+		Source:     testRepo,
+	})
+	require.NoError(t, err)
+
+	_, err = repoClient.ReplicateRepository(injectedCtx, &gitalypb.ReplicateRepositoryRequest{
+		Repository: &targetRepo,
+		Source:     testRepo,
+	})
+	require.Error(t, err)
+}
+
+func runServerWithBadFetchInternalRemote(t *testing.T) (*grpc.Server, string) {
+	server := testhelper.NewTestGrpcServer(t, nil, nil)
+	serverSocketPath := testhelper.GetTemporaryGitalySocketFileName()
+
+	listener, err := net.Listen("unix", serverSocketPath)
+	require.NoError(t, err)
+
+	internalListener, err := net.Listen("unix", config.GitalyInternalSocketPath())
+	require.NoError(t, err)
+
+	gitalypb.RegisterRepositoryServiceServer(server, repository.NewServer(repository.RubyServer))
+	gitalypb.RegisterRemoteServiceServer(server, &mockRemoteServer{})
+	reflection.Register(server)
+
+	go server.Serve(listener)
+	go server.Serve(internalListener)
+
+	return server, "unix://" + serverSocketPath
+}
+
+type mockRemoteServer struct {
+	gitalypb.UnimplementedRemoteServiceServer
+}
+
+func (m *mockRemoteServer) FetchInternalRemote(ctx context.Context, req *gitalypb.FetchInternalRemoteRequest) (*gitalypb.FetchInternalRemoteResponse, error) {
+	return &gitalypb.FetchInternalRemoteResponse{
+		Result: false,
+	}, nil
+}