Merge branch '30631-storing-scheduling-type-for-builds' into 'master'

Store scheduling type of builds

See merge request gitlab-org/gitlab!22245

app/models/ci/bridge.rb

@@ -8,7 +8,6 @@ module Ci
     include Importable
     include AfterCommitQueue
     include HasRef
-    include Gitlab::Utils::StrongMemoize
 
     InvalidBridgeTypeError = Class.new(StandardError)
 

app/models/ci/build.rb

@@ -10,7 +10,6 @@ module Ci
     include ObjectStorage::BackgroundMove
     include Presentable
     include Importable
-    include Gitlab::Utils::StrongMemoize
     include HasRef
     include IgnorableColumns
 

app/models/ci/processable.rb

@@ -2,10 +2,14 @@
 
 module Ci
   class Processable < ::CommitStatus
+    include Gitlab::Utils::StrongMemoize
+
     has_many :needs, class_name: 'Ci::BuildNeed', foreign_key: :build_id, inverse_of: :build
 
     accepts_nested_attributes_for :needs
 
+    enum scheduling_type: { stage: 0, dag: 1 }, _prefix: true
+
     scope :preload_needs, -> { preload(:needs) }
 
     def self.select_with_aggregated_needs(project)
@@ -23,6 +27,7 @@ module Ci
     end
 
     validates :type, presence: true
+    validates :scheduling_type, presence: true, on: :create, if: :validate_scheduling_type?
 
     def aggregated_needs_names
       read_attribute(:aggregated_needs_names)
@@ -47,5 +52,19 @@ module Ci
     def scoped_variables_hash
       raise NotImplementedError
     end
+
+    # scheduling_type column of previous builds/bridges have not been populated,
+    # so we calculate this value on runtime when we need it.
+    def find_legacy_scheduling_type
+      strong_memoize(:find_legacy_scheduling_type) do
+        needs.exists? ? :dag : :stage
+      end
+    end
+
+    private
+
+    def validate_scheduling_type?
+      !importing? && Feature.enabled?(:validate_scheduling_type_of_processables, project)
+    end
   end
 end

app/services/ci/retry_build_service.rb

@@ -5,7 +5,7 @@ module Ci
     CLONE_ACCESSORS = %i[pipeline project ref tag options name
                          allow_failure stage stage_id stage_idx trigger_request
                          yaml_variables when environment coverage_regex
-                         description tag_list protected needs resource_group].freeze
+                         description tag_list protected needs resource_group scheduling_type].freeze
 
     def execute(build)
       reprocess!(build).tap do |new_build|
@@ -27,9 +27,10 @@ module Ci
 
       attributes = CLONE_ACCESSORS.map do |attribute|
         [attribute, build.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend
-      end
+      end.to_h
 
-      attributes.push([:user, current_user])
+      attributes[:user] = current_user
+      attributes[:scheduling_type] ||= build.find_legacy_scheduling_type
 
       Ci::Build.transaction do
         # mark all other builds of that name as retried
@@ -49,7 +50,7 @@ module Ci
     private
 
     def create_build!(attributes)
-      build = project.builds.new(Hash[attributes])
+      build = project.builds.new(attributes)
       build.deployment = ::Gitlab::Ci::Pipeline::Seed::Deployment.new(build).to_resource
       build.retried = false
       build.save!

db/fixtures/development/14_pipelines.rb

@@ -165,9 +165,10 @@ class Gitlab::Seeder::Pipelines
   end
 
   def job_attributes(pipeline, opts)
-    { name: 'test build', stage: 'test', stage_idx: stage_index(opts[:stage]),
+    {
+      name: 'test build', stage: 'test', stage_idx: stage_index(opts[:stage]),
       ref: pipeline.ref, tag: false, user: build_user, project: @project, pipeline: pipeline,
-      created_at: Time.now, updated_at: Time.now
+      scheduling_type: :stage, created_at: Time.now, updated_at: Time.now
     }.merge(opts)
   end
 

db/migrate/20191223124940_add_scheduling_type_to_ci_builds.rb

+# frozen_string_literal: true
+
+class AddSchedulingTypeToCiBuilds < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_builds, :scheduling_type, :integer, limit: 2
+  end
+end

db/schema.rb

@@ -677,6 +677,7 @@ ActiveRecord::Schema.define(version: 2020_02_11_152410) do
     t.bigint "resource_group_id"
     t.datetime_with_timezone "waiting_for_resource_at"
     t.boolean "processed"
+    t.integer "scheduling_type", limit: 2
     t.index ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)"
     t.index ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id"
     t.index ["commit_id", "artifacts_expire_at", "id"], name: "index_ci_builds_on_commit_id_and_artifacts_expireatandidpartial", where: "(((type)::text = 'Ci::Build'::text) AND ((retried = false) OR (retried IS NULL)) AND ((name)::text = ANY (ARRAY[('sast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('sast:container'::character varying)::text, ('container_scanning'::character varying)::text, ('dast'::character varying)::text])))"

ee/app/services/ci/create_web_ide_terminal_service.rb

@@ -65,7 +65,8 @@ module Ci
       terminal.merge(
         name: TERMINAL_NAME,
         stage: TERMINAL_NAME,
-        user: current_user)
+        user: current_user,
+        scheduling_type: :stage)
     end
 
     def load_terminal_config!

