Move repository when project is removed

Ths commit does next:

* When we remove project we move repository to path+deleted.git
* Then we schedule removal of path+deleted with sidekiq
* If repository move failed we abort project removal

This should help us with NFS issue when project get removed but
repository stayed. The full explanation of problem is below:

* rm -rf project.git
* rm -rf removes project.git/objects/foo
* NFS server renames foo to foo.nfsXXXX because some NFS client (think
* Unicorn) still has the file open
* rm -rf exits, but project.git/objects/foo.nfsXXX still exists
* Unicorn closes the file, the NFS client closes the file (foo), and the
* NFS server removes foo.nfsXXX
* the directory project.git/objects/ still exists => problem

So now we move repository and even if repository removal failed

Repository directory is moved so no bugs with project removed but
repository directory taken. User still able to create new project with
same name. From administrator perspective you can easily find stalled
repositories by searching `*+deleted.git`

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

CHANGELOG

@@ -40,6 +40,7 @@ v 7.12.0 (unreleased)
   - Add an option to automatically sign-in with an Omniauth provider
   - Better performance for web editor (switched from satellites to rugged)
   - GitLab CI service sends .gitlab-ci.yaml in each push call
+  - When remove project - move repository and schedule it removal
 v 7.11.4
   - Fix missing bullets when creating lists

app/controllers/projects_controller.rb

@@ -97,18 +97,15 @@ class ProjectsController < ApplicationController
     return access_denied! unless can?(current_user, :remove_project, @project)
     ::Projects::DestroyService.new(@project, current_user, {}).execute
-    respond_to do |format|
-      format.html do
-        flash[:alert] = 'Project deleted.'
-
-        if request.referer.include?('/admin')
-          redirect_to admin_namespaces_projects_path
-        else
-          redirect_to dashboard_path
-        end
-      end
+    flash[:alert] = 'Project deleted.'
+    if request.referer.include?('/admin')
+      redirect_to admin_namespaces_projects_path
+    else
+      redirect_to dashboard_path
     end
+  rescue Projects::DestroyService::DestroyError => ex
+    redirect_to edit_project_path(@project), alert: ex.message
   end
   def autocomplete_sources

app/services/projects/destroy_service.rb

 module Projects
   class DestroyService < BaseService
+    include Gitlab::ShellAdapter
+
+    class DestroyError < StandardError; end
+
+    DELETED_FLAG = '+deleted'
+
     def execute
       return false unless can?(current_user, :remove_project, project)
       project.team.truncate
       project.repository.expire_cache unless project.empty_repo?
-      if project.destroy
-        GitlabShellWorker.perform_async(
-          :remove_repository,
-          project.path_with_namespace
-        )
-        GitlabShellWorker.perform_async(
-          :remove_repository,
-          project.path_with_namespace + ".wiki"
-        )
-        project.satellite.destroy
-        log_info("Project \"#{project.name}\" was removed")
-        system_hook_service.execute_hooks_for(project, :destroy)
-        true
+      repo_path = project.path_with_namespace
+      wiki_path = repo_path + '.wiki'
+
+      Project.transaction do
+        project.destroy!
+        unless remove_repository(repo_path)
+          raise_error('Failed to remove project repository. Please try again or contact administrator')
+        end
+
+        unless remove_repository(wiki_path)
+          raise_error('Failed to remove wiki repository. Please try again or contact administrator')
+        end
+      end
+
+      project.satellite.destroy
+      log_info("Project \"#{project.name}\" was removed")
+      system_hook_service.execute_hooks_for(project, :destroy)
+      true
+    end
+    private
+    def remove_repository(path)
+      unless gitlab_shell.exists?(path + '.git')
+        return true
       end
+
+      new_path = removal_path(path)
+
+      if gitlab_shell.mv_repository(path, new_path)
+        log_info("Repository \"#{path}\" moved to \"#{new_path}\"")
+        GitlabShellWorker.perform_in(30.seconds, :remove_repository, new_path)
+      else
+        false
+      end
+    end
+
+    def raise_error(message)
+      raise DestroyError.new(message)
+    end
+
+    # Build a path for removing repositories
+    # We use `+` because its not allowed by GitLab so user can not create
+    # project with name cookies+119+deleted and capture someone stalled repository
+    #
+    # gitlab/cookies.git -> gitlab/cookies+119+deleted.git
+    #
+    def removal_path(path)
+      "#{path}+#{project.id}#{DELETED_FLAG}"
     end
   end
 end

lib/gitlab/backend/shell.rb

@@ -244,6 +244,16 @@ module Gitlab
       end
     end
+    # Check if such directory exists in repositories.
+    #
+    # Usage:
+    #   exists?('gitlab')
+    #   exists?('gitlab/cookies.git')
+    #
+    def exists?(dir_name)
+      File.exists?(full_path(dir_name))
+    end
+
     protected
     def gitlab_shell_path
@@ -264,10 +274,6 @@ module Gitlab
       File.join(repos_path, dir_name)
     end
-    def exists?(dir_name)
-      File.exists?(full_path(dir_name))
-    end
-
     def gitlab_shell_projects_path
       File.join(gitlab_shell_path, 'bin', 'gitlab-projects')
     end