Merge branch 'sh-namespace-cleanup-deleted-projects' into 'master'

Fix a number of race conditions that can occur during namespace deletion

See merge request !9294

3 files changed, 43 insertions, 2 deletions

diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index c5713fb7818..2afa2ad8d20 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -35,7 +35,7 @@ class Namespace < ActiveRecord::Base
   after_commit :refresh_access_of_projects_invited_groups, on: :update, if: -> { previous_changes.key?('share_with_group_lock') }
 
   # Save the storage paths before the projects are destroyed to use them on after destroy
-  before_destroy(prepend: true) { @old_repository_storage_paths = repository_storage_paths }
+  before_destroy(prepend: true) { prepare_for_destroy }
   after_destroy :rm_dir
 
   scope :root, -> { where('type IS NULL') }
@@ -217,6 +217,18 @@ class Namespace < ActiveRecord::Base
     [owner_id]
   end
 
+  def parent_changed?
+    parent_id_changed?
+  end
+
+  def prepare_for_destroy
+    old_repository_storage_paths
+  end
+
+  def old_repository_storage_paths
+    @old_repository_storage_paths ||= repository_storage_paths
+  end
+
   private
 
   def repository_storage_paths
@@ -230,7 +242,7 @@ class Namespace < ActiveRecord::Base
 
   def rm_dir
     # Remove the namespace directory in all storages paths used by member projects
-    @old_repository_storage_paths.each do |repository_storage_path|
+    old_repository_storage_paths.each do |repository_storage_path|
       # Move namespace directory into trash.
       # We will remove it later async
       new_path = "#{path}+#{id}+deleted"
diff --git a/app/models/project.rb b/app/models/project.rb
index b45f22d94d9..5fdc4377309 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -214,6 +214,8 @@ class Project < ActiveRecord::Base
   # Scopes
   default_scope { where(pending_delete: false) }
 
+  scope :with_deleted, -> { unscope(where: :pending_delete) }
+
   scope :sorted_by_activity, -> { reorder(last_activity_at: :desc) }
   scope :sorted_by_stars, -> { reorder('projects.star_count DESC') }
 
diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb
new file mode 100644
index 00000000000..2e2d7f884ac
--- /dev/null
+++ b/app/services/groups/destroy_service.rb
@@ -0,0 +1,27 @@
+module Groups
+  class DestroyService < Groups::BaseService
+    def async_execute
+      # Soft delete via paranoia gem
+      group.destroy
+      job_id = GroupDestroyWorker.perform_async(group.id, current_user.id)
+      Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}")
+    end
+
+    def execute
+      group.prepare_for_destroy
+
+      group.projects.with_deleted.each do |project|
+        # Execute the destruction of the models immediately to ensure atomic cleanup.
+        # Skip repository removal because we remove directory with namespace
+        # that contain all these repositories
+        ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
+      end
+
+      group.children.each do |group|
+        DestroyService.new(group, current_user).async_execute
+      end
+
+      group.really_destroy!
+    end
+  end
+end