Commit 0178ad61 authored by Matthias Kaeppler's avatar Matthias Kaeppler

Fail batch-aborted pipelines with reason

Cancellation should not be used for automated
abandonment of pipelines. We should fail them
instead with a reason string.
parent 97a52748
...@@ -20,6 +20,8 @@ module Enums ...@@ -20,6 +20,8 @@ module Enums
scheduler_failure: 11, scheduler_failure: 11,
data_integrity_failure: 12, data_integrity_failure: 12,
forward_deployment_failure: 13, forward_deployment_failure: 13,
user_blocked: 14,
project_deleted: 15,
insufficient_bridge_permissions: 1_001, insufficient_bridge_permissions: 1_001,
downstream_bridge_project_not_found: 1_002, downstream_bridge_project_not_found: 1_002,
invalid_bridge_trigger: 1_003, invalid_bridge_trigger: 1_003,
......
...@@ -13,7 +13,9 @@ module Enums ...@@ -13,7 +13,9 @@ module Enums
activity_limit_exceeded: 20, activity_limit_exceeded: 20,
size_limit_exceeded: 21, size_limit_exceeded: 21,
job_activity_limit_exceeded: 22, job_activity_limit_exceeded: 22,
deployments_limit_exceeded: 23 deployments_limit_exceeded: 23,
user_blocked: 24,
project_deleted: 25
} }
end end
......
...@@ -354,7 +354,7 @@ class User < ApplicationRecord ...@@ -354,7 +354,7 @@ class User < ApplicationRecord
# this state transition object in order to do a rollback. # this state transition object in order to do a rollback.
# For this reason the tradeoff is to disable this cop. # For this reason the tradeoff is to disable this cop.
after_transition any => :blocked do |user| after_transition any => :blocked do |user|
Ci::AbortPipelinesService.new.execute(user.pipelines) Ci::AbortPipelinesService.new.execute(user.pipelines, :user_blocked)
Ci::DisableUserPipelineSchedulesService.new.execute(user) Ci::DisableUserPipelineSchedulesService.new.execute(user)
end end
# rubocop: enable CodeReuse/ServiceClass # rubocop: enable CodeReuse/ServiceClass
......
...@@ -14,7 +14,9 @@ module Ci ...@@ -14,7 +14,9 @@ module Ci
activity_limit_exceeded: 'Pipeline activity limit exceeded!', activity_limit_exceeded: 'Pipeline activity limit exceeded!',
size_limit_exceeded: 'Pipeline size limit exceeded!', size_limit_exceeded: 'Pipeline size limit exceeded!',
job_activity_limit_exceeded: 'Pipeline job activity limit exceeded!', job_activity_limit_exceeded: 'Pipeline job activity limit exceeded!',
deployments_limit_exceeded: 'Pipeline deployments limit exceeded!' } deployments_limit_exceeded: 'Pipeline deployments limit exceeded!',
project_deleted: 'The associated project was deleted',
user_blocked: 'The user who created this pipeline is blocked' }
end end
presents :pipeline presents :pipeline
......
...@@ -21,7 +21,9 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated ...@@ -21,7 +21,9 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
bridge_pipeline_is_child_pipeline: 'This job belongs to a child pipeline and cannot create further child pipelines', bridge_pipeline_is_child_pipeline: 'This job belongs to a child pipeline and cannot create further child pipelines',
downstream_pipeline_creation_failed: 'The downstream pipeline could not be created', downstream_pipeline_creation_failed: 'The downstream pipeline could not be created',
secrets_provider_not_found: 'The secrets provider can not be found', secrets_provider_not_found: 'The secrets provider can not be found',
reached_max_descendant_pipelines_depth: 'Maximum child pipeline depth has been reached' reached_max_descendant_pipelines_depth: 'You reached the maximum depth of child pipelines',
project_deleted: 'The job belongs to a deleted project',
user_blocked: 'The user who created this job is blocked'
}.freeze }.freeze
private_constant :CALLOUT_FAILURE_MESSAGES private_constant :CALLOUT_FAILURE_MESSAGES
......
...@@ -2,28 +2,27 @@ ...@@ -2,28 +2,27 @@
module Ci module Ci
class AbortPipelinesService class AbortPipelinesService
# Danger: Cancels in bulk without callbacks # NOTE: This call fails pipelines in bulk without running callbacks.
# Only for pipeline abandonment scenarios (examples: project delete, user block) # Only for pipeline abandonment scenarios (examples: project delete, user block)
def execute(pipelines) def execute(pipelines, failure_reason)
@time = Time.current pipelines.cancelable.each_batch(of: 100) do |pipeline_batch|
now = Time.current
bulk_abort!(pipelines.cancelable, { status: :canceled }) basic_attributes = { status: :failed }
all_attributes = basic_attributes.merge(failure_reason: failure_reason, finished_at: now)
ServiceResponse.success(message: 'Pipelines canceled') bulk_fail_for(Ci::Stage, pipeline_batch, basic_attributes)
end bulk_fail_for(CommitStatus, pipeline_batch, all_attributes)
private
def bulk_abort!(pipelines, attributes)
pipelines.each_batch(of: 100) do |pipeline_batch|
update_status_for(Ci::Stage, pipeline_batch, attributes)
update_status_for(CommitStatus, pipeline_batch, attributes.merge(finished_at: @time))
pipeline_batch.update_all(attributes.merge(finished_at: @time)) pipeline_batch.update_all(all_attributes)
end end
ServiceResponse.success(message: 'Pipelines stopped')
end end
def update_status_for(klass, pipelines, attributes) private
def bulk_fail_for(klass, pipelines, attributes)
klass.in_pipelines(pipelines) klass.in_pipelines(pipelines)
.cancelable .cancelable
.in_batches(of: 150) # rubocop:disable Cop/InBatches .in_batches(of: 150) # rubocop:disable Cop/InBatches
......
...@@ -28,7 +28,7 @@ module Projects ...@@ -28,7 +28,7 @@ module Projects
flush_caches(project) flush_caches(project)
if Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: :yaml) if Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: :yaml)
::Ci::AbortPipelinesService.new.execute(project.all_pipelines) ::Ci::AbortPipelinesService.new.execute(project.all_pipelines, :project_deleted)
end end
Projects::UnlinkForkService.new(project, current_user).execute Projects::UnlinkForkService.new(project, current_user).execute
......
---
title: Fail batch-aborted pipelines with reason
merge_request: 57838
author:
type: changed
...@@ -26,7 +26,9 @@ module Gitlab ...@@ -26,7 +26,9 @@ module Gitlab
bridge_pipeline_is_child_pipeline: 'creation of child pipeline not allowed from another child pipeline', bridge_pipeline_is_child_pipeline: 'creation of child pipeline not allowed from another child pipeline',
downstream_pipeline_creation_failed: 'downstream pipeline can not be created', downstream_pipeline_creation_failed: 'downstream pipeline can not be created',
secrets_provider_not_found: 'secrets provider can not be found', secrets_provider_not_found: 'secrets provider can not be found',
reached_max_descendant_pipelines_depth: 'reached maximum depth of child pipelines' reached_max_descendant_pipelines_depth: 'reached maximum depth of child pipelines',
project_deleted: 'pipeline project was deleted',
user_blocked: 'pipeline user was blocked'
}.freeze }.freeze
private_constant :REASONS private_constant :REASONS
......
...@@ -1804,7 +1804,7 @@ RSpec.describe User do ...@@ -1804,7 +1804,7 @@ RSpec.describe User do
it 'aborts all running pipelines and related jobs' do it 'aborts all running pipelines and related jobs' do
expect(user).to receive(:pipelines).and_return(pipelines) expect(user).to receive(:pipelines).and_return(pipelines)
expect(Ci::AbortPipelinesService).to receive(:new).and_return(service) expect(Ci::AbortPipelinesService).to receive(:new).and_return(service)
expect(service).to receive(:execute).with(pipelines) expect(service).to receive(:execute).with(pipelines, :user_blocked)
user.block user.block
end end
......
...@@ -17,33 +17,38 @@ RSpec.describe Ci::AbortPipelinesService do ...@@ -17,33 +17,38 @@ RSpec.describe Ci::AbortPipelinesService do
describe '#execute' do describe '#execute' do
def expect_correct_cancellations def expect_correct_cancellations
expect(cancelable_pipeline.finished_at).not_to be_nil expect(cancelable_pipeline.finished_at).not_to be_nil
expect(cancelable_pipeline).to be_canceled expect(cancelable_pipeline.status).to eq('failed')
expect(cancelable_pipeline.stages - [non_cancelable_stage]).to all(be_canceled) expect((cancelable_pipeline.stages - [non_cancelable_stage]).map(&:status)).to all(eq('failed'))
expect(cancelable_build).to be_canceled expect(cancelable_build.status).to eq('failed')
expect(cancelable_build.finished_at).not_to be_nil
expect(manual_pipeline).not_to be_canceled
expect(non_cancelable_stage).not_to be_canceled expect(manual_pipeline.status).not_to eq('failed')
expect(non_cancelable_build).not_to be_canceled expect(non_cancelable_stage.status).not_to eq('failed')
expect(non_cancelable_build.status).not_to eq('failed')
end end
context 'with project pipelines' do context 'with project pipelines' do
it 'cancels all running pipelines and related jobs' do def abort_project_pipelines
expect(described_class.new.execute(project.all_pipelines)).to be_success described_class.new.execute(project.all_pipelines, :project_deleted)
end
it 'fails all running pipelines and related jobs' do
expect(abort_project_pipelines).to be_success
expect_correct_cancellations expect_correct_cancellations
expect(other_users_pipeline).to be_canceled expect(other_users_pipeline.status).to eq('failed')
expect(other_users_pipeline.stages).to all(be_canceled) expect(other_users_pipeline.failure_reason).to eq('project_deleted')
expect(other_users_pipeline.stages.map(&:status)).to all(eq('failed'))
end end
it 'avoids N+1 queries' do it 'avoids N+1 queries' do
project_pipelines = project.all_pipelines control_count = ActiveRecord::QueryRecorder.new { abort_project_pipelines }.count
control_count = ActiveRecord::QueryRecorder.new { described_class.new.execute(project_pipelines) }.count
pipelines = create_list(:ci_pipeline, 5, :running, project: project) pipelines = create_list(:ci_pipeline, 5, :running, project: project)
create_list(:ci_build, 5, :running, pipeline: pipelines.first) create_list(:ci_build, 5, :running, pipeline: pipelines.first)
expect { described_class.new.execute(project_pipelines) }.not_to exceed_query_limit(control_count) expect { abort_project_pipelines }.not_to exceed_query_limit(control_count)
end end
context 'with live build logs' do context 'with live build logs' do
...@@ -51,11 +56,11 @@ RSpec.describe Ci::AbortPipelinesService do ...@@ -51,11 +56,11 @@ RSpec.describe Ci::AbortPipelinesService do
create(:ci_build_trace_chunk, build: cancelable_build) create(:ci_build_trace_chunk, build: cancelable_build)
end end
it 'makes canceled builds with stale trace visible' do it 'makes failed builds with stale trace visible' do
expect(Ci::Build.with_stale_live_trace.count).to eq 0 expect(Ci::Build.with_stale_live_trace.count).to eq 0
travel_to(2.days.ago) do travel_to(2.days.ago) do
described_class.new.execute(project.all_pipelines) abort_project_pipelines
end end
expect(Ci::Build.with_stale_live_trace.count).to eq 1 expect(Ci::Build.with_stale_live_trace.count).to eq 1
...@@ -64,22 +69,25 @@ RSpec.describe Ci::AbortPipelinesService do ...@@ -64,22 +69,25 @@ RSpec.describe Ci::AbortPipelinesService do
end end
context 'with user pipelines' do context 'with user pipelines' do
it 'cancels all running pipelines and related jobs' do def abort_user_pipelines
expect(described_class.new.execute(user.pipelines)).to be_success described_class.new.execute(user.pipelines, :user_blocked)
end
it 'fails all running pipelines and related jobs' do
expect(abort_user_pipelines).to be_success
expect_correct_cancellations expect_correct_cancellations
expect(other_users_pipeline).not_to be_canceled expect(other_users_pipeline.status).not_to eq('failed')
end end
it 'avoids N+1 queries' do it 'avoids N+1 queries' do
user_pipelines = user.pipelines control_count = ActiveRecord::QueryRecorder.new { abort_user_pipelines }.count
control_count = ActiveRecord::QueryRecorder.new { described_class.new.execute(user_pipelines) }.count
pipelines = create_list(:ci_pipeline, 5, :running, project: project, user: user) pipelines = create_list(:ci_pipeline, 5, :running, project: project, user: user)
create_list(:ci_build, 5, :running, pipeline: pipelines.first) create_list(:ci_build, 5, :running, pipeline: pipelines.first)
expect { described_class.new.execute(user_pipelines) }.not_to exceed_query_limit(control_count) expect { abort_user_pipelines }.not_to exceed_query_limit(control_count)
end end
end end
end end
......
...@@ -109,7 +109,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -109,7 +109,7 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
pipelines = build_list(:ci_pipeline, 3, :running) pipelines = build_list(:ci_pipeline, 3, :running)
allow(project).to receive(:all_pipelines).and_return(pipelines) allow(project).to receive(:all_pipelines).and_return(pipelines)
expect(::Ci::AbortPipelinesService).to receive_message_chain(:new, :execute).with(pipelines) expect(::Ci::AbortPipelinesService).to receive_message_chain(:new, :execute).with(pipelines, :project_deleted)
destroy_project(project, user, {}) destroy_project(project, user, {})
end end
......
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