Merge branch 'gitaly-internal-mirroring' into 'master'

Use repository mirroring fetches to move projects between storages

Closes gitaly#587

See merge request gitlab-org/gitlab-ee!3152

app/models/concerns/repository_mirroring.rb

-module RepositoryMirroring
-  IMPORT_HEAD_REFS = '+refs/heads/*:refs/heads/*'.freeze
-  IMPORT_TAG_REFS = '+refs/tags/*:refs/tags/*'.freeze
-
-  def storage_path
-    @project.repository_storage_path
-  end
-
-  def push_remote_branches(remote, branches)
-    gitlab_shell.push_remote_branches(storage_path, disk_path, remote, branches)
-  end
-
-  def delete_remote_branches(remote, branches)
-    gitlab_shell.delete_remote_branches(storage_path, disk_path, remote, branches)
-  end
-
-  def set_remote_as_mirror(name)
-    # This is used to define repository as equivalent as "git clone --mirror"
-    raw_repository.rugged.config["remote.#{name}.fetch"] = 'refs/*:refs/*'
-    raw_repository.rugged.config["remote.#{name}.mirror"] = true
-    raw_repository.rugged.config["remote.#{name}.prune"] = true
-  end
-
-  def set_import_remote_as_mirror(remote_name)
-    # Add first fetch with Rugged so it does not create its own.
-    raw_repository.rugged.config["remote.#{remote_name}.fetch"] = IMPORT_HEAD_REFS
-
-    add_remote_fetch_config(remote_name, IMPORT_TAG_REFS)
-
-    raw_repository.rugged.config["remote.#{remote_name}.mirror"] = true
-    raw_repository.rugged.config["remote.#{remote_name}.prune"] = true
-  end
-
-  def add_remote_fetch_config(remote_name, refspec)
-    run_git(%W[config --add remote.#{remote_name}.fetch #{refspec}])
-  end
-
-  def fetch_mirror(remote, url)
-    add_remote(remote, url)
-    set_remote_as_mirror(remote)
-    fetch_remote(remote, forced: true)
-    remove_remote(remote)
-  end
-
-  def remote_tags(remote)
-    gitlab_shell.list_remote_tags(storage_path, disk_path, remote).map do |name, target|
-      target_commit = Gitlab::Git::Commit.find(raw_repository, target)
-      Gitlab::Git::Tag.new(raw_repository, name, target, target_commit)
-    end
-  end
-
-  def remote_branches(remote_name)
-    branches = []
-
-    rugged.references.each("refs/remotes/#{remote_name}/*").map do |ref|
-      name = ref.name.sub(/\Arefs\/remotes\/#{remote_name}\//, '')
-
-      begin
-        target_commit = Gitlab::Git::Commit.find(raw_repository, ref.target)
-        branches << Gitlab::Git::Branch.new(raw_repository, name, ref.target, target_commit)
-      rescue Rugged::ReferenceError
-        # Omit invalid branch
-      end
-    end
-
-    branches
-  end
-end

app/models/project.rb

@@ -1679,6 +1679,10 @@ class Project < ActiveRecord::Base
     Gitlab::GlRepository.gl_repository(self, is_wiki)
   end
 
+  def reference_counter(wiki: false)
+    Gitlab::ReferenceCounter.new(gl_repository(is_wiki: wiki))
+  end
+
   private
 
   def storage
@@ -1697,11 +1701,11 @@ class Project < ActiveRecord::Base
   end
 
   def repo_reference_count
-    Gitlab::ReferenceCounter.new(gl_repository(is_wiki: false)).value
+    reference_counter.value
   end
 
   def wiki_reference_count
-    Gitlab::ReferenceCounter.new(gl_repository(is_wiki: true)).value
+    reference_counter(wiki: true).value
   end
 
   def check_repository_absence!

app/models/project_wiki.rb

@@ -150,7 +150,7 @@ class ProjectWiki
   end
 
   def repository
-    @repository ||= Repository.new(full_path, @project, disk_path: disk_path)
+    @repository ||= Repository.new(full_path, @project, disk_path: disk_path, is_wiki: true)
   end
 
   def default_branch

app/models/repository.rb

@@ -18,10 +18,9 @@ class Repository
 
   include Gitlab::ShellAdapter
   include Elastic::RepositoriesSearch
-  include RepositoryMirroring
   prepend EE::Repository
 
-  attr_accessor :full_path, :disk_path, :project
+  attr_accessor :full_path, :disk_path, :project, :is_wiki
 
   delegate :ref_name_for_sha, to: :raw_repository
 
@@ -79,11 +78,12 @@ class Repository
     end
   end
 
-  def initialize(full_path, project, disk_path: nil)
+  def initialize(full_path, project, disk_path: nil, is_wiki: false)
     @full_path = full_path
     @disk_path = disk_path || full_path
     @project = project
     @commit_cache = {}
+    @is_wiki = is_wiki
   end
 
   def ==(other)
@@ -1020,19 +1020,6 @@ class Repository
     run_git(args).first.lines.map(&:strip)
   end
 
-  def add_remote(name, url)
-    raw_repository.remote_add(name, url)
-  rescue Rugged::ConfigError
-    raw_repository.remote_update(name, url: url)
-  end
-
-  def remove_remote(name)
-    raw_repository.remote_delete(name)
-    true
-  rescue Rugged::ConfigError
-    false
-  end
-
   def fetch_remote(remote, forced: false, ssh_auth: nil, no_tags: false)
     gitlab_shell.fetch_remote(raw_repository, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags)
   end
@@ -1202,7 +1189,7 @@ class Repository
   end
 
   def initialize_raw_repository
-    Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, false))
+    Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, is_wiki))
   end
 
   def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)