ee/spec/lib/ee/gitlab/ci/config/entry/bridge_spec.rb

@@ -46,7 +46,28 @@ describe Gitlab::Ci::Config::Entry::Bridge do
                                       needs: { bridge: [{ pipeline: 'some/project' }] },
                                       ignore: false,
                                       stage: 'test',
-                                      only: { refs: %w[branches tags] })
+                                      only: { refs: %w[branches tags] },
+                                      scheduling_type: :stage)
+        end
+      end
+    end
+
+    context 'when needs config is a job' do
+      let(:config) { { trigger: { project: 'some/project' }, needs: ['some_job'] } }
+
+      describe '#valid?' do
+        it { is_expected.to be_valid }
+      end
+
+      describe '#value' do
+        it 'is returns a bridge job configuration' do
+          expect(subject.value).to eq(name: :my_bridge,
+                                      trigger: { project: 'some/project' },
+                                      needs: { job: [{ name: 'some_job', artifacts: true }] },
+                                      ignore: false,
+                                      stage: 'test',
+                                      only: { refs: %w[branches tags] },
+                                      scheduling_type: :dag)
         end
       end
     end

ee/spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -28,7 +28,8 @@ describe Gitlab::Ci::YamlProcessor do
           },
           when: "on_success",
           allow_failure: false,
-          yaml_variables: []
+          yaml_variables: [],
+          scheduling_type: :stage
         )
         expect(subject.builds[1]).to eq(
           stage: "test",
@@ -40,7 +41,8 @@ describe Gitlab::Ci::YamlProcessor do
           },
           when: "on_success",
           allow_failure: false,
-          yaml_variables: []
+          yaml_variables: [],
+          scheduling_type: :stage
         )
       end
     end
@@ -60,7 +62,8 @@ describe Gitlab::Ci::YamlProcessor do
           },
           when: "on_success",
           allow_failure: false,
-          yaml_variables: []
+          yaml_variables: [],
+          scheduling_type: :stage
         )
         expect(subject.builds[1]).to eq(
           stage: "test",
@@ -75,7 +78,8 @@ describe Gitlab::Ci::YamlProcessor do
           ],
           when: "on_success",
           allow_failure: false,
-          yaml_variables: []
+          yaml_variables: [],
+          scheduling_type: :stage
         )
       end
     end
@@ -148,7 +152,8 @@ describe Gitlab::Ci::YamlProcessor do
           only: { refs: %w[branches tags] },
           when: 'on_success',
           allow_failure: false,
-          yaml_variables: []
+          yaml_variables: [],
+          scheduling_type: :dag
         )
       end
     end

ee/spec/services/ci/create_pipeline_service/needs_spec.rb

@@ -49,4 +49,10 @@ describe Ci::CreatePipelineService do
 
     expect(bridge.downstream_project).to eq downstream_project
   end
+
+  it "sets scheduling_type of bridge_dag_job as 'dag'" do
+    bridge = execute.stages.last.bridges.first
+
+    expect(bridge.scheduling_type).to eq('dag')
+  end
 end

lib/gitlab/ci/config/entry/bridge.rb

@@ -132,7 +132,8 @@ module Gitlab
               variables: (variables_value if variables_defined?),
               rules: (rules_value if has_rules?),
               only: only_value,
-              except: except_value }.compact
+              except: except_value,
+              scheduling_type: needs_defined? && !bridge_needs ? :dag : :stage }.compact
           end
 
           def bridge_needs

lib/gitlab/ci/config/entry/job.rb

