Merge branch 'port-trigger-pipeline-behavior-to-core' into 'master'

Port `trigger` keyword to Core (2/4) - move Bridge-Pipeline interactions

See merge request gitlab-org/gitlab!24382

app/models/ci/pipeline.rb

@@ -16,6 +16,8 @@ module Ci
     include FromUnion
     include UpdatedAtFilterable
 
+    BridgeStatusError = Class.new(StandardError)
+
     sha_attribute :source_sha
     sha_attribute :target_sha
 
@@ -64,6 +66,7 @@ module Ci
     has_one :triggered_by_pipeline, through: :source_pipeline, source: :source_pipeline
     has_one :parent_pipeline, -> { merge(Ci::Sources::Pipeline.same_project) }, through: :source_pipeline, source: :source_pipeline
     has_one :source_job, through: :source_pipeline, source: :source_job
+    has_one :source_bridge, through: :source_pipeline, source: :source_bridge
 
     has_one :pipeline_config, class_name: 'Ci::PipelineConfig', inverse_of: :pipeline
 
@@ -204,6 +207,22 @@ module Ci
         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
+
       after_transition any => [:success, :failed] do |pipeline|
         pipeline.run_after_commit do
           PipelineNotificationWorker.perform_async(pipeline.id)
@@ -722,6 +741,21 @@ module Ci
       end
     end
 
+    def update_bridge_status!
+      raise ArgumentError unless bridge_triggered?
+      raise BridgeStatusError unless source_bridge.active?
+
+      source_bridge.success!
+    end
+
+    def bridge_triggered?
+      source_bridge.present?
+    end
+
+    def bridge_waiting?
+      source_bridge&.dependent?
+    end
+
     def child?
       parent_pipeline.present?
     end

app/models/ci/sources/pipeline.rb

@@ -10,6 +10,7 @@ module Ci
 
       belongs_to :source_project, class_name: "Project", foreign_key: :source_project_id
       belongs_to :source_job, class_name: "CommitStatus", foreign_key: :source_job_id
+      belongs_to :source_bridge, class_name: "Ci::Bridge", foreign_key: :source_job_id
       belongs_to :source_pipeline, class_name: "Ci::Pipeline", foreign_key: :source_pipeline_id
 
       validates :project, presence: true
@@ -23,5 +24,3 @@ module Ci
     end
   end
 end
-
-::Ci::Sources::Pipeline.prepend_if_ee('::EE::Ci::Sources::Pipeline')

ee/app/services/ci/pipeline_bridge_status_service.rb → app/services/ci/pipeline_bridge_status_service.rb

@@ -3,11 +3,11 @@
 module Ci
   class PipelineBridgeStatusService < ::BaseService
     def execute(pipeline)
-      pipeline.downstream_bridges.each(&:inherit_status_from_upstream!)
+      return unless pipeline.bridge_triggered?
 
-      if pipeline.bridge_triggered?
-        pipeline.source_bridge.inherit_status_from_downstream!(pipeline)
-      end
+      pipeline.source_bridge.inherit_status_from_downstream!(pipeline)
     end
   end
 end
+
+Ci::PipelineBridgeStatusService.prepend_if_ee('EE::Ci::PipelineBridgeStatusService')

app/workers/all_queues.yml

@@ -531,6 +531,12 @@
   :latency_sensitive: 
   :resource_boundary: :unknown
   :weight: 3
+- :name: pipeline_default:ci_pipeline_bridge_status
+  :feature_category: :continuous_integration
+  :has_external_dependencies: 
+  :latency_sensitive: true
+  :resource_boundary: :cpu
+  :weight: 3
 - :name: pipeline_default:pipeline_metrics
   :feature_category: :continuous_integration
   :has_external_dependencies: 

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

@@ -6,8 +6,6 @@ module EE
       extend ActiveSupport::Concern
       extend ::Gitlab::Utils::Override
 
