Merge branch 'gitaly-mandatory-fetch-ref' into 'master'

More Gitaly cleanup: fetch_ref, allow disk access blocks

Closes gitaly#1295

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

lib/gitlab/git/conflict/resolver.rb

@@ -17,7 +17,7 @@ module Gitlab
           end
         rescue GRPC::FailedPrecondition => e
           raise Gitlab::Git::Conflict::Resolver::ConflictSideMissing.new(e.message)
-        rescue Rugged::ReferenceError, Rugged::OdbError, GRPC::BadStatus => e
+        rescue GRPC::BadStatus => e
           raise Gitlab::Git::CommandError.new(e)
         end
 

lib/gitlab/git/repository.rb

@@ -558,10 +558,12 @@ module Gitlab
           if is_enabled
             gitaly_operation_client.user_update_branch(branch_name, user, newrev, oldrev)
           else
+            Gitlab::GitalyClient::StorageSettings.allow_disk_access do
               OperationService.new(user, self).update_branch(branch_name, newrev, oldrev)
             end
           end
         end
+      end
 
       def rm_branch(branch_name, user:)
         wrapped_gitaly_errors do
@@ -832,18 +834,9 @@ module Gitlab
         Gitlab::Git.check_namespace!(source_repository)
         source_repository = RemoteRepository.new(source_repository) unless source_repository.is_a?(RemoteRepository)
 
-        message, status = GitalyClient.migrate(:fetch_ref) do |is_enabled|
-          if is_enabled
-            gitaly_fetch_ref(source_repository, source_ref: source_ref, target_ref: target_ref)
-          else
-            # When removing this code, also remove source_repository#path
-            # to remove deprecated method calls
-            local_fetch_ref(source_repository.path, source_ref: source_ref, target_ref: target_ref)
-          end
-        end
-
-        # Make sure ref was created, and raise Rugged::ReferenceError when not
-        raise Rugged::ReferenceError, message if status != 0
+        args = %W(fetch --no-tags -f #{GITALY_INTERNAL_URL} #{source_ref}:#{target_ref})
+        message, status = run_git(args, env: source_repository.fetch_env)
+        raise Gitlab::Git::CommandError, message if status != 0
 
         target_ref
       end
@@ -1244,17 +1237,6 @@ module Gitlab
         gitaly_repository_client.apply_gitattributes(revision)
       end
 
-      def local_fetch_ref(source_path, source_ref:, target_ref:)
-        args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
-        run_git(args)
-      end
-
-      def gitaly_fetch_ref(source_repository, source_ref:, target_ref:)
-        args = %W(fetch --no-tags -f #{GITALY_INTERNAL_URL} #{source_ref}:#{target_ref})
-
-        run_git(args, env: source_repository.fetch_env)
-      end
-
       def gitaly_delete_refs(*ref_names)
         gitaly_ref_client.delete_refs(refs: ref_names) if ref_names.any?
       end

lib/gitlab/git/repository_mirroring.rb

@@ -6,10 +6,12 @@ module Gitlab
           if is_enabled
             gitaly_ref_client.remote_branches(remote_name)
           else
+            Gitlab::GitalyClient::StorageSettings.allow_disk_access do
               rugged_remote_branches(remote_name)
             end
           end
         end
+      end
 
       private
 

lib/gitlab/import_export/merge_request_parser.rb

@@ -27,7 +27,11 @@ module Gitlab
 
       # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1295
       def fetch_ref
-        @project.repository.fetch_ref(@project.repository, source_ref: @diff_head_sha, target_ref: @merge_request.source_branch)
+        target_ref = Gitlab::Git::BRANCH_REF_PREFIX + @merge_request.source_branch
+
+        unless @project.repository.fetch_source_branch!(@project.repository, @diff_head_sha, target_ref)
+          Rails.logger.warn("Import/Export warning: Failed to create #{target_ref} for MR: #{@merge_request.iid}")
+        end
       end
 
       def branch_exists?(branch_name)

spec/lib/gitlab/import_export/project_tree_restorer_spec.rb

@@ -16,7 +16,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         @shared = @project.import_export_shared
         allow(@shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/')
 
-        allow_any_instance_of(Repository).to receive(:fetch_ref).and_return(true)
+        allow_any_instance_of(Repository).to receive(:fetch_source_branch!).and_return(true)
         allow_any_instance_of(Gitlab::Git::Repository).to receive(:branch_exists?).and_return(false)
 
         expect_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch).with('feature', 'DCBA')

spec/models/repository_spec.rb

@@ -1129,16 +1129,12 @@ describe Repository do
       end
 
       it 'raises Rugged::ReferenceError' do
-        raise_reference_error = raise_error(Rugged::ReferenceError) do |err|
-          expect(err.cause).to be_nil
-        end
-
         expect do
           Gitlab::Git::OperationService.new(git_user, target_project.repository.raw_repository)
             .with_branch('feature',
                          start_repository: project.repository.raw_repository,
                          &:itself)
-        end.to raise_reference_error
+        end.to raise_error(Gitlab::Git::CommandError)
       end
     end