diff options
| -rw-r--r-- | app/models/ci/build.rb | 1 | ||||
| -rw-r--r-- | app/models/ci/pipeline.rb | 8 | ||||
| -rw-r--r-- | app/models/concerns/ci/metadatable.rb | 9 | ||||
| -rw-r--r-- | app/models/concerns/has_status.rb | 1 | ||||
| -rw-r--r-- | app/services/ci/create_pipeline_service.rb | 20 | ||||
| -rw-r--r-- | changelogs/unreleased/issue-32741.yml | 5 | ||||
| -rw-r--r-- | db/migrate/20190905223800_add_interruptible_to_builds_metadata.rb | 9 | ||||
| -rw-r--r-- | db/migrate/20190905223900_add_concurrent_index_to_builds_metadata.rb | 19 | ||||
| -rw-r--r-- | db/schema.rb | 4 | ||||
| -rw-r--r-- | doc/ci/yaml/README.md | 41 | ||||
| -rw-r--r-- | lib/gitlab/ci/config/entry/job.rb | 9 | ||||
| -rw-r--r-- | lib/gitlab/ci/yaml_processor.rb | 1 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/yaml_processor_spec.rb | 26 | ||||
| -rw-r--r-- | spec/lib/gitlab/import_export/safe_model_attributes.yml | 1 | ||||
| -rw-r--r-- | spec/models/concerns/has_status_spec.rb | 12 | ||||
| -rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 202 |
16 files changed, 356 insertions, 12 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d558f66154e..72782827906 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(: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 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/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index 304cc71e9dc..a0ca8a34c6d 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 @@ -50,6 +51,14 @@ module Ci write_metadata_attribute(:yaml_variables, :config_variables, value) end + def interruptible + metadata&.interruptible + end + + def interruptible=(value) + ensure_metadata.interruptible = value + end + private def read_metadata_attribute(legacy_key, metadata_key, default_value = nil) 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..8f8582afb43 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -91,11 +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)) - .created_or_pending + # 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 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/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/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 9d7cdaed70c..61f7787f192 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_223900) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -619,9 +619,11 @@ 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 + 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 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. diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 6e11c582750..3009c7e8329 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 @@ -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 } @@ -122,10 +123,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 +209,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..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], diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 91c559dcd9b..cf496b79a62 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[:interruptible]).to be_truthy } + end + + describe 'interruptible job with default value' do + let(:config) do + YAML.dump(rspec: { script: 'rspec' }) + end + + it { expect(subject).not_to have_key(:interruptible) } + end + + describe 'uninterruptible job' do + let(:config) do + YAML.dump(rspec: { script: 'rspec', interruptible: false }) + end + + it { expect(subject[:interruptible]).to be_falsy } + end + end + describe 'retry entry' do context 'when retry count is specified' do let(:config) do diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 1386f8822ce..d34c6d2421b 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 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 } diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index fad865a4811..6cec93a53fd 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,202 @@ 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 + 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 + 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 + 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 + 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 + 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 + + 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 + + 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! + + pipeline + + expect(pipeline_on_previous_commit.reload).to have_attributes( + status: 'canceled', auto_canceled_by_id: pipeline.id) + end + 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! + + pipeline + + 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 is running' do + it 'does not cancel running outdated pipelines' do + 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) + end + end + + context 'when an build is waiting on an interruptible scheduled task' do + it 'cancels running outdated pipelines' do + allow(Ci::BuildScheduleWorker).to receive(:perform_at) + + pipeline_on_previous_commit + .builds + .find_by_name('build_2_1') + .schedule! + + 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 finished' do + it 'does not cancel running outdated pipelines' do + 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) + end + end + end end context 'auto-cancel disabled' do |