Merge branch 'zj-fork-mandatory' into 'master'

Move forking to Gitaly

Closes gitaly#817 and gitaly#1236

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

app/workers/repository_fork_worker.rb

@@ -8,28 +8,12 @@ class RepositoryForkWorker
     target_project_id = args.shift
     target_project = Project.find(target_project_id)
 
-    # By v10.8, we should've drained the queue of all jobs using the old arguments.
-    # We can remove the else clause if we're no longer logging the message in that clause.
-    # See https://gitlab.com/gitlab-org/gitaly/issues/1110
-    if args.empty?
-      source_project = target_project.forked_from_project
-      unless source_project
-        return target_project.mark_import_as_failed('Source project cannot be found.')
-      end
-
-      fork_repository(target_project, source_project.repository_storage, source_project.disk_path)
-    else
-      Rails.logger.info("Project #{target_project.id} is being forked using old-style arguments.")
-
-      source_repository_storage_path, source_disk_path = *args
-
-      source_repository_storage_name = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-        Gitlab.config.repositories.storages.find do |_, info|
-          info.legacy_disk_path == source_repository_storage_path
-        end&.first || raise("no shard found for path '#{source_repository_storage_path}'")
-      end
-      fork_repository(target_project, source_repository_storage_name, source_disk_path)
+    source_project = target_project.forked_from_project
+    unless source_project
+      return target_project.mark_import_as_failed('Source project cannot be found.')
     end
+
+    fork_repository(target_project, source_project.repository_storage, source_project.disk_path)
   end
 
   private

lib/gitlab/shell.rb

-# Gitaly note: JV: two sets of straightforward RPC's. 1 Hard RPC: fork_repository.
-# SSH key operations are not part of Gitaly so will never be migrated.
+# Gitaly note: SSH key operations are not part of Gitaly so will never be migrated.
 
 require 'securerandom'
 
@@ -153,8 +152,6 @@ module Gitlab
     #
     # Ex.
     #   mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new")
-    #
-    # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873
     def mv_repository(storage, path, new_path)
       return false if path.empty? || new_path.empty?
 
@@ -169,19 +166,11 @@ module Gitlab
     #
     # Ex.
     #  fork_repository("nfs-file06", "gitlab/gitlab-ci", "nfs-file07", "new-namespace/gitlab-ci")
-    #
-    # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/817
     def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path)
       forked_from_relative_path = "#{forked_from_disk_path}.git"
       fork_args = [forked_to_storage, "#{forked_to_disk_path}.git"]
 
-      gitaly_migrate(:fork_repository, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
-        if is_enabled
-          GitalyGitlabProjects.new(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args)
-        else
-          gitlab_projects(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args)
-        end
-      end
+      GitalyGitlabProjects.new(forked_from_storage, forked_from_relative_path).fork_repository(*fork_args)
     end
 
     # Removes a repository from file system, using rm_diretory which is an alias
@@ -193,8 +182,6 @@ module Gitlab
     #
     # Ex.
     #   remove_repository("/path/to/storage", "gitlab/gitlab-ci")
-    #
-    # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873
     def remove_repository(storage, name)
       return false if name.empty?
 

spec/lib/gitlab/shell_spec.rb

@@ -498,34 +498,18 @@ describe Gitlab::Shell do
         )
       end
 
-      context 'with gitaly' do
-        it 'returns true when the command succeeds' do
-          expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository)
-            .with(repository.raw_repository) { :gitaly_response_object }
-
-          is_expected.to be_truthy
-        end
-
-        it 'return false when the command fails' do
-          expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository)
-            .with(repository.raw_repository) { raise GRPC::BadStatus, 'bla' }
+      it 'returns true when the command succeeds' do
+        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository)
+          .with(repository.raw_repository) { :gitaly_response_object }
 
-          is_expected.to be_falsy
-        end
+        is_expected.to be_truthy
       end
 
-      context 'without gitaly', :disable_gitaly do
-        it 'returns true when the command succeeds' do
-          expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { true }
-
-          is_expected.to be_truthy
-        end
-
-        it 'return false when the command fails' do
-          expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { false }
+      it 'return false when the command fails' do
+        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:fork_repository)
+          .with(repository.raw_repository) { raise GRPC::BadStatus, 'bla' }
 
-          is_expected.to be_falsy
-        end
+        is_expected.to be_falsy
       end
     end
 
@@ -665,7 +649,7 @@ describe Gitlab::Shell do
 
         subject do
           gitlab_shell.fetch_remote(repository.raw_repository, remote_name,
-            forced: true, no_tags: true, ssh_auth: ssh_auth)
+                                    forced: true, no_tags: true, ssh_auth: ssh_auth)
         end
 
         it 'passes the correct params to the gitaly service' do

spec/workers/repository_fork_worker_spec.rb

@@ -92,16 +92,6 @@ describe RepositoryForkWorker do
       end
 
       it_behaves_like 'RepositoryForkWorker performing'
-
-      it 'logs a message about forking with old-style arguments' do
-        allow(subject).to receive(:gitlab_shell).and_return(shell)
-        expect(shell).to receive(:fork_repository) { true }
-
-        allow(Rails.logger).to receive(:info).with(anything) # To compensate for other logs
-        expect(Rails.logger).to receive(:info).with("Project #{fork_project.id} is being forked using old-style arguments.")
-
-        perform!
-      end
     end
   end
 end