Merge branch '212435-fix-upstream-pipeline-result' into 'master'

Fix upstream pipeline status when strategy dependent

See merge request gitlab-org/gitlab!41930

app/models/ci/bridge.rb

@@ -35,6 +35,10 @@ module Ci
         end
       end
 
+      event :pending do
+        transition all => :pending
+      end
+
       event :manual do
         transition all => :manual
       end

app/models/ci/pipeline.rb

@@ -237,7 +237,6 @@ module Ci
       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
@@ -1072,6 +1071,18 @@ module Ci
         .base_and_ancestors
     end
 
+    # We need `base_and_ancestors` in a specific order to "break" when needed.
+    # If we use `find_each`, then the order is broken.
+    # rubocop:disable Rails/FindEach
+    def reset_ancestor_bridges!
+      base_and_ancestors.includes(:source_bridge).each do |pipeline|
+        break unless pipeline.bridge_waiting?
+
+        pipeline.source_bridge.pending!
+      end
+    end
+    # rubocop:enable Rails/FindEach
+
     private
 
     def add_message(severity, content)

app/services/ci/retry_build_service.rb

@@ -15,6 +15,7 @@ module Ci
 
       reprocess!(build).tap do |new_build|
         mark_subsequent_stages_as_processable(build)
+        build.pipeline.reset_ancestor_bridges!
 
         Gitlab::OptimisticLocking.retry_lock(new_build, &:enqueue)
 

app/services/ci/retry_pipeline_service.rb

@@ -26,6 +26,8 @@ module Ci
         retry_optimistic_lock(skipped) { |build| build.process }
       end
 
+      pipeline.reset_ancestor_bridges!
+
       MergeRequests::AddTodoWhenBuildFailsService
         .new(project, current_user)
         .close_all(pipeline)

changelogs/unreleased/212435-fix-upstream-pipeline-result.yml

+---
+title: Fix upstream pipeline status when strategy dependent
+merge_request: 41930
+author:
+type: fixed

spec/factories/ci/bridge.rb

@@ -58,5 +58,9 @@ FactoryBot.define do
       started
       status { 'skipped' }
     end
+
+    trait :strategy_depend do
+      options { { trigger: { strategy: 'depend' } } }
+    end
   end
 end

spec/models/ci/pipeline_spec.rb

@@ -1360,34 +1360,126 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
         end
       end
 
-      context 'when pipeline is bridge triggered' do
-        before do
-          pipeline.source_bridge = create(:ci_bridge)
+      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
+    end
+
+    describe 'bridge triggered pipeline' do
+      shared_examples 'upstream downstream pipeline' do
+        let!(:source_pipeline) { create(:ci_sources_pipeline, pipeline: downstream_pipeline, source_job: bridge) }
+        let!(:job) { downstream_pipeline.builds.first }
 
         context 'when source bridge is dependent on pipeline status' do
+          let!(:bridge) { create(:ci_bridge, :strategy_depend, pipeline: upstream_pipeline) }
+
+          it 'schedules the pipeline bridge worker' do
+            expect(::Ci::PipelineBridgeStatusWorker).to receive(:perform_async).with(downstream_pipeline.id)
+
+            downstream_pipeline.succeed!
+          end
+
+          context 'when the downstream pipeline first fails then retries and succeeds' do
+            it 'makes the upstream pipeline successful' do
+              Sidekiq::Testing.inline! { job.drop! }
+
+              expect(downstream_pipeline.reload).to be_failed
+              expect(upstream_pipeline.reload).to be_failed
+
+              Sidekiq::Testing.inline! do
+                new_job = Ci::Build.retry(job, project.users.first)
+
+                expect(downstream_pipeline.reload).to be_running
+                expect(upstream_pipeline.reload).to be_running
+
+                new_job.success!
+              end
+
+              expect(downstream_pipeline.reload).to be_success
+              expect(upstream_pipeline.reload).to be_success
+            end
+          end
+
+          context 'when the downstream pipeline first succeeds then retries and fails' do
+            it 'makes the upstream pipeline failed' do
+              Sidekiq::Testing.inline! { job.success! }
+
+              expect(downstream_pipeline.reload).to be_success
+              expect(upstream_pipeline.reload).to be_success
+
+              Sidekiq::Testing.inline! do
+                new_job = Ci::Build.retry(job, project.users.first)
+
+                expect(downstream_pipeline.reload).to be_running
+                expect(upstream_pipeline.reload).to be_running
+
+                new_job.drop!
+              end
+
+              expect(downstream_pipeline.reload).to be_failed
+              expect(upstream_pipeline.reload).to be_failed
+            end
+          end
+
+          context 'when the upstream pipeline has another dependent upstream pipeline' do
+            let!(:upstream_of_upstream_pipeline) { create(:ci_pipeline) }
+
             before do
-            allow(pipeline.source_bridge).to receive(:dependent?).and_return(true)
+              upstream_bridge = create(:ci_bridge, :strategy_depend, pipeline: upstream_of_upstream_pipeline)
+              create(:ci_sources_pipeline, pipeline: upstream_pipeline,
+                                           source_job: upstream_bridge)
             end
 
-          it 'schedules the pipeline bridge worker' do
-            expect(::Ci::PipelineBridgeStatusWorker).to receive(:perform_async)
+            context 'when the downstream pipeline first fails then retries and succeeds' do
+              it 'makes upstream pipelines successful' do
+                Sidekiq::Testing.inline! { job.drop! }
 
