Commit 0e2b9e65 authored by Allison Browne's avatar Allison Browne Committed by GitLab Release Tools Bot

Cancel all cancelable jobs on project delete

To avoid using runner resources on deleted projects we
cancel all cancelable jobs as the first step in deletion
parent a3b77cd9
...@@ -16,6 +16,7 @@ module Ci ...@@ -16,6 +16,7 @@ module Ci
include ShaAttribute include ShaAttribute
include FromUnion include FromUnion
include UpdatedAtFilterable include UpdatedAtFilterable
include EachBatch
MAX_OPEN_MERGE_REQUESTS_REFS = 4 MAX_OPEN_MERGE_REQUESTS_REFS = 4
......
...@@ -55,6 +55,7 @@ class CommitStatus < ApplicationRecord ...@@ -55,6 +55,7 @@ class CommitStatus < ApplicationRecord
scope :for_ids, -> (ids) { where(id: ids) } scope :for_ids, -> (ids) { where(id: ids) }
scope :for_ref, -> (ref) { where(ref: ref) } scope :for_ref, -> (ref) { where(ref: ref) }
scope :by_name, -> (name) { where(name: name) } scope :by_name, -> (name) { where(name: name) }
scope :in_pipelines, ->(pipelines) { where(pipeline: pipelines) }
scope :for_project_paths, -> (paths) do scope :for_project_paths, -> (paths) do
where(project: Project.where_full_path_in(Array(paths))) where(project: Project.where_full_path_in(Array(paths)))
......
# frozen_string_literal: true
module Ci
class AbortProjectPipelinesService
# Danger: Cancels in bulk without callbacks
# Only for pipeline abandonment scenarios (current example: project delete)
def execute(project)
return unless Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: :yaml)
pipelines = project.all_pipelines.cancelable
bulk_abort!(pipelines, status: :canceled)
ServiceResponse.success(message: 'Pipelines canceled')
end
private
def bulk_abort!(pipelines, status:)
pipelines.each_batch do |pipeline_batch|
CommitStatus.in_pipelines(pipeline_batch).in_batches.update_all(status: status) # rubocop: disable Cop/InBatches
pipeline_batch.update_all(status: status)
end
end
end
end
...@@ -6,6 +6,7 @@ module Ci ...@@ -6,6 +6,7 @@ module Ci
# This is a bug with CodeReuse/ActiveRecord cop # This is a bug with CodeReuse/ActiveRecord cop
# https://gitlab.com/gitlab-org/gitlab/issues/32332 # https://gitlab.com/gitlab-org/gitlab/issues/32332
def execute(user) def execute(user)
# TODO: fix N+1 queries https://gitlab.com/gitlab-org/gitlab/-/issues/300685
user.pipelines.cancelable.find_each(&:cancel_running) user.pipelines.cancelable.find_each(&:cancel_running)
ServiceResponse.success(message: 'Pipeline canceled') ServiceResponse.success(message: 'Pipeline canceled')
......
...@@ -21,11 +21,14 @@ module Projects ...@@ -21,11 +21,14 @@ module Projects
def execute def execute
return false unless can?(current_user, :remove_project, project) return false unless can?(current_user, :remove_project, project)
project.update_attribute(:pending_delete, true)
# Flush the cache for both repositories. This has to be done _before_ # Flush the cache for both repositories. This has to be done _before_
# removing the physical repositories as some expiration code depends on # removing the physical repositories as some expiration code depends on
# Git data (e.g. a list of branch names). # Git data (e.g. a list of branch names).
flush_caches(project) flush_caches(project)
::Ci::AbortProjectPipelinesService.new.execute(project)
Projects::UnlinkForkService.new(project, current_user).execute Projects::UnlinkForkService.new(project, current_user).execute
attempt_destroy(project) attempt_destroy(project)
......
---
title: Cancel running and pending jobs when a project is deleted
merge_request: 1220
author:
type: security
---
name: abort_deleted_project_pipelines
introduced_by_url: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1220
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/301106
milestone: '13.9'
type: development
group: group::continuous integration
default_enabled: true
...@@ -10,6 +10,10 @@ module Gitlab ...@@ -10,6 +10,10 @@ module Gitlab
include Chain::Helpers include Chain::Helpers
def perform! def perform!
if project.pending_delete?
return error('Project is deleted!')
end
unless project.builds_enabled? unless project.builds_enabled?
return error('Pipelines are disabled!') return error('Pipelines are disabled!')
end end
......
...@@ -74,6 +74,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do ...@@ -74,6 +74,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do
it 'does not break the chain' do it 'does not break the chain' do
expect(step.break?).to eq false expect(step.break?).to eq false
end end
context 'when project is deleted' do
before do
project.update!(pending_delete: true)
end
specify { expect(step.perform!).to contain_exactly('Project is deleted!') }
end
end end
describe '#allowed_to_write_ref?' do describe '#allowed_to_write_ref?' do
......
...@@ -34,6 +34,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -34,6 +34,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
it { is_expected.to have_many(:auto_canceled_jobs) } it { is_expected.to have_many(:auto_canceled_jobs) }
it { is_expected.to have_many(:sourced_pipelines) } it { is_expected.to have_many(:sourced_pipelines) }
it { is_expected.to have_many(:triggered_pipelines) } it { is_expected.to have_many(:triggered_pipelines) }
it { is_expected.to have_many(:pipeline_artifacts) }
it { is_expected.to have_one(:chat_data) } it { is_expected.to have_one(:chat_data) }
it { is_expected.to have_one(:source_pipeline) } it { is_expected.to have_one(:source_pipeline) }
...@@ -41,14 +42,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do ...@@ -41,14 +42,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
it { is_expected.to have_one(:source_job) } it { is_expected.to have_one(:source_job) }
it { is_expected.to have_one(:pipeline_config) } it { is_expected.to have_one(:pipeline_config) }
it { is_expected.to validate_presence_of(:sha) }
it { is_expected.to validate_presence_of(:status) }
it { is_expected.to respond_to :git_author_name } it { is_expected.to respond_to :git_author_name }
it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :git_author_email }
it { is_expected.to respond_to :short_sha } it { is_expected.to respond_to :short_sha }
it { is_expected.to delegate_method(:full_path).to(:project).with_prefix } it { is_expected.to delegate_method(:full_path).to(:project).with_prefix }
it { is_expected.to have_many(:pipeline_artifacts) }
describe 'validations' do
it { is_expected.to validate_presence_of(:sha) }
it { is_expected.to validate_presence_of(:status) }
end
describe 'associations' do describe 'associations' do
it 'has a bidirectional relationship with projects' do it 'has a bidirectional relationship with projects' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Ci::AbortProjectPipelinesService do
let_it_be(:project) { create(:project) }
let_it_be(:pipeline) { create(:ci_pipeline, :running, project: project) }
let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) }
describe '#execute' do
it 'cancels all running pipelines and related jobs' do
result = described_class.new.execute(project)
expect(result).to be_success
expect(pipeline.reload).to be_canceled
expect(build.reload).to be_canceled
end
it 'avoids N+1 queries' do
control_count = ActiveRecord::QueryRecorder.new { described_class.new.execute(project) }.count
pipelines = create_list(:ci_pipeline, 5, :running, project: project)
create_list(:ci_build, 5, :running, pipeline: pipelines.first)
expect { described_class.new.execute(project) }.not_to exceed_query_limit(control_count)
end
end
context 'when feature disabled' do
before do
stub_feature_flags(abort_deleted_project_pipelines: false)
end
it 'does not abort the pipeline' do
result = described_class.new.execute(project)
expect(result).to be(nil)
expect(pipeline.reload).to be_running
expect(build.reload).to be_running
end
end
end
...@@ -69,6 +69,12 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do ...@@ -69,6 +69,12 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
destroy_project(project, user, {}) destroy_project(project, user, {})
end end
it 'performs cancel for project ci pipelines' do
expect(::Ci::AbortProjectPipelinesService).to receive_message_chain(:new, :execute).with(project)
destroy_project(project, user, {})
end
context 'when project has remote mirrors' do context 'when project has remote mirrors' do
let!(:project) do let!(:project) do
create(:project, :repository, namespace: user.namespace).tap do |project| create(:project, :repository, namespace: user.namespace).tap do |project|
......
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