diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2017-08-21 11:47:44 +0000 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2017-08-21 11:47:44 +0000 |
commit | e00e62c2c8ddf8d12145660a3478065caab4c4ca (patch) | |
tree | df5600704090efccaa399816556696e4487c4e36 | |
parent | d6547ce0e78e2eecac76af8509cc5f7eea5fc369 (diff) | |
parent | 3a1103fd9173e8cb7a70c871d6a54a846f6eee4a (diff) | |
download | gitlab-ce-e00e62c2c8ddf8d12145660a3478065caab4c4ca.tar.gz |
Merge branch 'backstage/gb/migrate-stages-statuses' into 'master'
Migrate CI/CD stages statuses
Closes #33453
See merge request !12584
-rw-r--r-- | app/models/ci/stage.rb | 59 | ||||
-rw-r--r-- | app/models/commit_status.rb | 9 | ||||
-rw-r--r-- | app/models/concerns/has_status.rb | 2 | ||||
-rw-r--r-- | app/workers/stage_update_worker.rb | 10 | ||||
-rw-r--r-- | db/migrate/20170711145320_add_status_to_ci_stages.rb | 9 | ||||
-rw-r--r-- | db/migrate/20170720111708_add_lock_version_to_ci_stages.rb | 9 | ||||
-rw-r--r-- | db/post_migrate/20170711145558_migrate_stages_statuses.rb | 33 | ||||
-rw-r--r-- | db/schema.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/background_migration/migrate_stage_status.rb | 77 | ||||
-rw-r--r-- | spec/factories/ci/stages.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/background_migration/migrate_stage_status_spec.rb | 80 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/safe_model_attributes.yml | 2 | ||||
-rw-r--r-- | spec/migrations/migrate_stage_id_reference_in_background_spec.rb | 13 | ||||
-rw-r--r-- | spec/migrations/migrate_stages_statuses_spec.rb | 67 | ||||
-rw-r--r-- | spec/models/ci/stage_spec.rb | 79 | ||||
-rw-r--r-- | spec/models/commit_status_spec.rb | 6 | ||||
-rw-r--r-- | spec/support/background_migrations_matchers.rb | 13 | ||||
-rw-r--r-- | spec/workers/stage_update_worker_spec.rb | 22 |
18 files changed, 478 insertions, 22 deletions
diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index 59570924c8d..4ee972fa68d 100644 --- a/app/models/ci/stage.rb +++ b/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 diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 07cec63b939..842c6e5cb50 100644 --- a/app/models/commit_status.rb +++ b/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 diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index 32af5566135..3803e18a96e 100644 --- a/app/models/concerns/has_status.rb +++ b/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 diff --git a/app/workers/stage_update_worker.rb b/app/workers/stage_update_worker.rb new file mode 100644 index 00000000000..eef0b11e70b --- /dev/null +++ b/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 diff --git a/db/migrate/20170711145320_add_status_to_ci_stages.rb b/db/migrate/20170711145320_add_status_to_ci_stages.rb new file mode 100644 index 00000000000..d497a61a959 --- /dev/null +++ b/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 diff --git a/db/migrate/20170720111708_add_lock_version_to_ci_stages.rb b/db/migrate/20170720111708_add_lock_version_to_ci_stages.rb new file mode 100644 index 00000000000..e1c4f033286 --- /dev/null +++ b/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 diff --git a/db/post_migrate/20170711145558_migrate_stages_statuses.rb b/db/post_migrate/20170711145558_migrate_stages_statuses.rb new file mode 100644 index 00000000000..5a24fb1307f --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 80f8cde1818..c31bff3a8f2 100644 --- a/db/schema.rb +++ b/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 diff --git a/lib/gitlab/background_migration/migrate_stage_status.rb b/lib/gitlab/background_migration/migrate_stage_status.rb new file mode 100644 index 00000000000..b1ff0900709 --- /dev/null +++ b/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 diff --git a/spec/factories/ci/stages.rb b/spec/factories/ci/stages.rb index d3c8bf9d54f..b2ded945738 100644 --- a/spec/factories/ci/stages.rb +++ b/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 diff --git a/spec/lib/gitlab/background_migration/migrate_stage_status_spec.rb b/spec/lib/gitlab/background_migration/migrate_stage_status_spec.rb new file mode 100644 index 00000000000..878158910be --- /dev/null +++ b/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 diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index ae3b0173160..a5e03e149a7 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/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 diff --git a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb b/spec/migrations/migrate_stage_id_reference_in_background_spec.rb index 260378adaa7..9b92f4b70b0 100644 --- a/spec/migrations/migrate_stage_id_reference_in_background_spec.rb +++ b/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) } diff --git a/spec/migrations/migrate_stages_statuses_spec.rb b/spec/migrations/migrate_stages_statuses_spec.rb new file mode 100644 index 00000000000..4102d57e368 --- /dev/null +++ b/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 diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb new file mode 100644 index 00000000000..74c9d6145e2 --- /dev/null +++ b/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 diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 8c4a366ef8f..f7583645e69 100644 --- a/spec/models/commit_status_spec.rb +++ b/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) } diff --git a/spec/support/background_migrations_matchers.rb b/spec/support/background_migrations_matchers.rb new file mode 100644 index 00000000000..423c0e4cefc --- /dev/null +++ b/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 diff --git a/spec/workers/stage_update_worker_spec.rb b/spec/workers/stage_update_worker_spec.rb new file mode 100644 index 00000000000..7bc76c79464 --- /dev/null +++ b/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 |