Merge branch 'mc/feature/downstream-pipeline-tracking' into 'master'

Add status checking behaviors to pipeline triggers

Closes #11238

See merge request gitlab-org/gitlab-ee!15580

doc/ci/multi_project_pipelines.md

@@ -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,
 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
 
 You can mirror the pipeline status from an upstream pipeline to a bridge job by

ee/app/models/ee/ci/bridge.rb

@@ -83,6 +83,17 @@ module EE
         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
         self.user
       end
@@ -107,6 +118,12 @@ module EE
         options&.dig(:trigger, :branch)
       end
 
+      def dependent?
+        strong_memoize(:dependent) do
+          options&.dig(:trigger, :strategy) == 'depend'
+        end
+      end
+
       def downstream_variables
         scoped_variables.to_runner_variables.yield_self do |all_variables|
           yaml_variables.to_a.map do |hash|

ee/app/models/ee/ci/pipeline.rb

@@ -105,8 +105,18 @@ module EE
             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|
             next unless pipeline.bridge_triggered?
+            next if pipeline.bridge_waiting?
 
             pipeline.update_bridge_status!
           end
@@ -117,6 +127,10 @@ module EE
         source_bridge.present?
       end
 
+      def bridge_waiting?
+        source_bridge.dependent?
+      end
+
       def update_bridge_status!
         raise ArgumentError unless bridge_triggered?
         raise BridgeStatusError unless source_bridge.active?

ee/app/services/ci/pipeline_bridge_status_service.rb

@@ -4,6 +4,10 @@ module Ci
   class PipelineBridgeStatusService < ::BaseService
     def execute(pipeline)
       pipeline.downstream_bridges.each(&:inherit_status_from_upstream!)
+
+      if pipeline.bridge_triggered?
+        pipeline.source_bridge.inherit_status_from_downstream!(pipeline)
+      end
     end
   end
 end

ee/changelogs/unreleased/mc-feature-downstream-pipeline-tracking.yml

+---
+title: Add status checking behaviors to pipeline triggers.
+merge_request: 15580
+author:
+type: changed

ee/lib/ee/gitlab/ci/config/entry/trigger.rb

@@ -26,14 +26,15 @@ module EE
               include ::Gitlab::Config::Entry::Validatable
               include ::Gitlab::Config::Entry::Attributable
 
-              ALLOWED_KEYS = %i[project branch].freeze
-              attributes :project, :branch
+              ALLOWED_KEYS = %i[project branch strategy].freeze
+              attributes :project, :branch, :strategy
 
               validations do
                 validates :config, presence: true
                 validates :config, allowed_keys: ALLOWED_KEYS
                 validates :project, presence: 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
 

ee/spec/lib/ee/gitlab/ci/config/entry/trigger_spec.rb

@@ -34,8 +34,8 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
   end
 
   context 'when trigger is a hash' do
-    context 'when branch is not provided' do
-      let(:config) { { project: 'some/project' } }
+    context 'when branch is provided' do
+      let(:config) { { project: 'some/project', branch: 'feature' } }
 
       describe '#valid?' do
         it { is_expected.to be_valid }
@@ -43,22 +43,40 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
 
       describe '#value' 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
 
-    context 'when branch is provided' do
-      let(:config) { { project: 'some/project', branch: 'feature' } }
+    context 'when strategy is provided' do
+      context 'when strategy is depend' do
+        let(:config) { { project: 'some/project', strategy: 'depend' } }
 
-      describe '#valid?' do
-        it { is_expected.to be_valid }
+        describe '#valid?' do
+          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
 
-      describe '#value' do
-        it 'is returns a trigger configuration hash' do
-          expect(subject.value)
-            .to eq(project: 'some/project', branch: 'feature')
+      context 'when strategy is invalid' do
+        let(:config) { { project: 'some/project', strategy: 'notdepend' } }
+
+        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

ee/spec/models/ci/bridge_spec.rb

@@ -79,9 +79,19 @@ describe Ci::Bridge do
     end
 
     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
 
     context 'when status is supported' do
@@ -92,15 +102,53 @@ describe Ci::Bridge do
           it 'inherits the upstream status' do
             expect { subject }.to change { bridge.status }.from('pending').to(status)
           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