-            pipeline.succeed!
+                expect(downstream_pipeline.reload).to be_failed
+                expect(upstream_pipeline.reload).to be_failed
+                expect(upstream_of_upstream_pipeline.reload).to be_failed
+
+                Sidekiq::Testing.inline! do
+                  new_job = Ci::Build.retry(job, project.users.first)
+
+                  expect(downstream_pipeline.reload).to be_running
+                  expect(upstream_pipeline.reload).to be_running
+                  expect(upstream_of_upstream_pipeline.reload).to be_running
+
+                  new_job.success!
+                end
+
+                expect(downstream_pipeline.reload).to be_success
+                expect(upstream_pipeline.reload).to be_success
+                expect(upstream_of_upstream_pipeline.reload).to be_success
+              end
+            end
           end
         end
 
         context 'when source bridge is not dependent on pipeline status' do
+          let!(:bridge) { create(:ci_bridge, pipeline: upstream_pipeline) }
+
           it 'does not schedule the pipeline bridge worker' do
             expect(::Ci::PipelineBridgeStatusWorker).not_to receive(:perform_async)
 
-            pipeline.succeed!
+            downstream_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)
+      context 'multi-project pipelines' do
+        let!(:downstream_project) { create(:project, :repository) }
+        let!(:upstream_pipeline) { create(:ci_pipeline, project: project) }
+        let!(:downstream_pipeline) { create(:ci_pipeline, :with_job, project: downstream_project) }
+
+        it_behaves_like 'upstream downstream pipeline'
+      end
+
+      context 'parent-child pipelines' do
+        let!(:upstream_pipeline) { create(:ci_pipeline, project: project) }
+        let!(:downstream_pipeline) { create(:ci_pipeline, :with_job, project: project) }
+
+        it_behaves_like 'upstream downstream pipeline'
       end
     end
 
@@ -3611,4 +3703,65 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
       end
     end
   end
+
+  describe 'reset_ancestor_bridges!' do
+    context 'when the pipeline is a child pipeline and the bridge is depended' do
+      let!(:parent_pipeline) { create(:ci_pipeline, project: project) }
+      let!(:bridge) { create_bridge(parent_pipeline, pipeline, true) }
+
+      it 'marks source bridge as pending' do
+        pipeline.reset_ancestor_bridges!
+
+        expect(bridge.reload).to be_pending
+      end
+
+      context 'when the parent pipeline has a dependent upstream pipeline' do
+        let!(:upstream_bridge) do
+          create_bridge(create(:ci_pipeline, project: create(:project)), parent_pipeline, true)
+        end
+
+        it 'marks all source bridges as pending' do
+          pipeline.reset_ancestor_bridges!
+
+          expect(bridge.reload).to be_pending
+          expect(upstream_bridge.reload).to be_pending
+        end
+      end
+    end
+
+    context 'when the pipeline is a child pipeline and the bridge is not depended' do
+      let!(:parent_pipeline) { create(:ci_pipeline, project: project) }
+      let!(:bridge) { create_bridge(parent_pipeline, pipeline, false) }
+
+      it 'does not touch source bridge' do
+        pipeline.reset_ancestor_bridges!
+
+        expect(bridge.reload).to be_success
+      end
+
+      context 'when the parent pipeline has a dependent upstream pipeline' do
+        let!(:upstream_bridge) do
+          create_bridge(create(:ci_pipeline, project: create(:project)), parent_pipeline, true)
+        end
+
+        it 'does not touch any source bridge' do
+          pipeline.reset_ancestor_bridges!
+
+          expect(bridge.reload).to be_success
+          expect(upstream_bridge.reload).to be_success
+        end
+      end
+    end
+
+    private
+
+    def create_bridge(upstream, downstream, depend = false)
+      options = depend ? { trigger: { strategy: 'depend' } } : {}
+
+      bridge = create(:ci_bridge, pipeline: upstream, status: 'success', options: options)
+      create(:ci_sources_pipeline, pipeline: downstream, source_job: bridge)
+
+      bridge
+    end
+  end
 end

spec/services/ci/retry_build_service_spec.rb

@@ -232,6 +232,19 @@ RSpec.describe Ci::RetryBuildService do
           end
         end
       end
+
+      context 'when the pipeline is a child pipeline and the bridge is depended' do
+        let!(:parent_pipeline) { create(:ci_pipeline, project: project) }
+        let!(:pipeline) { create(:ci_pipeline, project: project) }
+        let!(:bridge) { create(:ci_bridge, :strategy_depend, pipeline: parent_pipeline, status: 'success') }
+        let!(:source_pipeline) { create(:ci_sources_pipeline, pipeline: pipeline, source_job: bridge) }
+
+        it 'marks source bridge as pending' do
+          service.execute(build)
+
+          expect(bridge.reload).to be_pending
+        end
+      end
     end
 
     context 'when user does not have ability to execute build' do

spec/services/ci/retry_pipeline_service_spec.rb

@@ -280,6 +280,20 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do
         expect(build3.reload.scheduling_type).to eq('dag')
       end
     end
+
+    context 'when the pipeline is a downstream pipeline and the bridge is depended' do
+      let!(:bridge) { create(:ci_bridge, :strategy_depend, status: 'success') }
+
+      before do
+        create(:ci_sources_pipeline, pipeline: pipeline, source_job: bridge)
+      end
+
+      it 'marks source bridge as pending' do
+        service.execute(pipeline)
+
+        expect(bridge.reload).to be_pending
+      end
+    end
   end
 
   context 'when user is not allowed to retry pipeline' do