Commit 4971e334 authored by Stan Hu's avatar Stan Hu

Merge branch '273131-fix-mr-create-push-option-race' into 'master'

Fix intermittent 'branch not found' errors when pushing creates an MR

See merge request gitlab-org/gitlab!68362
parents 84e39afc efdb7f7a
......@@ -65,7 +65,7 @@ module MergeRequests
end
if push_options[:target] && !target_project.repository.branch_exists?(push_options[:target])
errors << "Branch #{push_options[:target]} does not exist"
errors << "Target branch #{target_project.full_path}:#{push_options[:target]} does not exist"
end
end
......
......@@ -17,13 +17,18 @@ class PostReceiveService
response = Gitlab::InternalPostReceive::Response.new
push_options = Gitlab::PushOptions.new(params[:push_options])
mr_options = push_options.get(:merge_request)
response.reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease
# The PostReceive worker will normally invalidate the cache. However, it
# runs asynchronously. If push options require us to create a new merge
# request synchronously, we can't rely on that, so invalidate the cache here
repository&.expire_branches_cache if mr_options&.fetch(:create, false)
PostReceive.perform_async(params[:gl_repository], params[:identifier],
params[:changes], push_options.as_json)
mr_options = push_options.get(:merge_request)
if mr_options.present?
message = process_mr_push_options(mr_options, params[:changes])
response.add_alert_message(message)
......
......@@ -25,18 +25,6 @@ module QA
push.file_content = "Target branch test target branch #{SecureRandom.hex(8)}"
end
# Confirm the target branch can be checked out to avoid a race condition
# where the subsequent push option attempts to create an MR before the target branch is ready.
Support::Retrier.retry_on_exception(sleep_interval: 5) do
Git::Repository.perform do |repository|
repository.uri = project.repository_http_location.uri
repository.use_default_credentials
repository.clone
repository.configure_identity('GitLab QA', 'root@gitlab.com')
repository.checkout(target_branch)
end
end
Resource::Repository::ProjectPush.fabricate! do |push|
push.project = project
push.branch_name = "push-options-test-#{SecureRandom.hex(8)}"
......
......@@ -743,7 +743,7 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it 'records an error' do
service.execute
expect(service.errors).to eq(['Branch my-branch does not exist'])
expect(service.errors).to eq(["Target branch #{project.full_path}:my-branch does not exist"])
end
end
......
......@@ -131,6 +131,12 @@ RSpec.describe PostReceiveService do
project.add_developer(user)
end
it 'invalidates the branch name cache' do
expect(service.repository).to receive(:expire_branches_cache).and_call_original
subject
end
it 'invokes MergeRequests::PushOptionsHandlerService' do
expect(MergeRequests::PushOptionsHandlerService).to receive(:new).and_call_original
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment