Commit 01063d64 authored by Furkan Ayhan's avatar Furkan Ayhan

Move populate_scheduling_type logic

We need this logic where only retrying mechanism occurs.
With this movement, we'll achieve to remove redis calls.
We added "ci_ensure_scheduling_type" FF just in case.
parent a96ef15b
...@@ -961,6 +961,14 @@ module Ci ...@@ -961,6 +961,14 @@ module Ci
end end
end end
# Set scheduling type of processables if they were created before scheduling_type
# data was deployed (https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22246).
def ensure_scheduling_type!
return unless ::Gitlab::Ci::Features.ensure_scheduling_type_enabled?
processables.populate_scheduling_type!
end
private private
def pipeline_data def pipeline_data
......
...@@ -99,5 +99,13 @@ module Ci ...@@ -99,5 +99,13 @@ module Ci
needs.map { |need| need.attributes.except('id', 'build_id') } needs.map { |need| need.attributes.except('id', 'build_id') }
end end
end end
def ensure_scheduling_type!
# If this has a scheduling_type, it means all processables in the pipeline already have.
return if scheduling_type
pipeline.ensure_scheduling_type!
reset
end
end end
end end
...@@ -10,7 +10,6 @@ module Ci ...@@ -10,7 +10,6 @@ module Ci
def execute(trigger_build_ids = nil, initial_process: false) def execute(trigger_build_ids = nil, initial_process: false)
update_retried update_retried
ensure_scheduling_type_for_processables
if Feature.enabled?(:ci_atomic_processing, pipeline.project) if Feature.enabled?(:ci_atomic_processing, pipeline.project)
Ci::PipelineProcessing::AtomicProcessingService Ci::PipelineProcessing::AtomicProcessingService
...@@ -44,17 +43,5 @@ module Ci ...@@ -44,17 +43,5 @@ module Ci
.update_all(retried: true) if latest_statuses.any? .update_all(retried: true) if latest_statuses.any?
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
# Set scheduling type of processables if they were created before scheduling_type
# data was deployed (https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22246).
# Given that this service runs multiple times during the pipeline
# life cycle we need to ensure we populate the data once.
# See more: https://gitlab.com/gitlab-org/gitlab/issues/205426
def ensure_scheduling_type_for_processables
lease = Gitlab::ExclusiveLease.new("set-scheduling-types:#{pipeline.id}", timeout: 1.hour.to_i)
return unless lease.try_obtain
pipeline.processables.populate_scheduling_type!
end
end end
end end
...@@ -9,6 +9,8 @@ module Ci ...@@ -9,6 +9,8 @@ module Ci
resource_group scheduling_type].freeze resource_group scheduling_type].freeze
def execute(build) def execute(build)
build.ensure_scheduling_type!
reprocess!(build).tap do |new_build| reprocess!(build).tap do |new_build|
build.pipeline.mark_as_processable_after_stage(build.stage_idx) build.pipeline.mark_as_processable_after_stage(build.stage_idx)
...@@ -31,6 +33,9 @@ module Ci ...@@ -31,6 +33,9 @@ module Ci
end.to_h end.to_h
attributes[:user] = current_user attributes[:user] = current_user
# TODO: we can probably remove this logic
# see: https://gitlab.com/gitlab-org/gitlab/-/issues/217930
attributes[:scheduling_type] ||= build.find_legacy_scheduling_type attributes[:scheduling_type] ||= build.find_legacy_scheduling_type
Ci::Build.transaction do Ci::Build.transaction do
......
...@@ -11,6 +11,8 @@ module Ci ...@@ -11,6 +11,8 @@ module Ci
needs = Set.new needs = Set.new
pipeline.ensure_scheduling_type!
pipeline.retryable_builds.preload_needs.find_each do |build| 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)
......
...@@ -9,6 +9,10 @@ module Gitlab ...@@ -9,6 +9,10 @@ module Gitlab
def self.artifacts_exclude_enabled? def self.artifacts_exclude_enabled?
::Feature.enabled?(:ci_artifacts_exclude, default_enabled: false) ::Feature.enabled?(:ci_artifacts_exclude, default_enabled: false)
end end
def self.ensure_scheduling_type_enabled?
::Feature.enabled?(:ci_ensure_scheduling_type, default_enabled: true)
end
end end
end end
end end
...@@ -33,25 +33,6 @@ describe Ci::ProcessPipelineService do ...@@ -33,25 +33,6 @@ describe Ci::ProcessPipelineService do
end end
end end
context 'with a pipeline which has processables with nil scheduling_type', :clean_gitlab_redis_shared_state do
let!(:build1) { create_build('build1') }
let!(:build2) { create_build('build2') }
let!(:build3) { create_build('build3', scheduling_type: :dag) }
let!(:build3_on_build2) { create(:ci_build_need, build: build3, name: 'build2') }
before do
pipeline.processables.update_all(scheduling_type: nil)
end
it 'populates scheduling_type before processing' do
process_pipeline
expect(build1.scheduling_type).to eq('stage')
expect(build2.scheduling_type).to eq('stage')
expect(build3.scheduling_type).to eq('dag')
end
end
def process_pipeline def process_pipeline
described_class.new(pipeline).execute described_class.new(pipeline).execute
end end
......
...@@ -190,6 +190,35 @@ describe Ci::RetryBuildService do ...@@ -190,6 +190,35 @@ describe Ci::RetryBuildService do
expect(subsequent_build.reload).to be_created expect(subsequent_build.reload).to be_created
end end
end end
context 'when pipeline has other builds' do
let!(:stage2) { create(:ci_stage_entity, project: project, pipeline: pipeline, name: 'deploy') }
let!(:build2) { create(:ci_build, pipeline: pipeline, stage_id: stage.id ) }
let!(:deploy) { create(:ci_build, pipeline: pipeline, stage_id: stage2.id) }
let!(:deploy_needs_build2) { create(:ci_build_need, build: deploy, name: build2.name) }
context 'when build has nil scheduling_type' do
before do
build.pipeline.processables.update_all(scheduling_type: nil)
build.reload
end
it 'populates scheduling_type of processables' do
expect(new_build.scheduling_type).to eq('stage')
expect(build.reload.scheduling_type).to eq('stage')
expect(build2.reload.scheduling_type).to eq('stage')
expect(deploy.reload.scheduling_type).to eq('dag')
end
end
context 'when build has scheduling_type' do
it 'does not call populate_scheduling_type!' do
expect_any_instance_of(Ci::Pipeline).not_to receive(:ensure_scheduling_type!)
expect(new_build.scheduling_type).to eq('stage')
end
end
end
end end
context 'when user does not have ability to execute build' do context 'when user does not have ability to execute build' do
......
...@@ -261,6 +261,25 @@ describe Ci::RetryPipelineService, '#execute' do ...@@ -261,6 +261,25 @@ describe Ci::RetryPipelineService, '#execute' do
service.execute(pipeline) service.execute(pipeline)
end end
context 'when pipeline has processables with nil scheduling_type' do
let!(:build1) { create_build('build1', :success, 0) }
let!(:build2) { create_build('build2', :failed, 0) }
let!(:build3) { create_build('build3', :failed, 1) }
let!(:build3_needs_build1) { create(:ci_build_need, build: build3, name: build1.name) }
before do
statuses.update_all(scheduling_type: nil)
end
it 'populates scheduling_type of processables' do
service.execute(pipeline)
expect(build1.reload.scheduling_type).to eq('stage')
expect(build2.reload.scheduling_type).to eq('stage')
expect(build3.reload.scheduling_type).to eq('dag')
end
end
end end
context 'when user is not allowed to retry pipeline' do context 'when user is not allowed to retry pipeline' do
......
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