@@ -258,7 +258,8 @@ module Gitlab
               after_script: after_script_value,
               ignore: ignored?,
               needs: needs_defined? ? needs_value : nil,
-              resource_group: resource_group }
+              resource_group: resource_group,
+              scheduling_type: needs_defined? ? :dag : :stage }
           end
         end
       end

lib/gitlab/ci/yaml_processor.rb

@@ -65,6 +65,7 @@ module Gitlab
           rules: job[:rules],
           cache: job[:cache],
           resource_group_key: job[:resource_group],
+          scheduling_type: job[:scheduling_type],
           options: {
             image: job[:image],
             services: job[:services],

spec/factories/ci/bridge.rb

@@ -9,6 +9,7 @@ FactoryBot.define do
     tag { false }
     created_at { 'Di 29. Okt 09:50:00 CET 2013' }
     status { :created }
+    scheduling_type { 'stage' }
 
     pipeline factory: :ci_pipeline
 

spec/factories/ci/builds.rb

@@ -11,6 +11,7 @@ FactoryBot.define do
     tag { false }
     add_attribute(:protected) { false }
     created_at { 'Di 29. Okt 09:50:00 CET 2013' }
+    scheduling_type { 'stage' }
     pending
 
     options do

spec/lib/gitlab/ci/config/entry/bridge_spec.rb

@@ -105,7 +105,8 @@ describe Gitlab::Ci::Config::Entry::Bridge do
                                       trigger: { project: 'some/project' },
                                       ignore: false,
                                       stage: 'test',
-                                      only: { refs: %w[branches tags] })
+                                      only: { refs: %w[branches tags] },
+                                      scheduling_type: :stage)
         end
       end
     end
@@ -126,7 +127,8 @@ describe Gitlab::Ci::Config::Entry::Bridge do
                                                  branch: 'feature' },
                                       ignore: false,
                                       stage: 'test',
-                                      only: { refs: %w[branches tags] })
+                                      only: { refs: %w[branches tags] },
+                                      scheduling_type: :stage)
         end
       end
     end

spec/lib/gitlab/ci/config/entry/job_spec.rb

@@ -110,6 +110,10 @@ describe Gitlab::Ci::Config::Entry::Job do
 
         it { expect(entry).to be_valid }
 
+        it "returns scheduling_type as :dag" do
+          expect(entry.value[:scheduling_type]).to eq(:dag)
+        end
+
         context 'when has dependencies' do
           let(:config) do
             {
@@ -598,7 +602,8 @@ describe Gitlab::Ci::Config::Entry::Job do
                    ignore: false,
                    after_script: %w[cleanup],
                    only: { refs: %w[branches tags] },
-                   variables: {})
+                   variables: {},
+                   scheduling_type: :stage)
         end
       end
     end

spec/lib/gitlab/ci/config/entry/jobs_spec.rb

@@ -98,7 +98,8 @@ describe Gitlab::Ci::Config::Entry::Jobs do
               name: :my_trigger,
               only: { refs: %w[branches tags] },
               stage: 'test',
-              trigger: { project: 'my/project' }
+              trigger: { project: 'my/project' },
+              scheduling_type: :stage
             },
             regular_job: {
               ignore: false,
@@ -106,7 +107,8 @@ describe Gitlab::Ci::Config::Entry::Jobs do
               only: { refs: %w[branches tags] },
               script: ['something'],
               stage: 'test',
-              variables: {}
+              variables: {},
+              scheduling_type: :stage
             })
         end
       end

spec/lib/gitlab/ci/config/entry/root_spec.rb

@@ -130,7 +130,8 @@ describe Gitlab::Ci::Config::Entry::Root do
                        variables: {},
                        ignore: false,
                        after_script: ['make clean'],
-                       only: { refs: %w[branches tags] } }
+                       only: { refs: %w[branches tags] },
+                       scheduling_type: :stage }
             )
             expect(root.jobs_value[:spinach]).to eq(
               { name: :spinach,
@@ -143,7 +144,8 @@ describe Gitlab::Ci::Config::Entry::Root do
                          variables: {},
                          ignore: false,
                          after_script: ['make clean'],
-                         only: { refs: %w[branches tags] } }
+                         only: { refs: %w[branches tags] },
+                         scheduling_type: :stage }
             )
             expect(root.jobs_value[:release]).to eq(
               { name: :release,
@@ -157,7 +159,8 @@ describe Gitlab::Ci::Config::Entry::Root do
                          only: { refs: %w(branches tags) },
                          variables: {},
                          after_script: [],
-                         ignore: false }
+                         ignore: false,
+                         scheduling_type: :stage }
             )
           end
         end
