diff --git a/app/services/projects/after_import_service.rb b/app/services/projects/after_import_service.rb index 6fc15db9b4cfd82564494d8d01d5c3afb2246db5..a2270f1054778a3792027cb522db79315ea56a97 100644 --- a/app/services/projects/after_import_service.rb +++ b/app/services/projects/after_import_service.rb @@ -12,7 +12,9 @@ def execute service = Projects::HousekeepingService.new(@project) service.execute do - repository.delete_all_refs_except(RESERVED_REF_PREFIXES) + import_failure_service.with_retry(action: 'delete_all_refs') do + repository.delete_all_refs_except(RESERVED_REF_PREFIXES) + end end # Right now we don't actually have a way to know if a project @@ -26,6 +28,10 @@ def execute private + def import_failure_service + @import_failure_service ||= Gitlab::ImportExport::ImportFailureService.new(@project) + end + def repository @repository ||= @project.repository end diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb index 27e8f3c45ba27b2c36644083929d7ab14813b16c..4830e71224de4507c1c4fb9fe014628567b5c7a7 100644 --- a/spec/services/projects/after_import_service_spec.rb +++ b/spec/services/projects/after_import_service_spec.rb @@ -20,7 +20,7 @@ allow(housekeeping_service) .to receive(:execute).and_yield - expect(housekeeping_service).to receive(:increment!) + allow(housekeeping_service).to receive(:increment!) end it 'performs housekeeping' do @@ -58,6 +58,53 @@ end end + context 'when after import action throw non-retriable exception' do + let(:exception) { StandardError.new('after import error') } + + before do + allow(repository) + .to receive(:delete_all_refs_except) + .and_raise(exception) + end + + it 'throws after import error' do + expect { subject.execute }.to raise_exception('after import error') + end + end + + context 'when after import action throw retriable exception one time' do + let(:exception) { GRPC::DeadlineExceeded.new } + + before do + repository.write_ref('refs/pull/1/head', sha) + expect(repository) + .to receive(:delete_all_refs_except) + .and_raise(exception) + expect(repository) + .to receive(:delete_all_refs_except) + .and_call_original + + subject.execute + end + + it 'removes refs/pull/**/*' do + expect(rugged.references.map(&:name)) + .not_to include(%r{\Arefs/pull/}) + end + + it 'records the failures in the database', :aggregate_failures do + import_failure = ImportFailure.last + + expect(import_failure.source).to eq('delete_all_refs') + expect(import_failure.project_id).to eq(project.id) + expect(import_failure.relation_key).to be_nil + expect(import_failure.relation_index).to be_nil + expect(import_failure.exception_class).to eq('GRPC::DeadlineExceeded') + expect(import_failure.exception_message).to be_present + expect(import_failure.correlation_id_value).not_to be_empty + end + end + def rugged rugged_repo(repository) end