Commit 14c31b6f authored by Furkan Ayhan's avatar Furkan Ayhan Committed by Heinrich Lee Yu

Implement allowing child pipeline to have child pipeline

In this commit, we are just allowing parent->child->child nesting.
This is controlled by MAX_DESCENDANTS_DEPTH constant.

This feature is also behind a FF ci_child_of_child_pipeline for now.
parent cc86d001
...@@ -821,12 +821,16 @@ module Ci ...@@ -821,12 +821,16 @@ module Ci
all_merge_requests.order(id: :desc) all_merge_requests.order(id: :desc)
end end
def same_family_pipeline_ids
if ::Gitlab::Ci::Features.child_of_child_pipeline_enabled?(project)
::Gitlab::Ci::PipelineObjectHierarchy.new(base_and_ancestors).base_and_descendants.select(:id)
else
# If pipeline is a child of another pipeline, include the parent # If pipeline is a child of another pipeline, include the parent
# and the siblings, otherwise return only itself and children. # and the siblings, otherwise return only itself and children.
def same_family_pipeline_ids
parent = parent_pipeline || self parent = parent_pipeline || self
[parent.id] + parent.child_pipelines.pluck(:id) [parent.id] + parent.child_pipelines.pluck(:id)
end end
end
def bridge_triggered? def bridge_triggered?
source_bridge.present? source_bridge.present?
...@@ -1058,6 +1062,14 @@ module Ci ...@@ -1058,6 +1062,14 @@ module Ci
self.ci_ref = Ci::Ref.ensure_for(self) self.ci_ref = Ci::Ref.ensure_for(self)
end end
def base_and_ancestors
# Without using `unscoped`, caller scope is also included into the query.
# Using `unscoped` here will be redundant after Rails 6.1
::Gitlab::Ci::PipelineObjectHierarchy
.new(self.class.unscoped.where(id: id))
.base_and_ancestors
end
private private
def add_message(severity, content) def add_message(severity, content)
......
...@@ -23,9 +23,10 @@ module Enums ...@@ -23,9 +23,10 @@ module Enums
insufficient_bridge_permissions: 1_001, insufficient_bridge_permissions: 1_001,
downstream_bridge_project_not_found: 1_002, downstream_bridge_project_not_found: 1_002,
invalid_bridge_trigger: 1_003, invalid_bridge_trigger: 1_003,
bridge_pipeline_is_child_pipeline: 1_006, bridge_pipeline_is_child_pipeline: 1_006, # not used anymore, but cannot be deleted because of old data
downstream_pipeline_creation_failed: 1_007, downstream_pipeline_creation_failed: 1_007,
secrets_provider_not_found: 1_008 secrets_provider_not_found: 1_008,
reached_max_descendant_pipelines_depth: 1_009
} }
end end
end end
......
...@@ -20,7 +20,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated ...@@ -20,7 +20,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
insufficient_bridge_permissions: 'This job could not be executed because of insufficient permissions to create a downstream pipeline', insufficient_bridge_permissions: 'This job could not be executed because of insufficient permissions to create a downstream pipeline',
bridge_pipeline_is_child_pipeline: 'This job belongs to a child pipeline and cannot create further child pipelines', bridge_pipeline_is_child_pipeline: 'This job belongs to a child pipeline and cannot create further child pipelines',
downstream_pipeline_creation_failed: 'The downstream pipeline could not be created', downstream_pipeline_creation_failed: 'The downstream pipeline could not be created',
secrets_provider_not_found: 'The secrets provider can not be found' secrets_provider_not_found: 'The secrets provider can not be found',
reached_max_descendant_pipelines_depth: 'Maximum child pipeline depth has been reached'
}.freeze }.freeze
private_constant :CALLOUT_FAILURE_MESSAGES private_constant :CALLOUT_FAILURE_MESSAGES
......
...@@ -9,6 +9,8 @@ module Ci ...@@ -9,6 +9,8 @@ module Ci
DuplicateDownstreamPipelineError = Class.new(StandardError) DuplicateDownstreamPipelineError = Class.new(StandardError)
MAX_DESCENDANTS_DEPTH = 2
def execute(bridge) def execute(bridge)
@bridge = bridge @bridge = bridge
...@@ -75,10 +77,17 @@ module Ci ...@@ -75,10 +77,17 @@ module Ci
# TODO: Remove this condition if favour of model validation # TODO: Remove this condition if favour of model validation
# https://gitlab.com/gitlab-org/gitlab/issues/38338 # https://gitlab.com/gitlab-org/gitlab/issues/38338
if ::Gitlab::Ci::Features.child_of_child_pipeline_enabled?(project)
if has_max_descendants_depth?
@bridge.drop!(:reached_max_descendant_pipelines_depth)
return false
end
else
if @bridge.triggers_child_pipeline? && @bridge.pipeline.parent_pipeline.present? if @bridge.triggers_child_pipeline? && @bridge.pipeline.parent_pipeline.present?
@bridge.drop!(:bridge_pipeline_is_child_pipeline) @bridge.drop!(:bridge_pipeline_is_child_pipeline)
return false return false
end end
end
unless can_create_downstream_pipeline?(target_ref) unless can_create_downstream_pipeline?(target_ref)
@bridge.drop!(:insufficient_bridge_permissions) @bridge.drop!(:insufficient_bridge_permissions)
...@@ -108,5 +117,12 @@ module Ci ...@@ -108,5 +117,12 @@ module Ci
@bridge.downstream_project @bridge.downstream_project
end end
end end
def has_max_descendants_depth?
return false unless @bridge.triggers_child_pipeline?
ancestors_of_new_child = @bridge.pipeline.base_and_ancestors
ancestors_of_new_child.count > MAX_DESCENDANTS_DEPTH
end
end end
end end
---
name: ci_child_of_child_pipeline
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41102
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/243747
group: 'group::continuous integration'
type: development
default_enabled: false
...@@ -152,6 +152,10 @@ This is [resolved in GitLab 12.10](https://gitlab.com/gitlab-org/gitlab/-/issues ...@@ -152,6 +152,10 @@ This is [resolved in GitLab 12.10](https://gitlab.com/gitlab-org/gitlab/-/issues
## Limitations ## Limitations
A parent pipeline can trigger many child pipelines, but a child pipeline cannot trigger In GitLab 13.3 and older, a parent pipeline can trigger many child pipelines, but
further child pipelines. See the [related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/29651) those child pipeline cannot trigger further child pipelines.
for discussion on possible future improvements.
In GitLab 13.4 and newer, the [maximum depth of child pipelines was increased to 2](https://gitlab.com/gitlab-org/gitlab/-/issues/29651).
A parent pipeline can trigger many child pipelines. These child pipelines can trigger
their own child pipelines. This second layer of child pipelines cannot trigger further
child pipelines.
...@@ -74,6 +74,10 @@ module Gitlab ...@@ -74,6 +74,10 @@ module Gitlab
def self.coverage_report_view?(project) def self.coverage_report_view?(project)
::Feature.enabled?(:coverage_report_view, project) ::Feature.enabled?(:coverage_report_view, project)
end end
def self.child_of_child_pipeline_enabled?(project)
::Feature.enabled?(:ci_child_of_child_pipeline, project, default_enabled: false)
end
end end
end end
end end
......
# frozen_string_literal: true
module Gitlab
module Ci
class PipelineObjectHierarchy < ::Gitlab::ObjectHierarchy
private
def middle_table
::Ci::Sources::Pipeline.arel_table
end
def from_tables(cte)
[objects_table, cte.table, middle_table]
end
def parent_id_column(_cte)
middle_table[:source_pipeline_id]
end
def ancestor_conditions(cte)
middle_table[:source_pipeline_id].eq(objects_table[:id]).and(
middle_table[:pipeline_id].eq(cte.table[:id])
)
end
def descendant_conditions(cte)
middle_table[:pipeline_id].eq(objects_table[:id]).and(
middle_table[:source_pipeline_id].eq(cte.table[:id])
)
end
end
end
end
...@@ -25,7 +25,8 @@ module Gitlab ...@@ -25,7 +25,8 @@ module Gitlab
insufficient_bridge_permissions: 'no permissions to trigger downstream pipeline', insufficient_bridge_permissions: 'no permissions to trigger downstream pipeline',
bridge_pipeline_is_child_pipeline: 'creation of child pipeline not allowed from another child pipeline', bridge_pipeline_is_child_pipeline: 'creation of child pipeline not allowed from another child pipeline',
downstream_pipeline_creation_failed: 'downstream pipeline can not be created', downstream_pipeline_creation_failed: 'downstream pipeline can not be created',
secrets_provider_not_found: 'secrets provider can not be found' secrets_provider_not_found: 'secrets provider can not be found',
reached_max_descendant_pipelines_depth: 'reached maximum depth of child pipelines'
}.freeze }.freeze
private_constant :REASONS private_constant :REASONS
......
...@@ -133,8 +133,8 @@ module Gitlab ...@@ -133,8 +133,8 @@ module Gitlab
# Recursively get all the ancestors of the base set. # Recursively get all the ancestors of the base set.
parent_query = model parent_query = model
.from([objects_table, cte.table]) .from(from_tables(cte))
.where(objects_table[:id].eq(cte.table[:parent_id])) .where(ancestor_conditions(cte))
.except(:order) .except(:order)
if hierarchy_order if hierarchy_order
...@@ -148,7 +148,7 @@ module Gitlab ...@@ -148,7 +148,7 @@ module Gitlab
).where(cte.table[:tree_cycle].eq(false)) ).where(cte.table[:tree_cycle].eq(false))
end end
parent_query = parent_query.where(cte.table[:parent_id].not_eq(stop_id)) if stop_id parent_query = parent_query.where(parent_id_column(cte).not_eq(stop_id)) if stop_id
cte << parent_query cte << parent_query
cte cte
...@@ -166,8 +166,8 @@ module Gitlab ...@@ -166,8 +166,8 @@ module Gitlab
# Recursively get all the descendants of the base set. # Recursively get all the descendants of the base set.
descendants_query = model descendants_query = model
.from([objects_table, cte.table]) .from(from_tables(cte))
.where(objects_table[:parent_id].eq(cte.table[:id])) .where(descendant_conditions(cte))
.except(:order) .except(:order)
if with_depth if with_depth
...@@ -190,6 +190,22 @@ module Gitlab ...@@ -190,6 +190,22 @@ module Gitlab
model.arel_table model.arel_table
end end
def parent_id_column(cte)
cte.table[:parent_id]
end
def from_tables(cte)
[objects_table, cte.table]
end
def ancestor_conditions(cte)
objects_table[:id].eq(cte.table[:parent_id])
end
def descendant_conditions(cte)
objects_table[:parent_id].eq(cte.table[:id])
end
def read_only(relation) def read_only(relation)
# relations using a CTE are not safe to use with update_all as it will # relations using a CTE are not safe to use with update_all as it will
# throw away the CTE, hence we mark them as read-only. # throw away the CTE, hence we mark them as read-only.
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Ci::PipelineObjectHierarchy do
let_it_be(:project) { create_default(:project, :repository) }
let_it_be(:pipeline) { create(:ci_empty_pipeline, status: :created, project: project) }
let_it_be(:ancestor) { create(:ci_pipeline, project: pipeline.project) }
let_it_be(:parent) { create(:ci_pipeline, project: pipeline.project) }
let_it_be(:child) { create(:ci_pipeline, project: pipeline.project) }
let_it_be(:cousin_parent) { create(:ci_pipeline, project: pipeline.project) }
let_it_be(:cousin) { create(:ci_pipeline, project: pipeline.project) }
before_all do
create_source_relation(ancestor, parent)
create_source_relation(ancestor, cousin_parent)
create_source_relation(parent, child)
create_source_relation(cousin_parent, cousin)
end
describe '#base_and_ancestors' do
it 'includes the base and its ancestors' do
relation = described_class.new(::Ci::Pipeline.where(id: parent.id)).base_and_ancestors
expect(relation).to contain_exactly(ancestor, parent)
end
it 'can find ancestors upto a certain level' do
relation = described_class.new(::Ci::Pipeline.where(id: child.id)).base_and_ancestors(upto: ancestor.id)
expect(relation).to contain_exactly(parent, child)
end
describe 'hierarchy_order option' do
let(:relation) do
described_class.new(::Ci::Pipeline.where(id: child.id)).base_and_ancestors(hierarchy_order: hierarchy_order)
end
context ':asc' do
let(:hierarchy_order) { :asc }
it 'orders by child to ancestor' do
expect(relation).to eq([child, parent, ancestor])
end
end
context ':desc' do
let(:hierarchy_order) { :desc }
it 'orders by ancestor to child' do
expect(relation).to eq([ancestor, parent, child])
end
end
end
end
describe '#base_and_descendants' do
it 'includes the base and its descendants' do
relation = described_class.new(::Ci::Pipeline.where(id: parent.id)).base_and_descendants
expect(relation).to contain_exactly(parent, child)
end
context 'when with_depth is true' do
let(:relation) do
described_class.new(::Ci::Pipeline.where(id: ancestor.id)).base_and_descendants(with_depth: true)
end
it 'includes depth in the results' do
object_depths = {
ancestor.id => 1,
parent.id => 2,
cousin_parent.id => 2,
child.id => 3,
cousin.id => 3
}
relation.each do |object|
expect(object.depth).to eq(object_depths[object.id])
end
end
end
end
describe '#all_objects' do
it 'includes its ancestors and descendants' do
relation = described_class.new(::Ci::Pipeline.where(id: parent.id)).all_objects
expect(relation).to contain_exactly(ancestor, parent, child)
end
it 'returns all family tree' do
relation = described_class.new(
::Ci::Pipeline.where(id: child.id),
described_class.new(::Ci::Pipeline.where(id: child.id)).base_and_ancestors
).all_objects
expect(relation).to contain_exactly(ancestor, parent, cousin_parent, child, cousin)
end
end
private
def create_source_relation(parent, child)
create(:ci_sources_pipeline,
source_job: create(:ci_build, pipeline: parent),
source_project: parent.project,
pipeline: child,
project: child.project)
end
end
...@@ -2616,11 +2616,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -2616,11 +2616,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end end
describe '#same_family_pipeline_ids' do describe '#same_family_pipeline_ids' do
subject(:same_family_pipeline_ids) { pipeline.same_family_pipeline_ids } subject { pipeline.same_family_pipeline_ids.map(&:id) }
context 'when pipeline is not child nor parent' do context 'when pipeline is not child nor parent' do
it 'returns just the pipeline id' do it 'returns just the pipeline id' do
expect(same_family_pipeline_ids).to contain_exactly(pipeline.id) expect(subject).to contain_exactly(pipeline.id)
end end
end end
...@@ -2643,7 +2643,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -2643,7 +2643,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end end
it 'returns parent sibling and self ids' do it 'returns parent sibling and self ids' do
expect(same_family_pipeline_ids).to contain_exactly(parent.id, pipeline.id, sibling.id) expect(subject).to contain_exactly(parent.id, pipeline.id, sibling.id)
end end
end end
...@@ -2659,7 +2659,46 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -2659,7 +2659,46 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end end
it 'returns self and child ids' do it 'returns self and child ids' do
expect(same_family_pipeline_ids).to contain_exactly(pipeline.id, child.id) expect(subject).to contain_exactly(pipeline.id, child.id)
end
end
context 'when pipeline is a child of a child pipeline' do
let(:ancestor) { create(:ci_pipeline, project: pipeline.project) }
let(:parent) { create(:ci_pipeline, project: pipeline.project) }
let(:cousin_parent) { create(:ci_pipeline, project: pipeline.project) }
let(:cousin) { create(:ci_pipeline, project: pipeline.project) }
before do
create(:ci_sources_pipeline,
source_job: create(:ci_build, pipeline: ancestor),
source_project: ancestor.project,
pipeline: parent,
project: parent.project)
create(:ci_sources_pipeline,
source_job: create(:ci_build, pipeline: ancestor),
source_project: ancestor.project,
pipeline: cousin_parent,
project: cousin_parent.project)
create(:ci_sources_pipeline,
source_job: create(:ci_build, pipeline: parent),
source_project: parent.project,
pipeline: pipeline,
project: pipeline.project)
create(:ci_sources_pipeline,
source_job: create(:ci_build, pipeline: cousin_parent),
source_project: cousin_parent.project,
pipeline: cousin,
project: cousin.project)
end
it 'returns all family ids' do
expect(subject).to contain_exactly(
ancestor.id, parent.id, cousin_parent.id, cousin.id, pipeline.id
)
end end
end end
end end
......
...@@ -311,7 +311,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do ...@@ -311,7 +311,7 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end end
end end
context 'when upstream pipeline is a child pipeline' do context 'when upstream pipeline is a first descendant pipeline' do
let!(:pipeline_source) do let!(:pipeline_source) do
create(:ci_sources_pipeline, create(:ci_sources_pipeline,
source_pipeline: create(:ci_pipeline, project: upstream_pipeline.project), source_pipeline: create(:ci_pipeline, project: upstream_pipeline.project),
...@@ -323,6 +323,18 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do ...@@ -323,6 +323,18 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
upstream_pipeline.update!(source: :parent_pipeline) upstream_pipeline.update!(source: :parent_pipeline)
end end
it 'creates the pipeline' do
expect { service.execute(bridge) }
.to change { Ci::Pipeline.count }.by(1)
expect(bridge.reload).to be_success
end
context 'when FF ci_child_of_child_pipeline is disabled' do
before do
stub_feature_flags(ci_child_of_child_pipeline: false)
end
it 'does not create a further child pipeline' do it 'does not create a further child pipeline' do
expect { service.execute(bridge) } expect { service.execute(bridge) }
.not_to change { Ci::Pipeline.count } .not_to change { Ci::Pipeline.count }
...@@ -332,6 +344,35 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do ...@@ -332,6 +344,35 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
end end
end end
end end
context 'when upstream pipeline is a second descendant pipeline' do
let!(:pipeline_source) do
parent_of_upstream_pipeline = create(:ci_pipeline, project: upstream_pipeline.project)
create(:ci_sources_pipeline,
source_pipeline: create(:ci_pipeline, project: upstream_pipeline.project),
pipeline: parent_of_upstream_pipeline
)
create(:ci_sources_pipeline,
source_pipeline: parent_of_upstream_pipeline,
pipeline: upstream_pipeline
)
end
before do
upstream_pipeline.update!(source: :parent_pipeline)
end
it 'does not create a second descendant pipeline' do
expect { service.execute(bridge) }
.not_to change { Ci::Pipeline.count }
expect(bridge.reload).to be_failed
expect(bridge.failure_reason).to eq 'reached_max_descendant_pipelines_depth'
end
end
end
end end
context 'when downstream pipeline creation errors out' do context 'when downstream pipeline creation errors out' 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