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.

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