@@ -203,7 +206,8 @@ describe Gitlab::Ci::Config::Entry::Root do
                        variables: {},
                        ignore: false,
                        after_script: ['make clean'],
-                       only: { refs: %w[branches tags] } },
+                       only: { refs: %w[branches tags] },
+                       scheduling_type: :stage },
               spinach: { name: :spinach,
                          before_script: [],
                          script: %w[spinach],
@@ -214,7 +218,8 @@ describe Gitlab::Ci::Config::Entry::Root do
                          variables: { 'VAR' => 'AA' },
                          ignore: false,
                          after_script: ['make clean'],
-                         only: { refs: %w[branches tags] } }
+                         only: { refs: %w[branches tags] },
+                         scheduling_type: :stage }
             )
           end
         end

spec/lib/gitlab/ci/pipeline/seed/build_spec.rb

@@ -6,7 +6,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
   let(:project) { create(:project, :repository) }
   let(:head_sha) { project.repository.head_commit.id }
   let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: head_sha) }
-  let(:attributes) { { name: 'rspec', ref: 'master' } }
+  let(:attributes) { { name: 'rspec', ref: 'master', scheduling_type: :stage } }
   let(:previous_stages) { [] }
 
   let(:seed_build) { described_class.new(pipeline, attributes, previous_stages) }
@@ -244,7 +244,9 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
     context 'when job is a bridge' do
       let(:attributes) do
-        { name: 'rspec', ref: 'master', options: { trigger: 'my/project' } }
+        {
+          name: 'rspec', ref: 'master', options: { trigger: 'my/project' }, scheduling_type: :stage
+        }
       end
 
       it { is_expected.to be_a(::Ci::Bridge) }

spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb

@@ -10,9 +10,9 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do
   let(:attributes) do
     { name: 'test',
       index: 0,
-      builds: [{ name: 'rspec' },
-               { name: 'spinach' },
-               { name: 'deploy', only: { refs: ['feature'] } }] }
+      builds: [{ name: 'rspec', scheduling_type: :stage },
+               { name: 'spinach', scheduling_type: :stage },
+               { name: 'deploy', only: { refs: ['feature'] } }], scheduling_type: :stage }
   end
 
   subject do

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -36,7 +36,8 @@ module Gitlab
               interruptible: true,
               allow_failure: false,
               when: "on_success",
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
         end
@@ -66,7 +67,8 @@ module Gitlab
               ],
               allow_failure: false,
               when: 'on_success',
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
         end
@@ -126,7 +128,8 @@ module Gitlab
               interruptible: true,
               allow_failure: false,
               when: "on_success",
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
         end
@@ -282,6 +285,7 @@ module Gitlab
                   allow_failure: false,
                   when: "on_success",
                   yaml_variables: [],
+                  scheduling_type: :stage,
                   options: { script: ["rspec"] },
                   only: { refs: ["branches"] } }] },
            { name: "deploy",
@@ -293,6 +297,7 @@ module Gitlab
                   allow_failure: false,
                   when: "on_success",
                   yaml_variables: [],
+                  scheduling_type: :stage,
                   options: { script: ["cap prod"] },
                   only: { refs: ["tags"] } }] },
            { name: ".post",
@@ -642,7 +647,8 @@ module Gitlab
               },
               allow_failure: false,
               when: "on_success",
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
 
@@ -674,7 +680,8 @@ module Gitlab
               },
               allow_failure: false,
               when: "on_success",
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
         end
@@ -702,7 +709,8 @@ module Gitlab
               },
               allow_failure: false,
               when: "on_success",
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
 
@@ -728,7 +736,8 @@ module Gitlab
               },
               allow_failure: false,
               when: "on_success",
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
         end
@@ -1250,7 +1259,8 @@ module Gitlab
             },
             when: "on_success",
             allow_failure: false,
-            yaml_variables: []
+            yaml_variables: [],
+            scheduling_type: :stage
           })
         end
 
@@ -1604,7 +1614,8 @@ module Gitlab
               },
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             )
             expect(subject.builds[4]).to eq(
               stage: "test",
@@ -1618,7 +1629,8 @@ module Gitlab
               ],
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :dag
             )
           end
         end
