summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2019-07-02 09:01:36 +0700
committerShinya Maeda <shinya@gitlab.com>2019-07-08 14:36:32 +0700
commitf8d6f7322ea43ff7844d006885db9f0c36ad7cc9 (patch)
treeb1177546270245395ca30479ada1e75da903a4a9
parentc03f23c48311f0663458bbed33131fa3a6c80842 (diff)
downloadgitlab-ce-run-pipeline-for-merge-train-at-train-ref-ce.tar.gz
Fix race condition on merge train ref generationrun-pipeline-for-merge-train-at-train-ref-ce
Today, Pipelines for merge train run on `refs/merge`, however, this causes a race condition that it can be overwritten by CheckMergeabilityService. This patch fixes the problem by generating `refs/train` for those pipelines.
-rw-r--r--app/models/merge_request.rb13
-rw-r--r--app/models/repository.rb4
-rw-r--r--spec/models/merge_request_spec.rb30
-rw-r--r--spec/services/merge_requests/merge_to_ref_service_spec.rb9
4 files changed, 56 insertions, 0 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index e96e26cc773..53977748c30 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -1127,6 +1127,19 @@ class MergeRequest < ApplicationRecord
"refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/merge"
end
+ def train_ref_path
+ "refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/train"
+ end
+
+ def cleanup_refs(only: :all)
+ target_refs = []
+ target_refs << ref_path if %i[all head].include?(only)
+ target_refs << merge_ref_path if %i[all merge].include?(only)
+ target_refs << train_ref_path if %i[all train].include?(only)
+
+ project.repository.delete_refs(*target_refs)
+ end
+
def self.merge_request_ref?(ref)
ref.start_with?("refs/#{Repository::REF_MERGE_REQUEST}/")
end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index a408db7ebbe..a25d5abfa64 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -845,6 +845,10 @@ class Repository
raw.merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
end
+ def delete_refs(*ref_names)
+ raw.delete_refs(*ref_names)
+ end
+
def ff_merge(user, source, target_branch, merge_request: nil)
their_commit_id = commit(source)&.id
raise 'Invalid merge source' if their_commit_id.nil?
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index fe6d68aff3f..9b0c232f370 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -3220,4 +3220,34 @@ describe MergeRequest do
it { is_expected.to be_truthy }
end
end
+
+ describe '#cleanup_refs' do
+ subject { merge_request.cleanup_refs(only: only) }
+
+ let(:merge_request) { build(:merge_request) }
+
+ context 'when removing all refs' do
+ let(:only) { :all }
+
+ it 'deletes all refs from the target project' do
+ expect(merge_request.target_project.repository)
+ .to receive(:delete_refs)
+ .with(merge_request.ref_path, merge_request.merge_ref_path, merge_request.train_ref_path)
+
+ subject
+ end
+ end
+
+ context 'when removing only train ref' do
+ let(:only) { :train }
+
+ it 'deletes train ref from the target project' do
+ expect(merge_request.target_project.repository)
+ .to receive(:delete_refs)
+ .with(merge_request.train_ref_path)
+
+ subject
+ end
+ end
+ end
end
diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb
index e2f201677fa..14012b4ea2d 100644
--- a/spec/services/merge_requests/merge_to_ref_service_spec.rb
+++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb
@@ -191,6 +191,15 @@ describe MergeRequests::MergeToRefService do
it { expect(todo).not_to be_done }
end
+ context 'when target ref is passed as a parameter' do
+ let(:params) { { commit_message: 'merge train', target_ref: target_ref } }
+
+ it_behaves_like 'successfully merges to ref with merge method' do
+ let(:first_parent_ref) { 'refs/heads/master' }
+ let(:target_ref) { 'refs/merge-requests/1/train' }
+ end
+ end
+
describe 'cascading merge refs' do
set(:project) { create(:project, :repository) }
let(:params) { { commit_message: 'Cascading merge', first_parent_ref: first_parent_ref, target_ref: target_ref } }