Commit 060d64bf authored by Allison Browne's avatar Allison Browne

Introduce feature flag for cancel on delete

Introduce canceling on delete flag and start

to move some logic to be sync. Do not enable
the feature flag without completing TODO.
parent fb9f861a
......@@ -7,6 +7,8 @@ module Ci
Ci::ExpirePipelineCacheService.new.execute(pipeline, delete: true)
pipeline.cancel_running if pipeline.cancelable? && ::Feature.enabled?(:cancel_pipelines_prior_to_destroy, default_enabled: :yaml)
pipeline.destroy!
ServiceResponse.success(message: 'Pipeline not found')
......
---
name: cancel_pipelines_prior_to_destroy
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65586
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/335338
milestone: '14.1'
type: development
group: group::continuous integration
default_enabled: false
......@@ -48,6 +48,21 @@ module EE
.for_ref(ref)
.for_project_paths(project_path)
end
state_machine :status do
after_transition any => [:success, :failed, :canceled] do |build|
build.run_after_commit do
# TODO(Issue #331891): before enabling this feature flag. Move update consumption to async while keeping consumption calculation sync.
# This will ensure consumption is calculated before related records are deleted.
if ::Feature.enabled?(:cancel_pipelines_prior_to_destroy, default_enabled: :yaml)
::Ci::Minutes::UpdateBuildMinutesService.new(build.project, nil).execute(build)
# We need to use `reset` on `project` because their AR associations have been cached
# and `Namespace#namespace_statistics` will return stale data.
::Ci::Minutes::EmailNotificationService.new(build.project.reset).execute if ::Gitlab.com?
end
end
end
end
end
override :variables
......
......@@ -3,10 +3,12 @@
module EE
module BuildFinishedWorker
def process_build(build)
unless ::Feature.enabled?(:cancel_pipelines_prior_to_destroy, default_enabled: :yaml)
::Ci::Minutes::UpdateBuildMinutesService.new(build.project, nil).execute(build)
# We need to use `reset` on `project` because their AR associations have been cached
# and `Namespace#namespace_statistics` will return stale data.
::Ci::Minutes::EmailNotificationService.new(build.project.reset).execute if ::Gitlab.com?
end
RequirementsManagement::ProcessRequirementsReportsWorker.perform_async(build.id)
......
......@@ -88,11 +88,12 @@ RSpec.describe Ci::Build do
it_behaves_like 'depends on runner presence and type'
end
shared_context 'updates minutes' do
context 'updates pipeline minutes' do
let(:job) { create(:ci_build, :running, pipeline: pipeline) }
%w(success drop cancel).each do |event|
it "for event #{event}", :sidekiq_might_not_need_inline do
it "for event #{event}" do
expect(Ci::Minutes::UpdateBuildMinutesService)
.to receive(:new).and_call_original
......@@ -100,6 +101,19 @@ RSpec.describe Ci::Build do
end
end
end
end
context 'when cancel_pipelines_prior_to_destroy is enabled' do
include_context 'updates minutes'
end
context 'when cancel_pipelines_prior_to_destroy is disabled', :sidekiq_inline do
before do
stub_feature_flags(cancel_pipelines_prior_to_destroy: false)
end
include_context 'updates minutes'
end
describe '#variables' do
subject { job.variables }
......
......@@ -27,6 +27,11 @@ RSpec.describe BuildFinishedWorker do
allow_any_instance_of(EE::Project).to receive(:shared_runners_minutes_limit_enabled?).and_return(true)
end
context 'when cancel_pipelines_prior_to_destroy is disabled' do
before do
stub_feature_flags(cancel_pipelines_prior_to_destroy: false)
end
it 'updates the project stats' do
expect { subject }.to change { project_stats.reload.shared_runners_seconds }
end
......@@ -44,6 +49,7 @@ RSpec.describe BuildFinishedWorker do
subject
end
end
end
context 'when not on .com' do
before do
......
......@@ -67,6 +67,30 @@ RSpec.describe ::Ci::DestroyPipelineService do
end
end
end
context 'when pipeline is in cancelable state' do
before do
allow(pipeline).to receive(:cancelable?).and_return(true)
end
it 'cancels the pipeline' do
expect(pipeline).to receive(:cancel_running)
subject
end
context 'when cancel_pipelines_prior_to_destroy is disabled' do
before do
stub_feature_flags(cancel_pipelines_prior_to_destroy: false)
end
it "doesn't cancel the pipeline" do
expect(pipeline).not_to receive(:cancel_running)
subject
end
end
end
end
context 'user is not owner' 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