From 22535ae2c2bd19b95b6265f97b6f3911a23eabaa Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Sat, 11 Jul 2020 19:45:47 +0300 Subject: [PATCH 01/20] Migrate all artifacts to be license scanning https://gitlab.com/gitlab-org/gitlab/-/issues/213571 We removed license_management artifacts, so we need to migrate them to a new license_scanning type --- ...anagement_artifacts_to_license_scanning.rb | 37 ++++++++++++++++ ...ment_artifacts_to_license_scanning_spec.rb | 42 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb create mode 100644 ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb diff --git a/db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb new file mode 100644 index 00000000000000..7b7184fb539c36 --- /dev/null +++ b/db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MigrateLicenseManagementArtifactsToLicenseScanning < ActiveRecord::Migration[6.0] + # include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + disable_ddl_transaction! + + class Build < ActiveRecord::Base + self.table_name = 'ci_builds' + + has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, inverse_of: :job + + scope :with_both_artifacts, -> { } + end + + class JobArtifact < ActiveRecord::Base + self.table_name = 'ci_job_artifacts' + + belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id + + scope :with_both_artifacts, -> { where(file_type: [10, 101]).group(:job_id).having('count(*) > 1') } + end + + def up + + JobArtifact.with_both_artifacts.each do |pair| + pair.last.destroy + end + JobArtifact.where(file_type: 101).update(file_type: 10) + end + + def down; end +end diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb new file mode 100644 index 00000000000000..586cb5c7b43af2 --- /dev/null +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20200707124528_migrate_license_management_artifacts_to_license_scanning.rb') + +RSpec.describe MigrateLicenseManagementArtifactsToLicenseScanning, :migration, :sidekiq do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:job_artifacts) { table(:ci_job_artifacts) } + let(:builds) { table(:ci_builds) } + let(:license_management_type) { 10 } + let(:license_scanning_type) { 101 } + + + before do + namespaces.create!(id: 1, name: 'tanuki', path: 'tanuki') + projects.create!(id: 42, name: 'tanuki', path: 'tanuki', namespace_id: 1) + builds.create!(id: 1) + job_artifacts.create!(project_id: 42, job_id: 1, file_type: 10) + end + + + context 'with two types of the report' do + before do + job_artifacts.create!(project_id: 42, job_id: 1, file_type: 101) + end + + it 'leaves only one artifact' do + migrate! + + expect(job_artifacts.where(file_type: 101).count).to be_nil + end + end + + context 'with only license_management report' do + it 'change' do + migrate! + + expect(job_artifacts.where(file_type: 101).count).to be_nil + end + end +end -- GitLab From 6ae3c09b4b541a5c6889b07b6c8ff23c6e0a43e8 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Sun, 12 Jul 2020 15:46:34 +0300 Subject: [PATCH 02/20] Update migration so it delete only duplicates --- ...anagement_artifacts_to_license_scanning.rb | 20 ++++--------------- ...ment_artifacts_to_license_scanning_spec.rb | 12 +++++++++-- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb index 7b7184fb539c36..ef8bce1c651534 100644 --- a/db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb @@ -4,33 +4,21 @@ # for more information on how to write migrations for GitLab. class MigrateLicenseManagementArtifactsToLicenseScanning < ActiveRecord::Migration[6.0] - # include Gitlab::Database::MigrationHelpers - DOWNTIME = false disable_ddl_transaction! - class Build < ActiveRecord::Base - self.table_name = 'ci_builds' - - has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, inverse_of: :job - - scope :with_both_artifacts, -> { } - end - class JobArtifact < ActiveRecord::Base self.table_name = 'ci_job_artifacts' belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id - scope :with_both_artifacts, -> { where(file_type: [10, 101]).group(:job_id).having('count(*) > 1') } + scope :duplicates, -> { select("MIN(id) as id").group(:job_id).map(&:id) } + scope :license_compliance, -> { where(file_type: [10, 101]) } end def up - - JobArtifact.with_both_artifacts.each do |pair| - pair.last.destroy - end - JobArtifact.where(file_type: 101).update(file_type: 10) + JobArtifact.license_compliance.where.not(id: JobArtifact.license_compliance.duplicates).delete_all + JobArtifact.where(file_type: 10).update(file_type: 101) end def down; end diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index 586cb5c7b43af2..69d6c4285313a0 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -16,7 +16,11 @@ namespaces.create!(id: 1, name: 'tanuki', path: 'tanuki') projects.create!(id: 42, name: 'tanuki', path: 'tanuki', namespace_id: 1) builds.create!(id: 1) + builds.create!(id: 2) + builds.create!(id: 3) job_artifacts.create!(project_id: 42, job_id: 1, file_type: 10) + job_artifacts.create!(project_id: 42, job_id: 2, file_type: 9) + job_artifacts.create!(project_id: 42, job_id: 2, file_type: 10) end @@ -28,7 +32,9 @@ it 'leaves only one artifact' do migrate! - expect(job_artifacts.where(file_type: 101).count).to be_nil + expect(job_artifacts.where(file_type: 10).count).to eq 0 + expect(job_artifacts.where(file_type: 101).count).to eq 2 + expect(job_artifacts.where(file_type: 9).count).to eq 1 end end @@ -36,7 +42,9 @@ it 'change' do migrate! - expect(job_artifacts.where(file_type: 101).count).to be_nil + expect(job_artifacts.where(file_type: 10).count).to eq 0 + expect(job_artifacts.where(file_type: 101).count).to eq 2 + expect(job_artifacts.where(file_type: 9).count).to eq 1 end end end -- GitLab From 3a37c7afd44ef8ad2902163a8a1940ceffed26dc Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 14 Jul 2020 15:00:11 +0300 Subject: [PATCH 03/20] Update migration name --- ...migrate_license_management_artifacts_to_license_scanning.rb} | 0 ...ate_license_management_artifacts_to_license_scanning_spec.rb | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename db/post_migrate/{20200707124528_migrate_license_management_artifacts_to_license_scanning.rb => 20200714124528_migrate_license_management_artifacts_to_license_scanning.rb} (100%) diff --git a/db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb similarity index 100% rename from db/post_migrate/20200707124528_migrate_license_management_artifacts_to_license_scanning.rb rename to db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index 69d6c4285313a0..bcffdb2bc987b4 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20200707124528_migrate_license_management_artifacts_to_license_scanning.rb') +require Rails.root.join('db', 'post_migrate', '20200714124528_migrate_license_management_artifacts_to_license_scanning.rb') RSpec.describe MigrateLicenseManagementArtifactsToLicenseScanning, :migration, :sidekiq do let(:namespaces) { table(:namespaces) } -- GitLab From da1e679fa34aa4017e3a605be5ef204907417cc2 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 14 Jul 2020 15:11:03 +0300 Subject: [PATCH 04/20] Add changelog entry --- changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml diff --git a/changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml b/changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml new file mode 100644 index 00000000000000..b8b2d3abf6923c --- /dev/null +++ b/changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml @@ -0,0 +1,5 @@ +--- +title: Migrate license_management artifacts to license_scanning type +merge_request: +author: +type: changed -- GitLab From fac5ff64cad7f613d0604d9138b49af1d8db860b Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 14 Jul 2020 16:43:15 +0300 Subject: [PATCH 05/20] Fix failed cops --- ...ate_license_management_artifacts_to_license_scanning_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index bcffdb2bc987b4..6c74ffee9c3b54 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -11,7 +11,6 @@ let(:license_management_type) { 10 } let(:license_scanning_type) { 101 } - before do namespaces.create!(id: 1, name: 'tanuki', path: 'tanuki') projects.create!(id: 42, name: 'tanuki', path: 'tanuki', namespace_id: 1) @@ -23,7 +22,6 @@ job_artifacts.create!(project_id: 42, job_id: 2, file_type: 10) end - context 'with two types of the report' do before do job_artifacts.create!(project_id: 42, job_id: 1, file_type: 101) -- GitLab From 5af4100db7d38c89ed53e01a3104c24269bd3a7f Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 21 Jul 2020 14:35:55 +0300 Subject: [PATCH 06/20] Address review comments --- ...license_management_artifacts_to_license_scanning.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb index ef8bce1c651534..ccd05ebc165fb5 100644 --- a/db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb @@ -12,14 +12,16 @@ class JobArtifact < ActiveRecord::Base belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id - scope :duplicates, -> { select("MIN(id) as id").group(:job_id).map(&:id) } + scope :original_reports, -> { select("MIN(id) as id").group(:job_id).pluck(&:id) } scope :license_compliance, -> { where(file_type: [10, 101]) } end def up - JobArtifact.license_compliance.where.not(id: JobArtifact.license_compliance.duplicates).delete_all - JobArtifact.where(file_type: 10).update(file_type: 101) + JobArtifact.license_compliance.where.not(id: JobArtifact.license_compliance.original_reports).delete_all + JobArtifact.where(file_type: 10).update_all(file_type: 101) end - def down; end + def down + # this one way migration as we're updating data + end end -- GitLab From 3403b2321bde2837fd3855cbe86ccc7b629bbc31 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Thu, 23 Jul 2020 16:49:55 +0300 Subject: [PATCH 07/20] Bump migration version --- ...migrate_license_management_artifacts_to_license_scanning.rb} | 2 +- ...ate_license_management_artifacts_to_license_scanning_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename db/post_migrate/{20200714124528_migrate_license_management_artifacts_to_license_scanning.rb => 20200723133628_migrate_license_management_artifacts_to_license_scanning.rb} (97%) diff --git a/db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb similarity index 97% rename from db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb rename to db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb index ccd05ebc165fb5..849ff03b16aaad 100644 --- a/db/post_migrate/20200714124528_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb @@ -12,7 +12,7 @@ class JobArtifact < ActiveRecord::Base belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id - scope :original_reports, -> { select("MIN(id) as id").group(:job_id).pluck(&:id) } + scope :original_reports, -> { select("MIN(id) as id").group(:job_id).map(&:id) } scope :license_compliance, -> { where(file_type: [10, 101]) } end diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index 6c74ffee9c3b54..9f62df2facc188 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20200714124528_migrate_license_management_artifacts_to_license_scanning.rb') +require Rails.root.join('db', 'post_migrate', '20200723133628_migrate_license_management_artifacts_to_license_scanning.rb') RSpec.describe MigrateLicenseManagementArtifactsToLicenseScanning, :migration, :sidekiq do let(:namespaces) { table(:namespaces) } -- GitLab From aac8b8cdb8d7b6061e11a495c26a87e9c7881099 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Thu, 23 Jul 2020 13:58:55 +0000 Subject: [PATCH 08/20] Apply 1 suggestion(s) to 1 file(s) --- changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml b/changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml index b8b2d3abf6923c..29e6668351973b 100644 --- a/changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml +++ b/changelogs/unreleased/213571-migrate-lm-to-ls-artifacts.yml @@ -1,5 +1,5 @@ --- title: Migrate license_management artifacts to license_scanning type -merge_request: +merge_request: 36817 author: type: changed -- GitLab From e2f6219f289655a0ba600d3239a8c3a4b41ecb5b Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Fri, 24 Jul 2020 18:32:19 +0300 Subject: [PATCH 09/20] Add comments and update in batches --- ...cense_management_artifacts_to_license_scanning.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb index 849ff03b16aaad..a93fac49ba8cba 100644 --- a/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb @@ -8,6 +8,8 @@ class MigrateLicenseManagementArtifactsToLicenseScanning < ActiveRecord::Migrati disable_ddl_transaction! class JobArtifact < ActiveRecord::Base + include EachBatch + self.table_name = 'ci_job_artifacts' belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id @@ -16,12 +18,18 @@ class JobArtifact < ActiveRecord::Base scope :license_compliance, -> { where(file_type: [10, 101]) } end + # We're updating file_type of ci artifacts from license_management to license_scanning + # But before that we need to delete "rogue" artifacts for CI builds that have associated with them + # both license_scanning and license_management artifacts. It's an edge case and usually, we don't have + # such builds in the database. def up JobArtifact.license_compliance.where.not(id: JobArtifact.license_compliance.original_reports).delete_all - JobArtifact.where(file_type: 10).update_all(file_type: 101) + JobArtifact.where(file_type: 10).each_batch do |relation| + relation.update_all(file_type: 101) + end end def down - # this one way migration as we're updating data + # this one way migration as we're deleting duplicating artifacts and updating their file_type end end -- GitLab From 5ca5cce7f9250042c84c440db01bf664654fe485 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Sun, 26 Jul 2020 16:49:25 +0300 Subject: [PATCH 10/20] Simplify query plan --- ..._migrate_license_management_artifacts_to_license_scanning.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb index a93fac49ba8cba..93ccd606414903 100644 --- a/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb @@ -14,7 +14,7 @@ class JobArtifact < ActiveRecord::Base belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id - scope :original_reports, -> { select("MIN(id) as id").group(:job_id).map(&:id) } + scope :original_reports, -> { select("MIN(id) as id").group(:job_id) } scope :license_compliance, -> { where(file_type: [10, 101]) } end -- GitLab From f6a40170842292514ffb3dad93dd5d055064b890 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Mon, 27 Jul 2020 16:13:03 +0300 Subject: [PATCH 11/20] Update artifacts migration Bump version number and switch to the new schema numbers model --- ...igrate_license_management_artifacts_to_license_scanning.rb} | 3 ++- db/schema_migrations/20200727133628 | 1 + ...te_license_management_artifacts_to_license_scanning_spec.rb | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) rename db/post_migrate/{20200723133628_migrate_license_management_artifacts_to_license_scanning.rb => 20200727133628_migrate_license_management_artifacts_to_license_scanning.rb} (91%) create mode 100644 db/schema_migrations/20200727133628 diff --git a/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb similarity index 91% rename from db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb rename to db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb index 93ccd606414903..334460111dfffa 100644 --- a/db/post_migrate/20200723133628_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb @@ -30,6 +30,7 @@ def up end def down - # this one way migration as we're deleting duplicating artifacts and updating their file_type + # no-op + # we're deleting duplicating artifacts and updating file_type for license_management artifacts end end diff --git a/db/schema_migrations/20200727133628 b/db/schema_migrations/20200727133628 new file mode 100644 index 00000000000000..61fb7122e26f1e --- /dev/null +++ b/db/schema_migrations/20200727133628 @@ -0,0 +1 @@ +ac6e11b70527250b291f948a27a484cb68adef115bccfac77397016c423c36d7 \ No newline at end of file diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index 9f62df2facc188..e78b4440a1165d 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20200723133628_migrate_license_management_artifacts_to_license_scanning.rb') +require Rails.root.join('db', 'post_migrate', '20200727133628_migrate_license_management_artifacts_to_license_scanning.rb') RSpec.describe MigrateLicenseManagementArtifactsToLicenseScanning, :migration, :sidekiq do let(:namespaces) { table(:namespaces) } -- GitLab From 66612edccfefd1bb77bc9d5d5e2d0d70c188a978 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 28 Jul 2020 15:27:01 +0000 Subject: [PATCH 12/20] Apply 1 suggestion(s) to 1 file(s) --- ...migrate_license_management_artifacts_to_license_scanning.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb index 334460111dfffa..3d3bd7d817c644 100644 --- a/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb @@ -22,7 +22,8 @@ class JobArtifact < ActiveRecord::Base # But before that we need to delete "rogue" artifacts for CI builds that have associated with them # both license_scanning and license_management artifacts. It's an edge case and usually, we don't have # such builds in the database. - def up +def up + return unless Gitlab.ee? JobArtifact.license_compliance.where.not(id: JobArtifact.license_compliance.original_reports).delete_all JobArtifact.where(file_type: 10).each_batch do |relation| relation.update_all(file_type: 101) -- GitLab From 94e40ed810dff8a639dfff4daf8bf12a70a3abca Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Fri, 31 Jul 2020 18:43:22 +0300 Subject: [PATCH 13/20] Wrap delete in batch --- ...te_license_management_artifacts_to_license_scanning.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb index 3d3bd7d817c644..244c7de0b37797 100644 --- a/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb @@ -22,9 +22,13 @@ class JobArtifact < ActiveRecord::Base # But before that we need to delete "rogue" artifacts for CI builds that have associated with them # both license_scanning and license_management artifacts. It's an edge case and usually, we don't have # such builds in the database. -def up + def up return unless Gitlab.ee? - JobArtifact.license_compliance.where.not(id: JobArtifact.license_compliance.original_reports).delete_all + + JobArtifact.license_compliance.where.not(id: JobArtifact.license_compliance.original_reports).each_batch do |relation| + relation.delete_all + end + JobArtifact.where(file_type: 10).each_batch do |relation| relation.update_all(file_type: 101) end -- GitLab From cd9bdd07823c30693913cfa370ca0eb2fdd48dad Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 4 Aug 2020 22:23:20 +0300 Subject: [PATCH 14/20] Bump migration version This migration depends on an index that should be created right before --- ...igrate_license_management_artifacts_to_license_scanning.rb} | 3 --- db/schema_migrations/20200804221641 | 1 + ...te_license_management_artifacts_to_license_scanning_spec.rb | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) rename db/post_migrate/{20200727133628_migrate_license_management_artifacts_to_license_scanning.rb => 20200804221641_migrate_license_management_artifacts_to_license_scanning.rb} (90%) create mode 100644 db/schema_migrations/20200804221641 diff --git a/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200804221641_migrate_license_management_artifacts_to_license_scanning.rb similarity index 90% rename from db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb rename to db/post_migrate/20200804221641_migrate_license_management_artifacts_to_license_scanning.rb index 244c7de0b37797..4e851e58bcb013 100644 --- a/db/post_migrate/20200727133628_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200804221641_migrate_license_management_artifacts_to_license_scanning.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class MigrateLicenseManagementArtifactsToLicenseScanning < ActiveRecord::Migration[6.0] DOWNTIME = false disable_ddl_transaction! diff --git a/db/schema_migrations/20200804221641 b/db/schema_migrations/20200804221641 new file mode 100644 index 00000000000000..ca01b4f3361856 --- /dev/null +++ b/db/schema_migrations/20200804221641 @@ -0,0 +1 @@ +0dd7aa62807bc98595047a96b3b131d6a9dfb1875b9218f8b3d6d430e548d4e2 \ No newline at end of file diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index e78b4440a1165d..748fe28d7c5e39 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20200727133628_migrate_license_management_artifacts_to_license_scanning.rb') +require Rails.root.join('db', 'post_migrate', '20200804221641_migrate_license_management_artifacts_to_license_scanning.rb') RSpec.describe MigrateLicenseManagementArtifactsToLicenseScanning, :migration, :sidekiq do let(:namespaces) { table(:namespaces) } -- GitLab From 8a43ffec5aa4b9343d833fbc467a7b1a038b11f2 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Sun, 9 Aug 2020 16:34:30 +0300 Subject: [PATCH 15/20] Rework migration of license_management artifacts - Add temp index to improve performance - Update query to delete items --- ..._index_for_license_compliance_artifacts.rb | 18 +++++++++++++++++ ...nagement_artifacts_to_license_scanning.rb} | 20 ++++++++++++++++--- db/schema_migrations/20200808221641 | 1 + db/schema_migrations/20200809221641 | 1 + db/structure.sql | 2 ++ ...ment_artifacts_to_license_scanning_spec.rb | 2 +- 6 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 db/post_migrate/20200808221641_add_index_for_license_compliance_artifacts.rb rename db/post_migrate/{20200804221641_migrate_license_management_artifacts_to_license_scanning.rb => 20200809221641_migrate_license_management_artifacts_to_license_scanning.rb} (56%) create mode 100644 db/schema_migrations/20200808221641 create mode 100644 db/schema_migrations/20200809221641 diff --git a/db/post_migrate/20200808221641_add_index_for_license_compliance_artifacts.rb b/db/post_migrate/20200808221641_add_index_for_license_compliance_artifacts.rb new file mode 100644 index 00000000000000..efa764c9c65c77 --- /dev/null +++ b/db/post_migrate/20200808221641_add_index_for_license_compliance_artifacts.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddIndexForLicenseComplianceArtifacts < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_ci_job_artifacts_on_license_compliance_file_types' + + disable_ddl_transaction! + + def up + add_index :ci_job_artifacts, [:job_id, :file_type], where: 'file_type = 10 OR file_type = 101', name: INDEX_NAME + end + + def down + remove_index :ci_job_artifacts, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20200804221641_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb similarity index 56% rename from db/post_migrate/20200804221641_migrate_license_management_artifacts_to_license_scanning.rb rename to db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb index 4e851e58bcb013..aec01d0308d4ba 100644 --- a/db/post_migrate/20200804221641_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb @@ -22,11 +22,25 @@ class JobArtifact < ActiveRecord::Base def up return unless Gitlab.ee? - JobArtifact.license_compliance.where.not(id: JobArtifact.license_compliance.original_reports).each_batch do |relation| - relation.delete_all + JobArtifact + .where("file_type = 10 OR file_type = 101") + .each_batch(column: :job_id, of: 1000) do |relation| + + min, max = relation.pluck('MIN(job_id)', 'MAX(job_id)').flatten + + ActiveRecord::Base.connection.execute <<~SQL + WITH ci_job_artifacts_with_row_number as ( + SELECT job_id, id, ROW_NUMBER() OVER (PARTITION BY job_id ORDER BY id ASC) as row_number + FROM ci_job_artifacts + WHERE (file_type = 101 OR file_type = 10) + AND job_id >= #{Integer(min)} AND job_id < #{Integer(max)} + ) + DELETE FROM ci_job_artifacts + WHERE ci_job_artifacts.id IN (SELECT id from ci_job_artifacts_with_row_number WHERE ci_job_artifacts_with_row_number.row_number > 1) + SQL end - JobArtifact.where(file_type: 10).each_batch do |relation| + JobArtifact.where(file_type: 10).each_batch(of: 1000) do |relation| relation.update_all(file_type: 101) end end diff --git a/db/schema_migrations/20200808221641 b/db/schema_migrations/20200808221641 new file mode 100644 index 00000000000000..05e3769247e9f1 --- /dev/null +++ b/db/schema_migrations/20200808221641 @@ -0,0 +1 @@ +a24aa5052d37bff1bffc5df076d8422ea90f3781ae01ecf626bc59f3e299c90b \ No newline at end of file diff --git a/db/schema_migrations/20200809221641 b/db/schema_migrations/20200809221641 new file mode 100644 index 00000000000000..4e43680663a4a4 --- /dev/null +++ b/db/schema_migrations/20200809221641 @@ -0,0 +1 @@ +4e360aa1b375e391ec1202f1fe2eb26d64895faf326ec9c7a9b8d8351b6f4dc3 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 86fa7a40734b78..c215ba1eba5255 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19202,6 +19202,8 @@ CREATE INDEX index_ci_job_artifacts_on_file_store ON public.ci_job_artifacts USI CREATE UNIQUE INDEX index_ci_job_artifacts_on_job_id_and_file_type ON public.ci_job_artifacts USING btree (job_id, file_type); +CREATE INDEX index_ci_job_artifacts_on_license_compliance_file_types ON public.ci_job_artifacts USING btree (job_id, file_type) WHERE ((file_type = 10) OR (file_type = 101)); + CREATE INDEX index_ci_job_artifacts_on_project_id ON public.ci_job_artifacts USING btree (project_id); CREATE INDEX index_ci_job_artifacts_on_project_id_for_security_reports ON public.ci_job_artifacts USING btree (project_id) WHERE (file_type = ANY (ARRAY[5, 6, 7, 8])); diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index 748fe28d7c5e39..1870690cacb5f2 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20200804221641_migrate_license_management_artifacts_to_license_scanning.rb') +require Rails.root.join('db', 'post_migrate', '20200809221641_migrate_license_management_artifacts_to_license_scanning.rb') RSpec.describe MigrateLicenseManagementArtifactsToLicenseScanning, :migration, :sidekiq do let(:namespaces) { table(:namespaces) } -- GitLab From a8fa912d57d94616da88bd307d889dfc6fed07e1 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Mon, 10 Aug 2020 20:11:52 +0000 Subject: [PATCH 16/20] Apply 2 suggestion(s) to 1 file(s) --- ...e_license_management_artifacts_to_license_scanning_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index 1870690cacb5f2..bebda9fdc7d39e 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -8,8 +8,8 @@ let(:projects) { table(:projects) } let(:job_artifacts) { table(:ci_job_artifacts) } let(:builds) { table(:ci_builds) } - let(:license_management_type) { 10 } - let(:license_scanning_type) { 101 } + let(:license_management_type) { Ci::JobArtifact.file_types[:license_management] } + let(:license_scanning_type) { Ci::JobArtifact.file_types[:license_scanning] } before do namespaces.create!(id: 1, name: 'tanuki', path: 'tanuki') -- GitLab From 0d6a61aadfeaf80e087178ab9d1a770bb8865063 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Mon, 10 Aug 2020 23:59:49 +0300 Subject: [PATCH 17/20] Cleanup with review suggestions --- ...se_management_artifacts_to_license_scanning.rb | 5 ----- db/schema_migrations/20200804221641 | 1 - ...nagement_artifacts_to_license_scanning_spec.rb | 15 ++++++++++++++- 3 files changed, 14 insertions(+), 7 deletions(-) delete mode 100644 db/schema_migrations/20200804221641 diff --git a/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb index aec01d0308d4ba..4da020880d52a1 100644 --- a/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb @@ -8,11 +8,6 @@ class JobArtifact < ActiveRecord::Base include EachBatch self.table_name = 'ci_job_artifacts' - - belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id - - scope :original_reports, -> { select("MIN(id) as id").group(:job_id) } - scope :license_compliance, -> { where(file_type: [10, 101]) } end # We're updating file_type of ci artifacts from license_management to license_scanning diff --git a/db/schema_migrations/20200804221641 b/db/schema_migrations/20200804221641 deleted file mode 100644 index ca01b4f3361856..00000000000000 --- a/db/schema_migrations/20200804221641 +++ /dev/null @@ -1 +0,0 @@ -0dd7aa62807bc98595047a96b3b131d6a9dfb1875b9218f8b3d6d430e548d4e2 \ No newline at end of file diff --git a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb index bebda9fdc7d39e..f72f94215322de 100644 --- a/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb +++ b/ee/spec/migrations/migrate_license_management_artifacts_to_license_scanning_spec.rb @@ -37,7 +37,7 @@ end context 'with only license_management report' do - it 'change' do + it 'changes license_management to license_scanning' do migrate! expect(job_artifacts.where(file_type: 10).count).to eq 0 @@ -45,4 +45,17 @@ expect(job_artifacts.where(file_type: 9).count).to eq 1 end end + + context 'with FOSS version of GitLab' do + before do + allow(Gitlab).to receive(:ee?).and_return(false) + end + + it 'skips this migration' do + migrate! + + expect(job_artifacts.where(file_type: 10).count).to eq 2 + expect(job_artifacts.where(file_type: 101).count).to eq 0 + end + end end -- GitLab From 64529753f08e5a56e2f62e6a6e257578786a9ce6 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 11 Aug 2020 15:50:07 +0300 Subject: [PATCH 18/20] Use concurrent index --- ...200808221641_add_index_for_license_compliance_artifacts.rb | 4 ++-- db/structure.sql | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/db/post_migrate/20200808221641_add_index_for_license_compliance_artifacts.rb b/db/post_migrate/20200808221641_add_index_for_license_compliance_artifacts.rb index efa764c9c65c77..fce4ee095bde4e 100644 --- a/db/post_migrate/20200808221641_add_index_for_license_compliance_artifacts.rb +++ b/db/post_migrate/20200808221641_add_index_for_license_compliance_artifacts.rb @@ -9,10 +9,10 @@ class AddIndexForLicenseComplianceArtifacts < ActiveRecord::Migration[6.0] disable_ddl_transaction! def up - add_index :ci_job_artifacts, [:job_id, :file_type], where: 'file_type = 10 OR file_type = 101', name: INDEX_NAME + add_concurrent_index :ci_job_artifacts, [:job_id, :file_type], where: 'file_type = 10 OR file_type = 101', name: INDEX_NAME end def down - remove_index :ci_job_artifacts, name: INDEX_NAME + remove_concurrent_index_by_name :ci_job_artifacts, name: INDEX_NAME end end diff --git a/db/structure.sql b/db/structure.sql index c215ba1eba5255..aba6a4656e0a02 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19200,6 +19200,8 @@ CREATE INDEX index_ci_job_artifacts_on_expire_at_and_job_id ON public.ci_job_art CREATE INDEX index_ci_job_artifacts_on_file_store ON public.ci_job_artifacts USING btree (file_store); +CREATE INDEX index_ci_job_artifacts_on_file_type ON public.ci_job_artifacts USING btree (file_type) WHERE (file_type = ANY (ARRAY[10, 101])); + CREATE UNIQUE INDEX index_ci_job_artifacts_on_job_id_and_file_type ON public.ci_job_artifacts USING btree (job_id, file_type); CREATE INDEX index_ci_job_artifacts_on_license_compliance_file_types ON public.ci_job_artifacts USING btree (job_id, file_type) WHERE ((file_type = 10) OR (file_type = 101)); -- GitLab From 423a85fc25b9739b8e6089630a6836281d58e11b Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 11 Aug 2020 16:03:36 +0300 Subject: [PATCH 19/20] Use constants and fix rebase issues --- ...e_license_management_artifacts_to_license_scanning.rb | 9 ++++++--- db/schema_migrations/20200727133628 | 1 - db/structure.sql | 2 -- 3 files changed, 6 insertions(+), 6 deletions(-) delete mode 100644 db/schema_migrations/20200727133628 diff --git a/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb index 4da020880d52a1..9d563dd29be764 100644 --- a/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb @@ -2,6 +2,9 @@ class MigrateLicenseManagementArtifactsToLicenseScanning < ActiveRecord::Migration[6.0] DOWNTIME = false + LICENSE_MANAGEMENT_FILE_TYPE = 10 + LICENSE_SCANNING_FILE_TYPE = 101 + disable_ddl_transaction! class JobArtifact < ActiveRecord::Base @@ -27,7 +30,7 @@ def up WITH ci_job_artifacts_with_row_number as ( SELECT job_id, id, ROW_NUMBER() OVER (PARTITION BY job_id ORDER BY id ASC) as row_number FROM ci_job_artifacts - WHERE (file_type = 101 OR file_type = 10) + WHERE (file_type = #{LICENSE_SCANNING_FILE_TYPE} OR file_type = #{LICENSE_MANAGEMENT_FILE_TYPE}) AND job_id >= #{Integer(min)} AND job_id < #{Integer(max)} ) DELETE FROM ci_job_artifacts @@ -35,8 +38,8 @@ def up SQL end - JobArtifact.where(file_type: 10).each_batch(of: 1000) do |relation| - relation.update_all(file_type: 101) + JobArtifact.where(file_type: LICENSE_MANAGEMENT_FILE_TYPE).each_batch(of: 1000) do |relation| + relation.update_all(file_type: LICENSE_SCANNING_FILE_TYPE) end end diff --git a/db/schema_migrations/20200727133628 b/db/schema_migrations/20200727133628 deleted file mode 100644 index 61fb7122e26f1e..00000000000000 --- a/db/schema_migrations/20200727133628 +++ /dev/null @@ -1 +0,0 @@ -ac6e11b70527250b291f948a27a484cb68adef115bccfac77397016c423c36d7 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index aba6a4656e0a02..c215ba1eba5255 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -19200,8 +19200,6 @@ CREATE INDEX index_ci_job_artifacts_on_expire_at_and_job_id ON public.ci_job_art CREATE INDEX index_ci_job_artifacts_on_file_store ON public.ci_job_artifacts USING btree (file_store); -CREATE INDEX index_ci_job_artifacts_on_file_type ON public.ci_job_artifacts USING btree (file_type) WHERE (file_type = ANY (ARRAY[10, 101])); - CREATE UNIQUE INDEX index_ci_job_artifacts_on_job_id_and_file_type ON public.ci_job_artifacts USING btree (job_id, file_type); CREATE INDEX index_ci_job_artifacts_on_license_compliance_file_types ON public.ci_job_artifacts USING btree (job_id, file_type) WHERE ((file_type = 10) OR (file_type = 101)); -- GitLab From 46462dd4e5eceea8d37daac55805004900e86bd2 Mon Sep 17 00:00:00 2001 From: Tetiana Chupryna Date: Tue, 11 Aug 2020 16:54:54 +0300 Subject: [PATCH 20/20] Remove empty line --- ...1_migrate_license_management_artifacts_to_license_scanning.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb b/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb index 9d563dd29be764..dcc8e4d11b2623 100644 --- a/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb +++ b/db/post_migrate/20200809221641_migrate_license_management_artifacts_to_license_scanning.rb @@ -23,7 +23,6 @@ def up JobArtifact .where("file_type = 10 OR file_type = 101") .each_batch(column: :job_id, of: 1000) do |relation| - min, max = relation.pluck('MIN(job_id)', 'MAX(job_id)').flatten ActiveRecord::Base.connection.execute <<~SQL -- GitLab