Merge branch 'latest-successful-build-including-child-pipelines' into 'master'

Include child pipeline builds in latest successful builds for ref/sha

See merge request gitlab-org/gitlab!29710

app/models/ci/pipeline.rb

@@ -841,6 +841,25 @@ module Ci
       end
     end
 
+    def build_with_artifacts_in_self_and_descendants(name)
+      builds_in_self_and_descendants
+        .ordered_by_pipeline # find job in hierarchical order
+        .with_downloadable_artifacts
+        .find_by_name(name)
+    end
+
+    def builds_in_self_and_descendants
+      Ci::Build.latest.where(pipeline: self_and_descendants)
+    end
+
+    # Without using `unscoped`, caller scope is also included into the query.
+    # Using `unscoped` here will be redundant after Rails 6.1
+    def self_and_descendants
+      ::Gitlab::Ci::PipelineObjectHierarchy
+        .new(self.class.unscoped.where(id: id), options: { same_project: true })
+        .base_and_descendants
+    end
+
     def bridge_triggered?
       source_bridge.present?
     end

app/models/commit_status.rb

@@ -48,6 +48,7 @@ class CommitStatus < ApplicationRecord
   scope :ordered_by_stage, -> { order(stage_idx: :asc) }
   scope :latest_ordered, -> { latest.ordered.includes(project: :namespace) }
   scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) }
+  scope :ordered_by_pipeline, -> { order(pipeline_id: :asc) }
   scope :before_stage, -> (index) { where('stage_idx < ?', index) }
   scope :for_stage, -> (index) { where(stage_idx: index) }
   scope :after_stage, -> (index) { where('stage_idx > ?', index) }

app/models/project.rb

@@ -951,7 +951,7 @@ class Project < ApplicationRecord
     latest_pipeline = ci_pipelines.latest_successful_for_ref(ref)
     return unless latest_pipeline
 
-    latest_pipeline.builds.latest.with_downloadable_artifacts.find_by(name: job_name)
+    latest_pipeline.build_with_artifacts_in_self_and_descendants(job_name)
   end
 
   def latest_successful_build_for_sha(job_name, sha)
@@ -960,7 +960,7 @@ class Project < ApplicationRecord
     latest_pipeline = ci_pipelines.latest_successful_for_sha(sha)
     return unless latest_pipeline
 
-    latest_pipeline.builds.latest.with_downloadable_artifacts.find_by(name: job_name)
+    latest_pipeline.build_with_artifacts_in_self_and_descendants(job_name)
   end
 
   def latest_successful_build_for_ref!(job_name, ref = default_branch)

changelogs/unreleased/latest-successful-build-including-child-pipelines.yml

+---
+title: Include builds from child pipelines in latest sucessful build for ref/sha
+merge_request: 29710
+author:
+type: fixed

doc/api/job_artifacts.md

