Commit 94029df5 authored by Matija Čupić's avatar Matija Čupić Committed by Kamil Trzciński

Update source birdge status on pipeline completion

Mirrors the pipeline status to the source bridge when pipeline
completes.
parent 45a83f98
...@@ -176,6 +176,18 @@ Upstream pipelines take precedence over downstream ones. If there are two ...@@ -176,6 +176,18 @@ Upstream pipelines take precedence over downstream ones. If there are two
variables with the same name defined in both upstream and downstream projects, variables with the same name defined in both upstream and downstream projects,
the ones defined in the upstream project will take precedence. the ones defined in the upstream project will take precedence.
### Mirroring status from triggered pipeline
You can mirror the pipeline status from the triggered pipeline to the source
bridge job by using `strategy: depend`. For example:
```yaml
trigger_job:
trigger:
project: my/project
strategy: depend
```
### Mirroring status from upstream pipeline ### Mirroring status from upstream pipeline
You can mirror the pipeline status from an upstream pipeline to a bridge job by You can mirror the pipeline status from an upstream pipeline to a bridge job by
......
...@@ -83,6 +83,17 @@ module EE ...@@ -83,6 +83,17 @@ module EE
end end
end end
def inherit_status_from_downstream!(pipeline)
case pipeline.status
when 'success'
self.success!
when 'failed', 'canceled', 'skipped'
self.drop!
else
false
end
end
def target_user def target_user
self.user self.user
end end
...@@ -107,6 +118,12 @@ module EE ...@@ -107,6 +118,12 @@ module EE
options&.dig(:trigger, :branch) options&.dig(:trigger, :branch)
end end
def dependent?
strong_memoize(:dependent) do
options&.dig(:trigger, :strategy) == 'depend'
end
end
def downstream_variables def downstream_variables
scoped_variables.to_runner_variables.yield_self do |all_variables| scoped_variables.to_runner_variables.yield_self do |all_variables|
yaml_variables.to_a.map do |hash| yaml_variables.to_a.map do |hash|
......
...@@ -105,8 +105,18 @@ module EE ...@@ -105,8 +105,18 @@ module EE
end end
end end
after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline|
next unless pipeline.bridge_triggered?
next unless pipeline.bridge_waiting?
pipeline.run_after_commit do
::Ci::PipelineBridgeStatusWorker.perform_async(pipeline.id)
end
end
after_transition created: :pending do |pipeline| after_transition created: :pending do |pipeline|
next unless pipeline.bridge_triggered? next unless pipeline.bridge_triggered?
next if pipeline.bridge_waiting?
pipeline.update_bridge_status! pipeline.update_bridge_status!
end end
...@@ -117,6 +127,10 @@ module EE ...@@ -117,6 +127,10 @@ module EE
source_bridge.present? source_bridge.present?
end end
def bridge_waiting?
source_bridge.dependent?
end
def update_bridge_status! def update_bridge_status!
raise ArgumentError unless bridge_triggered? raise ArgumentError unless bridge_triggered?
raise BridgeStatusError unless source_bridge.active? raise BridgeStatusError unless source_bridge.active?
......
...@@ -4,6 +4,10 @@ module Ci ...@@ -4,6 +4,10 @@ module Ci
class PipelineBridgeStatusService < ::BaseService class PipelineBridgeStatusService < ::BaseService
def execute(pipeline) def execute(pipeline)
pipeline.downstream_bridges.each(&:inherit_status_from_upstream!) pipeline.downstream_bridges.each(&:inherit_status_from_upstream!)
if pipeline.bridge_triggered?
pipeline.source_bridge.inherit_status_from_downstream!(pipeline)
end
end end
end end
end end
---
title: Add status checking behaviors to pipeline triggers.
merge_request: 15580
author:
type: changed
...@@ -26,14 +26,15 @@ module EE ...@@ -26,14 +26,15 @@ module EE
include ::Gitlab::Config::Entry::Validatable include ::Gitlab::Config::Entry::Validatable
include ::Gitlab::Config::Entry::Attributable include ::Gitlab::Config::Entry::Attributable
ALLOWED_KEYS = %i[project branch].freeze ALLOWED_KEYS = %i[project branch strategy].freeze
attributes :project, :branch attributes :project, :branch, :strategy
validations do validations do
validates :config, presence: true validates :config, presence: true
validates :config, allowed_keys: ALLOWED_KEYS validates :config, allowed_keys: ALLOWED_KEYS
validates :project, presence: true validates :project, presence: true
validates :branch, type: String, allow_nil: true validates :branch, type: String, allow_nil: true
validates :strategy, type: String, inclusion: { in: %w[depend], message: 'should be depend' }, allow_nil: true
end end
end end
......
...@@ -34,8 +34,8 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -34,8 +34,8 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
end end
context 'when trigger is a hash' do context 'when trigger is a hash' do
context 'when branch is not provided' do context 'when branch is provided' do
let(:config) { { project: 'some/project' } } let(:config) { { project: 'some/project', branch: 'feature' } }
describe '#valid?' do describe '#valid?' do
it { is_expected.to be_valid } it { is_expected.to be_valid }
...@@ -43,22 +43,40 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do ...@@ -43,22 +43,40 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
describe '#value' do describe '#value' do
it 'is returns a trigger configuration hash' do it 'is returns a trigger configuration hash' do
expect(subject.value).to eq(project: 'some/project') expect(subject.value)
.to eq(project: 'some/project', branch: 'feature')
end end
end end
end end
context 'when branch is provided' do context 'when strategy is provided' do
let(:config) { { project: 'some/project', branch: 'feature' } } context 'when strategy is depend' do
let(:config) { { project: 'some/project', strategy: 'depend' } }
describe '#valid?' do describe '#valid?' do
it { is_expected.to be_valid } it { is_expected.to be_valid }
end
describe '#value' do
it 'is returns a trigger configuration hash' do
expect(subject.value)
.to eq(project: 'some/project', strategy: 'depend')
end
end
end end
describe '#value' do context 'when strategy is invalid' do
it 'is returns a trigger configuration hash' do let(:config) { { project: 'some/project', strategy: 'notdepend' } }
expect(subject.value)
.to eq(project: 'some/project', branch: 'feature') describe '#valid?' do
it { is_expected.not_to be_valid }
end
describe '#errors' do
it 'is returns an error about unknown config key' do
expect(subject.errors.first)
.to match /trigger strategy should be depend/
end
end end
end end
end end
......
...@@ -79,9 +79,19 @@ describe Ci::Bridge do ...@@ -79,9 +79,19 @@ describe Ci::Bridge do
end end
context 'when status is not supported' do context 'when status is not supported' do
let(:upstream_pipeline) { build(:ci_pipeline, status: 'preparing') } (::Ci::Pipeline::AVAILABLE_STATUSES - ::Ci::Pipeline.bridgeable_statuses).each do |status|
context "when status is #{status}" do
let(:upstream_pipeline) { build(:ci_pipeline, status: status) }
it { is_expected.to be false } it 'returns false' do
expect(subject).to eq(false)
end
it 'does not change the bridge status' do
expect { subject }.not_to change { bridge.status }.from('pending')
end
end
end
end end
context 'when status is supported' do context 'when status is supported' do
...@@ -92,15 +102,53 @@ describe Ci::Bridge do ...@@ -92,15 +102,53 @@ describe Ci::Bridge do
it 'inherits the upstream status' do it 'inherits the upstream status' do
expect { subject }.to change { bridge.status }.from('pending').to(status) expect { subject }.to change { bridge.status }.from('pending').to(status)
end end
end
end
end
end
describe '#inherit_status_from_downstream!' do
let(:downstream_pipeline) { build(:ci_pipeline, status: downstream_status) }
before do
bridge.status = 'pending'
create(:ci_sources_pipeline, pipeline: downstream_pipeline, source_job: bridge)
end
subject { bridge.inherit_status_from_downstream!(downstream_pipeline) }
context 'when status is not supported' do
(::Ci::Pipeline::AVAILABLE_STATUSES - ::Ci::Pipeline::COMPLETED_STATUSES).map(&:to_s).each do |status|
context "when status is #{status}" do
let(:downstream_status) { status }
it 'persists the bridge' do it 'returns false' do
subject expect(subject).to eq(false)
end
expect(bridge).to be_persisted it 'does not change the bridge status' do
expect { subject }.not_to change { bridge.status }.from('pending')
end end
end end
end end
end end
context 'when status is supported' do
using RSpec::Parameterized::TableSyntax
where(:downstream_status, :upstream_status) do
[
%w[success success],
*::Ci::Pipeline.completed_statuses.without(:success).map { |status| [status.to_s, 'failed'] }
]
end
with_them do
it 'inherits the downstream status' do
expect { subject }.to change { bridge.status }.from('pending').to(upstream_status)
end
end
end
end end
describe '#target_user' do describe '#target_user' do
...@@ -141,6 +189,20 @@ describe Ci::Bridge do ...@@ -141,6 +189,20 @@ describe Ci::Bridge do
end end
end end
describe '#dependent?' do
subject { bridge.dependent? }
context 'when bridge has strategy depend' do
let(:options) { { trigger: { project: 'my/project', strategy: 'depend' } } }
it { is_expected.to be true }
end
context 'when bridge does not have strategy depend' do
it { is_expected.to be false }
end
end
describe '#yaml_variables' do describe '#yaml_variables' do
it 'returns YAML variables' do it 'returns YAML variables' do
expect(bridge.yaml_variables) expect(bridge.yaml_variables)
......
...@@ -423,8 +423,10 @@ describe Ci::Pipeline do ...@@ -423,8 +423,10 @@ describe Ci::Pipeline do
end end
end end
end end
end
context 'when pipeline has bridged jobs' do describe 'state machine transitions' do
context 'when pipeline has downstream bridges' do
before do before do
pipeline.downstream_bridges << create(:ci_bridge) pipeline.downstream_bridges << create(:ci_bridge)
end end
...@@ -445,6 +447,32 @@ describe Ci::Pipeline do ...@@ -445,6 +447,32 @@ describe Ci::Pipeline do
end end
end end
end end
context 'when pipeline is bridge triggered' do
before do
pipeline.source_bridge = create(:ci_bridge)
end
context 'when source bridge is dependent on pipeline status' do
before do
allow(pipeline.source_bridge).to receive(:dependent?).and_return(true)
end
it 'schedules the pipeline bridge worker' do
expect(::Ci::PipelineBridgeStatusWorker).to receive(:perform_async)
pipeline.succeed!
end
end
context 'when source bridge is not dependent on pipeline status' do
it 'does not schedule the pipeline bridge worker' do
expect(::Ci::PipelineBridgeStatusWorker).not_to receive(:perform_async)
pipeline.succeed!
end
end
end
end end
describe '#ci_yaml_file_path' do describe '#ci_yaml_file_path' do
......
...@@ -3,70 +3,66 @@ ...@@ -3,70 +3,66 @@
require 'spec_helper' require 'spec_helper'
describe Ci::PipelineBridgeStatusService do describe Ci::PipelineBridgeStatusService do
let(:user) { create(:user) } let(:user) { build(:user) }
let(:project) { create(:project) } let(:project) { build(:project) }
let(:pipeline) { create(:ci_pipeline, status: status, project: project) } let(:pipeline) { build(:ci_pipeline, project: project) }
describe '#execute' do describe '#execute' do
subject { described_class.new(project, user).execute(pipeline) } subject { described_class.new(project, user).execute(pipeline) }
context 'when pipeline has bridged jobs' do context 'when pipeline has downstream bridges' do
let(:bridge) { create(:ci_bridge, status: 'pending') } let(:bridge) { build(:ci_bridge) }
before do before do
pipeline.downstream_bridges << bridge pipeline.downstream_bridges << bridge
end end
context 'when pipeline has the same status as the bridge' do it 'calls inherit_status_from_upstream on downstream bridges' do
let(:status) { 'running' } expect(bridge).to receive(:inherit_status_from_upstream!)
before do subject
bridge.status = 'running' end
end end
it 'does not update the bridge status' do context 'when pipeline has upstream bridge' do
expect { subject }.not_to change { bridge.status } let(:bridge) { build(:ci_bridge) }
end
it 'does not save the bridge' do before do
expect(bridge).not_to receive(:save!) pipeline.source_bridge = bridge
end
end end
context 'when pipeline starts running' do it 'calls inherit_status_from_downstream on upstream bridge' do
let(:status) { 'running' } expect(bridge).to receive(:inherit_status_from_downstream!).with(pipeline)
it 'updates the bridge status with the pipeline status' do subject
expect { subject }.to change { bridge.status }.from('pending').to('running') end
end end
context 'when pipeline has both downstream and upstream bridge' do
let(:downstream_bridge) { build(:ci_bridge) }
let(:upstream_bridge) { build(:ci_bridge) }
it 'persists the status change' do before do
expect(bridge).to be_persisted pipeline.downstream_bridges << downstream_bridge
end pipeline.source_bridge = upstream_bridge
end end
context 'when pipeline succeeds' do it 'only calls inherit_status_from_downstream on upstream bridge' do
let(:status) { 'success' } allow(downstream_bridge).to receive(:inherit_status_from_upstream!)
it 'updates the bridge status with the pipeline status' do expect(upstream_bridge).to receive(:inherit_status_from_downstream!).with(pipeline)
expect { subject }.to change { bridge.status }.from('pending').to('success') expect(downstream_bridge).not_to receive(:inherit_status_from_downstream!)
end
it 'persists the status change' do subject
expect(bridge).to be_persisted
end
end end
context 'when pipeline gets blocked' do it 'only calls inherit_status_from_upstream on downstream bridge' do
let(:status) { 'manual' } allow(upstream_bridge).to receive(:inherit_status_from_downstream!)
it 'updates the bridge status with the pipeline status' do expect(upstream_bridge).not_to receive(:inherit_status_from_upstream!)
expect { subject }.to change { bridge.status }.from('pending').to('manual') expect(downstream_bridge).to receive(:inherit_status_from_upstream!)
end
it 'persists the status change' do subject
expect(bridge).to be_persisted
end
end end
end end
end end
......
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