From 2d2f9e51594e33f04bfda4012e3d4c7e48539fe3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 6 Jan 2018 14:44:43 +0100 Subject: [PATCH] Do not attempt to migrate legacy pipeline stages --- .../migrate_build_stage.rb | 33 ++++++++++++------- .../migrate_build_stage_spec.rb | 5 ++- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/lib/gitlab/background_migration/migrate_build_stage.rb b/lib/gitlab/background_migration/migrate_build_stage.rb index a870609e68d..7139ac03737 100644 --- a/lib/gitlab/background_migration/migrate_build_stage.rb +++ b/lib/gitlab/background_migration/migrate_build_stage.rb @@ -6,9 +6,9 @@ module Gitlab module BackgroundMigration class MigrateBuildStage def perform(id) - Ci::Build.find_by(id: id).try do |build| - Stage.new(build).tap do |stage| - break if stage.exists? + DatabaseBuild.find_by(id: id).try do |build| + MigratableStage.new(build).tap do |stage| + break if stage.exists? || stage.legacy? stage.ensure! stage.migrate_reference! @@ -17,15 +17,15 @@ module Gitlab end end - class Ci::Stage < ActiveRecord::Base + class DatabaseStage < ActiveRecord::Base self.table_name = 'ci_stages' end - class Ci::Build < ActiveRecord::Base + class DatabaseBuild < ActiveRecord::Base self.table_name = 'ci_builds' end - class Stage + class MigratableStage def initialize(build) @build = build end @@ -34,20 +34,29 @@ module Gitlab @build.reload.stage_id.present? end + ## + # We can have some very old stages that do not have `ci_builds.stage` set. + # + # In that case we just don't migrate such stage. + # + def legacy? + @build.stage.nil? + end + def ensure! find || create! end def find - Ci::Stage.find_by(name: @build.stage, - pipeline_id: @build.commit_id, - project_id: @build.project_id) + DatabaseStage.find_by(name: @build.stage, + pipeline_id: @build.commit_id, + project_id: @build.project_id) end def create! - Ci::Stage.create!(name: @build.stage, - pipeline_id: @build.commit_id, - project_id: @build.project_id) + DatabaseStage.create!(name: @build.stage, + pipeline_id: @build.commit_id, + project_id: @build.project_id) end def migrate_reference! diff --git a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb index b675db463af..d20b5c29a71 100644 --- a/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_build_stage_spec.rb @@ -29,6 +29,8 @@ describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 201 stage_idx: 1, stage: 'test', status: :success) jobs.create!(id: 5, commit_id: 1, project_id: 123, stage_idx: 3, stage: 'deploy', status: :pending) + jobs.create!(id: 6, commit_id: 1, project_id: 123, + stage_idx: 3, stage: nil, status: :pending) end it 'correctly migrates builds stages' do @@ -40,7 +42,8 @@ describe Gitlab::BackgroundMigration::MigrateBuildStage, :migration, schema: 201 expect(stages.count).to eq 3 expect(stages.all.pluck(:name)).to match_array %w[test build deploy] - expect(jobs.where(stage_id: nil)).to be_empty + expect(jobs.where(stage_id: nil)).to be_one + expect(jobs.find_by(stage_id: nil).id).to eq 6 expect(stages.all.pluck(:status)).to match_array [STATUSES[:success], STATUSES[:failed], STATUSES[:pending]]