From 6f169b23093d377d37d2f704a632a0987089648d Mon Sep 17 00:00:00 2001 From: Patrick Bajao <ebajao@gitlab.com> Date: Mon, 16 Nov 2020 10:27:35 +0800 Subject: [PATCH] Do not fail when cleaning up MR with no repository When we clean up refs of a merge request that is on a project with no repository anymore, the MergeRequests::CleanupRefsService will fail. Before this fix, it'll throw an error and retry and will not set the cleanup schedule as completed. When the cleanup schedule is not marked as completed, it'll be picked up by the scheduler again. To fix that, we are now checking if the repository exists before deleting refs. This way, we can still update the cleanup schedule. --- app/services/merge_requests/cleanup_refs_service.rb | 2 +- .../245263-fix-cleanup-service-no-repository.yml | 5 +++++ .../merge_requests/cleanup_refs_service_spec.rb | 13 +++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/245263-fix-cleanup-service-no-repository.yml diff --git a/app/services/merge_requests/cleanup_refs_service.rb b/app/services/merge_requests/cleanup_refs_service.rb index 92d5d96b7f7..23ac8e393f4 100644 --- a/app/services/merge_requests/cleanup_refs_service.rb +++ b/app/services/merge_requests/cleanup_refs_service.rb @@ -31,7 +31,7 @@ module MergeRequests return error('Failed to create keep around refs.') unless kept_around? return error('Failed to cache merge ref sha.') unless cache_merge_ref_sha - delete_refs + delete_refs if repository.exists? return error('Failed to update schedule.') unless update_schedule diff --git a/changelogs/unreleased/245263-fix-cleanup-service-no-repository.yml b/changelogs/unreleased/245263-fix-cleanup-service-no-repository.yml new file mode 100644 index 00000000000..7bb7f8366f9 --- /dev/null +++ b/changelogs/unreleased/245263-fix-cleanup-service-no-repository.yml @@ -0,0 +1,5 @@ +--- +title: Do not fail when cleaning up MR with no repository +merge_request: 47744 +author: +type: fixed diff --git a/spec/services/merge_requests/cleanup_refs_service_spec.rb b/spec/services/merge_requests/cleanup_refs_service_spec.rb index 70acb50bb29..38c0e204e54 100644 --- a/spec/services/merge_requests/cleanup_refs_service_spec.rb +++ b/spec/services/merge_requests/cleanup_refs_service_spec.rb @@ -115,6 +115,19 @@ RSpec.describe MergeRequests::CleanupRefsService do it_behaves_like 'service that does not clean up merge request refs' end + + context 'when repository no longer exists' do + before do + Repositories::DestroyService.new(merge_request.project.repository).execute + end + + it 'does not fail and still mark schedule as complete' do + aggregate_failures do + expect(result[:status]).to eq(:success) + expect(merge_request.cleanup_schedule.completed_at).to be_present + end + end + end end shared_examples_for 'service that does not clean up merge request refs' do -- 2.30.9