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

Gitaly mirroring migration prep

Closes gitaly#816

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

4 files changed, 46 insertions, 55 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 751306188a0..d1ae3304e4a 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -971,8 +971,7 @@ class Repository
       tmp_remote_name = true
     end
 
-    add_remote(remote_name, url)
-    set_remote_as_mirror(remote_name, refmap: refmap)
+    add_remote(remote_name, url, mirror_refmap: refmap)
     fetch_remote(remote_name, forced: forced)
   ensure
     remove_remote(remote_name) if tmp_remote_name
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 73889328f36..fbdd646ea13 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -20,6 +20,7 @@ module Gitlab
       SEARCH_CONTEXT_LINES = 3
       REBASE_WORKTREE_PREFIX = 'rebase'.freeze
       SQUASH_WORKTREE_PREFIX = 'squash'.freeze
+      GITALY_INTERNAL_URL = 'ssh://gitaly/internal.git'.freeze
 
       NoRepository = Class.new(StandardError)
       InvalidBlobName = Class.new(StandardError)
@@ -887,8 +888,11 @@ module Gitlab
         end
       end
 
-      def add_remote(remote_name, url)
+      # If `mirror_refmap` is present the remote is set as mirror with that mapping
+      def add_remote(remote_name, url, mirror_refmap: nil)
         rugged.remotes.create(remote_name, url)
+
+        set_remote_as_mirror(remote_name, refmap: mirror_refmap) if mirror_refmap
       rescue Rugged::ConfigError
         remote_update(remote_name, url: url)
       end
@@ -1128,12 +1132,24 @@ module Gitlab
         !has_visible_content?
       end
 
-      # Like all public `Gitlab::Git::Repository` methods, this method is part
-      # of `Repository`'s interface through `method_missing`.
-      # `Repository` has its own `fetch_remote` which uses `gitlab-shell` and
-      # takes some extra attributes, so we qualify this method name to prevent confusion.
-      def fetch_remote_without_shell(remote = 'origin')
-        run_git(['fetch', remote]).last.zero?
+      def fetch_repository_as_mirror(repository)
+        remote_name = "tmp-#{SecureRandom.hex}"
+
+        # Notice that this feature flag is not for `fetch_repository_as_mirror`
+        # as a whole but for the fetching mechanism (file path or gitaly-ssh).
+        url, env = gitaly_migrate(:fetch_internal) do |is_enabled|
+          if is_enabled
+            repository = RemoteRepository.new(repository) unless repository.is_a?(RemoteRepository)
+            [GITALY_INTERNAL_URL, repository.fetch_env]
+          else
+            [repository.path, nil]
+          end
+        end
+
+        add_remote(remote_name, url, mirror_refmap: :all_refs)
+        fetch_remote(remote_name, env: env)
+      ensure
+        remove_remote(remote_name)
       end
 
       def blob_at(sha, path)
@@ -1851,7 +1867,7 @@ module Gitlab
       end
 
       def gitaly_fetch_ref(source_repository, source_ref:, target_ref:)
-        args = %W(fetch --no-tags -f ssh://gitaly/internal.git #{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
@@ -1871,6 +1887,10 @@ module Gitlab
       rescue Rugged::ReferenceError
         raise ArgumentError, 'Invalid merge source'
       end
+
+      def fetch_remote(remote_name = 'origin', env: nil)
+        run_git(['fetch', remote_name], env: env).last.zero?
+      end
     end
   end
 end
diff --git a/lib/gitlab/git/repository_mirroring.rb b/lib/gitlab/git/repository_mirroring.rb
index 392bef69e80..effb1f0ca19 100644
--- a/lib/gitlab/git/repository_mirroring.rb
+++ b/lib/gitlab/git/repository_mirroring.rb
@@ -17,33 +17,6 @@ module Gitlab
         rugged.config["remote.#{remote_name}.prune"] = true
       end
 
-      def set_remote_refmap(remote_name, refmap)
-        Array(refmap).each_with_index do |refspec, i|
-          refspec = REFMAPS[refspec] || refspec
-
-          # We need multiple `fetch` entries, but Rugged only allows replacing a config, not adding to it.
-          # To make sure we start from scratch, we set the first using rugged, and use `git` for any others
-          if i == 0
-            rugged.config["remote.#{remote_name}.fetch"] = refspec
-          else
-            run_git(%W[config --add remote.#{remote_name}.fetch #{refspec}])
-          end
-        end
-      end
-
-      # Like all_refs public `Gitlab::Git::Repository` methods, this method is part
-      # of `Repository`'s interface through `method_missing`.
-      # `Repository` has its own `fetch_as_mirror` which uses `gitlab-shell` and
-      # takes some extra attributes, so we qualify this method name to prevent confusion.
-      def fetch_as_mirror_without_shell(url)
-        remote_name = "tmp-#{SecureRandom.hex}"
-        add_remote(remote_name, url)
-        set_remote_as_mirror(remote_name)
-        fetch_remote_without_shell(remote_name)
-      ensure
-        remove_remote(remote_name) if remote_name
-      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' }, ...]
@@ -85,6 +58,20 @@ module Gitlab
 
       private
 
+      def set_remote_refmap(remote_name, refmap)
+        Array(refmap).each_with_index do |refspec, i|
+          refspec = REFMAPS[refspec] || refspec
+
+          # We need multiple `fetch` entries, but Rugged only allows replacing a config, not adding to it.
+          # To make sure we start from scratch, we set the first using rugged, and use `git` for any others
+          if i == 0
+            rugged.config["remote.#{remote_name}.fetch"] = refspec
+          else
+            run_git(%W[config --add remote.#{remote_name}.fetch #{refspec}])
+          end
+        end
+      end
+
       def list_remote_tags(remote)
         tag_list, exit_code, error = nil
         cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path} ls-remote --tags #{remote})
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index f19b65a5f71..e6845420f7d 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -588,12 +588,12 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe '#fetch_as_mirror_without_shell' do
+  describe '#fetch_repository_as_mirror' do
     let(:new_repository) do
       Gitlab::Git::Repository.new('default', 'my_project.git', '')
     end
 
-    subject { new_repository.fetch_as_mirror_without_shell(repository.path) }
+    subject { new_repository.fetch_repository_as_mirror(repository) }
 
     before do
       Gitlab::Shell.new.add_repository('default', 'my_project')
@@ -603,7 +603,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
       Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project')
     end
 
-    it 'fetches a url as a mirror remote' do
+    it 'fetches a repository as a mirror remote' do
       subject
 
       expect(refs(new_repository.path)).to eq(refs(repository.path))
@@ -1662,21 +1662,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe '#fetch_remote_without_shell' do
-    let(:git_path) { Gitlab.config.git.bin_path }
-    let(:remote_name) { 'my_remote' }
-
-    subject { repository.fetch_remote_without_shell(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
-
   describe '#merge' do
     let(:repository) do
       Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')