-            subject
+          it 'returns false' do
+            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
+
+    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
 
   describe '#target_user' do
@@ -141,6 +189,20 @@ describe Ci::Bridge do
     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
     it 'returns YAML variables' do
       expect(bridge.yaml_variables)

ee/spec/models/ci/pipeline_spec.rb

@@ -423,8 +423,10 @@ describe Ci::Pipeline do
         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
         pipeline.downstream_bridges << create(:ci_bridge)
       end
@@ -445,6 +447,32 @@ describe Ci::Pipeline do
         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
 
   describe '#ci_yaml_file_path' do

ee/spec/services/ci/pipeline_bridge_status_service_spec.rb

@@ -3,70 +3,66 @@
 require 'spec_helper'
 
 describe Ci::PipelineBridgeStatusService do
-  let(:user) { create(:user) }
-  let(:project) { create(:project) }
-  let(:pipeline) { create(:ci_pipeline, status: status, project: project) }
+  let(:user) { build(:user) }
+  let(:project) { build(:project) }
+  let(:pipeline) { build(:ci_pipeline, project: project) }
 
   describe '#execute' do
     subject { described_class.new(project, user).execute(pipeline) }
 
-    context 'when pipeline has bridged jobs' do
-      let(:bridge) { create(:ci_bridge, status: 'pending') }
+    context 'when pipeline has downstream bridges' do
+      let(:bridge) { build(:ci_bridge) }
 
       before do
         pipeline.downstream_bridges << bridge
       end
 
-      context 'when pipeline has the same status as the bridge' do
-        let(:status) { 'running' }
+      it 'calls inherit_status_from_upstream on downstream bridges' do
+        expect(bridge).to receive(:inherit_status_from_upstream!)
 
-        before do
-          bridge.status = 'running'
-        end
+        subject
+      end
+    end
 
-        it 'does not update the bridge status' do
-          expect { subject }.not_to change { bridge.status }
-        end
+    context 'when pipeline has upstream bridge' do
+      let(:bridge) { build(:ci_bridge) }
 
-        it 'does not save the bridge' do
-          expect(bridge).not_to receive(:save!)
-        end
+      before do
+        pipeline.source_bridge = bridge
       end
 
-      context 'when pipeline starts running' do
-        let(:status) { 'running' }
+      it 'calls inherit_status_from_downstream on upstream bridge' do
+        expect(bridge).to receive(:inherit_status_from_downstream!).with(pipeline)
 
-        it 'updates the bridge status with the pipeline status' do
-          expect { subject }.to change { bridge.status }.from('pending').to('running')
-        end
+        subject
+      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
-          expect(bridge).to be_persisted
-        end
+      before do
+        pipeline.downstream_bridges << downstream_bridge
+        pipeline.source_bridge = upstream_bridge
       end
 
-      context 'when pipeline succeeds' do
-        let(:status) { 'success' }
+      it 'only calls inherit_status_from_downstream on upstream bridge' do
+        allow(downstream_bridge).to receive(:inherit_status_from_upstream!)
 
-        it 'updates the bridge status with the pipeline status' do
-          expect { subject }.to change { bridge.status }.from('pending').to('success')
-        end
+        expect(upstream_bridge).to receive(:inherit_status_from_downstream!).with(pipeline)
+        expect(downstream_bridge).not_to receive(:inherit_status_from_downstream!)
 
-        it 'persists the status change' do
-          expect(bridge).to be_persisted
-        end
+        subject
       end
 
-      context 'when pipeline gets blocked' do
-        let(:status) { 'manual' }
+      it 'only calls inherit_status_from_upstream on downstream bridge' do
+        allow(upstream_bridge).to receive(:inherit_status_from_downstream!)
 
-        it 'updates the bridge status with the pipeline status' do
-          expect { subject }.to change { bridge.status }.from('pending').to('manual')
-        end
+        expect(upstream_bridge).not_to receive(:inherit_status_from_upstream!)
+        expect(downstream_bridge).to receive(:inherit_status_from_upstream!)
 
-        it 'persists the status change' do
-          expect(bridge).to be_persisted
-        end
+        subject
       end
     end
   end