Merge branch 'fix/import-hooks' into 'master'

Fix import webhooks performance issue

Closes #27958

See merge request !9197

5 files changed, 49 insertions, 32 deletions

diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb
index 9db96347322..d0b991db112 100644
--- a/app/models/project_wiki.rb
+++ b/app/models/project_wiki.rb
@@ -160,6 +160,10 @@ class ProjectWiki
     }
   end
 
+  def repository_storage_path
+    project.repository_storage_path
+  end
+
   private
 
   def init_repo(path_with_namespace)
diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb
index f00c7460e82..90942774a2e 100644
--- a/lib/gitlab/import_export/command_line_util.rb
+++ b/lib/gitlab/import_export/command_line_util.rb
@@ -15,14 +15,6 @@ module Gitlab
         execute(%W(#{git_bin_path} --git-dir=#{repo_path} bundle create #{bundle_path} --all))
       end
 
-      def git_unbundle(repo_path:, bundle_path:)
-        execute(%W(#{git_bin_path} clone --bare #{bundle_path} #{repo_path}))
-      end
-
-      def git_restore_hooks
-        execute(%W(#{Gitlab.config.gitlab_shell.path}/bin/create-hooks) + repository_storage_paths_args)
-      end
-
       def mkdir_p(path)
         FileUtils.mkdir_p(path, mode: DEFAULT_MODE)
         FileUtils.chmod(DEFAULT_MODE, path)
@@ -56,10 +48,6 @@ module Gitlab
         FileUtils.copy_entry(source, destination)
         true
       end
-
-      def repository_storage_paths_args
-        Gitlab.config.repositories.storages.values
-      end
     end
   end
 end
diff --git a/lib/gitlab/import_export/repo_restorer.rb b/lib/gitlab/import_export/repo_restorer.rb
index 48a9a6fa5e2..c824d3ea9fc 100644
--- a/lib/gitlab/import_export/repo_restorer.rb
+++ b/lib/gitlab/import_export/repo_restorer.rb
@@ -2,6 +2,7 @@ module Gitlab
   module ImportExport
     class RepoRestorer
       include Gitlab::ImportExport::CommandLineUtil
+      include Gitlab::ShellAdapter
 
       def initialize(project:, shared:, path_to_bundle:)
         @project = project
@@ -12,29 +13,11 @@ module Gitlab
       def restore
         return true unless File.exist?(@path_to_bundle)
 
-        mkdir_p(path_to_repo)
-
-        git_unbundle(repo_path: path_to_repo, bundle_path: @path_to_bundle) && repo_restore_hooks
+        gitlab_shell.import_repository(@project.repository_storage_path, @project.path_with_namespace, @path_to_bundle)
       rescue => e
         @shared.error(e)
         false
       end
-
-      private
-
-      def path_to_repo
-        @project.repository.path_to_repo
-      end
-
-      def repo_restore_hooks
-        return true if wiki?
-
-        git_restore_hooks
-      end
-
-      def wiki?
-        @project.class.name == 'ProjectWiki'
-      end
     end
   end
 end
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index 942cedd6cd4..3faa336f142 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -80,8 +80,10 @@ module Gitlab
     #   import_repository("/path/to/storage", "gitlab/gitlab-ci", "https://github.com/randx/six.git")
     #
     def import_repository(storage, name, url)
+      # Timeout should be less than 900 ideally, to prevent the memory killer
+      # to silently kill the process without knowing we are timing out here.
       output, status = Popen::popen([gitlab_shell_projects_path, 'import-project',
-                                     storage, "#{name}.git", url, '900'])
+                                     storage, "#{name}.git", url, '800'])
       raise Error, output unless status.zero?
       true
     end
diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb
new file mode 100644
index 00000000000..168a59e5139
--- /dev/null
+++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::RepoRestorer, services: true do
+  describe 'bundle a project Git repo' do
+    let(:user) { create(:user) }
+    let!(:project_with_repo) { create(:project, :test_repo, name: 'test-repo-restorer', path: 'test-repo-restorer') }
+    let!(:project) { create(:empty_project) }
+    let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
+    let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.path_with_namespace) }
+    let(:bundler) { Gitlab::ImportExport::RepoSaver.new(project: project_with_repo, shared: shared) }
+    let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) }
+    let(:restorer) do
+      described_class.new(path_to_bundle: bundle_path,
+                          shared: shared,
+                          project: project)
+    end
+
+    before do
+      allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+
+      bundler.save
+    end
+
+    after do
+      FileUtils.rm_rf(export_path)
+      FileUtils.rm_rf(project_with_repo.repository.path_to_repo)
+      FileUtils.rm_rf(project.repository.path_to_repo)
+    end
+
+    it 'restores the repo successfully' do
+      expect(restorer.restore).to be true
+    end
+
+    it 'has the webhooks' do
+      restorer.restore
+
+      expect(Gitlab::Git::Hook.new('post-receive', project.repository.path_to_repo)).to exist
+    end
+  end
+end