Merge branch 'issue-67299' into 'master'

Jobs are considered as not interruptible by default

Closes #31776

See merge request gitlab-org/gitlab!16785

app/models/ci/build.rb

@@ -88,7 +88,11 @@ 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 :not_interruptible, -> do
+      joins(:metadata).where('ci_builds_metadata.id NOT IN (?)',
+        Ci::BuildMetadata.scoped_build.with_interruptible.select(:id))
+    end
+
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
     scope :with_artifacts_archive, ->() do

app/models/ci/build_metadata.rb

@@ -25,6 +25,9 @@ module Ci
 
     chronic_duration_attr_reader :timeout_human_readable, :timeout
 
+    scope :scoped_build, -> { where('ci_builds_metadata.build_id = ci_builds.id') }
+    scope :with_interruptible, -> { where(interruptible: true) }
+
     enum timeout_source: {
         unknown_timeout_source: 1,
         project_timeout_source: 2,

app/services/ci/create_pipeline_service.rb

@@ -95,7 +95,7 @@ module Ci
     # rubocop: disable CodeReuse/ActiveRecord
     def auto_cancelable_pipelines
       # TODO: Introduced by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23464
-      if Feature.enabled?(:ci_support_interruptible_pipelines, project, default_enabled: false)
+      if Feature.enabled?(:ci_support_interruptible_pipelines, project, default_enabled: true)
         project.ci_pipelines
           .where(ref: pipeline.ref)
           .where.not(id: pipeline.id)

db/migrate/20190914223900_modify_concurrent_index_to_builds_metadata.rb

+# frozen_string_literal: true
+
+class ModifyConcurrentIndexToBuildsMetadata < 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 = true",
+                         name: "index_ci_builds_metadata_on_build_id_and_interruptible"
+    remove_concurrent_index_by_name(:ci_builds_metadata, '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')
+    add_concurrent_index :ci_builds_metadata, [:build_id],
+                         where: "interruptible = false",
+                         name: "index_ci_builds_metadata_on_build_id_and_interruptible_false"
+  end
+end

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_12_061145) do
+ActiveRecord::Schema.define(version: 2019_09_14_223900) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -648,7 +648,7 @@ ActiveRecord::Schema.define(version: 2019_09_12_061145) 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 ["build_id"], name: "index_ci_builds_metadata_on_build_id_and_interruptible", where: "(interruptible = true)"
     t.index ["project_id"], name: "index_ci_builds_metadata_on_project_id"
   end
 

doc/ci/yaml/README.md

@@ -2104,7 +2104,7 @@ staging:
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23464) in GitLab 12.3.
 
-`interruptible` is used to indicate that a job should be canceled if made redundant by a newer run of the same job. Defaults to `true`.
+`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`.
 This value will only be used if the [automatic cancellation of redundant pipelines feature](../../user/project/pipelines/settings.md#auto-cancel-pending-pipelines)
 is enabled.
 

spec/services/ci/create_pipeline_service_spec.rb

@@ -287,51 +287,21 @@ describe Ci::CreatePipelineService do
               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
-            Feature.enable(:ci_support_interruptible_pipelines)
             stub_ci_pipeline_yaml_file(YAML.dump(config))
           end
 
-          after do
-            Feature.disable(:ci_support_interruptible_pipelines)
-          end
-
           let(:config) do
             {
               stages: %w[stage1 stage2 stage3 stage4],
 
               build_1_1: {
                 stage: 'stage1',
-                script: 'echo'
+                script: 'echo',
+                interruptible: true
               },
               build_1_2: {
                 stage: 'stage1',
@@ -342,7 +312,8 @@ describe Ci::CreatePipelineService do
                 stage: 'stage2',
                 script: 'echo',
                 when: 'delayed',
-                start_in: '10 minutes'
+                start_in: '10 minutes',
+                interruptible: true
               },
               build_3_1: {
                 stage: 'stage3',
@@ -364,9 +335,9 @@ describe Ci::CreatePipelineService do
                 .pluck(:name, 'ci_builds_metadata.interruptible')
 
             expect(interruptible_status).to contain_exactly(
-              ['build_1_1', nil],
+              ['build_1_1', true],
               ['build_1_2', true],
-              ['build_2_1', nil],
+              ['build_2_1', true],
               ['build_3_1', false],
               ['build_4_1', nil]
             )