Merge branch 'issue-32741' into 'master'

Interruptible builds for redundant pipelines

See merge request gitlab-org/gitlab-ce!23464

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