Commit 968e6ad5 authored by Stan Hu's avatar Stan Hu

Fix pipeline retry in a CI DAG

Previously a retried pipeline for builds in a DAG would cause the newly-
created builds to sit in the created state because it would be waiting
for the dependencies to trigger it. However, in the retry case, the
dependencies may have already completed, so they will never trigger new
builds.

We fix this by:

1. Looking up successful builds with the required names
2. Passing those build IDs to `Ci::ProcessPipelineService` to trigger
   builds with those dependencies.

Closes https://gitlab.com/gitlab-org/gitlab/issues/36068
parent 425c041e
...@@ -811,6 +811,10 @@ module Ci ...@@ -811,6 +811,10 @@ module Ci
@persistent_ref ||= PersistentRef.new(pipeline: self) @persistent_ref ||= PersistentRef.new(pipeline: self)
end end
def find_successful_build_ids_by_names(names)
statuses.latest.success.where(name: names).pluck(:id)
end
private private
def pipeline_data def pipeline_data
......
...@@ -14,6 +14,8 @@ module Ci ...@@ -14,6 +14,8 @@ module Ci
has_many :needs, class_name: 'Ci::BuildNeed', foreign_key: :build_id, inverse_of: :build has_many :needs, class_name: 'Ci::BuildNeed', foreign_key: :build_id, inverse_of: :build
accepts_nested_attributes_for :needs accepts_nested_attributes_for :needs
scope :preload_needs, -> { preload(:needs) }
end end
def schedulable? def schedulable?
......
...@@ -9,13 +9,23 @@ module Ci ...@@ -9,13 +9,23 @@ module Ci
raise Gitlab::Access::AccessDeniedError raise Gitlab::Access::AccessDeniedError
end end
pipeline.retryable_builds.find_each do |build| needs = Set.new
pipeline.retryable_builds.preload_needs.find_each do |build|
next unless can?(current_user, :update_build, build) next unless can?(current_user, :update_build, build)
Ci::RetryBuildService.new(project, current_user) Ci::RetryBuildService.new(project, current_user)
.reprocess!(build) .reprocess!(build)
needs += build.needs.map(&:name)
end end
# In a DAG, the dependencies may have already completed. Figure out
# which builds have succeeded and use them to update the pipeline. If we don't
# do this, then builds will be stuck in the created state since their dependencies
# will never run.
completed_build_ids = pipeline.find_successful_build_ids_by_names(needs) if needs.any?
pipeline.builds.latest.skipped.find_each do |skipped| pipeline.builds.latest.skipped.find_each do |skipped|
retry_optimistic_lock(skipped) { |build| build.process } retry_optimistic_lock(skipped) { |build| build.process }
end end
...@@ -26,7 +36,7 @@ module Ci ...@@ -26,7 +36,7 @@ module Ci
Ci::ProcessPipelineService Ci::ProcessPipelineService
.new(pipeline) .new(pipeline)
.execute .execute(completed_build_ids)
end end
end end
end end
---
title: Fix pipeline retry in a CI DAG
merge_request: 21296
author:
type: fixed
...@@ -91,6 +91,25 @@ describe Ci::RetryPipelineService, '#execute' do ...@@ -91,6 +91,25 @@ describe Ci::RetryPipelineService, '#execute' do
end end
end end
context 'when there is a failed test in a DAG' do
before do
create_build('build', :success, 0)
create_build('build2', :success, 0)
test_build = create_build('test', :failed, 1)
create(:ci_build_need, build: test_build, name: 'build')
create(:ci_build_need, build: test_build, name: 'build2')
end
it 'retries the test' do
service.execute(pipeline)
expect(build('build')).to be_success
expect(build('build2')).to be_success
expect(build('test')).to be_pending
expect(build('test').needs.map(&:name)).to match_array(%w(build build2))
end
end
context 'when the last stage was skipepd' do context 'when the last stage was skipepd' do
before do before do
create_build('build 1', :success, 0) create_build('build 1', :success, 0)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment