Merge branch 'backstage/gb/migrate-stages-statuses' into 'master'

Migrate CI/CD stages statuses

Closes #33453

See merge request !12584

app/models/ci/stage.rb

@@ -1,11 +1,66 @@
 module Ci
   class Stage < ActiveRecord::Base
     extend Ci::Model
+    include Importable
+    include HasStatus
+    include Gitlab::OptimisticLocking
+
+    enum status: HasStatus::STATUSES_ENUM
 
     belongs_to :project
     belongs_to :pipeline
 
-    has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id
-    has_many :builds, foreign_key: :commit_id
+    has_many :statuses, class_name: 'CommitStatus', foreign_key: :stage_id
+    has_many :builds, foreign_key: :stage_id
+
+    validates :project, presence: true, unless: :importing?
+    validates :pipeline, presence: true, unless: :importing?
+    validates :name, presence: true, unless: :importing?
+
+    state_machine :status, initial: :created do
+      event :enqueue do
+        transition created: :pending
+        transition [:success, :failed, :canceled, :skipped] => :running
+      end
+
+      event :run do
+        transition any - [:running] => :running
+      end
+
+      event :skip do
+        transition any - [:skipped] => :skipped
+      end
+
+      event :drop do
+        transition any - [:failed] => :failed
+      end
+
+      event :succeed do
+        transition any - [:success] => :success
+      end
+
+      event :cancel do
+        transition any - [:canceled] => :canceled
+      end
+
+      event :block do
+        transition any - [:manual] => :manual
+      end
+    end
+
+    def update_status
+      retry_optimistic_lock(self) do
+        case statuses.latest.status
+        when 'pending' then enqueue
+        when 'running' then run
+        when 'success' then succeed
+        when 'failed' then drop
+        when 'canceled' then cancel
+        when 'manual' then block
+        when 'skipped' then skip
+        else skip
+        end
+      end
+    end
   end
 end

app/models/commit_status.rb

@@ -39,14 +39,14 @@ class CommitStatus < ActiveRecord::Base
   scope :after_stage, -> (index) { where('stage_idx > ?', index) }
 
   state_machine :status do
-    event :enqueue do
-      transition [:created, :skipped, :manual] => :pending
-    end
-
     event :process do
       transition [:skipped, :manual] => :created
     end
 
+    event :enqueue do
+      transition [:created, :skipped, :manual] => :pending
+    end
+
     event :run do
       transition pending: :running
     end
@@ -91,6 +91,7 @@ class CommitStatus < ActiveRecord::Base
           end
         end
 
+        StageUpdateWorker.perform_async(commit_status.stage_id)
         ExpireJobCacheWorker.perform_async(commit_status.id)
       end
     end

app/models/concerns/has_status.rb

@@ -8,6 +8,8 @@ module HasStatus
   ACTIVE_STATUSES = %w[pending running].freeze
   COMPLETED_STATUSES = %w[success failed canceled skipped].freeze
   ORDERED_STATUSES = %w[failed pending running manual canceled success skipped created].freeze
+  STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3,
+                    failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze
 
   class_methods do
     def status_sql

app/workers/stage_update_worker.rb

@@ -0,0 +1,10 @@
+class StageUpdateWorker
+  include Sidekiq::Worker
+  include PipelineQueue
+
+  def perform(stage_id)
+    Ci::Stage.find_by(id: stage_id).try do |stage|
+      stage.update_status
+    end
+  end
+end

db/migrate/20170711145320_add_status_to_ci_stages.rb

@@ -0,0 +1,9 @@
+class AddStatusToCiStages < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_stages, :status, :integer
+  end
+end

db/migrate/20170720111708_add_lock_version_to_ci_stages.rb

@@ -0,0 +1,9 @@
+class AddLockVersionToCiStages < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_stages, :lock_version, :integer
+  end
+end

db/post_migrate/20170711145558_migrate_stages_statuses.rb

@@ -0,0 +1,33 @@
+class MigrateStagesStatuses < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  BATCH_SIZE = 10000
+  RANGE_SIZE = 1000
+  MIGRATION = 'MigrateStageStatus'.freeze
+
+  class Stage < ActiveRecord::Base
+    self.table_name = 'ci_stages'
+    include ::EachBatch
+  end
+
+  def up
+    Stage.where(status: nil).each_batch(of: BATCH_SIZE) do |relation, index|
+      relation.each_batch(of: RANGE_SIZE) do |batch|
+        range = relation.pluck('MIN(id)', 'MAX(id)').first
+        schedule = index * 5.minutes
+
+        BackgroundMigrationWorker.perform_in(schedule, MIGRATION, range)
+      end
+    end
+  end
+
+  def down
+    disable_statement_timeout
+
+    update_column_in_batches(:ci_stages, :status, nil)
+  end
+end

db/schema.rb

@@ -379,6 +379,8 @@ ActiveRecord::Schema.define(version: 20170820100558) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string "name"
+    t.integer "status"
+    t.integer "lock_version"
   end
 
   add_index "ci_stages", ["pipeline_id", "name"], name: "index_ci_stages_on_pipeline_id_and_name", using: :btree

lib/gitlab/background_migration/migrate_stage_status.rb

