Merge branch 'migrate-fork-repo-to-gitaly' into 'master'

Migrate repo forking to Gitaly

Closes gitaly#825

See merge request gitlab-org/gitlab-ce!16018

GITALY_SERVER_VERSION

-0.64.0
+0.65.0

Gemfile

@@ -402,7 +402,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.62.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.64.0', require: 'gitaly'
 
 gem 'toml-rb', '~> 0.3.15', require: false
 

Gemfile.lock

@@ -284,7 +284,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     gherkin-ruby (0.3.2)
-    gitaly-proto (0.62.0)
+    gitaly-proto (0.64.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.0)
     github-linguist (4.7.6)
@@ -1046,7 +1046,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.2.0)
-  gitaly-proto (~> 0.62.0)
+  gitaly-proto (~> 0.64.0)
   github-linguist (~> 4.7.0)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-markup (~> 1.6.2)

lib/gitlab/git/gitlab_projects.rb

@@ -2,6 +2,9 @@ module Gitlab
   module Git
     class GitlabProjects
       include Gitlab::Git::Popen
+      include Gitlab::Utils::StrongMemoize
+
+      ShardNameNotFoundError = Class.new(StandardError)
 
       # Absolute path to directory where repositories are stored.
       # Example: /home/git/repositories
@@ -97,22 +100,13 @@ module Gitlab
       end
 
       def fork_repository(new_shard_path, new_repository_relative_path)
-        from_path = repository_absolute_path
-        to_path = File.join(new_shard_path, new_repository_relative_path)
-
-        # The repository cannot already exist
-        if File.exist?(to_path)
-          logger.error "fork-repository failed: destination repository <#{to_path}> already exists."
-          return false
+        Gitlab::GitalyClient.migrate(:fork_repository) do |is_enabled|
+          if is_enabled
+            gitaly_fork_repository(new_shard_path, new_repository_relative_path)
+          else
+            git_fork_repository(new_shard_path, new_repository_relative_path)
+          end
         end