@@ -1644,7 +1656,8 @@ module Gitlab
               },
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             )
             expect(subject.builds[4]).to eq(
               stage: "test",
@@ -1660,7 +1673,8 @@ module Gitlab
               ],
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :dag
             )
           end
         end
@@ -1682,7 +1696,8 @@ module Gitlab
               ],
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :dag
             )
           end
         end
@@ -1712,7 +1727,8 @@ module Gitlab
               ],
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :dag
             )
           end
         end
@@ -1849,7 +1865,8 @@ module Gitlab
               },
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
         end
@@ -1895,7 +1912,8 @@ module Gitlab
               },
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
             expect(subject.second).to eq({
               stage: "build",
@@ -1907,7 +1925,8 @@ module Gitlab
               },
               when: "on_success",
               allow_failure: false,
-              yaml_variables: []
+              yaml_variables: [],
+              scheduling_type: :stage
             })
           end
         end

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -360,6 +360,7 @@ CommitStatus:
 - upstream_pipeline_id
 - interruptible
 - processed
+- scheduling_type
 Ci::Variable:
 - id
 - project_id

spec/models/ci/build_spec.rb

@@ -3007,7 +3007,8 @@ describe Ci::Build do
           stage: 'test',
           ref: 'feature',
           project: project,
-          pipeline: pipeline
+          pipeline: pipeline,
+          scheduling_type: :stage
         )
       end
 

spec/models/ci/processable_spec.rb

@@ -52,4 +52,72 @@ describe Ci::Processable do
       end
     end
   end
+
+  describe 'validate presence of scheduling_type' do
+    context 'on create' do
+      let(:processable) do
+        build(
+          :ci_build, :created, project: project, pipeline: pipeline,
+          importing: importing, scheduling_type: nil
+        )
+      end
+
+      context 'when importing' do
+        let(:importing) { true }
+
+        context 'when validate_scheduling_type_of_processables is true' do
+          before do
+            stub_feature_flags(validate_scheduling_type_of_processables: true)
+          end
+
+          it 'does not validate' do
+            expect(processable).to be_valid
+          end
+        end
+
+        context 'when validate_scheduling_type_of_processables is false' do
+          before do
+            stub_feature_flags(validate_scheduling_type_of_processables: false)
+          end
+
+          it 'does not validate' do
+            expect(processable).to be_valid
+          end
+        end
+      end
+
+      context 'when not importing' do
+        let(:importing) { false }
+
+        context 'when validate_scheduling_type_of_processables is true' do
+          before do
+            stub_feature_flags(validate_scheduling_type_of_processables: true)
+          end
+
+          it 'validates' do
+            expect(processable).not_to be_valid
+          end
+        end
+
+        context 'when validate_scheduling_type_of_processables is false' do
+          before do
+            stub_feature_flags(validate_scheduling_type_of_processables: false)
+          end
+
+          it 'does not validate' do
+            expect(processable).to be_valid
+          end
+        end
+      end
+    end
+
+    context 'on update' do
+      let(:processable) { create(:ci_build, :created, project: project, pipeline: pipeline) }
+
+      it 'does not validate' do
+        processable.scheduling_type = nil
+        expect(processable).to be_valid
+      end
+    end
+  end
 end

spec/services/ci/create_pipeline_service/needs_spec.rb

@@ -131,6 +131,10 @@ describe Ci::CreatePipelineService do
             )
         end
       end
+
+      it "sets scheduling_type as 'dag'" do
+        expect(test_a_build.scheduling_type).to eq('dag')
+      end
     end
 
     context 'with an invalid config' do

spec/services/ci/retry_build_service_spec.rb

@@ -222,6 +222,28 @@ describe Ci::RetryBuildService do
           expect { new_build }.to change { Deployment.count }.by(1)
         end
       end
+
+      context 'when scheduling_type of build is nil' do
+        before do
+          build.update_columns(scheduling_type: nil)
+        end
+
+        context 'when build has not needs' do
+          it 'sets scheduling_type as :stage' do
+            expect(new_build.scheduling_type).to eq('stage')
+          end
+        end
+
+        context 'when build has needs' do
+          before do
+            create(:ci_build_need, build: build)
+          end
+
+          it 'sets scheduling_type as :dag' do
+            expect(new_build.scheduling_type).to eq('dag')
+          end
+        end
+      end
     end
 
     context 'when user does not have ability to execute build' do