@@ -63,6 +63,11 @@ the given reference name and job, provided the job finished successfully. This
 is the same as [getting the job's artifacts](#get-job-artifacts), but by
 defining the job's name instead of its ID.
 
+NOTE: **Note:**
+If a pipeline is [parent of other child pipelines](../ci/parent_child_pipelines.md), artifacts 
+are searched in hierarchical order from parent to child. For example, if both parent and
+child pipelines have a job with the same name, the artifact from the parent pipeline will be returned.
+
 ```plaintext
 GET /projects/:id/jobs/artifacts/:ref_name/download?job=name
 ```
@@ -157,6 +162,11 @@ Download a single artifact file for a specific job of the latest successful
 pipeline for the given reference name from within the job's artifacts archive.
 The file is extracted from the archive and streamed to the client.
 
+In [GitLab 13.5](https://gitlab.com/gitlab-org/gitlab/-/issues/201784) and later, artifacts
+for [parent and child pipelines](../ci/parent_child_pipelines.md) are searched in hierarchical
+order from parent to child. For example, if both parent and child pipelines have a
+job with the same name, the artifact from the parent pipeline is returned.
+
 ```plaintext
 GET /projects/:id/jobs/artifacts/:ref_name/raw/*artifact_path?job=name
 ```

doc/ci/pipelines/job_artifacts.md

@@ -343,6 +343,11 @@ The latest artifacts are created by jobs in the **most recent** successful pipel
 for the specific ref. If you run two types of pipelines for the same ref, timing determines the latest
 artifact. For example, if a merge request creates a branch pipeline at the same time as a scheduled pipeline, the pipeline that completed most recently creates the latest artifact.
 
+In [GitLab 13.5](https://gitlab.com/gitlab-org/gitlab/-/issues/201784) and later, artifacts
+for [parent and child pipelines](../parent_child_pipelines.md) are searched in hierarchical
+order from parent to child. For example, if both parent and child pipelines have a
+job with the same name, the artifact from the parent pipeline is returned.
+
 Artifacts for other pipelines can be accessed with direct access to them.
 
 The structure of the URL to download the whole artifacts archive is the following:

spec/factories/ci/pipelines.rb

@@ -15,9 +15,25 @@ FactoryBot.define do
     # on pipeline factories to avoid circular references
     transient { head_pipeline_of { nil } }
 
+    transient { child_of { nil } }
+
+    after(:build) do |pipeline, evaluator|
+      if evaluator.child_of
+        pipeline.project = evaluator.child_of.project
+        pipeline.source = :parent_pipeline
+      end
+    end
+
     after(:create) do |pipeline, evaluator|
       merge_request = evaluator.head_pipeline_of
       merge_request&.update!(head_pipeline: pipeline)
+
+      if evaluator.child_of
+        bridge = create(:ci_bridge, pipeline: evaluator.child_of)
+        create(:ci_sources_pipeline,
+          source_job: bridge,
+          pipeline: pipeline)
+      end
     end
 
     factory :ci_pipeline do

spec/models/ci/pipeline_spec.rb

@@ -2988,6 +2988,57 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
     end
   end
 
+  describe '#builds_in_self_and_descendants' do
+    subject(:builds) { pipeline.builds_in_self_and_descendants }
+
+    let(:pipeline) { create(:ci_pipeline, project: project) }
+    let!(:build) { create(:ci_build, pipeline: pipeline) }
+
+    context 'when pipeline is standalone' do
+      it 'returns the list of builds' do
+        expect(builds).to contain_exactly(build)
+      end
+    end
+
+    context 'when pipeline is parent of another pipeline' do
+      let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) }
+      let!(:child_build) { create(:ci_build, pipeline: child_pipeline) }
+
+      it 'returns the list of builds' do
+        expect(builds).to contain_exactly(build, child_build)
+      end
+    end
+
+    context 'when pipeline is parent of another parent pipeline' do
+      let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) }
+      let!(:child_build) { create(:ci_build, pipeline: child_pipeline) }
+      let(:child_of_child_pipeline) { create(:ci_pipeline, child_of: child_pipeline) }
+      let!(:child_of_child_build) { create(:ci_build, pipeline: child_of_child_pipeline) }
+
+      it 'returns the list of builds' do
+        expect(builds).to contain_exactly(build, child_build, child_of_child_build)
+      end
+    end
+  end
+
+  describe '#build_with_artifacts_in_self_and_descendants' do
+    let!(:build) { create(:ci_build, name: 'test', pipeline: pipeline) }
+    let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) }
+    let!(:child_build) { create(:ci_build, :artifacts, name: 'test', pipeline: child_pipeline) }
+
+    it 'returns the build with a given name, having artifacts' do
+      expect(pipeline.build_with_artifacts_in_self_and_descendants('test')).to eq(child_build)
+    end
+
+    context 'when same job name is present in both parent and child pipeline' do
+      let!(:build) { create(:ci_build, :artifacts, name: 'test', pipeline: pipeline) }
+
+      it 'returns the job in the parent pipeline' do
+        expect(pipeline.build_with_artifacts_in_self_and_descendants('test')).to eq(build)
+      end
+    end
+  end
+
   describe '#find_job_with_archive_artifacts' do
     let!(:old_job) { create(:ci_build, name: 'rspec', retried: true, pipeline: pipeline) }
     let!(:job_without_artifacts) { create(:ci_build, name: 'rspec', pipeline: pipeline) }

spec/requests/api/jobs_spec.rb

@@ -465,12 +465,14 @@ RSpec.describe API::Jobs do
     end
 
     context 'find proper job' do
+      let(:job_with_artifacts) { job }
+
       shared_examples 'a valid file' do
         context 'when artifacts are stored locally', :sidekiq_might_not_need_inline do
           let(:download_headers) do
             { 'Content-Transfer-Encoding' => 'binary',
               'Content-Disposition' =>
-              %Q(attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) }
+              %Q(attachment; filename="#{job_with_artifacts.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) }
           end
 
           it { expect(response).to have_gitlab_http_status(:ok) }
@@ -518,6 +520,18 @@ RSpec.describe API::Jobs do
 
         it_behaves_like 'a valid file'
       end
+
+      context 'with job name in a child pipeline' do
+        let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) }
+        let!(:child_job) { create(:ci_build, :artifacts, :success, name: 'rspec', pipeline: child_pipeline) }
+        let(:job_with_artifacts) { child_job }
+
+        before do
+          get_for_ref('master', child_job.name)
+        end
+
+        it_behaves_like 'a valid file'
+      end
     end
   end
 

spec/support/shared_examples/models/project_latest_successful_build_for_shared_examples.rb

@@ -60,4 +60,20 @@ RSpec.shared_examples 'latest successful build for sha or ref' do
       expect(subject).to be_nil
     end
   end
+
+  context 'with build belonging to a child pipeline' do
+    let(:child_pipeline) { create_pipeline(project) }
+    let(:parent_bridge) { create(:ci_bridge, pipeline: pipeline, project: pipeline.project) }
+    let!(:pipeline_source) { create(:ci_sources_pipeline, source_job: parent_bridge, pipeline: child_pipeline)}
+    let!(:child_build) { create_build(child_pipeline, 'child-build') }
+    let(:build_name) { child_build.name }
+
+    before do
+      child_pipeline.update!(source: :parent_pipeline)
+    end
+
+    it 'returns the child build' do
+      expect(subject).to eq(child_build)
+    end
+  end
 end