Wrap deleting project with Gitlab::OptimisticLocking.retry_lock

We need to retry upon ActiveRecord::StaleObjectError because
pipelines or builds could be updated and raising an error,
and it doesn't hurt to remove everything while CI is still running.

Tests incoming.

Closes #24766

1 files changed, 76 insertions, 58 deletions

diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index a08c6fcd94b..f7cadb6a8ab 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -2,97 +2,115 @@ module Projects
   class DestroyService < BaseService
     include Gitlab::ShellAdapter
 
-    class DestroyError < StandardError; end
-
     DELETED_FLAG = '+deleted'
 
-    def async_execute
-      project.transaction do
-        project.update_attribute(:pending_delete, true)
-        job_id = ProjectDestroyWorker.perform_async(project.id, current_user.id, params)
-        Rails.logger.info("User #{current_user.id} scheduled destruction of project #{project.path_with_namespace} with job ID #{job_id}")
-      end
-    end
-
-    def execute
-      return false unless can?(current_user, :remove_project, project)
+    class DestroyError < StandardError; end
 
-      project.team.truncate
+    Executor = Struct.new(:service, :project) do
+      def execute
+        project.team.truncate
 
-      repo_path = project.path_with_namespace
-      wiki_path = repo_path + '.wiki'
+        repo_path = project.path_with_namespace
+        wiki_path = repo_path + '.wiki'
 
-      # Flush the cache for both repositories. This has to be done _before_
-      # removing the physical repositories as some expiration code depends on
-      # Git data (e.g. a list of branch names).
-      flush_caches(project, wiki_path)
+        # Flush the cache for both repositories. This has to be done _before_
+        # removing the physical repositories as some expiration code depends on
+        # Git data (e.g. a list of branch names).
+        flush_caches(wiki_path)
 
-      Projects::UnlinkForkService.new(project, current_user).execute
+        Projects::UnlinkForkService.new(project, service.current_user).execute
 
-      Project.transaction do
         project.destroy!
 
         unless remove_registry_tags
-          raise_error('Failed to remove project container registry. Please try again or contact administrator')
+          raise_error('Failed to remove project container registry')
         end
 
         unless remove_repository(repo_path)
-          raise_error('Failed to remove project repository. Please try again or contact administrator')
+          raise_error('Failed to remove project repository')
         end
 
         unless remove_repository(wiki_path)
-          raise_error('Failed to remove wiki repository. Please try again or contact administrator')
+          raise_error('Failed to remove wiki repository')
         end
+
+        service.log_info(
+          "Project \"#{project.path_with_namespace}\" was removed")
+        service.system_hook_service.execute_hooks_for(project, :destroy)
+
+        true
       end
 
-      log_info("Project \"#{project.path_with_namespace}\" was removed")
-      system_hook_service.execute_hooks_for(project, :destroy)
-      true
-    end
+      private
 
-    private
+      def flush_caches(wiki_path)
+        project.repository.before_delete
 
-    def remove_repository(path)
-      # Skip repository removal. We use this flag when remove user or group
-      return true if params[:skip_repo] == true
+        Repository.new(wiki_path, project).before_delete
+      end
 
-      # There is a possibility project does not have repository or wiki
-      return true unless gitlab_shell.exists?(project.repository_storage_path, path + '.git')
+      def remove_registry_tags
+        return true unless Gitlab.config.registry.enabled
 
-      new_path = removal_path(path)
+        project.container_registry_repository.delete_tags
+      end
 
-      if gitlab_shell.mv_repository(project.repository_storage_path, path, new_path)
-        log_info("Repository \"#{path}\" moved to \"#{new_path}\"")
-        GitlabShellWorker.perform_in(5.minutes, :remove_repository, project.repository_storage_path, new_path)
-      else
-        false
+      def remove_repository(path)
+        # Skip repository removal. We use this flag when remove user or group
+        return true if service.params[:skip_repo] == true
+
+        # There is a possibility project does not have repository or wiki
+        return true unless
+          service.gitlab_shell.exists?(
+            project.repository_storage_path, path + '.git')
+
+        new_path = removal_path(path)
+
+        if service.gitlab_shell.mv_repository(
+          project.repository_storage_path, path, new_path)
+          service.log_info("Repository \"#{path}\" moved to \"#{new_path}\"")
+          GitlabShellWorker.perform_in(
+            5.minutes,
+            :remove_repository,
+            project.repository_storage_path,
+            new_path)
+        else
+          false
+        end
       end
-    end
 
-    def remove_registry_tags
-      return true unless Gitlab.config.registry.enabled
+      def raise_error(message)
+        raise DestroyError.new(
+          "#{message}. Please try again or contact administrator")
+      end
 
-      project.container_registry_repository.delete_tags
+      # 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
 
-    def raise_error(message)
-      raise DestroyError.new(message)
+    def async_execute
+      Project.transaction do
+        project.update_attribute(:pending_delete, true)
+        job_id = ProjectDestroyWorker.perform_async(project.id, current_user.id, params)
+        Rails.logger.info("User #{current_user.id} scheduled destruction of project #{project.path_with_namespace} with job ID #{job_id}")
+      end
     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
+    def execute
+      return false unless can?(current_user, :remove_project, project)
 
-    def flush_caches(project, wiki_path)
-      project.repository.before_delete
+      Gitlab::OptimisticLocking.retry_lock(project) do |subject|
+        Executor.new(self, subject).execute
+      end
 
-      Repository.new(wiki_path, project).before_delete
+      true
     end
   end
 end