app/services/geo/repository_sync_service.rb

@@ -21,7 +21,9 @@ module Geo
         log_info("Finished repository sync",
                  update_delay_s: update_delay_in_seconds,
                  download_time_s: download_time_in_seconds)
-      rescue Gitlab::Shell::Error, Geo::EmptyCloneUrlPrefixError => e
+      rescue Gitlab::Shell::Error,
+             Gitlab::Git::RepositoryMirroring::RemoteError,
+             Geo::EmptyCloneUrlPrefixError => e
         log_error('Error syncing repository', e)
       rescue Gitlab::Git::Repository::NoRepository => e
         log_error('Invalid repository', e)

app/services/geo/wiki_sync_service.rb

@@ -21,6 +21,7 @@ module Geo
                  update_delay_s: update_delay_in_seconds,
                  download_time_s: download_time_in_seconds)
       rescue Gitlab::Git::Repository::NoRepository,
+             Gitlab::Git::RepositoryMirroring::RemoteError,
              Gitlab::Shell::Error,
              ProjectWiki::CouldNotCreateWikiError,
              Geo::EmptyCloneUrlPrefixError => e

app/services/projects/import_service.rb

@@ -44,7 +44,7 @@ module Projects
         else
           clone_repository
         end
-      rescue Gitlab::Shell::Error => e
+      rescue Gitlab::Shell::Error, Gitlab::Git::RepositoryMirroring::RemoteError => e
         # Expire cache to prevent scenarios such as:
         # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true
         # 2. Retried import, repo is broken or not imported but +exists?+ still returns true

app/services/projects/update_mirror_service.rb

@@ -19,7 +19,7 @@ module Projects
       update_branches
 
       success
-    rescue Gitlab::Shell::Error, UpdateError => e
+    rescue Gitlab::Shell::Error, Gitlab::Git::RepositoryMirroring::RemoteError, UpdateError => e
       error(e.message)
     end
 

app/services/projects/update_repository_storage_service.rb

@@ -7,11 +7,10 @@ module Projects
     end
 
     def execute(new_repository_storage_key)
-      new_storage_path = Gitlab.config.repositories.storages[new_repository_storage_key]['path']
-      result = move_storage(project.disk_path, new_storage_path)
+      result = mirror_repository(new_repository_storage_key)
 
       if project.wiki.repository_exists?
-        result &&= move_storage(project.wiki.disk_path, new_storage_path)
+        result &&= mirror_repository(new_repository_storage_key, wiki: true)
       end
 
       if result
@@ -25,8 +24,18 @@ module Projects
 
     private
 
-    def move_storage(project_path, new_storage_path)
-      gitlab_shell.mv_storage(project.repository_storage_path, project_path, new_storage_path)
+    def mirror_repository(new_storage_key, wiki: false)
+      return false unless wait_for_pushes(wiki)
+
+      repository = (wiki ? project.wiki.repository : project.repository).raw
+
+      # Initialize a git repository on the target path
+      gitlab_shell.add_repository(new_storage_key, repository.relative_path)
+      new_repository = Gitlab::Git::Repository.new(new_storage_key,
+                                                   repository.relative_path,
+                                                   repository.gl_repository)
+
+      new_repository.fetch_mirror(repository.path)
     end
 
     def mark_old_paths_for_archive
