Merge branch 'infrastructure_155' into 'master'

Add missing `storage` argument to some methods in `Gitlab::Shell`

The affected methods are EE only and were basically related to the sync
of local and remote mirrors.

Fixes: https://gitlab.com/gitlab-com/infrastructure/issues/155

Ref: https://gitlab.com/gitlab-com/support-forum/issues/816

See merge request !532

app/models/repository.rb

@@ -17,8 +17,6 @@ class Repository
 
   attr_accessor :path_with_namespace, :project
 
-  delegate :push_remote_branches, :delete_remote_branches, to: :gitlab_shell
-
   def self.clean_old_archives
     Gitlab::Metrics.measure(:clean_old_archives) do
       repository_downloads_path = Gitlab.config.gitlab.repository_downloads_path
@@ -44,10 +42,14 @@ class Repository
     raw_repository.autocrlf = :input if raw_repository.autocrlf != :input
   end
 
+  def storage_path
+    @project.repository_storage_path
+  end
+
   # Return absolute path to repository
   def path_to_repo
     @path_to_repo ||= File.expand_path(
-      File.join(@project.repository_storage_path, path_with_namespace + ".git")
+      File.join(storage_path, path_with_namespace + ".git")
     )
   end
 
@@ -162,6 +164,10 @@ class Repository
     find_branch(branch_name)
   end
 
+  def push_remote_branches(remote, branches)
+    gitlab_shell.push_remote_branches(storage_path, path_with_namespace, remote, branches)
+  end
+
   def add_tag(user, tag_name, target, message = nil)
     oldrev = Gitlab::Git::BLANK_SHA
     ref    = Gitlab::Git::TAG_REF_PREFIX + tag_name
@@ -194,6 +200,10 @@ class Repository
     true
   end
 
+  def delete_remote_branches(remote, branches)
+    gitlab_shell.delete_remote_branches(storage_path, path_with_namespace, remote, branches)
+  end
+
   def rm_tag(tag_name)
     before_remove_tag
 
@@ -230,17 +240,17 @@ class Repository
   end
 
   def fetch_remote(remote, forced: false, no_tags: false)
-    gitlab_shell.fetch_remote(path_with_namespace, remote, forced: forced, no_tags: no_tags)
+    gitlab_shell.fetch_remote(storage_path, path_with_namespace, remote, forced: forced, no_tags: no_tags)
   end
 
   def remote_tags(remote)
-    gitlab_shell.list_remote_tags(path_with_namespace, remote).map do |name, target|
+    gitlab_shell.list_remote_tags(storage_path, path_with_namespace, remote).map do |name, target|
       Gitlab::Git::Tag.new(name, target)
     end
   end
 
   def fetch_remote_forced!(remote)
-    gitlab_shell.fetch_remote(path_with_namespace, remote, forced: true)
+    gitlab_shell.fetch_remote(storage_path, path_with_namespace, remote, forced: true)
   end
 
   def ref_names

app/services/projects/update_remote_mirror_service.rb

@@ -49,11 +49,11 @@ module Projects
       # Push the default branch first so it works fine when remote mirror is empty.
       branches.unshift(*default_branch)
 
-      repository.push_remote_branches(project.path_with_namespace, mirror.ref_name, branches)
+      repository.push_remote_branches(mirror.ref_name, branches)
     end
 
     def delete_branches
-      repository.delete_remote_branches(project.path_with_namespace, mirror.ref_name, deleted_branches)
+      repository.delete_remote_branches(mirror.ref_name, deleted_branches)
     end
 
     def deleted_branches
@@ -101,11 +101,11 @@ module Projects
     end
 
     def push_tags
-      repository.push_remote_branches(project.path_with_namespace, mirror.ref_name, changed_tags)
+      repository.push_remote_branches(mirror.ref_name, changed_tags)
     end
 
     def delete_tags
-      repository.delete_remote_branches(project.path_with_namespace, mirror.ref_name, deleted_tags)
+      repository.delete_remote_branches(mirror.ref_name, deleted_tags)
     end
 
     def changed_tags

lib/gitlab/backend/shell.rb

@@ -46,8 +46,8 @@ module Gitlab
       true
     end
 
-    def list_remote_tags(name, remote)
-      output, status = Popen::popen([gitlab_shell_projects_path, 'list-remote-tags', "#{name}.git", remote])
+    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?
@@ -83,8 +83,8 @@ module Gitlab
     # Ex.
     #   fetch_remote("gitlab/gitlab-ci", "upstream")
     #
-    def fetch_remote(name, remote, forced: false, no_tags: false)
-      args = [gitlab_shell_projects_path, 'fetch-remote', "#{name}.git", remote, '600']
+    def fetch_remote(storage, name, remote, forced: false, no_tags: false)
+      args = [gitlab_shell_projects_path, 'fetch-remote', storage, "#{name}.git", remote, '600']
       args << '--force' if forced
       args << '--no-tags' if no_tags
 
@@ -261,8 +261,8 @@ module Gitlab
     # Ex.
     #   push_remote_branches('upstream', 'feature')
     #
-    def push_remote_branches(project_name, remote_name, branch_names)
-      args = [gitlab_shell_projects_path, 'push-branches', "#{project_name}.git", remote_name, *branch_names]
+    def push_remote_branches(storage, project_name, remote_name, branch_names)
+      args = [gitlab_shell_projects_path, 'push-branches', storage, "#{project_name}.git", remote_name, *branch_names]
       output, status = Popen::popen(args)
       raise Error, output unless status.zero?
       true