@@ -0,0 +1,77 @@
+module Gitlab
+  module BackgroundMigration
+    class MigrateStageStatus
+      STATUSES = { created: 0, pending: 1, running: 2, success: 3,
+                   failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze
+
+      class Build < ActiveRecord::Base
+        self.table_name = 'ci_builds'
+
+        scope :latest, -> { where(retried: [false, nil]) }
+        scope :created, -> { where(status: 'created') }
+        scope :running, -> { where(status: 'running') }
+        scope :pending, -> { where(status: 'pending') }
+        scope :success, -> { where(status: 'success') }
+        scope :failed, -> { where(status: 'failed')  }
+        scope :canceled, -> { where(status: 'canceled')  }
+        scope :skipped, -> { where(status: 'skipped')  }
+        scope :manual, -> { where(status: 'manual')  }
+
+        scope :failed_but_allowed, -> do
+          where(allow_failure: true, status: [:failed, :canceled])
+        end
+
+        scope :exclude_ignored, -> do
+          where("allow_failure = ? OR status IN (?)",
+            false, %w[created pending running success skipped])
+        end
+
+        def self.status_sql
+          scope_relevant = latest.exclude_ignored
+          scope_warnings = latest.failed_but_allowed
+
+          builds = scope_relevant.select('count(*)').to_sql
+          created = scope_relevant.created.select('count(*)').to_sql
+          success = scope_relevant.success.select('count(*)').to_sql
+          manual = scope_relevant.manual.select('count(*)').to_sql
+          pending = scope_relevant.pending.select('count(*)').to_sql
+          running = scope_relevant.running.select('count(*)').to_sql
+          skipped = scope_relevant.skipped.select('count(*)').to_sql
+          canceled = scope_relevant.canceled.select('count(*)').to_sql
+          warnings = scope_warnings.select('count(*) > 0').to_sql
+
+          <<-SQL.strip_heredoc
+            (CASE
+              WHEN (#{builds}) = (#{skipped}) AND (#{warnings}) THEN #{STATUSES[:success]}
+              WHEN (#{builds}) = (#{skipped}) THEN #{STATUSES[:skipped]}
+              WHEN (#{builds}) = (#{success}) THEN #{STATUSES[:success]}
+              WHEN (#{builds}) = (#{created}) THEN #{STATUSES[:created]}
+              WHEN (#{builds}) = (#{success}) + (#{skipped}) THEN #{STATUSES[:success]}
+              WHEN (#{builds}) = (#{success}) + (#{skipped}) + (#{canceled}) THEN #{STATUSES[:canceled]}
+              WHEN (#{builds}) = (#{created}) + (#{skipped}) + (#{pending}) THEN #{STATUSES[:pending]}
+              WHEN (#{running}) + (#{pending}) > 0 THEN #{STATUSES[:running]}
+              WHEN (#{manual}) > 0 THEN #{STATUSES[:manual]}
+              WHEN (#{created}) > 0 THEN #{STATUSES[:running]}
+              ELSE #{STATUSES[:failed]}
+            END)
+          SQL
+        end
+      end
+
+      def perform(start_id, stop_id)
+        status_sql = Build
+          .where('ci_builds.commit_id = ci_stages.pipeline_id')
+          .where('ci_builds.stage = ci_stages.name')
+          .status_sql
+
+        sql = <<-SQL
+          UPDATE ci_stages SET status = (#{status_sql})
+            WHERE ci_stages.status IS NULL
+            AND ci_stages.id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}
+        SQL
+
+        ActiveRecord::Base.connection.execute(sql)
+      end
+    end
+  end
+end

spec/factories/ci/stages.rb

@@ -15,4 +15,12 @@ FactoryGirl.define do
                                     warnings: warnings)
     end
   end
+
+  factory :ci_stage_entity, class: Ci::Stage do
+    project factory: :project
+    pipeline factory: :ci_empty_pipeline
+
+    name 'test'
+    status 'pending'
+  end
 end

spec/lib/gitlab/background_migration/migrate_stage_status_spec.rb

@@ -0,0 +1,80 @@
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::MigrateStageStatus, :migration, schema: 20170711145320 do
+  let(:projects) { table(:projects) }
+  let(:pipelines) { table(:ci_pipelines) }
+  let(:stages) { table(:ci_stages) }
+  let(:jobs) { table(:ci_builds) }
+
+  STATUSES = { created: 0, pending: 1, running: 2, success: 3,
+               failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze
+
+  before do
+    projects.create!(id: 1, name: 'gitlab1', path: 'gitlab1')
+    pipelines.create!(id: 1, project_id: 1, ref: 'master', sha: 'adf43c3a')
+    stages.create!(id: 1, pipeline_id: 1, project_id: 1, name: 'test', status: nil)
+    stages.create!(id: 2, pipeline_id: 1, project_id: 1, name: 'deploy', status: nil)
+  end
+
+  context 'when stage status is known' do
+    before do
+      create_job(project: 1, pipeline: 1, stage: 'test', status: 'success')
+      create_job(project: 1, pipeline: 1, stage: 'test', status: 'running')
+      create_job(project: 1, pipeline: 1, stage: 'deploy', status: 'failed')
+    end
+
+    it 'sets a correct stage status' do
+      described_class.new.perform(1, 2)
+
+      expect(stages.first.status).to eq STATUSES[:running]
+      expect(stages.second.status).to eq STATUSES[:failed]
+    end
+  end
+
+  context 'when stage status is not known' do
+    it 'sets a skipped stage status' do
+      described_class.new.perform(1, 2)
+
+      expect(stages.first.status).to eq STATUSES[:skipped]
+      expect(stages.second.status).to eq STATUSES[:skipped]
+    end
+  end
+
+  context 'when stage status includes status of a retried job' do
+    before do
+      create_job(project: 1, pipeline: 1, stage: 'test', status: 'canceled')
+      create_job(project: 1, pipeline: 1, stage: 'deploy', status: 'failed', retried: true)
+      create_job(project: 1, pipeline: 1, stage: 'deploy', status: 'success')
+    end
+
+    it 'sets a correct stage status' do
+      described_class.new.perform(1, 2)
+
+      expect(stages.first.status).to eq STATUSES[:canceled]
+      expect(stages.second.status).to eq STATUSES[:success]
+    end
+  end
+
+  context 'when some job in the stage is blocked / manual' do
+    before do
+      create_job(project: 1, pipeline: 1, stage: 'test', status: 'failed')
+      create_job(project: 1, pipeline: 1, stage: 'test', status: 'manual')
+      create_job(project: 1, pipeline: 1, stage: 'deploy', status: 'success', when: 'manual')
+    end
+
+    it 'sets a correct stage status' do
+      described_class.new.perform(1, 2)
+
+      expect(stages.first.status).to eq STATUSES[:manual]
+      expect(stages.second.status).to eq STATUSES[:success]
+    end
+  end
+
+  def create_job(project:, pipeline:, stage:, status:, **opts)
+    stages = { test: 1, build: 2, deploy: 3 }
+
+    jobs.create!(project_id: project, commit_id: pipeline,
+                 stage_idx: stages[stage.to_sym], stage: stage,
+                 status: status, **opts)
+  end
+end

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -227,6 +227,8 @@ Ci::Pipeline:
 Ci::Stage:
 - id
 - name
+- status
+- lock_version
 - project_id
 - pipeline_id
 - created_at

spec/migrations/migrate_stage_id_reference_in_background_spec.rb

@@ -2,19 +2,6 @@ require 'spec_helper'
 require Rails.root.join('db', 'post_migrate', '20170628080858_migrate_stage_id_reference_in_background')
 
 describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do
-  matcher :be_scheduled_migration do |delay, *expected|
-    match do |migration|
-      BackgroundMigrationWorker.jobs.any? do |job|
-        job['args'] == [migration, expected] &&
-          job['at'].to_i == (delay.to_i + Time.now.to_i)
-      end
-    end
-
-    failure_message do |migration|
-      "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!"
-    end
-  end
-
   let(:jobs) { table(:ci_builds) }
   let(:stages) { table(:ci_stages) }
   let(:pipelines) { table(:ci_pipelines) }

spec/migrations/migrate_stages_statuses_spec.rb

@@ -0,0 +1,67 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170711145558_migrate_stages_statuses.rb')
+
+describe MigrateStagesStatuses, :migration do
+  let(:jobs) { table(:ci_builds) }
+  let(:stages) { table(:ci_stages) }
+  let(:pipelines) { table(:ci_pipelines) }
+  let(:projects) { table(:projects) }
+
+  STATUSES = { created: 0, pending: 1, running: 2, success: 3,
+               failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze
+
+  before do
+    stub_const("#{described_class.name}::BATCH_SIZE", 2)
+    stub_const("#{described_class.name}::RANGE_SIZE", 2)
+
+    projects.create!(id: 1, name: 'gitlab1', path: 'gitlab1')
+    projects.create!(id: 2, name: 'gitlab2', path: 'gitlab2')
+
+    pipelines.create!(id: 1, project_id: 1, ref: 'master', sha: 'adf43c3a')
+    pipelines.create!(id: 2, project_id: 2, ref: 'feature', sha: '21a3deb')
+
+    create_job(project: 1, pipeline: 1, stage: 'test', status: 'success')
+    create_job(project: 1, pipeline: 1, stage: 'test', status: 'running')
+    create_job(project: 1, pipeline: 1, stage: 'build', status: 'success')
+    create_job(project: 1, pipeline: 1, stage: 'build', status: 'failed')
+    create_job(project: 2, pipeline: 2, stage: 'test', status: 'success')
+    create_job(project: 2, pipeline: 2, stage: 'test', status: 'success')
+    create_job(project: 2, pipeline: 2, stage: 'test', status: 'failed', retried: true)
+
+    stages.create!(id: 1, pipeline_id: 1, project_id: 1, name: 'test', status: nil)
+    stages.create!(id: 2, pipeline_id: 1, project_id: 1, name: 'build', status: nil)
+    stages.create!(id: 3, pipeline_id: 2, project_id: 2, name: 'test', status: nil)
+  end
+
+  it 'correctly migrates stages statuses' do
+    Sidekiq::Testing.inline! do
+      expect(stages.where(status: nil).count).to eq 3
+
+      migrate!
+
+      expect(stages.where(status: nil)).to be_empty
+      expect(stages.all.order('id ASC').pluck(:status))
+        .to eq [STATUSES[:running], STATUSES[:failed], STATUSES[:success]]
+    end
+  end
+
+  it 'correctly schedules background migrations' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 2)
+        expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 3, 3)
+        expect(BackgroundMigrationWorker.jobs.size).to eq 2
+      end
+    end
+  end
+
+  def create_job(project:, pipeline:, stage:, status:, **opts)
+    stages = { test: 1, build: 2, deploy: 3 }
+
+    jobs.create!(project_id: project, commit_id: pipeline,
+                 stage_idx: stages[stage.to_sym], stage: stage,
+                 status: status, **opts)
+  end
+end

spec/models/ci/stage_spec.rb

@@ -0,0 +1,79 @@
+require 'spec_helper'
+
+describe Ci::Stage, :models do
+  let(:stage) { create(:ci_stage_entity) }
+
+  describe 'associations' do
+    before do
+      create(:ci_build, stage_id: stage.id)
+      create(:commit_status, stage_id: stage.id)
+    end
+
+    describe '#statuses' do
+      it 'returns all commit statuses' do
+        expect(stage.statuses.count).to be 2
+      end
+    end
+
+    describe '#builds' do
+      it 'returns only builds' do
+        expect(stage.builds).to be_one
+      end
+    end
+  end
+
+  describe '#status' do
+    context 'when stage is pending' do
+      let(:stage) { create(:ci_stage_entity, status: 'pending') }
+
+      it 'has a correct status value' do
+        expect(stage.status).to eq 'pending'
+      end
+    end
+
+    context 'when stage is success' do
+      let(:stage) { create(:ci_stage_entity, status: 'success') }
+
+      it 'has a correct status value' do
+        expect(stage.status).to eq 'success'
+      end
+    end
+  end
+
+  describe 'update_status' do
+    context 'when stage objects needs to be updated' do
+      before do
+        create(:ci_build, :success, stage_id: stage.id)
+        create(:ci_build, :running, stage_id: stage.id)
+      end
+
+      it 'updates stage status correctly' do
+        expect { stage.update_status }
+          .to change { stage.reload.status }
+          .to 'running'
+      end
+    end
+
+    context 'when stage is skipped' do
+      it 'updates status to skipped' do
+        expect { stage.update_status }
+          .to change { stage.reload.status }
+          .to 'skipped'
+      end
+    end
+
+    context 'when stage object is locked' do
+      before do
+        create(:ci_build, :failed, stage_id: stage.id)
+      end
+
+      it 'retries a lock to update a stage status' do
+        stage.lock_version = 100
+
+        stage.update_status
+
+        expect(stage.reload).to be_failed
+      end
+    end
+  end
+end

spec/models/commit_status_spec.rb

@@ -7,10 +7,10 @@ describe CommitStatus do
     create(:ci_pipeline, project: project, sha: project.commit.id)
   end
 
-  let(:commit_status) { create_status }
+  let(:commit_status) { create_status(stage: 'test') }
 
-  def create_status(args = {})
-    create(:commit_status, args.merge(pipeline: pipeline))
+  def create_status(**opts)
+    create(:commit_status, pipeline: pipeline, **opts)
   end
 
   it { is_expected.to belong_to(:pipeline) }

spec/support/background_migrations_matchers.rb

@@ -0,0 +1,13 @@
+RSpec::Matchers.define :be_scheduled_migration do |delay, *expected|
+  match do |migration|
+    BackgroundMigrationWorker.jobs.any? do |job|
+      job['args'] == [migration, expected] &&
+        job['at'].to_i == (delay.to_i + Time.now.to_i)
+    end
+  end
+
+  failure_message do |migration|
+    "Migration `#{migration}` with args `#{expected.inspect}` " \
+      'not scheduled in expected time!'
+  end
+end

spec/workers/stage_update_worker_spec.rb

@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe StageUpdateWorker do
+  describe '#perform' do
+    context 'when stage exists' do
+      let(:stage) { create(:ci_stage_entity) }
+
+      it 'updates stage status' do
+        expect_any_instance_of(Ci::Stage).to receive(:update_status)
+
+        described_class.new.perform(stage.id)
+      end
+    end
+
+    context 'when stage does not exist' do
+      it 'does not raise exception' do
+        expect { described_class.new.perform(123) }
+          .not_to raise_error
+      end
+    end
+  end
+end