Add configurable timeout for git fetch and clone operationsmrchrisw-import-shell-timeout

GitLab uses the import_project method in GitLab Shell,
This method uses a timeout for the operation, hardcoded to 800 seconds.
With this MR the timeout is now configurable in the gitlab_shell
settings.

5 files changed, 51 insertions, 2 deletions

diff --git a/changelogs/unreleased/mrchrisw-import-shell-timeout.yml b/changelogs/unreleased/mrchrisw-import-shell-timeout.yml
new file mode 100644
index 00000000000..e43409109d6
--- /dev/null
+++ b/changelogs/unreleased/mrchrisw-import-shell-timeout.yml
@@ -0,0 +1,4 @@
+---
+title: Add configurable timeout for git fetch and clone operations
+merge_request: 10697
+author: 
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 06c9f734c2a..ce477001b6f 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -502,6 +502,9 @@ production: &base
     upload_pack: true
     receive_pack: true
 
+    # Git import/fetch timeout
+    # git_timeout: 800
+
     # If you use non-standard ssh port you need to specify it
     # ssh_port: 22
 
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 87bf48a3dcd..16bc08cf48f 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -386,6 +386,7 @@ Settings.gitlab_shell['ssh_port']     ||= 22
 Settings.gitlab_shell['ssh_user']     ||= Settings.gitlab.user
 Settings.gitlab_shell['owner_group']  ||= Settings.gitlab.user
 Settings.gitlab_shell['ssh_path_prefix'] ||= Settings.__send__(:build_gitlab_shell_ssh_path_prefix)
+Settings.gitlab_shell['git_timeout'] ||= 800
 
 #
 # Repositories
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index 36a871e5bbc..b1d6ea665b7 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -83,7 +83,7 @@ module Gitlab
       # 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, '800'])
+                                    storage, "#{name}.git", url, "#{Gitlab.config.gitlab_shell.git_timeout}"])
       raise Error, output unless status.zero?
       true
     end
@@ -99,7 +99,7 @@ module Gitlab
     #   fetch_remote("gitlab/gitlab-ci", "upstream")
     #
     def fetch_remote(storage, name, remote, forced: false, no_tags: false)
-      args = [gitlab_shell_projects_path, 'fetch-remote', storage, "#{name}.git", remote, '800']
+      args = [gitlab_shell_projects_path, 'fetch-remote', storage, "#{name}.git", remote, "#{Gitlab.config.gitlab_shell.git_timeout}"]
       args << '--force' if forced
       args << '--no-tags' if no_tags
 
diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb
index 6675d26734e..a97a0f8452b 100644
--- a/spec/lib/gitlab/backend/shell_spec.rb
+++ b/spec/lib/gitlab/backend/shell_spec.rb
@@ -91,4 +91,45 @@ describe Gitlab::Shell, lib: true do
       end
     end
   end
+
+  describe 'projects commands' do
+    let(:projects_path) { 'tmp/tests/shell-projects-test/bin/gitlab-projects' }
+
+    before do
+      allow(Gitlab.config.gitlab_shell).to receive(:path).and_return('tmp/tests/shell-projects-test')
+      allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800)
+    end
+
+    describe '#fetch_remote' do
+      it 'returns true when the command succeeds' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'fetch-remote', 'current/storage', 'project/path.git', 'new/storage', '800']).and_return([nil, 0])
+
+        expect(gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage')).to be true
+      end
+
+      it 'raises an exception when the command fails' do
+        expect(Gitlab::Popen).to receive(:popen)
+        .with([projects_path, 'fetch-remote', 'current/storage', 'project/path.git', 'new/storage', '800']).and_return(["error", 1])
+
+        expect { gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage') }.to raise_error(Gitlab::Shell::Error, "error")
+      end
+    end
+
+    describe '#import_repository' do
+      it 'returns true when the command succeeds' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'import-project', 'current/storage', 'project/path.git', 'https://gitlab.com/gitlab-org/gitlab-ce.git', "800"]).and_return([nil, 0])
+
+        expect(gitlab_shell.import_repository('current/storage', 'project/path', 'https://gitlab.com/gitlab-org/gitlab-ce.git')).to be true
+      end
+
+      it 'raises an exception when the command fails' do
+        expect(Gitlab::Popen).to receive(:popen)
+        .with([projects_path, 'import-project', 'current/storage', 'project/path.git', 'https://gitlab.com/gitlab-org/gitlab-ce.git', "800"]).and_return(["error", 1])
+
+        expect { gitlab_shell.import_repository('current/storage', 'project/path', 'https://gitlab.com/gitlab-org/gitlab-ce.git') }.to raise_error(Gitlab::Shell::Error, "error")
+      end
+    end
+  end
 end