-      BridgeStatusError = Class.new(StandardError)
-
       EE_FAILURE_REASONS = {
         activity_limit_exceeded: 20,
         size_limit_exceeded: 21
@@ -23,11 +21,10 @@ module EE
         has_many :auto_canceled_pipelines, class_name: 'Ci::Pipeline', foreign_key: 'auto_canceled_by_id'
         has_many :auto_canceled_jobs, class_name: 'CommitStatus', foreign_key: 'auto_canceled_by_id'
 
+        # Subscriptions to this pipeline
         has_many :downstream_bridges, class_name: '::Ci::Bridge', foreign_key: :upstream_pipeline_id
         has_many :security_scans, class_name: 'Security::Scan', through: :builds
 
-        has_one :source_bridge, through: :source_pipeline, source: :source_bridge
-
         # Legacy way to fetch security reports based on job name. This has been replaced by the reports feature.
         scope :with_legacy_security_reports, -> do
           joins(:artifacts).where(ci_builds: { name: %w[sast dependency_scanning sast:container container_scanning dast] })
@@ -68,44 +65,13 @@ module EE
               ::Ci::PipelineBridgeStatusWorker.perform_async(pipeline.id)
             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
         end
       end
 
-      def bridge_triggered?
-        source_bridge.present?
-      end
-
-      def bridge_waiting?
-        source_bridge&.dependent?
-      end
-
       def retryable?
         !merge_train_pipeline? && super
       end
 
-      def update_bridge_status!
-        raise ArgumentError unless bridge_triggered?
-        raise BridgeStatusError unless source_bridge.active?
-
-        source_bridge.success!
-      end
-
       def batch_lookup_report_artifact_for_file_type(file_type)
         return unless available_licensed_report_type?(file_type)
 

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

-# frozen_string_literal: true
-
-module EE
-  module Ci
-    module Sources
-      module Pipeline
-        extend ActiveSupport::Concern
-
-        prepended do
-          belongs_to :source_bridge,
-            class_name: "Ci::Bridge",
-            foreign_key: :source_job_id
-        end
-      end
-    end
-  end
-end

ee/app/services/ee/ci/pipeline_bridge_status_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Ci
+    module PipelineBridgeStatusService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute(pipeline)
+        pipeline.downstream_bridges.each(&:inherit_status_from_upstream!)
+
+        super
+      end
+    end
+  end
+end

ee/app/workers/all_queues.yml

@@ -345,12 +345,6 @@
   :latency_sensitive: 
   :resource_boundary: :cpu
   :weight: 3
-- :name: pipeline_default:ci_pipeline_bridge_status
-  :feature_category: :continuous_integration
-  :has_external_dependencies: 
-  :latency_sensitive: true
-  :resource_boundary: :cpu
-  :weight: 3
 - :name: pipeline_default:store_security_reports
   :feature_category: :continuous_integration
   :has_external_dependencies: 

ee/spec/models/ci/pipeline_spec.rb

@@ -351,84 +351,6 @@ describe Ci::Pipeline do
     end
   end
 
-  describe 'upstream status interactions' do
-    context 'when a pipeline has an upstream status' do
-      context 'when an upstream status is a bridge' do
-        let(:bridge) { create(:ci_bridge, status: :pending) }
-
-        before do
-          create(:ci_sources_pipeline, pipeline: pipeline, source_job: bridge)
-        end
-
-        describe '#bridge_triggered?' do
-          it 'is a pipeline triggered by a bridge' do
-            expect(pipeline).to be_bridge_triggered
-          end
-        end
-
-        describe '#source_job' do
-          it 'has a correct source job' do
-            expect(pipeline.source_job).to eq bridge
-          end
-        end
-
-        describe '#source_bridge' do
-          it 'has a correct bridge source' do
-            expect(pipeline.source_bridge).to eq bridge
-          end
-        end
-
-        describe '#update_bridge_status!' do
-          it 'can update bridge status if it is running' do
-            pipeline.update_bridge_status!
-
-            expect(bridge.reload).to be_success
-          end
-
-          it 'can not update bridge status if is not active' do
-            bridge.success!
-
-            expect { pipeline.update_bridge_status! }
-              .to raise_error EE::Ci::Pipeline::BridgeStatusError
-          end
-        end
-      end
-
-      context 'when an upstream status is a build' do
-        let(:build) { create(:ci_build) }
-
-        before do
-          create(:ci_sources_pipeline, pipeline: pipeline, source_job: build)
-        end
-
-        describe '#bridge_triggered?' do
-          it 'is a pipeline that has not been triggered by a bridge' do
-            expect(pipeline).not_to be_bridge_triggered
-          end
-        end
-
-        describe '#source_job' do
-          it 'has a correct source job' do
-            expect(pipeline.source_job).to eq build
-          end
-        end
-
-        describe '#source_bridge' do
-          it 'does not have a bridge source' do
-            expect(pipeline.source_bridge).to be_nil
-          end
-        end
-
-        describe '#update_bridge_status!' do
-          it 'can not update upstream job status' do
-            expect { pipeline.update_bridge_status! }
-              .to raise_error ArgumentError
-          end
-        end
-      end
-    end
-  end
-
   describe 'state machine transitions' do
     context 'when pipeline has downstream bridges' do
       before do
@@ -451,32 +373,6 @@ 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 '#latest_merge_request_pipeline?' do

ee/spec/models/ci/sources/pipeline_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Ci::Sources::Pipeline do
-  it { is_expected.to belong_to(:source_bridge) }
-end

ee/spec/services/ci/pipeline_bridge_status_service_spec.rb

@@ -24,20 +24,6 @@ describe Ci::PipelineBridgeStatusService do
       end
     end
 
-    context 'when pipeline has upstream bridge' do
-      let(:bridge) { build(:ci_bridge) }
-
-      before do
-        pipeline.source_bridge = bridge
-      end
-
-      it 'calls inherit_status_from_downstream on upstream bridge' do
-        expect(bridge).to receive(:inherit_status_from_downstream!).with(pipeline)
-
-        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) }