@@ -277,8 +277,8 @@ module Gitlab
     # Ex.
     #   delete_remote_branches('upstream', 'feature')
     #
-    def delete_remote_branches(project_name, remote_name, branch_names)
-      args = [gitlab_shell_projects_path, 'delete-remote-branches', "#{project_name}.git", remote_name, *branch_names]
+    def delete_remote_branches(storage, project_name, remote_name, branch_names)
+      args = [gitlab_shell_projects_path, 'delete-remote-branches', storage, "#{project_name}.git", remote_name, *branch_names]
       output, status = Popen::popen(args)
       raise Error, output unless status.zero?
       true

spec/models/repository_spec.rb

@@ -1133,18 +1133,18 @@ describe Repository, models: true do
   describe '#push_remote_branches' do
     it 'push branches to the remote repo' do
       expect_any_instance_of(Gitlab::Shell).to receive(:push_remote_branches).
-        with('project_name', 'remote_name', ['branch'])
+        with(repository.storage_path, repository.path_with_namespace, 'remote_name', ['branch'])
 
-      repository.push_remote_branches('project_name', 'remote_name', ['branch'])
+      repository.push_remote_branches('remote_name', ['branch'])
     end
   end
 
   describe '#delete_remote_branches' do
     it 'delete branches to the remote repo' do
       expect_any_instance_of(Gitlab::Shell).to receive(:delete_remote_branches).
-        with('project_name', 'remote_name', ['branch'])
+        with(repository.storage_path, repository.path_with_namespace, 'remote_name', ['branch'])
 
-      repository.delete_remote_branches('project_name', 'remote_name', ['branch'])
+      repository.delete_remote_branches('remote_name', ['branch'])
     end
   end
 
@@ -1161,7 +1161,7 @@ describe Repository, models: true do
       masterrev = repository.find_branch('master').target
 
       expect_any_instance_of(Gitlab::Shell).to receive(:list_remote_tags).
-        with(repository.path_with_namespace, 'upstream').
+        with(repository.storage_path, repository.path_with_namespace, 'upstream').
         and_return({ 'v0.0.1' => masterrev })
 
       tags = repository.remote_tags('upstream')

spec/services/projects/update_remote_mirror_service_spec.rb

@@ -34,7 +34,7 @@ describe Projects::UpdateRemoteMirrorService do
       it "push all the branches the first time" do
         allow(repository).to receive(:fetch_remote)
 
-        expect(repository).to receive(:push_remote_branches).with(project.path_with_namespace, remote_mirror.ref_name, local_branch_names)
+        expect(repository).to receive(:push_remote_branches).with(remote_mirror.ref_name, local_branch_names)
 
         subject.execute(remote_mirror)
       end
@@ -53,7 +53,7 @@ describe Projects::UpdateRemoteMirrorService do
         allow(repository).to receive(:fetch_remote) { sync_remote(repository, remote_mirror.ref_name, current_branches) }
         create_branch(repository, 'my-new-branch')
 
-        expect(repository).to receive(:push_remote_branches).with(project.path_with_namespace, remote_mirror.ref_name, ['my-new-branch'])
+        expect(repository).to receive(:push_remote_branches).with(remote_mirror.ref_name, ['my-new-branch'])
 
         subject.execute(remote_mirror)
       end
@@ -64,7 +64,7 @@ describe Projects::UpdateRemoteMirrorService do
           update_branch(repository, 'existing-branch')
         end
 
-        expect(repository).to receive(:push_remote_branches).with(project.path_with_namespace, remote_mirror.ref_name, ['existing-branch'])
+        expect(repository).to receive(:push_remote_branches).with(remote_mirror.ref_name, ['existing-branch'])
 
         subject.execute(remote_mirror)
       end
@@ -75,7 +75,7 @@ describe Projects::UpdateRemoteMirrorService do
           delete_branch(repository, 'existing-branch')
         end
 
-        expect(repository).to receive(:delete_remote_branches).with(project.path_with_namespace, remote_mirror.ref_name, ['existing-branch'])
+        expect(repository).to receive(:delete_remote_branches).with(remote_mirror.ref_name, ['existing-branch'])
 
         subject.execute(remote_mirror)
       end
@@ -101,9 +101,7 @@ describe Projects::UpdateRemoteMirrorService do
         it 'should push tags to remote' do
           allow(repository).to receive(:remote_tags) { {} }
 
-          expect(repository).to receive(:push_remote_branches).with(
-            project.path_with_namespace, remote_mirror.ref_name, ['v1.0.0', 'v1.1.0']
-          )
+          expect(repository).to receive(:push_remote_branches).with(remote_mirror.ref_name, ['v1.0.0', 'v1.1.0'])
 
           subject.execute(remote_mirror)
         end
@@ -114,9 +112,7 @@ describe Projects::UpdateRemoteMirrorService do
           allow(repository).to receive(:remote_tags) { generate_tags(repository, 'v1.0.0', 'v1.1.0') }
           repository.rm_tag('v1.0.0')
 
-          expect(repository).to receive(:delete_remote_branches).with(
-            project.path_with_namespace, remote_mirror.ref_name, ['v1.0.0']
-          )
+          expect(repository).to receive(:delete_remote_branches).with(remote_mirror.ref_name, ['v1.0.0'])
 
           subject.execute(remote_mirror)
         end