-
-        # Ensure the namepsace / hashed storage directory exists
-        FileUtils.mkdir_p(File.dirname(to_path), mode: 0770)
-
-        logger.info "Forking repository from <#{from_path}> to <#{to_path}>."
-        cmd = %W(git clone --bare --no-local -- #{from_path} #{to_path})
-
-        run(cmd, nil) && Gitlab::Git::Repository.create_hooks(to_path, global_hooks_path)
       end
 
       def fetch_remote(name, timeout, force:, tags:, ssh_key: nil, known_hosts: nil)
@@ -253,6 +247,48 @@ module Gitlab
         known_hosts_file&.close!
         script&.close!
       end
+
+      private
+
+      def shard_name
+        strong_memoize(:shard_name) do
+          shard_name_from_shard_path(shard_path)
+        end
+      end
+
+      def shard_name_from_shard_path(shard_path)
+        Gitlab.config.repositories.storages.find { |_, info| info['path'] == shard_path }&.first ||
+          raise(ShardNameNotFoundError, "no shard found for path '#{shard_path}'")
+      end
+
+      def git_fork_repository(new_shard_path, new_repository_relative_path)
+        from_path = repository_absolute_path
+        to_path = File.join(new_shard_path, new_repository_relative_path)
+
+        # The repository cannot already exist
+        if File.exist?(to_path)
+          logger.error "fork-repository failed: destination repository <#{to_path}> already exists."
+          return false
+        end
+
+        # Ensure the namepsace / hashed storage directory exists
+        FileUtils.mkdir_p(File.dirname(to_path), mode: 0770)
+
+        logger.info "Forking repository from <#{from_path}> to <#{to_path}>."
+        cmd = %W(git clone --bare --no-local -- #{from_path} #{to_path})
+
+        run(cmd, nil) && Gitlab::Git::Repository.create_hooks(to_path, global_hooks_path)
+      end
+
+      def gitaly_fork_repository(new_shard_path, new_repository_relative_path)
+        target_repository = Gitlab::Git::Repository.new(shard_name_from_shard_path(new_shard_path), new_repository_relative_path, nil)
+        raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil)
+
+        Gitlab::GitalyClient::RepositoryService.new(target_repository).fork_repository(raw_repository)
+      rescue GRPC::BadStatus => e
+        logger.error "fork-repository failed: #{e.message}"
+        false
+      end
     end
   end
 end

lib/gitlab/gitaly_client/repository_service.rb

@@ -81,6 +81,22 @@ module Gitlab
         response.base.presence
       end
 
+      def fork_repository(source_repository)
+        request = Gitaly::CreateForkRequest.new(
+          repository: @gitaly_repo,
+          source_repository: source_repository.gitaly_repository
+        )
+
+        GitalyClient.call(
+          @storage,
+          :repository_service,
+          :create_fork,
+          request,
+          remote_storage: source_repository.storage,
+          timeout: GitalyClient.default_timeout
+        )
+      end
+
       def fetch_source_branch(source_repository, source_branch, local_ref)
         request = Gitaly::FetchSourceBranchRequest.new(
           repository: @gitaly_repo,

lib/gitlab/gitaly_client/util.rb

@@ -12,7 +12,7 @@ module Gitlab
           Gitaly::Repository.new(
             storage_name: repository_storage,
             relative_path: relative_path,
-            gl_repository: gl_repository,
+            gl_repository: gl_repository.to_s,
             git_object_directory: git_object_directory.to_s,
             git_alternate_object_directories: git_alternate_object_directories
           )

spec/lib/gitlab/git/gitlab_projects_spec.rb

@@ -243,7 +243,6 @@ describe Gitlab::Git::GitlabProjects do
     let(:dest_repos_path) { tmp_repos_path }
     let(:dest_repo_name) { File.join('@hashed', 'aa', 'bb', 'xyz.git') }
     let(:dest_repo) { File.join(dest_repos_path, dest_repo_name) }
-    let(:dest_namespace) { File.dirname(dest_repo) }
 
     subject { gl_projects.fork_repository(dest_repos_path, dest_repo_name) }
 
@@ -255,37 +254,64 @@ describe Gitlab::Git::GitlabProjects do
       FileUtils.rm_rf(dest_repos_path)
     end
 
-    it 'forks the repository' do
-      message = "Forking repository from <#{tmp_repo_path}> to <#{dest_repo}>."
-      expect(logger).to receive(:info).with(message)
+    shared_examples 'forking a repository' do
+      it 'forks the repository' do
+        is_expected.to be_truthy
 
-      is_expected.to be_truthy
+        expect(File.exist?(dest_repo)).to be_truthy
+        expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy
+        expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy
+      end
+
+      it 'does not fork if a project of the same name already exists' do
+        # create a fake project at the intended destination
+        FileUtils.mkdir_p(dest_repo)
 
-      expect(File.exist?(dest_repo)).to be_truthy
-      expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy
-      expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy
+        is_expected.to be_falsy
+      end
     end
 
-    it 'does not fork if a project of the same name already exists' do
-      # create a fake project at the intended destination
-      FileUtils.mkdir_p(dest_repo)
+    context 'when Gitaly fork_repository feature is enabled' do
+      it_behaves_like 'forking a repository'
+    end
 
-      # trying to fork again should fail as the repo already exists
-      message = "fork-repository failed: destination repository <#{dest_repo}> already exists."
-      expect(logger).to receive(:error).with(message)
+    context 'when Gitaly fork_repository feature is disabled', :disable_gitaly do
+      it_behaves_like 'forking a repository'
 
-      is_expected.to be_falsy
-    end
+      # We seem to be stuck to having only one working Gitaly storage in tests, changing
+      # that is not very straight-forward so I'm leaving this test here for now till
+      # https://gitlab.com/gitlab-org/gitlab-ce/issues/41393 is fixed.
+      context 'different storages' do
+        let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), 'alternative') }
 
-    context 'different storages' do
-      let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), 'alternative') }
+        it 'forks the repo' do
+          is_expected.to be_truthy
 
-      it 'forks the repo' do
-        is_expected.to be_truthy
+          expect(File.exist?(dest_repo)).to be_truthy
+          expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy
+          expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy
+        end
+      end
 
-        expect(File.exist?(dest_repo)).to be_truthy
-        expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy
-        expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy
+      describe 'log messages' do
+        describe 'successful fork' do
+          it do
+            message = "Forking repository from <#{tmp_repo_path}> to <#{dest_repo}>."
+            expect(logger).to receive(:info).with(message)
+
+            subject
+          end
+        end
+
+        describe 'failed fork due existing destination' do
+          it do
+            FileUtils.mkdir_p(dest_repo)
+            message = "fork-repository failed: destination repository <#{dest_repo}> already exists."
+            expect(logger).to receive(:error).with(message)
+
+            subject
+          end
+        end
       end
     end
   end