Commit 12e8e7b8 authored by Shinya Maeda's avatar Shinya Maeda

Use merge request MERGE ref for attached merge request pipelines

Merge request pipelines are for prevalidating merge pipelines before
an actual merge.
parent 7e676b3c
...@@ -55,15 +55,7 @@ module MergeRequests ...@@ -55,15 +55,7 @@ module MergeRequests
end end
def create_pipeline_for(merge_request, user) def create_pipeline_for(merge_request, user)
return unless Feature.enabled?(:ci_merge_request_pipeline, return unless can_create_pipeline_for?(merge_request)
merge_request.source_project,
default_enabled: true)
##
# UpdateMergeRequestsWorker could be retried by an exception.
# MR pipelines should not be recreated in such case.
return if merge_request.merge_request_pipeline_exists?
return if merge_request.has_no_commits?
create_detached_merge_request_pipeline(merge_request, user) create_detached_merge_request_pipeline(merge_request, user)
end end
...@@ -80,6 +72,16 @@ module MergeRequests ...@@ -80,6 +72,16 @@ module MergeRequests
end end
end end
def can_create_pipeline_for?(merge_request)
##
# UpdateMergeRequestsWorker could be retried by an exception.
# pipelines for merge request should not be recreated in such case.
return false if merge_request.merge_request_pipeline_exists?
return false if merge_request.has_no_commits?
true
end
def can_use_merge_request_ref?(merge_request) def can_use_merge_request_ref?(merge_request)
Feature.enabled?(:ci_use_merge_request_ref, project, default_enabled: true) && Feature.enabled?(:ci_use_merge_request_ref, project, default_enabled: true) &&
!merge_request.for_fork? !merge_request.for_fork?
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
module EE module EE
module MergeRequests module MergeRequests
module BaseService module BaseService
extend ::Gitlab::Utils::Override
private private
def filter_params(merge_request) def filter_params(merge_request)
...@@ -16,6 +18,34 @@ module EE ...@@ -16,6 +18,34 @@ module EE
super super
end end
override :create_pipeline_for
def create_pipeline_for(merge_request, user)
create_merge_request_pipeline_for(merge_request, user) || super
end
def create_merge_request_pipeline_for(merge_request, user)
return unless can_create_merge_request_pipeline_for?(merge_request)
ret = ::MergeRequests::MergeToRefService.new(merge_request.project, user)
.execute(merge_request)
if ret[:status] == :success
::Ci::CreatePipelineService.new(merge_request.source_project, user,
ref: merge_request.merge_ref_path,
checkout_sha: ret[:commit_id],
target_sha: ret[:target_id],
source_sha: ret[:source_id])
.execute(:merge_request_event, merge_request: merge_request)
end
end
def can_create_merge_request_pipeline_for?(merge_request)
return false unless merge_request.project.merge_pipelines_enabled?
return false unless can_use_merge_request_ref?(merge_request)
can_create_pipeline_for?(merge_request)
end
end end
end end
end end
---
title: Use merge request MERGE ref for attached merge request pipelines
merge_request: 9622
author:
type: added
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
describe MergeRequests::BaseService do describe MergeRequests::BaseService do
include ProjectForksHelper
subject { MergeRequests::CreateService.new(project, project.owner, params) } subject { MergeRequests::CreateService.new(project, project.owner, params) }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
...@@ -34,4 +36,114 @@ describe MergeRequests::BaseService do ...@@ -34,4 +36,114 @@ describe MergeRequests::BaseService do
end end
end end
end end
describe '#create_pipeline_for' do
subject { service.execute }
let(:service) { MergeRequests::CreateService.new(source_project, user, opts) }
let(:user) { create(:user) }
let(:title) { 'Awesome merge request' }
let(:merge_pipelines_enabled) { true }
let(:merge_pipelines_license) { true }
let(:source_project) { project }
let(:source_branch) { 'feature' }
let(:target_project) { project }
let(:target_branch) { 'master' }
let(:opts) do
{
title: title,
description: 'A new fix',
source_branch: source_branch,
source_project: source_project,
target_branch: target_branch,
target_project: target_project
}
end
let(:ci_yaml) do
{
test: {
stage: 'test',
script: 'echo',
only: ['merge_requests']
}
}
end
before do
source_project.add_developer(user)
target_project.add_developer(user)
source_project.merge_pipelines_enabled = merge_pipelines_enabled
stub_licensed_features(merge_pipelines: merge_pipelines_license)
stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml))
end
shared_examples_for 'creates a merge requst pipeline' do
it do
expect(subject).to be_persisted
expect(subject.all_pipelines.count).to eq(1)
expect(subject.all_pipelines.last).to be_merge_request_pipeline
expect(subject.all_pipelines.last).not_to be_detached_merge_request_pipeline
end
end
shared_examples_for 'creates a detached merge requst pipeline' do
it do
expect(subject).to be_persisted
expect(subject.all_pipelines.count).to eq(1)
expect(subject.all_pipelines.last).not_to be_merge_request_pipeline
expect(subject.all_pipelines.last).to be_detached_merge_request_pipeline
end
end
it_behaves_like 'creates a merge requst pipeline'
context 'when merge request is WIP' do
let(:title) { 'WIP: Awesome merge request' }
it_behaves_like 'creates a detached merge requst pipeline'
end
context 'when project setting for merge request pipelines is disabled' do
let(:merge_pipelines_enabled) { false }
it_behaves_like 'creates a detached merge requst pipeline'
end
context 'when ci_use_merge_request_ref feature flag is disabled' do
before do
stub_feature_flags(ci_use_merge_request_ref: false)
end
it_behaves_like 'creates a detached merge requst pipeline'
end
context 'when merge request is submitted from fork' do
let(:source_project) { fork_project(project, nil, repository: true) }
it_behaves_like 'creates a detached merge requst pipeline'
end
context 'when the CreateService is retried' do
it 'does not create a merge request pipeline twice' do
expect do
2.times { MergeRequests::CreateService.new(source_project, user, opts).execute }
end.to change { Ci::Pipeline.count }.by(1)
end
end
context 'when merge request has no commit' do
let(:source_branch) { 'empty-branch' }
it_behaves_like 'creates a detached merge requst pipeline'
end
context 'when merge request has a conflict' do
let(:source_branch) { 'feature' }
let(:target_branch) { 'feature_conflict' }
it_behaves_like 'creates a detached merge requst pipeline'
end
end
end end
...@@ -263,19 +263,6 @@ describe MergeRequests::CreateService do ...@@ -263,19 +263,6 @@ describe MergeRequests::CreateService do
expect(merge_request.actual_head_pipeline).to be_merge_request_event expect(merge_request.actual_head_pipeline).to be_merge_request_event
end end
end end
context "when the 'ci_merge_request_pipeline' feature flag is disabled" do
before do
stub_feature_flags(ci_merge_request_pipeline: false)
end
it 'does not create a detached merge request pipeline' do
expect(merge_request).to be_persisted
merge_request.reload
expect(merge_request.merge_request_pipelines.count).to eq(0)
end
end
end end
context "when .gitlab-ci.yml does not have merge_requests keywords" do context "when .gitlab-ci.yml does not have merge_requests keywords" do
......
...@@ -236,17 +236,6 @@ describe MergeRequests::RefreshService do ...@@ -236,17 +236,6 @@ describe MergeRequests::RefreshService do
end.not_to change { @merge_request.merge_request_pipelines.count } end.not_to change { @merge_request.merge_request_pipelines.count }
end end
end end
context "when the 'ci_merge_request_pipeline' feature flag is disabled" do
before do
stub_feature_flags(ci_merge_request_pipeline: false)
end
it 'does not create a detached merge request pipeline' do
expect { subject }
.not_to change { @merge_request.merge_request_pipelines.count }
end
end
end end
context "when .gitlab-ci.yml does not have merge_requests keywords" do context "when .gitlab-ci.yml does not have merge_requests keywords" do
......
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