spec/models/ci/pipeline_spec.rb

@@ -1210,6 +1210,32 @@ describe Ci::Pipeline, :mailer do
         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
+
       def auto_devops_pipelines_completed_total(status)
         Gitlab::Metrics.counter(:auto_devops_pipelines_completed_total, 'Number of completed auto devops pipelines').get(status: status)
       end
@@ -2883,4 +2909,82 @@ describe Ci::Pipeline, :mailer do
       end
     end
   end
+
+  describe 'upstream status interactions' do
+    context 'when a pipeline has an upstream status' do
+      context 'when an upstream status is a bridge' do
+        let(:bridge) { create(:ci_bridge, status: :pending) }
+
+        before do
+          create(:ci_sources_pipeline, pipeline: pipeline, source_job: bridge)
+        end
+
+        describe '#bridge_triggered?' do
+          it 'is a pipeline triggered by a bridge' do
+            expect(pipeline).to be_bridge_triggered
+          end
+        end
+
+        describe '#source_job' do
+          it 'has a correct source job' do
+            expect(pipeline.source_job).to eq bridge
+          end
+        end
+
+        describe '#source_bridge' do
+          it 'has a correct bridge source' do
+            expect(pipeline.source_bridge).to eq bridge
+          end
+        end
+
+        describe '#update_bridge_status!' do
+          it 'can update bridge status if it is running' do
+            pipeline.update_bridge_status!
+
+            expect(bridge.reload).to be_success
+          end
+
+          it 'can not update bridge status if is not active' do
+            bridge.success!
+
+            expect { pipeline.update_bridge_status! }
+              .to raise_error Ci::Pipeline::BridgeStatusError
+          end
+        end
+      end
+
+      context 'when an upstream status is a build' do
+        let(:build) { create(:ci_build) }
+
+        before do
+          create(:ci_sources_pipeline, pipeline: pipeline, source_job: build)
+        end
+
+        describe '#bridge_triggered?' do
+          it 'is a pipeline that has not been triggered by a bridge' do
+            expect(pipeline).not_to be_bridge_triggered
+          end
+        end
+
+        describe '#source_job' do
+          it 'has a correct source job' do
+            expect(pipeline.source_job).to eq build
+          end
+        end
+
+        describe '#source_bridge' do
+          it 'does not have a bridge source' do
+            expect(pipeline.source_bridge).to be_nil
+          end
+        end
+
+        describe '#update_bridge_status!' do
+          it 'can not update upstream job status' do
+            expect { pipeline.update_bridge_status! }
+              .to raise_error ArgumentError
+          end
+        end
+      end
+    end
+  end
 end

spec/models/ci/sources/pipeline_spec.rb

@@ -8,6 +8,7 @@ describe Ci::Sources::Pipeline do
 
   it { is_expected.to belong_to(:source_project) }
   it { is_expected.to belong_to(:source_job) }
+  it { is_expected.to belong_to(:source_bridge) }
   it { is_expected.to belong_to(:source_pipeline) }
 
   it { is_expected.to validate_presence_of(:project) }

spec/services/ci/pipeline_bridge_status_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::PipelineBridgeStatusService do
+  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 upstream bridge' do
+      let(:bridge) { build(:ci_bridge) }
+
+      before do
+        pipeline.source_bridge = bridge
+      end
+
+      it 'calls inherit_status_from_downstream on upstream bridge' do
+        expect(bridge).to receive(:inherit_status_from_downstream!).with(pipeline)
+
+        subject
+      end
+    end
+  end
+end