Merge branch '31526-skip-if-needed-job-skipped' into 'master'

Prevent DAG builds to run after skipped need build

See merge request gitlab-org/gitlab!23405

app/services/ci/process_build_service.rb

@@ -3,7 +3,7 @@
 module Ci
   class ProcessBuildService < BaseService
     def execute(build, current_status)
-      if valid_statuses_for_when(build.when).include?(current_status)
+      if valid_statuses_for_build(build).include?(current_status)
         if build.schedulable?
           build.schedule
         elsif build.action?
@@ -25,10 +25,10 @@ module Ci
       build.enqueue
     end
 
-    def valid_statuses_for_when(value)
-      case value
+    def valid_statuses_for_build(build)
+      case build.when
       when 'on_success'
-        %w[success skipped]
+        build.scheduling_type_dag? ? %w[success] : %w[success skipped]
       when 'on_failure'
         %w[failed]
       when 'always'

changelogs/unreleased/31526-skip-if-needed-job-skipped.yml

+---
+title: Prevent DAG builds to run after skipped need build
+merge_request: 23405
+author:
+type: fixed

spec/services/ci/pipeline_processing/shared_processing_service.rb

@@ -880,6 +880,31 @@ shared_examples 'Pipeline Processing Service' do
     end
   end
 
+  context 'when a needed job is skipped', :sidekiq_inline do
+    let!(:linux_build) { create_build('linux:build', stage: 'build', stage_idx: 0) }
+    let!(:linux_rspec) { create_build('linux:rspec', stage: 'test', stage_idx: 1) }
+    let!(:deploy) do
+      create_build('deploy', stage: 'deploy', stage_idx: 2, scheduling_type: :dag, needs: [
+        create(:ci_build_need, name: 'linux:rspec')
+      ])
+    end
+
+    it 'skips the jobs depending on it' do
+      expect(process_pipeline).to be_truthy
+
+      expect(stages).to eq(%w(pending created created))
+      expect(all_builds.pending).to contain_exactly(linux_build)
+
+      linux_build.reset.drop!
+
+      expect(stages).to eq(%w(failed skipped skipped))
+      expect(all_builds.failed).to contain_exactly(linux_build)
+      expect(all_builds.skipped).to contain_exactly(linux_rspec, deploy)
+    end
+  end
+
+  private
+
   def all_builds
     pipeline.builds.order(:stage_idx, :id)
   end

spec/services/ci/process_build_service_spec.rb

@@ -22,6 +22,14 @@ describe Ci::ProcessBuildService, '#execute' do
       end
     end
 
+    context 'when current status is skipped' do
+      let(:current_status) { 'skipped' }
+
+      it 'changes the build status' do
+        expect { subject }.to change { build.status }.to('pending')
+      end
+    end
+
     context 'when current status is failed' do
       let(:current_status) { 'failed' }
 
@@ -114,4 +122,27 @@ describe Ci::ProcessBuildService, '#execute' do
       end
     end
   end
+
+  context 'when build is scheduled with DAG' do
+    let(:pipeline) { create(:ci_pipeline, ref: 'master', project: project) }
+    let!(:build) { create(:ci_build, :created, when: :on_success, pipeline: pipeline, scheduling_type: :dag) }
+    let!(:other_build) { create(:ci_build, :created, when: :on_success, pipeline: pipeline) }
+    let!(:build_on_other_build) { create(:ci_build_need, build: build, name: other_build.name) }
+
+    context 'when current status is success' do
+      let(:current_status) { 'success' }
+
+      it 'enqueues the build' do
+        expect { subject }.to change { build.status }.to('pending')
+      end
+    end
+
+    context 'when current status is skipped' do
+      let(:current_status) { 'skipped' }
+
+      it 'skips the build' do
+        expect { subject }.to change { build.status }.to('skipped')
+      end
+    end
+  end
 end