@@ -53,5 +62,17 @@ module Projects
     def moved_path(path)
       "#{path}+#{project.id}+moved+#{Time.now.to_i}"
     end
+
+    def wait_for_pushes(wiki)
+      reference_counter = project.reference_counter(wiki: wiki)
+
+      # Try for 30 seconds, polling every 10
+      3.times do
+        return true if reference_counter.value == 0
+        sleep 10
+      end
+
+      false
+    end
   end
 end

ee/app/models/concerns/ee/repository_mirroring.rb

+module EE
+  module RepositoryMirroring
+    def storage_path
+      @project.repository_storage_path
+    end
+
+    def push_remote_branches(remote, branches)
+      gitlab_shell.push_remote_branches(storage_path, disk_path, remote, branches)
+    end
+
+    def delete_remote_branches(remote, branches)
+      gitlab_shell.delete_remote_branches(storage_path, disk_path, remote, branches)
+    end
+  end
+end

ee/app/models/ee/repository.rb

@@ -4,6 +4,7 @@ module EE
   # This module is intended to encapsulate EE-specific model logic
   # and be prepended in the `Repository` model
   module Repository
+    prepend RepositoryMirroring
     extend ActiveSupport::Concern
 
     # Transiently sets a configuration variable

lib/github/import.rb

@@ -60,7 +60,9 @@ module Github
         project.repository.set_import_remote_as_mirror('github')
         project.repository.add_remote_fetch_config('github', '+refs/pull/*/head:refs/merge-requests/*/head')
         fetch_remote(forced: true)
-      rescue Gitlab::Git::Repository::NoRepository, Gitlab::Shell::Error => e
+      rescue Gitlab::Git::Repository::NoRepository,
+             Gitlab::Git::RepositoryMirroring::RemoteError,
+             Gitlab::Shell::Error => e
         error(:project, repo_url, e.message)
         raise Github::RepositoryFetchError
       end

lib/gitlab/git/repository.rb

@@ -6,6 +6,7 @@ require "rubygems/package"
 module Gitlab
   module Git
     class Repository
