Merge branch '2701-geo-fix-repositories-removal' into 'master'

Geo: fix removal of repositories from disk on secondary nodes

Closes #2701

See merge request !2210

app/models/geo/deleted_project.rb

+class Geo::DeletedProject < ::Project
+  after_initialize :readonly!
+  attr_reader :full_path
+
+  def initialize(id:, name:, full_path:, repository_storage:)
+    repository_storage ||= current_application_settings.pick_repository_storage
+
+    super(id: id, name: name, repository_storage: repository_storage)
+    @full_path = full_path
+  end
+
+  def repository
+    @repository ||= Repository.new(full_path, self)
+  end
+end

app/workers/geo_repository_destroy_worker.rb

 class GeoRepositoryDestroyWorker
   include Sidekiq::Worker
   include GeoQueue
+  include Gitlab::ShellAdapter
+
+  def perform(id, name, full_path)
+    repository_storage = probe_repository_storage(full_path)
 
-  def perform(id, name, path_with_namespace)
     # We don't have access to the original model anymore, so we are
     # rebuilding only what our service class requires
-    project = FakeProject.new(id, name, path_with_namespace)
+    project = ::Geo::DeletedProject.new(id: id, name: name, full_path: full_path, repository_storage: repository_storage)
 
     ::Projects::DestroyService.new(project, nil).geo_replicate
   end
 
-  FakeProject = Struct.new(:id, :name, :path_with_namespace) do
-    def repository
-      @repository ||= Repository.new(path_with_namespace, self)
+  private
+
+  # Detect in which repository_storage the project repository is stored.
+  #
+  # As we don't have access to `repository_storage` from the data in the Hook notification
+  # we need to probe on all existing ones.
+  #
+  # if we don't find it means it has already been deleted and we just return
+  def probe_repository_storage(full_path)
+    Gitlab.config.repositories.storages.each do |repository_storage, rs_data|
+      return repository_storage if gitlab_shell.exists?(rs_data['path'], full_path + '.git')
     end
+
+    nil
   end
 end

changelogs/unreleased-ee/2701-geo-fix-repositories-removal.yml

+---
+title: 'Geo: fix removal of repositories from disk on secondary nodes'
+merge_request: 2210
+author:

spec/services/projects/destroy_service_spec.rb

@@ -15,8 +15,9 @@ describe Projects::DestroyService, services: true do
   shared_examples 'deleting the project' do
     it 'deletes the project' do
       expect(Project.unscoped.all).not_to include(project)
-      expect(Dir.exist?(path)).to be_falsey
-      expect(Dir.exist?(remove_path)).to be_falsey
+
+      expect(project.gitlab_shell.exists?(project.repository_storage_path, path + '.git')).to be_falsey
+      expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path + '.git')).to be_falsey
     end
   end
 

spec/workers/geo_repository_destroy_worker_spec.rb

+require 'spec_helper'
+
+describe GeoRepositoryDestroyWorker do
+  let!(:project) { create :project_empty_repo }
+  let!(:path) { project.repository.path_with_namespace }
+  let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") }
+  let(:perform!) { subject.perform(project.id, project.name, path) }
+
+  it 'delegates project removal to Projects::DestroyService' do
+    expect_any_instance_of(::Projects::DestroyService).to receive(:geo_replicate)
+
+    perform!
+  end
+
+  context 'sidekiq execution' do
+    before do
+      project.delete
+    end
+
+    it 'removes the repository from disk' do
+      expect(project.gitlab_shell.exists?(project.repository_storage_path, path + '.git')).to be_truthy
+
+      Sidekiq::Testing.inline! { perform! }
+
+      expect(project.gitlab_shell.exists?(project.repository_storage_path, path + '.git')).to be_falsey
+      expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path + '.git')).to be_falsey
+    end
+  end
+
+  describe '#probe_repository_storage' do
+    it 'returns a repository_storage when repository can be found' do
+      expect(subject.send(:probe_repository_storage, project.path_with_namespace)).to eq('default')
+    end
+
+    it 'returns nil when repository cannot be found in any existing repository_storage' do
+      expect(subject.send(:probe_repository_storage, 'nonexistent/project')).to eq(nil)
+    end
+  end
+end