Commit afe90d52 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'fix-cancelling-pipelines' into 'master'

Improve how we could cancel pipelines:

Improve how we could cancel pipelines:

* Introduce `HasStatus.cancelable` which we might be able to cancel
* Cancel and check upon `cancelable`
* Also cancel on `CommitStatus` rather than just `Ci::Build`

Fixes #23635

Fixes #17845

See merge request !7508
parents 90a3b3ab bd3a544a
...@@ -161,24 +161,28 @@ module Ci ...@@ -161,24 +161,28 @@ module Ci
end end
def retryable? def retryable?
builds.latest.any? do |build| builds.latest.failed_or_canceled.any?(&:retryable?)
(build.failed? || build.canceled?) && build.retryable?
end
end end
def cancelable? def cancelable?
builds.running_or_pending.any? statuses.cancelable.any?
end end
def cancel_running def cancel_running
builds.running_or_pending.each(&:cancel) Gitlab::OptimisticLocking.retry_lock(
statuses.cancelable) do |cancelable|
cancelable.each(&:cancel)
end
end end
def retry_failed(user) def retry_failed(user)
builds.latest.failed.select(&:retryable?).each do |build| Gitlab::OptimisticLocking.retry_lock(
builds.latest.failed_or_canceled) do |failed_or_canceled|
failed_or_canceled.select(&:retryable?).each do |build|
Ci::Build.retry(build, user) Ci::Build.retry(build, user)
end end
end end
end
def mark_as_processable_after_stage(stage_idx) def mark_as_processable_after_stage(stage_idx)
builds.skipped.where('stage_idx > ?', stage_idx).find_each(&:process) builds.skipped.where('stage_idx > ?', stage_idx).find_each(&:process)
......
...@@ -73,6 +73,11 @@ module HasStatus ...@@ -73,6 +73,11 @@ module HasStatus
scope :skipped, -> { where(status: 'skipped') } scope :skipped, -> { where(status: 'skipped') }
scope :running_or_pending, -> { where(status: [:running, :pending]) } scope :running_or_pending, -> { where(status: [:running, :pending]) }
scope :finished, -> { where(status: [:success, :failed, :canceled]) } scope :finished, -> { where(status: [:success, :failed, :canceled]) }
scope :failed_or_canceled, -> { where(status: [:failed, :canceled]) }
scope :cancelable, -> do
where(status: [:running, :pending, :created])
end
end end
def started? def started?
......
---
title: Fix cancelling created or external pipelines
merge_request: 7508
author:
...@@ -38,6 +38,10 @@ FactoryGirl.define do ...@@ -38,6 +38,10 @@ FactoryGirl.define do
status 'canceled' status 'canceled'
end end
trait :skipped do
status 'skipped'
end
trait :running do trait :running do
status 'running' status 'running'
end end
......
...@@ -19,6 +19,10 @@ FactoryGirl.define do ...@@ -19,6 +19,10 @@ FactoryGirl.define do
status 'canceled' status 'canceled'
end end
trait :skipped do
status 'skipped'
end
trait :running do trait :running do
status 'running' status 'running'
end end
......
...@@ -90,13 +90,20 @@ describe "Pipelines" do ...@@ -90,13 +90,20 @@ describe "Pipelines" do
visit namespace_project_pipelines_path(project.namespace, project) visit namespace_project_pipelines_path(project.namespace, project)
end end
it 'is not cancelable' do it 'is cancelable' do
expect(page).not_to have_link('Cancel') expect(page).to have_link('Cancel')
end end
it 'has pipeline running' do it 'has pipeline running' do
expect(page).to have_selector('.ci-running') expect(page).to have_selector('.ci-running')
end end
context 'when canceling' do
before { click_link('Cancel') }
it { expect(page).not_to have_link('Cancel') }
it { expect(page).to have_selector('.ci-canceled') }
end
end end
context 'when failed' do context 'when failed' do
......
...@@ -402,6 +402,160 @@ describe Ci::Pipeline, models: true do ...@@ -402,6 +402,160 @@ describe Ci::Pipeline, models: true do
end end
end end
describe '#cancelable?' do
%i[created running pending].each do |status0|
context "when there is a build #{status0}" do
before do
create(:ci_build, status0, pipeline: pipeline)
end
it 'is cancelable' do
expect(pipeline.cancelable?).to be_truthy
end
end
context "when there is an external job #{status0}" do
before do
create(:generic_commit_status, status0, pipeline: pipeline)
end
it 'is cancelable' do
expect(pipeline.cancelable?).to be_truthy
end
end
%i[success failed canceled].each do |status1|
context "when there are generic_commit_status jobs for #{status0} and #{status1}" do
before do
create(:generic_commit_status, status0, pipeline: pipeline)
create(:generic_commit_status, status1, pipeline: pipeline)
end
it 'is cancelable' do
expect(pipeline.cancelable?).to be_truthy
end
end
context "when there are generic_commit_status and ci_build jobs for #{status0} and #{status1}" do
before do
create(:generic_commit_status, status0, pipeline: pipeline)
create(:ci_build, status1, pipeline: pipeline)
end
it 'is cancelable' do
expect(pipeline.cancelable?).to be_truthy
end
end
context "when there are ci_build jobs for #{status0} and #{status1}" do
before do
create(:ci_build, status0, pipeline: pipeline)
create(:ci_build, status1, pipeline: pipeline)
end
it 'is cancelable' do
expect(pipeline.cancelable?).to be_truthy
end
end
end
end
%i[success failed canceled].each do |status|
context "when there is a build #{status}" do
before do
create(:ci_build, status, pipeline: pipeline)
end
it 'is not cancelable' do
expect(pipeline.cancelable?).to be_falsey
end
end
context "when there is an external job #{status}" do
before do
create(:generic_commit_status, status, pipeline: pipeline)
end
it 'is not cancelable' do
expect(pipeline.cancelable?).to be_falsey
end
end
end
end
describe '#cancel_running' do
let(:latest_status) { pipeline.statuses.pluck(:status) }
context 'when there is a running external job and created build' do
before do
create(:ci_build, :running, pipeline: pipeline)
create(:generic_commit_status, :running, pipeline: pipeline)
pipeline.cancel_running
end
it 'cancels both jobs' do
expect(latest_status).to contain_exactly('canceled', 'canceled')
end
end
context 'when builds are in different stages' do
before do
create(:ci_build, :running, stage_idx: 0, pipeline: pipeline)
create(:ci_build, :running, stage_idx: 1, pipeline: pipeline)
pipeline.cancel_running
end
it 'cancels both jobs' do
expect(latest_status).to contain_exactly('canceled', 'canceled')
end
end
end
describe '#retry_failed' do
let(:latest_status) { pipeline.statuses.latest.pluck(:status) }
context 'when there is a failed build and failed external status' do
before do
create(:ci_build, :failed, name: 'build', pipeline: pipeline)
create(:generic_commit_status, :failed, name: 'jenkins', pipeline: pipeline)
pipeline.retry_failed(create(:user))
end
it 'retries only build' do
expect(latest_status).to contain_exactly('pending', 'failed')
end
end
context 'when builds are in different stages' do
before do
create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline)
create(:ci_build, :failed, name: 'jenkins', stage_idx: 1, pipeline: pipeline)
pipeline.retry_failed(create(:user))
end
it 'retries both builds' do
expect(latest_status).to contain_exactly('pending', 'pending')
end
end
context 'when there are canceled and failed' do
before do
create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline)
create(:ci_build, :canceled, name: 'jenkins', stage_idx: 1, pipeline: pipeline)
pipeline.retry_failed(create(:user))
end
it 'retries both builds' do
expect(latest_status).to contain_exactly('pending', 'pending')
end
end
end
describe '#execute_hooks' do describe '#execute_hooks' do
let!(:build_a) { create_build('a', 0) } let!(:build_a) { create_build('a', 0) }
let!(:build_b) { create_build('b', 1) } let!(:build_b) { create_build('b', 1) }
......
...@@ -123,4 +123,100 @@ describe HasStatus do ...@@ -123,4 +123,100 @@ describe HasStatus do
it_behaves_like 'build status summary' it_behaves_like 'build status summary'
end end
end end
context 'for scope with one status' do
shared_examples 'having a job' do |status|
%i[ci_build generic_commit_status].each do |type|
context "when it's #{status} #{type} job" do
let!(:job) { create(type, status) }
describe ".#{status}" do
it 'contains the job' do
expect(CommitStatus.public_send(status).all).
to contain_exactly(job)
end
end
describe '.relevant' do
if status == :created
it 'contains nothing' do
expect(CommitStatus.relevant.all).to be_empty
end
else
it 'contains the job' do
expect(CommitStatus.relevant.all).to contain_exactly(job)
end
end
end
end
end
end
%i[created running pending success
failed canceled skipped].each do |status|
it_behaves_like 'having a job', status
end
end
context 'for scope with more statuses' do
shared_examples 'containing the job' do |status|
%i[ci_build generic_commit_status].each do |type|
context "when it's #{status} #{type} job" do
let!(:job) { create(type, status) }
it 'contains the job' do
is_expected.to contain_exactly(job)
end
end
end
end
shared_examples 'not containing the job' do |status|
%i[ci_build generic_commit_status].each do |type|
context "when it's #{status} #{type} job" do
let!(:job) { create(type, status) }
it 'contains nothing' do
is_expected.to be_empty
end
end
end
end
describe '.running_or_pending' do
subject { CommitStatus.running_or_pending }
%i[running pending].each do |status|
it_behaves_like 'containing the job', status
end
%i[created failed success].each do |status|
it_behaves_like 'not containing the job', status
end
end
describe '.finished' do
subject { CommitStatus.finished }
%i[success failed canceled].each do |status|
it_behaves_like 'containing the job', status
end
%i[created running pending].each do |status|
it_behaves_like 'not containing the job', status
end
end
describe '.cancelable' do
subject { CommitStatus.cancelable }
%i[running pending created].each do |status|
it_behaves_like 'containing the job', status
end
%i[failed success skipped canceled].each do |status|
it_behaves_like 'not containing the job', status
end
end
end
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