+      include Gitlab::Git::RepositoryMirroring
       include Gitlab::Git::Popen
 
       ALLOWED_OBJECT_DIRECTORIES_VARIABLES = %w[
@@ -898,16 +899,25 @@ module Gitlab
         end
       end
 
-      # Delete the specified remote from this repository.
-      def remote_delete(remote_name)
-        rugged.remotes.delete(remote_name)
-        nil
+      def add_remote(remote_name, url)
+        rugged.remotes.create(remote_name, url)
+      rescue Rugged::ConfigError
+        remote_update(remote_name, url: url)
       end
 
-      # Add a new remote to this repository.
-      def remote_add(remote_name, url)
-        rugged.remotes.create(remote_name, url)
-        nil
+      def remove_remote(remote_name)
+        # When a remote is deleted all its remote refs are deleted too, but in
+        # the case of mirrors we map its refs (that would usualy go under
+        # [remote_name]/) to the top level namespace. We clean the mapping so
+        # those don't get deleted.
+        if rugged.config["remote.#{remote_name}.mirror"]
+          rugged.config.delete("remote.#{remote_name}.fetch")
+        end
+
+        rugged.remotes.delete(remote_name)
+        true
+      rescue Rugged::ConfigError
+        false
       end
 
       # Update the specified remote using the values in the +options+ hash

lib/gitlab/git/repository_mirroring.rb

+module Gitlab
+  module Git
+    module RepositoryMirroring
+      IMPORT_HEAD_REFS = '+refs/heads/*:refs/heads/*'.freeze
+      IMPORT_TAG_REFS = '+refs/tags/*:refs/tags/*'.freeze
+      MIRROR_REMOTE = 'mirror'.freeze
+
+      RemoteError = Class.new(StandardError)
+
+      def set_remote_as_mirror(remote_name)
+        # This is used to define repository as equivalent as "git clone --mirror"
+        rugged.config["remote.#{remote_name}.fetch"] = 'refs/*:refs/*'
+        rugged.config["remote.#{remote_name}.mirror"] = true
+        rugged.config["remote.#{remote_name}.prune"] = true
+      end
+
+      def set_import_remote_as_mirror(remote_name)
+        # Add first fetch with Rugged so it does not create its own.
+        rugged.config["remote.#{remote_name}.fetch"] = IMPORT_HEAD_REFS
+
+        add_remote_fetch_config(remote_name, IMPORT_TAG_REFS)
+
+        rugged.config["remote.#{remote_name}.mirror"] = true
+        rugged.config["remote.#{remote_name}.prune"] = true
+      end
+
+      def add_remote_fetch_config(remote_name, refspec)
+        run_git(%W[config --add remote.#{remote_name}.fetch #{refspec}])
+      end
+
+      def fetch_mirror(url)
+        add_remote(MIRROR_REMOTE, url)
+        set_remote_as_mirror(MIRROR_REMOTE)
+        fetch(MIRROR_REMOTE)
+        remove_remote(MIRROR_REMOTE)
+      end
+
+      def remote_tags(remote)
+        # Each line has this format: "dc872e9fa6963f8f03da6c8f6f264d0845d6b092\trefs/tags/v1.10.0\n"
+        # We want to convert it to: [{ 'v1.10.0' => 'dc872e9fa6963f8f03da6c8f6f264d0845d6b092' }, ...]
+        list_remote_tags(remote).map do |line|
+          target, path = line.strip.split("\t")
+
+          # When the remote repo does not have tags.
+          if target.nil? || path.nil?
+            Rails.logger.info "Empty or invalid list of tags for remote: #{remote}. Output: #{output}"
+            return []
+          end
+
+          name = path.split('/', 3).last
+          # We're only interested in tag references
+          # See: http://stackoverflow.com/questions/15472107/when-listing-git-ls-remote-why-theres-after-the-tag-name
+          next if name =~ /\^\{\}\Z/
+
+          target_commit = Gitlab::Git::Commit.find(self, target)
+          Gitlab::Git::Tag.new(self, name, target, target_commit)
+        end.compact
+      end
+
+      def remote_branches(remote_name)
+        branches = []
+
+        rugged.references.each("refs/remotes/#{remote_name}/*").map do |ref|
+          name = ref.name.sub(/\Arefs\/remotes\/#{remote_name}\//, '')
+
+          begin
+            target_commit = Gitlab::Git::Commit.find(self, ref.target)
+            branches << Gitlab::Git::Branch.new(self, name, ref.target, target_commit)
+          rescue Rugged::ReferenceError
+            # Omit invalid branch
+          end
+        end
+
+        branches
+      end
+
+      private
+
+      def list_remote_tags(remote)
+        tag_list, exit_code, error = nil
+        cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{full_path} ls-remote --tags #{remote})
+
+        Open3.popen3(*cmd) do |stdin, stdout, stderr, wait_thr|
+          tag_list  = stdout.read
+          error     = stderr.read
+          exit_code = wait_thr.value.exitstatus
+        end
+
+        raise RemoteError, error unless exit_code.zero?
+
+        tag_list.split('\n')
+      end
+    end
+  end
+end

lib/gitlab/git/wiki.rb

@@ -10,6 +10,8 @@ module Gitlab
       end
       PageBlob = Struct.new(:name)
 
+      attr_reader :repository
+
       def self.default_ref
         'master'
       end

lib/gitlab/shell.rb

@@ -108,34 +108,6 @@ module Gitlab
       gitlab_shell_fast_execute_raise_error(cmd)
     end
 
-    def list_remote_tags(storage, name, remote)
-      output, status = Popen.popen([gitlab_shell_projects_path, 'list-remote-tags', storage, "#{name}.git", remote])
-      tags_with_targets = []
-
-      raise Error, output unless status.zero?
-
-      # Each line has this format: "dc872e9fa6963f8f03da6c8f6f264d0845d6b092\trefs/tags/v1.10.0\n"
-      # We want to convert it to: [{ 'v1.10.0' => 'dc872e9fa6963f8f03da6c8f6f264d0845d6b092' }, ...]
-      output.lines.each do |line|
-        target, path = line.strip!.split("\t")
-
-        # When the remote repo does not have tags.
-        if target.nil? || path.nil?
-          Rails.logger.info "Empty or invalid list of tags for remote: #{remote}. Output: #{output}"
-          break
-        end
-
-        name = path.split('/', 3).last
-        # We're only interested in tag references
-        # See: http://stackoverflow.com/questions/15472107/when-listing-git-ls-remote-why-theres-after-the-tag-name
-        next if name =~ /\^\{\}\Z/
-
-        tags_with_targets.concat([name, target])
-      end
-
-      Hash[*tags_with_targets]
-    end
-
     # Fetch remote for repository
     #
     # repository - an instance of Git::Repository

spec/lib/gitlab/git/repository_spec.rb

@@ -559,10 +559,10 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe "#remote_delete" do
+  describe "#remove_remote" do
     before(:all) do
       @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
-      @repo.remote_delete("expendable")
+      @repo.remove_remote("expendable")
     end
 
     it "should remove the remote" do
@@ -575,14 +575,16 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe "#remote_add" do
+  describe "#remote_update" do
     before(:all) do
       @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
-      @repo.remote_add("new_remote", SeedHelper::GITLAB_GIT_TEST_REPO_URL)
+      @repo.remote_update("expendable", url: TEST_NORMAL_REPO_PATH)
     end
 
     it "should add the remote" do
-      expect(@repo.rugged.remotes.each_name.to_a).to include("new_remote")
+      expect(@repo.rugged.remotes["expendable"].url).to(
+        eq(TEST_NORMAL_REPO_PATH)
+      )
     end
 
     after(:all) do
@@ -591,21 +593,58 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe "#remote_update" do
-    before(:all) do
-      @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
-      @repo.remote_update("expendable", url: TEST_NORMAL_REPO_PATH)
+  describe '#fetch_mirror' do
+    let(:new_repository) do
+      Gitlab::Git::Repository.new('default', 'my_project.git', '')
     end
 
-    it "should add the remote" do
-      expect(@repo.rugged.remotes["expendable"].url).to(
-        eq(TEST_NORMAL_REPO_PATH)
-      )
+    subject { new_repository.fetch_mirror(repository.path) }
+
+    before do
+      Gitlab::Shell.new.add_repository('default', 'my_project')
     end
 
-    after(:all) do
-      FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH)
-      ensure_seeds
+    after do
+      Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project')
+    end
+
+    it 'fetches a url as a mirror remote' do
+      subject
+
+      expect(refs(new_repository.path)).to eq(refs(repository.path))
+    end
+
+    context 'with keep-around refs' do
+      let(:sha) { SeedRepo::Commit::ID }
+      let(:keep_around_ref) { "refs/keep-around/#{sha}" }
+      let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" }
+
+      before do
+        repository.rugged.references.create(keep_around_ref, sha, force: true)
+        repository.rugged.references.create(tmp_ref, sha, force: true)
+      end
+
+      it 'includes the temporary and keep-around refs' do
+        subject
+
+        expect(refs(new_repository.path)).to include(keep_around_ref)
+        expect(refs(new_repository.path)).to include(tmp_ref)
+      end
+    end
+  end
+
+  describe '#remote_tags' do
+    let(:target_commit_id) { SeedRepo::Commit::ID }
+
+    subject { repository.remote_tags('upstream') }
+
+    it 'gets the remote tags' do
+      expect(repository).to receive(:list_remote_tags).with('upstream')
+        .and_return(["#{target_commit_id}\trefs/tags/v0.0.1\n"])
+
+      expect(subject.first).to be_an_instance_of(Gitlab::Git::Tag)
+      expect(subject.first.name).to eq('v0.0.1')
+      expect(subject.first.dereferenced_target.id).to eq(target_commit_id)
     end
   end
 
@@ -1685,6 +1724,21 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
+  describe '#fetch' do
+    let(:git_path) { Gitlab.config.git.bin_path }
+    let(:remote_name) { 'my_remote' }
+
+    subject { repository.fetch(remote_name) }
+
+    it 'fetches the remote and returns true if the command was successful' do
+      expect(repository).to receive(:popen)
+        .with(%W(#{git_path} fetch #{remote_name}), repository.path)
+        .and_return(['', 0])
+
+      expect(subject).to be(true)
+    end
+  end
+
   def create_remote_branch(repository, remote_name, branch_name, source_branch_name)
     source_branch = repository.branches.find { |branch| branch.name == source_branch_name }
     rugged = repository.rugged
@@ -1760,4 +1814,10 @@ describe Gitlab::Git::Repository, seed_helper: true do
     sha = Rugged::Commit.create(repo, options)
     repo.lookup(sha)
   end
+
+  def refs(dir)
+    IO.popen(%W[git -C #{dir} for-each-ref], &:read).split("\n").map do |line|
+      line.split("\t").last
+    end
+  end
 end

spec/models/repository_spec.rb

@@ -2148,30 +2148,6 @@ describe Repository do
     end
   end
 
-  describe '#remove_remote' do
-    it 'remove a remote reference' do
-      repository.add_remote('upstream', 'http://repo.test')
-
-      expect(repository.remove_remote('upstream')).to eq(true)
-    end
-  end
-
-  describe '#remote_tags' do
-    it 'gets the remote tags' do
-      masterrev = repository.find_branch('master').dereferenced_target.id
-
-      expect_any_instance_of(Gitlab::Shell).to receive(:list_remote_tags)
-        .with(repository.storage_path, repository.disk_path, 'upstream')
-        .and_return({ 'v0.0.1' => masterrev })
-
-      tags = repository.remote_tags('upstream')
-
-      expect(tags.first).to be_an_instance_of(Gitlab::Git::Tag)
-      expect(tags.first.name).to eq('v0.0.1')
-      expect(tags.first.dereferenced_target.id).to eq(masterrev)
-    end
-  end
-
   describe '#local_branches' do
     it 'returns the local branches' do
       masterrev = repository.find_branch('master').dereferenced_target
@@ -2430,4 +2406,24 @@ describe Repository do
       project.commit_by(oid: '1' * 40)
     end
   end
+
+  describe '#raw_repository' do
+    subject { repository.raw_repository }
+
+    it 'returns a Gitlab::Git::Repository representation of the repository' do
+      expect(subject).to be_a(Gitlab::Git::Repository)
+      expect(subject.relative_path).to eq(project.disk_path + '.git')
+      expect(subject.gl_repository).to eq("project-#{project.id}")
+    end
+
+    context 'with a wiki repository' do
+      let(:repository) { project.wiki.repository }
+
+      it 'creates a Gitlab::Git::Repository with the proper attributes' do
+        expect(subject).to be_a(Gitlab::Git::Repository)
+        expect(subject.relative_path).to eq(project.disk_path + '.wiki.git')
+        expect(subject.gl_repository).to eq("wiki-#{project.id}")
+      end
+    end
+  end
 end

spec/services/geo/repository_sync_service_spec.rb

@@ -75,6 +75,13 @@ RSpec.describe Geo::RepositorySyncService do
       expect { subject.execute }.not_to raise_error
     end
 
+    it 'rescues when Gitlab::Git::RepositoryMirroring::RemoteError is raised' do
+      allow(repository).to receive(:fetch_geo_mirror).with(url_to_repo)
+        .and_raise(Gitlab::Git::RepositoryMirroring::RemoteError)
+
+      expect { subject.execute }.not_to raise_error
+    end
+
     it 'rescues exception and fires after_create hook when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_geo_mirror).with(url_to_repo) { raise Gitlab::Git::Repository::NoRepository }
 

spec/services/geo/wiki_sync_service_spec.rb

@@ -58,6 +58,13 @@ RSpec.describe Geo::WikiSyncService do
       expect { subject.execute }.not_to raise_error
     end
 
+    it 'rescues exception when Gitlab::Git::RepositoryMirroring::RemoteError is raised' do
+      allow(repository).to receive(:fetch_geo_mirror).with(url_to_repo)
+        .and_raise(Gitlab::Git::RepositoryMirroring::RemoteError)
+
+      expect { subject.execute }.not_to raise_error
+    end
+
     it 'rescues exception when Gitlab::Git::Repository::NoRepository is raised' do
       allow(repository).to receive(:fetch_geo_mirror).with(url_to_repo) { raise Gitlab::Git::Repository::NoRepository }
 

spec/services/projects/update_repository_storage_service_spec.rb

@@ -6,7 +6,6 @@ describe Projects::UpdateRepositoryStorageService do
   subject { described_class.new(project) }
 
   describe "#execute" do
-    let(:gitlab_shell) { Gitlab::Shell.new }
     let(:time) { Time.now }
 
     before do
@@ -18,7 +17,6 @@ describe Projects::UpdateRepositoryStorageService do
         'b' => { 'path' => 'tmp/tests/storage_b' }
       }
       stub_storage_settings(storages)
-      allow(subject).to receive(:gitlab_shell).and_return(gitlab_shell)
 
       allow(Time).to receive(:now).and_return(time)
     end
@@ -33,9 +31,8 @@ describe Projects::UpdateRepositoryStorageService do
 
       context 'when the move succeeds' do
         it 'moves the repository to the new storage and unmarks the repository as read only' do
-          expect(gitlab_shell).to receive(:mv_storage)
-            .with('tmp/tests/storage_a', project.disk_path, 'tmp/tests/storage_b')
-            .and_return(true)
+          expect_any_instance_of(Gitlab::Git::Repository).to receive(:fetch_mirror)
+            .with(project.repository.raw.path).and_return(true)
           expect(GitlabShellWorker).to receive(:perform_async)
             .with(:mv_repository,
               'tmp/tests/storage_a',
@@ -51,9 +48,8 @@ describe Projects::UpdateRepositoryStorageService do
 
       context 'when the move fails' do
         it 'unmarks the repository as read-only without updating the repository storage' do
-          expect(gitlab_shell).to receive(:mv_storage)
-            .with('tmp/tests/storage_a', project.disk_path, 'tmp/tests/storage_b')
-            .and_return(false)
+          expect_any_instance_of(Gitlab::Git::Repository).to receive(:fetch_mirror)
+            .with(project.repository.raw.path).and_return(false)
           expect(GitlabShellWorker).not_to receive(:perform_async)
 
           subject.execute('b')
@@ -66,25 +62,38 @@ describe Projects::UpdateRepositoryStorageService do
 
     context 'with wiki', :skip_gitaly_mock do
       let(:project) { create(:project, :repository, repository_storage: 'a', repository_read_only: true, wiki_enabled: true) }
+      let(:repository_double) { double(:repository) }
+      let(:wiki_repository_double) { double(:repository) }
 
       before do
         project.create_wiki
+
+        # Default stub for non-specified params
+        allow(Gitlab::Git::Repository).to receive(:new).and_call_original
+
+        relative_path = project.repository.raw.relative_path
+        allow(Gitlab::Git::Repository).to receive(:new)
+          .with('b', relative_path, "project-#{project.id}")
+          .and_return(repository_double)
+
+        wiki_relative_path = project.wiki.repository.raw.relative_path
+        allow(Gitlab::Git::Repository).to receive(:new)
+          .with('b', wiki_relative_path, "wiki-#{project.id}")
+          .and_return(wiki_repository_double)
       end
 
       context 'when the move succeeds' do
         it 'moves the repository and its wiki to the new storage and unmarks the repository as read only' do
-          expect(gitlab_shell).to receive(:mv_storage)
-            .with('tmp/tests/storage_a', project.disk_path, 'tmp/tests/storage_b')
-            .and_return(true)
+          expect(repository_double).to receive(:fetch_mirror)
+            .with(project.repository.raw.path).and_return(true)
           expect(GitlabShellWorker).to receive(:perform_async)
             .with(:mv_repository,
               'tmp/tests/storage_a',
               project.disk_path,
               "#{project.disk_path}+#{project.id}+moved+#{time.to_i}")
 
-          expect(gitlab_shell).to receive(:mv_storage)
-            .with('tmp/tests/storage_a', project.wiki.disk_path, 'tmp/tests/storage_b')
-            .and_return(true)
+          expect(wiki_repository_double).to receive(:fetch_mirror)
+            .with(project.wiki.repository.raw.path).and_return(true)
           expect(GitlabShellWorker).to receive(:perform_async)
             .with(:mv_repository,
               'tmp/tests/storage_a',
@@ -100,12 +109,10 @@ describe Projects::UpdateRepositoryStorageService do
 
       context 'when the move of the wiki fails' do
         it 'unmarks the repository as read-only without updating the repository storage' do
-          expect(gitlab_shell).to receive(:mv_storage)
-            .with('tmp/tests/storage_a', project.disk_path, 'tmp/tests/storage_b')
-            .and_return(true)
-          expect(gitlab_shell).to receive(:mv_storage)
-            .with('tmp/tests/storage_a', project.wiki.disk_path, 'tmp/tests/storage_b')
-            .and_return(false)
+          expect(repository_double).to receive(:fetch_mirror)
+            .with(project.repository.raw.path).and_return(true)
+          expect(wiki_repository_double).to receive(:fetch_mirror)
+            .with(project.wiki.repository.raw.path).and_return(false)
           expect(GitlabShellWorker).not_to receive(:perform_async)
 
           subject.execute('b')