Commit ab1e5eb4 authored by Fabio Pitino's avatar Fabio Pitino

Merge branch '297678-marking-bridge-subsequent-jobs-processable' into 'master'

Fix resetting upstream bridge to allow subsequent jobs to run [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!60376
parents 086b720c ab27bb66
...@@ -1218,11 +1218,18 @@ module Ci ...@@ -1218,11 +1218,18 @@ module Ci
# We need `base_and_ancestors` in a specific order to "break" when needed. # We need `base_and_ancestors` in a specific order to "break" when needed.
# If we use `find_each`, then the order is broken. # If we use `find_each`, then the order is broken.
# rubocop:disable Rails/FindEach # rubocop:disable Rails/FindEach
def reset_ancestor_bridges! def reset_source_bridge!(current_user)
base_and_ancestors.includes(:source_bridge).each do |pipeline| if ::Feature.enabled?(:ci_reset_bridge_with_subsequent_jobs, project, default_enabled: :yaml)
break unless pipeline.bridge_waiting? return unless bridge_waiting?
pipeline.source_bridge.pending! source_bridge.pending!
Ci::AfterRequeueJobService.new(project, current_user).execute(source_bridge) # rubocop:disable CodeReuse/ServiceClass
else
base_and_ancestors.includes(:source_bridge).each do |pipeline|
break unless pipeline.bridge_waiting?
pipeline.source_bridge.pending!
end
end end
end end
# rubocop:enable Rails/FindEach # rubocop:enable Rails/FindEach
......
...@@ -4,7 +4,7 @@ module Ci ...@@ -4,7 +4,7 @@ module Ci
class AfterRequeueJobService < ::BaseService class AfterRequeueJobService < ::BaseService
def execute(processable) def execute(processable)
process_subsequent_jobs(processable) process_subsequent_jobs(processable)
reset_ancestor_bridges(processable) reset_source_bridge(processable)
end end
private private
...@@ -15,8 +15,8 @@ module Ci ...@@ -15,8 +15,8 @@ module Ci
end end
end end
def reset_ancestor_bridges(processable) def reset_source_bridge(processable)
processable.pipeline.reset_ancestor_bridges! processable.pipeline.reset_source_bridge!(current_user)
end end
def process(processable) def process(processable)
......
...@@ -26,7 +26,7 @@ module Ci ...@@ -26,7 +26,7 @@ module Ci
retry_optimistic_lock(skipped, name: 'ci_retry_pipeline') { |build| build.process(current_user) } retry_optimistic_lock(skipped, name: 'ci_retry_pipeline') { |build| build.process(current_user) }
end end
pipeline.reset_ancestor_bridges! pipeline.reset_source_bridge!(current_user)
::MergeRequests::AddTodoWhenBuildFailsService ::MergeRequests::AddTodoWhenBuildFailsService
.new(project, current_user) .new(project, current_user)
......
---
name: ci_reset_bridge_with_subsequent_jobs
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60376
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329194
milestone: '13.12'
type: development
group: group::pipeline authoring
default_enabled: false
...@@ -4303,26 +4303,80 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -4303,26 +4303,80 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end end
end end
describe 'reset_ancestor_bridges!' do describe '#reset_source_bridge!' do
let_it_be(:pipeline) { create(:ci_pipeline, :created) } let(:pipeline) { create(:ci_pipeline, :created, project: project) }
subject(:reset_bridge) { pipeline.reset_source_bridge!(project.owner) }
# This whole block will be removed by https://gitlab.com/gitlab-org/gitlab/-/issues/329194
# It contains some duplicate checks.
context 'when the FF ci_reset_bridge_with_subsequent_jobs is disabled' do
before do
stub_feature_flags(ci_reset_bridge_with_subsequent_jobs: false)
end
context 'when the pipeline is a child pipeline and the bridge is depended' do
let!(:parent_pipeline) { create(:ci_pipeline) }
let!(:bridge) { create_bridge(parent_pipeline, pipeline, true) }
it 'marks source bridge as pending' do
reset_bridge
expect(bridge.reload).to be_pending
end
context 'when the parent pipeline has subsequent jobs after the bridge' do
let!(:after_bridge_job) { create(:ci_build, :skipped, pipeline: parent_pipeline, stage_idx: bridge.stage_idx + 1) }
it 'does not touch subsequent jobs of the bridge' do
reset_bridge
expect(after_bridge_job.reload).to be_skipped
end
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
reset_bridge
expect(bridge.reload).to be_pending
expect(upstream_bridge.reload).to be_pending
end
end
end
end
context 'when the pipeline is a child pipeline and the bridge is depended' do context 'when the pipeline is a child pipeline and the bridge is depended' do
let!(:parent_pipeline) { create(:ci_pipeline) } let!(:parent_pipeline) { create(:ci_pipeline) }
let!(:bridge) { create_bridge(parent_pipeline, pipeline, true) } let!(:bridge) { create_bridge(parent_pipeline, pipeline, true) }
it 'marks source bridge as pending' do it 'marks source bridge as pending' do
pipeline.reset_ancestor_bridges! reset_bridge
expect(bridge.reload).to be_pending expect(bridge.reload).to be_pending
end end
context 'when the parent pipeline has subsequent jobs after the bridge' do
let!(:after_bridge_job) { create(:ci_build, :skipped, pipeline: parent_pipeline, stage_idx: bridge.stage_idx + 1) }
it 'marks subsequent jobs of the bridge as processable' do
reset_bridge
expect(after_bridge_job.reload).to be_created
end
end
context 'when the parent pipeline has a dependent upstream pipeline' do context 'when the parent pipeline has a dependent upstream pipeline' do
let!(:upstream_bridge) do let!(:upstream_bridge) do
create_bridge(create(:ci_pipeline, project: create(:project)), parent_pipeline, true) create_bridge(create(:ci_pipeline, project: create(:project)), parent_pipeline, true)
end end
it 'marks all source bridges as pending' do it 'marks all source bridges as pending' do
pipeline.reset_ancestor_bridges! reset_bridge
expect(bridge.reload).to be_pending expect(bridge.reload).to be_pending
expect(upstream_bridge.reload).to be_pending expect(upstream_bridge.reload).to be_pending
...@@ -4335,7 +4389,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -4335,7 +4389,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
let!(:bridge) { create_bridge(parent_pipeline, pipeline, false) } let!(:bridge) { create_bridge(parent_pipeline, pipeline, false) }
it 'does not touch source bridge' do it 'does not touch source bridge' do
pipeline.reset_ancestor_bridges! reset_bridge
expect(bridge.reload).to be_success expect(bridge.reload).to be_success
end end
...@@ -4346,7 +4400,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -4346,7 +4400,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
end end
it 'does not touch any source bridge' do it 'does not touch any source bridge' do
pipeline.reset_ancestor_bridges! reset_bridge
expect(bridge.reload).to be_success expect(bridge.reload).to be_success
expect(upstream_bridge.reload).to be_success expect(upstream_bridge.reload).to be_success
......
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