Automatic merge of gitlab-org/gitlab-ce master

app/services/projects/destroy_service.rb

@@ -8,9 +8,16 @@ module Projects
     DestroyError = Class.new(StandardError)
 
     DELETED_FLAG = '+deleted'.freeze
+    REPO_REMOVAL_DELAY = 5.minutes.to_i
 
     def async_execute
       project.update_attribute(:pending_delete, true)
+
+      # Ensure no repository +deleted paths are kept,
+      # regardless of any issue with the ProjectDestroyWorker
+      # job process.
+      schedule_stale_repos_removal
+
       job_id = ProjectDestroyWorker.perform_async(project.id, current_user.id, params)
       Rails.logger.info("User #{current_user.id} scheduled destruction of project #{project.full_path} with job ID #{job_id}")
     end
@@ -93,14 +100,23 @@ module Projects
         log_info(%Q{Repository "#{path}" moved to "#{new_path}" for project "#{project.full_path}"})
 
         project.run_after_commit do
-          # self is now project
-          GitlabShellWorker.perform_in(5.minutes, :remove_repository, self.repository_storage, new_path)
+          GitlabShellWorker.perform_in(REPO_REMOVAL_DELAY, :remove_repository, self.repository_storage, new_path)
         end
       else
         false
       end
     end
 
+    def schedule_stale_repos_removal
+      repo_paths = [removal_path(repo_path), removal_path(wiki_path)]
+
+      # Ideally it should wait until the regular removal phase finishes,
+      # so let's delay it a bit further.
+      repo_paths.each do |path|
+        GitlabShellWorker.perform_in(REPO_REMOVAL_DELAY * 2, :remove_repository, project.repository_storage, path)
+      end
+    end
+
     def rollback_repository(old_path, new_path)
       # There is a possibility project does not have repository or wiki
       return true unless repo_exists?(old_path)
@@ -114,13 +130,11 @@ module Projects
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    # rubocop: disable CodeReuse/ActiveRecord
     def mv_repository(from_path, to_path)
-      return true unless gitlab_shell.exists?(project.repository_storage, from_path + '.git')
+      return true unless repo_exists?(from_path)
 
       gitlab_shell.mv_repository(project.repository_storage, from_path, to_path)
     end
-    # rubocop: enable CodeReuse/ActiveRecord
 
     def attempt_rollback(project, message)
       return unless project

changelogs/unreleased/osw-enforces-project-removal-with-past-failed-attempts.yml

+---
+title: Cleanup stale +deleted repo paths on project removal (adjusts project removal bug)
+merge_request: 24269
+author:
+type: fixed

doc/development/contributing/issue_workflow.md

@@ -67,7 +67,7 @@ The current team labels are:
 - ~Geo
 - ~Gitaly
 - ~Manage
-- ~Monitoring
+- ~Monitor
 - ~Plan
 - ~Quality
 - ~Release

doc/update/11.6-to-11.7.md

@@ -66,7 +66,7 @@ from source at the nodejs.org website.
 
 <https://nodejs.org/en/download/>
 
-GitLab also requires the use of yarn `>= v1.2.0` to manage JavaScript
+GitLab also requires the use of yarn `>= v1.10.0` to manage JavaScript
 dependencies.
 
 ```bash

spec/services/projects/destroy_service_spec.rb

@@ -281,6 +281,40 @@ describe Projects::DestroyService do
     end
   end
 
+  context 'repository +deleted path removal' do
+    def removal_path(path)
+      "#{path}+#{project.id}#{described_class::DELETED_FLAG}"
+    end
+
+    context 'regular phase' do
+      it 'schedules +deleted removal of existing repos' do
+        service = described_class.new(project, user, {})
+        allow(service).to receive(:schedule_stale_repos_removal)
+
+        expect(GitlabShellWorker).to receive(:perform_in)
+          .with(5.minutes, :remove_repository, project.repository_storage, removal_path(project.disk_path))
+
+        service.execute
+      end
+    end
+
+    context 'stale cleanup' do
+      let!(:async) { true }
+
+      it 'schedules +deleted wiki and repo removal' do
+        allow(ProjectDestroyWorker).to receive(:perform_async)
+
+        expect(GitlabShellWorker).to receive(:perform_in)
+          .with(10.minutes, :remove_repository, project.repository_storage, removal_path(project.disk_path))
+
+        expect(GitlabShellWorker).to receive(:perform_in)
+          .with(10.minutes, :remove_repository, project.repository_storage, removal_path(project.wiki.disk_path))
+
+        destroy_project(project, user, {})
+      end
+    end
+  end
+
   context '#attempt_restore_repositories' do
     let(:path) { project.disk_path + '.git' }