From 4ecab0aaa2575ab4dd4654582f79e1b5448be966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sat, 6 Apr 2019 21:00:24 +0200 Subject: [PATCH 01/15] Adds YAML documentation and changelog. --- changelogs/unreleased/issue-32741.yml | 5 ++++ doc/ci/yaml/README.md | 41 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 changelogs/unreleased/issue-32741.yml diff --git a/changelogs/unreleased/issue-32741.yml b/changelogs/unreleased/issue-32741.yml new file mode 100644 index 00000000000..5d0a3ee2cc7 --- /dev/null +++ b/changelogs/unreleased/issue-32741.yml @@ -0,0 +1,5 @@ +--- +title: New interruptible attribute for CI/CD jobs +merge_request: 23464 +author: Cédric Tabin +type: added diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index bfba21646b5..8f2e95dbb10 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -116,6 +116,7 @@ The following table lists available parameters for jobs: | [`extends`](#extends) | Configuration entries that this job is going to inherit from. | | [`pages`](#pages) | Upload the result of a job to use with GitLab Pages. | | [`variables`](#variables) | Define job variables on a job level. | +| [interruptible](#interruptible) | Defines if a job can be canceled when made redundant by a newer run | NOTE: **Note:** Parameters `types` and `type` are [deprecated](#deprecated-parameters). @@ -2083,6 +2084,46 @@ staging: branch: stable ``` +### `interruptible` + +`interruptible` is used to indicate that a job should be canceled if made redundant by a newer run of the same job. Defaults to `false` if there is an environment defined and `true` otherwise. +This value will only be used if the [automatic cancellation of redundant pipelines feature](https://docs.gitlab.com/ee/user/project/pipelines/settings.html#auto-cancel-pending-pipelines) +is enabled. + +When enabled, a pipeline on the same branch will be canceled when: + +- It is made redundant by a newer pipeline run. +- Either all jobs are set as interruptible, or any uninterruptible jobs are not yet pending. + +Pending jobs are always considered interruptible. + +TIP: **Tip:** +Set jobs as uninterruptible that should behave atomically and should never be canceled once started. + +Here is a simple example: + +```yaml +stages: + - stage1 + - stage2 + +step-1: + stage: stage1 + script: + - echo "Can be canceled" + +step-2: + stage: stage2 + script: + - echo "Can not be canceled" + interruptible: false +``` + +In the example above, a new pipeline run will cause an existing running pipeline to be: + +- Canceled, if only `step-1` is running or pending. +- Not canceled, once `step-2` becomes pending. + ### `include` > - Introduced in [GitLab Premium](https://about.gitlab.com/pricing/) 10.5. -- 2.24.1 From 795720fb90276a892372a528bedc08ab7d8b18d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sun, 24 Feb 2019 21:43:40 +0100 Subject: [PATCH 02/15] New interruptible attribute supported in YAML parsing. Since it is not possible to dynamically detect if a job is automatically cancellable or not, a this new attribute is necessary. Moreover, it let the maintainer of the repo to adjust the behaviour of the auto cancellation feature to match exactly what he needs. --- lib/gitlab/ci/config/entry/job.rb | 8 ++++--- lib/gitlab/ci/yaml_processor.rb | 3 ++- spec/lib/gitlab/ci/yaml_processor_spec.rb | 26 +++++++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 6e11c582750..96ff688ccbb 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -15,7 +15,7 @@ module Gitlab ALLOWED_KEYS = %i[tags script only except rules type image services allow_failure type stage when start_in artifacts cache dependencies needs before_script after_script variables - environment coverage retry parallel extends].freeze + environment coverage retry parallel extends interruptible].freeze REQUIRED_BY_NEEDS = %i[stage].freeze @@ -122,10 +122,11 @@ module Gitlab helpers :before_script, :script, :stage, :type, :after_script, :cache, :image, :services, :only, :except, :variables, :artifacts, :environment, :coverage, :retry, - :parallel, :needs + :parallel, :needs, :interruptible attributes :script, :tags, :allow_failure, :when, :dependencies, - :needs, :retry, :parallel, :extends, :start_in, :rules + :needs, :retry, :parallel, :extends, :start_in, :rules, + :interruptible def self.matching?(name, config) !name.to_s.start_with?('.') && @@ -207,6 +208,7 @@ module Gitlab coverage: coverage_defined? ? coverage_value : nil, retry: retry_defined? ? retry_value : nil, parallel: parallel_defined? ? parallel_value.to_i : nil, + interruptible: interruptible_defined? ? interruptible_value : nil, artifacts: artifacts_value, after_script: after_script_value, ignore: ignored?, diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 2e1eab270ff..d30515ea1f8 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -56,7 +56,8 @@ module Gitlab instance: job[:instance], start_in: job[:start_in], trigger: job[:trigger], - bridge_needs: job[:needs] + bridge_needs: job[:needs], + interruptible: job[:interruptible] }.compact }.compact end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 91c559dcd9b..3ee85650d07 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -50,6 +50,32 @@ module Gitlab end end + describe 'interruptible entry' do + describe 'interruptible job' do + let(:config) do + YAML.dump(rspec: { script: 'rspec', interruptible: true }) + end + + it { expect(subject[:options][:interruptible]).to eq(true) } + end + + describe 'interruptible job with default value' do + let(:config) do + YAML.dump(rspec: { script: 'rspec' }) + end + + it { expect(subject[:options]).not_to have_key(:interruptible) } + end + + describe 'uninterruptible job' do + let(:config) do + YAML.dump(rspec: { script: 'rspec', interruptible: false }) + end + + it { expect(subject[:options][:interruptible]).to eq(false) } + end + end + describe 'retry entry' do context 'when retry count is specified' do let(:config) do -- 2.24.1 From ecf948255ba9300c964d6ebdf6bed6c15bd0acd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Fri, 5 Apr 2019 22:42:51 +0200 Subject: [PATCH 03/15] Adds 'interruptible' column to ci_builds_metadata. This attribute is added in this table in order to solve performance problems that are caused if this attribute is set in the ci_builds options or directly as a column in ci_builds. This allow to lookup for cancellable pipelines directly through SQL queries instead of having to iterate manually on all of them. --- ...0190905223800_add_interruptible_to_builds_metadata.rb | 9 +++++++++ db/schema.rb | 3 ++- spec/lib/gitlab/import_export/safe_model_attributes.yml | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20190905223800_add_interruptible_to_builds_metadata.rb diff --git a/db/migrate/20190905223800_add_interruptible_to_builds_metadata.rb b/db/migrate/20190905223800_add_interruptible_to_builds_metadata.rb new file mode 100644 index 00000000000..a666b11013b --- /dev/null +++ b/db/migrate/20190905223800_add_interruptible_to_builds_metadata.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddInterruptibleToBuildsMetadata < ActiveRecord::Migration[5.0] + DOWNTIME = false + + def change + add_column :ci_builds_metadata, :interruptible, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 98c4403efe1..c5459dea79e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_09_04_173203) do +ActiveRecord::Schema.define(version: 2019_09_05_223800) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -619,6 +619,7 @@ ActiveRecord::Schema.define(version: 2019_09_04_173203) do t.integer "project_id", null: false t.integer "timeout" t.integer "timeout_source", default: 1, null: false + t.boolean "interruptible" t.jsonb "config_options" t.jsonb "config_variables" t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id", unique: true diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 516e62c4728..9552f041efd 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -329,6 +329,7 @@ CommitStatus: - failure_reason - scheduled_at - upstream_pipeline_id +- interruptible Ci::Variable: - id - project_id -- 2.24.1 From ca8e010e050d600858397ef71dc2d3308c9d0925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sat, 6 Apr 2019 19:59:31 +0200 Subject: [PATCH 04/15] Adjust YAML parsing since the interruptible property has been moved in ci_builds_metadata --- lib/gitlab/ci/config/entry/job.rb | 1 + lib/gitlab/ci/yaml_processor.rb | 4 ++-- spec/lib/gitlab/ci/yaml_processor_spec.rb | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 96ff688ccbb..3009c7e8329 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -37,6 +37,7 @@ module Gitlab with_options allow_nil: true do validates :tags, array_of_strings: true validates :allow_failure, boolean: true + validates :interruptible, boolean: true validates :parallel, numericality: { only_integer: true, greater_than_or_equal_to: 2, less_than_or_equal_to: 50 } diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index d30515ea1f8..501d91fa9ad 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -41,6 +41,7 @@ module Gitlab coverage_regex: job[:coverage], yaml_variables: yaml_variables(name), needs_attributes: job[:needs]&.map { |need| { name: need } }, + interruptible: job[:interruptible], options: { image: job[:image], services: job[:services], @@ -56,8 +57,7 @@ module Gitlab instance: job[:instance], start_in: job[:start_in], trigger: job[:trigger], - bridge_needs: job[:needs], - interruptible: job[:interruptible] + bridge_needs: job[:needs] }.compact }.compact end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 3ee85650d07..cf496b79a62 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -56,7 +56,7 @@ module Gitlab YAML.dump(rspec: { script: 'rspec', interruptible: true }) end - it { expect(subject[:options][:interruptible]).to eq(true) } + it { expect(subject[:interruptible]).to be_truthy } end describe 'interruptible job with default value' do @@ -64,7 +64,7 @@ module Gitlab YAML.dump(rspec: { script: 'rspec' }) end - it { expect(subject[:options]).not_to have_key(:interruptible) } + it { expect(subject).not_to have_key(:interruptible) } end describe 'uninterruptible job' do @@ -72,7 +72,7 @@ module Gitlab YAML.dump(rspec: { script: 'rspec', interruptible: false }) end - it { expect(subject[:options][:interruptible]).to eq(false) } + it { expect(subject[:interruptible]).to be_falsy } end end -- 2.24.1 From eddd1df0dab465fb1f40211cf39e4112402348bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sat, 6 Apr 2019 20:59:52 +0200 Subject: [PATCH 05/15] Supports of interruptible attribute in build --- app/models/ci/build.rb | 6 +++++ app/models/concerns/ci/metadatable.rb | 11 +++++++- spec/models/ci/build_spec.rb | 36 +++++++++++++++++++++++++ spec/models/concerns/has_status_spec.rb | 12 +++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d558f66154e..48ec54cd939 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -685,6 +685,12 @@ module Ci super || project.try(:build_coverage_regex) end + def interruptible? + return !has_environment? if interruptible.nil? + + interruptible + end + def steps [Gitlab::Ci::Build::Step.from_commands(self), Gitlab::Ci::Build::Step.from_after_script(self)].compact diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index 304cc71e9dc..a061ade4de1 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -15,6 +15,7 @@ module Ci autosave: true delegate :timeout, to: :metadata, prefix: true, allow_nil: true + delegate :interruptible, to: :metadata, prefix: false, allow_nil: true before_create :ensure_metadata end @@ -28,7 +29,7 @@ module Ci def degenerate! self.class.transaction do - self.update!(options: nil, yaml_variables: nil) + self.update!(options: nil, yaml_variables: nil, interruptible: nil) self.needs.all.delete_all self.metadata&.destroy end @@ -50,6 +51,14 @@ module Ci write_metadata_attribute(:yaml_variables, :config_variables, value) end + def interruptible + ensure_metadata.read_attribute(:interruptible, value) + end + + def interruptible=(value) + ensure_metadata.write_attribute(:interruptible, value) + end + private def read_metadata_attribute(legacy_key, metadata_key, default_value = nil) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index bc853d45085..e1f885a08a0 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3797,6 +3797,42 @@ describe Ci::Build do end end + describe '#interruptible?' do + let(:build) { create(:ci_build) } + + it 'is interruptible by default' do + expect(build.interruptible).to be_nil + expect(build.interruptible?).to be_truthy + end + + context 'when build is uninterruptible' do + it 'is uninterruptible' do + build.update(interruptible: false) + + expect(build.interruptible).to be_falsy + expect(build.interruptible?).to be_falsy + end + end + + context 'when build has an environment' do + it 'is uninterruptible' do + build.update(environment: 'review') + + expect(build.interruptible).to be_nil + expect(build.interruptible?).to be_falsy + end + end + + context 'when build has an environment but is interruptible' do + it 'is interruptible' do + build.update(environment: 'review', interruptible: true) + + expect(build.interruptible).to be_truthy + expect(build.interruptible?).to be_truthy + end + end + end + describe '#archived?' do context 'when build is degenerated' do subject { create(:ci_build, :degenerated) } diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index a217dc42537..09fb2fff521 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -261,6 +261,18 @@ describe HasStatus do end end + describe '.alive_or_scheduled' do + subject { CommitStatus.alive_or_scheduled } + + %i[running pending preparing created scheduled].each do |status| + it_behaves_like 'containing the job', status + end + + %i[failed success canceled skipped].each do |status| + it_behaves_like 'not containing the job', status + end + end + describe '.created_or_pending' do subject { CommitStatus.created_or_pending } -- 2.24.1 From d1919206c0222773217f3e3bf22c8f689bbdb195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sat, 6 Apr 2019 21:01:36 +0200 Subject: [PATCH 06/15] Implements pipeline cancellation based on the interruptible attribute. A pipeline is cancelled when there is no uninterruptible build pending or finished yet. A typical uninterruptible build would be a deployment step whereas simple build/test steps could be considered as interruptible most of the time. This solves many uses cases, especially when the devs are only interested in the last pipeline results. It can also save a lot of CPU time when the pipeline builds are heavy and non-essential. --- app/models/ci/build.rb | 1 + app/models/ci/pipeline.rb | 7 + app/models/concerns/has_status.rb | 1 + app/services/ci/create_pipeline_service.rb | 3 +- .../ci/create_pipeline_service_spec.rb | 201 +++++++++++++++++- 5 files changed, 209 insertions(+), 4 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 48ec54cd939..8eb5ef412d3 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -88,6 +88,7 @@ module Ci validates :coverage, numericality: true, allow_blank: true validates :ref, presence: true + scope :not_interruptible, -> { joins('JOIN ci_builds_metadata ON ci_builds_metadata.build_id = ci_builds.id').where('ci_builds_metadata.interruptible = ?', false) } scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } scope :with_artifacts_archive, ->() do diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 2b6f10ef79f..ab2bbc5d54a 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -225,6 +225,13 @@ module Ci where('EXISTS (?)', ::Ci::Build.latest.with_reports(reports_scope).where('ci_pipelines.id=ci_builds.commit_id').select(1)) end + scope :non_interruptible_pipeline_ids_for, -> (ref) do + select("ci_pipelines.id") + .where('ci_pipelines.ref = ?', ref) + .joins("JOIN ci_builds ON ci_builds.commit_id = ci_pipelines.id AND ci_builds.status IN ('preparing','pending','running','success','failed','canceled','skipped','manual','scheduled')") + .joins("JOIN ci_builds_metadata ON ci_builds_metadata.build_id = ci_builds.id AND ci_builds_metadata.interruptible = 'FALSE'") + end + # Returns the pipelines in descending order (= newest first), optionally # limited to a number of references. # diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index cf88076ac74..bcbbb27a9a8 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -102,6 +102,7 @@ module HasStatus scope :manual, -> { with_status(:manual) } scope :scheduled, -> { with_status(:scheduled) } scope :alive, -> { with_status(:created, :preparing, :pending, :running) } + scope :alive_or_scheduled, -> { with_status(:created, :preparing, :pending, :running, :scheduled) } scope :created_or_pending, -> { with_status(:created, :pending) } scope :running_or_pending, -> { with_status(:running, :pending) } scope :finished, -> { with_status(:success, :failed, :canceled) } diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 29317f1176e..9806a19b881 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -95,7 +95,8 @@ module Ci .where(ref: pipeline.ref) .where.not(id: pipeline.id) .where.not(sha: project.commit(pipeline.ref).try(:id)) - .created_or_pending + .alive_or_scheduled + .where('ci_pipelines.id NOT IN (?)', Pipeline.non_interruptible_pipeline_ids_for(pipeline.ref)) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index fad865a4811..5ebf99bd355 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -220,11 +220,11 @@ describe Ci::CreatePipelineService do expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) end - it 'does not cancel running outdated pipelines' do + it 'cancels running outdated pipelines' do pipeline_on_previous_commit.run - execute_service + head_pipeline = execute_service - expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'running', auto_canceled_by_id: nil) + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: head_pipeline.id) end it 'cancel created outdated pipelines' do @@ -243,6 +243,201 @@ describe Ci::CreatePipelineService do expect(pending_pipeline.reload).to have_attributes(status: 'pending', auto_canceled_by_id: nil) end + + context 'when the interruptible attribute is' do + context 'not defined' do + before do + config = YAML.dump(rspec: { script: 'echo' }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_nil + expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_truthy + end + end + + context 'set to true' do + before do + config = YAML.dump(rspec: { script: 'echo', interruptible: true }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_truthy + expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_truthy + end + end + + context 'set to false' do + before do + config = YAML.dump(rspec: { script: 'echo', interruptible: false }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is not cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_falsy + expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_falsy + end + end + + context 'not defined, but an environment is' do + before do + config = YAML.dump(rspec: { script: 'echo', environment: { name: "review/$CI_COMMIT_REF_NAME" } }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is not cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_nil + expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_falsy + end + end + + context 'overriding the environment definition' do + before do + config = YAML.dump(rspec: { script: 'echo', environment: { name: "review/$CI_COMMIT_REF_NAME" }, interruptible: true }) + stub_ci_pipeline_yaml_file(config) + end + + it 'is cancelable' do + pipeline = execute_service + + expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_truthy + expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_truthy + end + end + end + + context 'interruptible builds' do + before do + stub_ci_pipeline_yaml_file(YAML.dump(config)) + end + + let(:config) do + { + stages: %w[stage1 stage2 stage3 stage4], + + build_1_1: { + stage: 'stage1', + script: 'echo' + }, + build_1_2: { + stage: 'stage1', + script: 'echo', + interruptible: true + }, + build_2_1: { + stage: 'stage2', + script: 'echo', + when: 'delayed', + start_in: '10 minutes' + }, + build_3_1: { + stage: 'stage3', + script: 'echo', + interruptible: false + }, + build_4_1: { + stage: 'stage4', + script: 'echo' + } + } + end + + def set_pipeline_status(status) + pipeline_on_previous_commit.run + + status.each_with_index do |s, i| + current_build = pipeline_on_previous_commit.builds[i] + current_build.update(status: 'created') + current_build.fire_events!('enqueue') + current_build.update(status: 'pending') + + if s != 'scheduled' + current_build.fire_events!('run') + end + + current_build.update(status: s) + end + + pipeline + end + + context 'when the status is updated' do + it 'applies the transitions accordingly' do + set_pipeline_status(%w[success success success running]) + + expect(pipeline_on_previous_commit.builds[0].started?).to be_truthy + expect(pipeline_on_previous_commit.builds[1].started?).to be_truthy + expect(pipeline_on_previous_commit.builds[2].started?).to be_truthy + expect(pipeline_on_previous_commit.builds[3].started?).to be_truthy + expect(pipeline_on_previous_commit.builds[4].started?).to be_falsy + expect(pipeline_on_previous_commit.builds[0].interruptible).to be_nil + expect(pipeline_on_previous_commit.builds[1].interruptible).to be_truthy + expect(pipeline_on_previous_commit.builds[2].interruptible).to be_nil + expect(pipeline_on_previous_commit.builds[3].interruptible).to be_falsy + expect(pipeline_on_previous_commit.builds[4].interruptible).to be_nil + expect(pipeline_on_previous_commit.builds[0].interruptible?).to be_truthy + expect(pipeline_on_previous_commit.builds[1].interruptible?).to be_truthy + expect(pipeline_on_previous_commit.builds[2].interruptible?).to be_truthy + expect(pipeline_on_previous_commit.builds[3].interruptible?).to be_falsy + expect(pipeline_on_previous_commit.builds[4].interruptible?).to be_truthy + end + end + + context 'when only interruptible builds are running' do + it 'cancels running outdated pipelines' do + pipeline_on_previous_commit.run + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + end + end + + context 'when an uninterruptible build has been reached' do + it 'does not cancel running outdated pipelines' do + set_pipeline_status(%w[success success success running]) + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'running', auto_canceled_by_id: nil) + end + end + + context 'when an build is waiting on an interruptible scheduled task' do + it 'cancels running outdated pipelines' do + set_pipeline_status(%w[success success scheduled]) + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + end + end + + context 'when an build is waiting on an interruptible scheduled task' do + it 'cancels running outdated pipelines' do + set_pipeline_status(%w[success success scheduled]) + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + end + end + + context 'when a uninterruptible build has been executed' do + it 'does not cancel running outdated pipelines' do + set_pipeline_status(%w[success success success success running]) + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'running', auto_canceled_by_id: nil) + end + end + end end context 'auto-cancel disabled' do -- 2.24.1 From 1dee7239d2bacda113d7326f5dc2f9ebc473820c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Thu, 20 Jun 2019 14:57:05 +0200 Subject: [PATCH 07/15] Adds the project id as boundary in the non_interruptible pipelines predicate --- app/models/ci/pipeline.rb | 3 ++- app/services/ci/create_pipeline_service.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ab2bbc5d54a..b6d35ab1dc9 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -225,8 +225,9 @@ module Ci where('EXISTS (?)', ::Ci::Build.latest.with_reports(reports_scope).where('ci_pipelines.id=ci_builds.commit_id').select(1)) end - scope :non_interruptible_pipeline_ids_for, -> (ref) do + scope :non_interruptible_pipeline_ids_for, -> (project_id, ref) do select("ci_pipelines.id") + .where('ci_pipelines.project_id = ?', project_id) .where('ci_pipelines.ref = ?', ref) .joins("JOIN ci_builds ON ci_builds.commit_id = ci_pipelines.id AND ci_builds.status IN ('preparing','pending','running','success','failed','canceled','skipped','manual','scheduled')") .joins("JOIN ci_builds_metadata ON ci_builds_metadata.build_id = ci_builds.id AND ci_builds_metadata.interruptible = 'FALSE'") diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 9806a19b881..53d01adf0a0 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -96,7 +96,7 @@ module Ci .where.not(id: pipeline.id) .where.not(sha: project.commit(pipeline.ref).try(:id)) .alive_or_scheduled - .where('ci_pipelines.id NOT IN (?)', Pipeline.non_interruptible_pipeline_ids_for(pipeline.ref)) + .where('ci_pipelines.id NOT IN (?)', Pipeline.non_interruptible_pipeline_ids_for(pipeline.project_id, pipeline.ref)) end # rubocop: enable CodeReuse/ActiveRecord -- 2.24.1 From 17f20488bbc840c3b177edfa00a4c25f9b524446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sat, 13 Jul 2019 10:39:34 +0200 Subject: [PATCH 08/15] Refactoring of SQL query to use a NOT EXISTS approach --- app/models/ci/pipeline.rb | 8 -------- app/services/ci/create_pipeline_service.rb | 8 +++++++- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index b6d35ab1dc9..2b6f10ef79f 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -225,14 +225,6 @@ module Ci where('EXISTS (?)', ::Ci::Build.latest.with_reports(reports_scope).where('ci_pipelines.id=ci_builds.commit_id').select(1)) end - scope :non_interruptible_pipeline_ids_for, -> (project_id, ref) do - select("ci_pipelines.id") - .where('ci_pipelines.project_id = ?', project_id) - .where('ci_pipelines.ref = ?', ref) - .joins("JOIN ci_builds ON ci_builds.commit_id = ci_pipelines.id AND ci_builds.status IN ('preparing','pending','running','success','failed','canceled','skipped','manual','scheduled')") - .joins("JOIN ci_builds_metadata ON ci_builds_metadata.build_id = ci_builds.id AND ci_builds_metadata.interruptible = 'FALSE'") - end - # Returns the pipelines in descending order (= newest first), optionally # limited to a number of references. # diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 53d01adf0a0..23ead12542e 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -96,7 +96,13 @@ module Ci .where.not(id: pipeline.id) .where.not(sha: project.commit(pipeline.ref).try(:id)) .alive_or_scheduled - .where('ci_pipelines.id NOT IN (?)', Pipeline.non_interruptible_pipeline_ids_for(pipeline.project_id, pipeline.ref)) + .where('NOT EXISTS ( + SELECT 1 FROM ci_builds + JOIN ci_builds_metadata ON ci_builds_metadata.build_id = ci_builds.id + WHERE ci_builds.commit_id = ci_pipelines.id + AND ci_builds_metadata.interruptible IS FALSE + AND ci_builds.status IN (\'running\',\'success\',\'failure\') + )') end # rubocop: enable CodeReuse/ActiveRecord -- 2.24.1 From ffb12054631da2674da6b8ee06cd3af9c8f31149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sat, 27 Jul 2019 21:13:08 +0200 Subject: [PATCH 09/15] Refactoring of the build.not_interruptible method to use explicit variables instead of fixed SQL query --- app/models/ci/build.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 8eb5ef412d3..828796e9708 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -88,7 +88,7 @@ module Ci validates :coverage, numericality: true, allow_blank: true validates :ref, presence: true - scope :not_interruptible, -> { joins('JOIN ci_builds_metadata ON ci_builds_metadata.build_id = ci_builds.id').where('ci_builds_metadata.interruptible = ?', false) } + scope :not_interruptible, -> { joins(:metadata).where(ci_builds_metadata: { interruptible: false }) } scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } scope :with_artifacts_archive, ->() do -- 2.24.1 From 1cf04ff59dc16023bbbe4374eab13465a72f107f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sat, 27 Jul 2019 21:21:48 +0200 Subject: [PATCH 10/15] Refactoring of metadatatable to avoid useless creation of ci_builds_metadata entries --- app/models/concerns/ci/metadatable.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index a061ade4de1..a0ca8a34c6d 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -29,7 +29,7 @@ module Ci def degenerate! self.class.transaction do - self.update!(options: nil, yaml_variables: nil, interruptible: nil) + self.update!(options: nil, yaml_variables: nil) self.needs.all.delete_all self.metadata&.destroy end @@ -52,11 +52,11 @@ module Ci end def interruptible - ensure_metadata.read_attribute(:interruptible, value) + metadata&.interruptible end def interruptible=(value) - ensure_metadata.write_attribute(:interruptible, value) + ensure_metadata.interruptible = value end private -- 2.24.1 From 9681e1404ec5250b8782267b14de4c72fe8e810c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Sat, 27 Jul 2019 21:40:35 +0200 Subject: [PATCH 11/15] Refactoring of SQL to cancel interruptible pipelines --- app/models/ci/pipeline.rb | 8 ++++++++ app/services/ci/create_pipeline_service.rb | 8 +------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 2b6f10ef79f..d620959b538 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -225,6 +225,14 @@ module Ci where('EXISTS (?)', ::Ci::Build.latest.with_reports(reports_scope).where('ci_pipelines.id=ci_builds.commit_id').select(1)) end + scope :without_interruptible_builds, -> do + where('NOT EXISTS (?)', + Ci::Build.where('ci_builds.commit_id = ci_pipelines.id') + .with_status(:running, :success, :failed) + .not_interruptible + ) + end + # Returns the pipelines in descending order (= newest first), optionally # limited to a number of references. # diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 23ead12542e..706c7fdd879 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -96,13 +96,7 @@ module Ci .where.not(id: pipeline.id) .where.not(sha: project.commit(pipeline.ref).try(:id)) .alive_or_scheduled - .where('NOT EXISTS ( - SELECT 1 FROM ci_builds - JOIN ci_builds_metadata ON ci_builds_metadata.build_id = ci_builds.id - WHERE ci_builds.commit_id = ci_pipelines.id - AND ci_builds_metadata.interruptible IS FALSE - AND ci_builds.status IN (\'running\',\'success\',\'failure\') - )') + .without_interruptible_builds end # rubocop: enable CodeReuse/ActiveRecord -- 2.24.1 From 1d4ff70531d34dfe10ef2bd4bde1cccd3e24e537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Tue, 13 Aug 2019 21:08:19 +0200 Subject: [PATCH 12/15] Adds a partial index on ci_builds_metadata where interruptible is false --- ...add_concurrent_index_to_builds_metadata.rb | 19 +++++++++++++++++++ db/schema.rb | 3 ++- 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20190905223900_add_concurrent_index_to_builds_metadata.rb diff --git a/db/migrate/20190905223900_add_concurrent_index_to_builds_metadata.rb b/db/migrate/20190905223900_add_concurrent_index_to_builds_metadata.rb new file mode 100644 index 00000000000..d394f995673 --- /dev/null +++ b/db/migrate/20190905223900_add_concurrent_index_to_builds_metadata.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddConcurrentIndexToBuildsMetadata < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_builds_metadata, [:build_id], + where: "interruptible = false", + name: "index_ci_builds_metadata_on_build_id_and_interruptible_false" + end + + def down + remove_concurrent_index_by_name(:ci_builds_metadata, 'index_ci_builds_metadata_on_build_id_and_interruptible_false') + end +end diff --git a/db/schema.rb b/db/schema.rb index c5459dea79e..501b8eac94e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_09_05_223800) do +ActiveRecord::Schema.define(version: 2019_09_05_223900) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -623,6 +623,7 @@ ActiveRecord::Schema.define(version: 2019_09_05_223800) do t.jsonb "config_options" t.jsonb "config_variables" t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id", unique: true + t.index ["build_id"], name: "index_ci_builds_metadata_on_build_id_and_interruptible_false", where: "(interruptible = false)" t.index ["project_id"], name: "index_ci_builds_metadata_on_project_id" end -- 2.24.1 From b0013e71a269ce5cb3a0afea74a159850947e0ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Tue, 3 Sep 2019 12:24:25 +0200 Subject: [PATCH 13/15] Adds a feature flag on when redundant pipelines are interrupted --- app/services/ci/create_pipeline_service.rb | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 706c7fdd879..8f8582afb43 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -91,12 +91,21 @@ module Ci # rubocop: disable CodeReuse/ActiveRecord def auto_cancelable_pipelines - project.ci_pipelines - .where(ref: pipeline.ref) - .where.not(id: pipeline.id) - .where.not(sha: project.commit(pipeline.ref).try(:id)) - .alive_or_scheduled - .without_interruptible_builds + # TODO: Introduced by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23464 + if Feature.enabled?(:ci_support_interruptible_pipelines, project, default_enabled: true) + project.ci_pipelines + .where(ref: pipeline.ref) + .where.not(id: pipeline.id) + .where.not(sha: project.commit(pipeline.ref).try(:id)) + .alive_or_scheduled + .without_interruptible_builds + else + project.ci_pipelines + .where(ref: pipeline.ref) + .where.not(id: pipeline.id) + .where.not(sha: project.commit(pipeline.ref).try(:id)) + .created_or_pending + end end # rubocop: enable CodeReuse/ActiveRecord -- 2.24.1 From 403a74c5cac36999466de935125f9836d8c360cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Thu, 5 Sep 2019 11:55:36 +0200 Subject: [PATCH 14/15] Removal of unused build.interruptible? method --- app/models/ci/build.rb | 6 ---- spec/models/ci/build_spec.rb | 36 ------------------- .../ci/create_pipeline_service_spec.rb | 10 ------ 3 files changed, 52 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 828796e9708..72782827906 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -686,12 +686,6 @@ module Ci super || project.try(:build_coverage_regex) end - def interruptible? - return !has_environment? if interruptible.nil? - - interruptible - end - def steps [Gitlab::Ci::Build::Step.from_commands(self), Gitlab::Ci::Build::Step.from_after_script(self)].compact diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index e1f885a08a0..bc853d45085 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3797,42 +3797,6 @@ describe Ci::Build do end end - describe '#interruptible?' do - let(:build) { create(:ci_build) } - - it 'is interruptible by default' do - expect(build.interruptible).to be_nil - expect(build.interruptible?).to be_truthy - end - - context 'when build is uninterruptible' do - it 'is uninterruptible' do - build.update(interruptible: false) - - expect(build.interruptible).to be_falsy - expect(build.interruptible?).to be_falsy - end - end - - context 'when build has an environment' do - it 'is uninterruptible' do - build.update(environment: 'review') - - expect(build.interruptible).to be_nil - expect(build.interruptible?).to be_falsy - end - end - - context 'when build has an environment but is interruptible' do - it 'is interruptible' do - build.update(environment: 'review', interruptible: true) - - expect(build.interruptible).to be_truthy - expect(build.interruptible?).to be_truthy - end - end - end - describe '#archived?' do context 'when build is degenerated' do subject { create(:ci_build, :degenerated) } diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 5ebf99bd355..d6577a4c13d 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -255,7 +255,6 @@ describe Ci::CreatePipelineService do pipeline = execute_service expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_nil - expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_truthy end end @@ -269,7 +268,6 @@ describe Ci::CreatePipelineService do pipeline = execute_service expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_truthy - expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_truthy end end @@ -283,7 +281,6 @@ describe Ci::CreatePipelineService do pipeline = execute_service expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_falsy - expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_falsy end end @@ -297,7 +294,6 @@ describe Ci::CreatePipelineService do pipeline = execute_service expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_nil - expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_falsy end end @@ -311,7 +307,6 @@ describe Ci::CreatePipelineService do pipeline = execute_service expect(pipeline.builds.find_by(name: 'rspec').interruptible).to be_truthy - expect(pipeline.builds.find_by(name: 'rspec').interruptible?).to be_truthy end end end @@ -385,11 +380,6 @@ describe Ci::CreatePipelineService do expect(pipeline_on_previous_commit.builds[2].interruptible).to be_nil expect(pipeline_on_previous_commit.builds[3].interruptible).to be_falsy expect(pipeline_on_previous_commit.builds[4].interruptible).to be_nil - expect(pipeline_on_previous_commit.builds[0].interruptible?).to be_truthy - expect(pipeline_on_previous_commit.builds[1].interruptible?).to be_truthy - expect(pipeline_on_previous_commit.builds[2].interruptible?).to be_truthy - expect(pipeline_on_previous_commit.builds[3].interruptible?).to be_falsy - expect(pipeline_on_previous_commit.builds[4].interruptible?).to be_truthy end end -- 2.24.1 From 87b6cb206176bbdf7ec6ff06f70910f7f87eab3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Tabin?= Date: Thu, 5 Sep 2019 13:51:10 +0200 Subject: [PATCH 15/15] Fixes tests to avoid random failure on the pipeline --- .../ci/create_pipeline_service_spec.rb | 111 ++++++++++-------- 1 file changed, 61 insertions(+), 50 deletions(-) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index d6577a4c13d..6cec93a53fd 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -347,84 +347,95 @@ describe Ci::CreatePipelineService do } end - def set_pipeline_status(status) - pipeline_on_previous_commit.run + it 'properly configures interruptible status' do + interruptible_status = + pipeline_on_previous_commit + .builds + .joins(:metadata) + .pluck(:name, 'ci_builds_metadata.interruptible') + + expect(interruptible_status).to contain_exactly( + ['build_1_1', nil], + ['build_1_2', true], + ['build_2_1', nil], + ['build_3_1', false], + ['build_4_1', nil] + ) + end - status.each_with_index do |s, i| - current_build = pipeline_on_previous_commit.builds[i] - current_build.update(status: 'created') - current_build.fire_events!('enqueue') - current_build.update(status: 'pending') + context 'when only interruptible builds are running' do + context 'when build marked explicitly by interruptible is running' do + it 'cancels running outdated pipelines' do + pipeline_on_previous_commit + .builds + .find_by_name('build_1_2') + .run! - if s != 'scheduled' - current_build.fire_events!('run') - end + pipeline - current_build.update(status: s) + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'canceled', auto_canceled_by_id: pipeline.id) + end end - pipeline - end + context 'when build that is not marked as interruptible is running' do + it 'cancels running outdated pipelines' do + pipeline_on_previous_commit + .builds + .find_by_name('build_2_1') + .tap(&:enqueue!) + .run! - context 'when the status is updated' do - it 'applies the transitions accordingly' do - set_pipeline_status(%w[success success success running]) - - expect(pipeline_on_previous_commit.builds[0].started?).to be_truthy - expect(pipeline_on_previous_commit.builds[1].started?).to be_truthy - expect(pipeline_on_previous_commit.builds[2].started?).to be_truthy - expect(pipeline_on_previous_commit.builds[3].started?).to be_truthy - expect(pipeline_on_previous_commit.builds[4].started?).to be_falsy - expect(pipeline_on_previous_commit.builds[0].interruptible).to be_nil - expect(pipeline_on_previous_commit.builds[1].interruptible).to be_truthy - expect(pipeline_on_previous_commit.builds[2].interruptible).to be_nil - expect(pipeline_on_previous_commit.builds[3].interruptible).to be_falsy - expect(pipeline_on_previous_commit.builds[4].interruptible).to be_nil - end - end + pipeline - context 'when only interruptible builds are running' do - it 'cancels running outdated pipelines' do - pipeline_on_previous_commit.run - pipeline - - expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'canceled', auto_canceled_by_id: pipeline.id) + end end end - context 'when an uninterruptible build has been reached' do + context 'when an uninterruptible build is running' do it 'does not cancel running outdated pipelines' do - set_pipeline_status(%w[success success success running]) + pipeline_on_previous_commit + .builds + .find_by_name('build_3_1') + .tap(&:enqueue!) + .run! + pipeline - expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'running', auto_canceled_by_id: nil) + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'running', auto_canceled_by_id: nil) end end context 'when an build is waiting on an interruptible scheduled task' do it 'cancels running outdated pipelines' do - set_pipeline_status(%w[success success scheduled]) - pipeline + allow(Ci::BuildScheduleWorker).to receive(:perform_at) - expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) - end - end + pipeline_on_previous_commit + .builds + .find_by_name('build_2_1') + .schedule! - context 'when an build is waiting on an interruptible scheduled task' do - it 'cancels running outdated pipelines' do - set_pipeline_status(%w[success success scheduled]) pipeline - expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'canceled', auto_canceled_by_id: pipeline.id) + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'canceled', auto_canceled_by_id: pipeline.id) end end - context 'when a uninterruptible build has been executed' do + context 'when a uninterruptible build has finished' do it 'does not cancel running outdated pipelines' do - set_pipeline_status(%w[success success success success running]) + pipeline_on_previous_commit + .builds + .find_by_name('build_3_1') + .success! + pipeline - expect(pipeline_on_previous_commit.reload).to have_attributes(status: 'running', auto_canceled_by_id: nil) + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'running', auto_canceled_by_id: nil) end end end -- 2.24.1