Merge branch 'sh-fix-backfill-project-repo-migration' into 'master'
Fix duplicate project disk path in BackfillLegacyProjectRepositories Closes #56061 See merge request gitlab-org/gitlab-ce!24213
This commit is contained in:
commit
ace4a88aa3
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Fix duplicate project disk path in BackfillLegacyProjectRepositories
|
||||||
|
merge_request: 24213
|
||||||
|
author:
|
||||||
|
type: changed
|
|
@ -83,7 +83,7 @@ module Gitlab
|
||||||
extend ActiveSupport::Concern
|
extend ActiveSupport::Concern
|
||||||
|
|
||||||
def full_path
|
def full_path
|
||||||
@full_path ||= build_full_path
|
route&.path || build_full_path
|
||||||
end
|
end
|
||||||
|
|
||||||
def build_full_path
|
def build_full_path
|
||||||
|
@ -99,7 +99,12 @@ module Gitlab
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Namespace model.
|
# Route model
|
||||||
|
class Route < ActiveRecord::Base
|
||||||
|
belongs_to :source, inverse_of: :route, polymorphic: true
|
||||||
|
end
|
||||||
|
|
||||||
|
# Namespace model
|
||||||
class Namespace < ActiveRecord::Base
|
class Namespace < ActiveRecord::Base
|
||||||
self.table_name = 'namespaces'
|
self.table_name = 'namespaces'
|
||||||
self.inheritance_column = nil
|
self.inheritance_column = nil
|
||||||
|
@ -108,6 +113,8 @@ module Gitlab
|
||||||
|
|
||||||
belongs_to :parent, class_name: 'Namespace', inverse_of: 'namespaces'
|
belongs_to :parent, class_name: 'Namespace', inverse_of: 'namespaces'
|
||||||
|
|
||||||
|
has_one :route, -> { where(source_type: 'Namespace') }, inverse_of: :source, foreign_key: :source_id
|
||||||
|
|
||||||
has_many :projects, inverse_of: :parent
|
has_many :projects, inverse_of: :parent
|
||||||
has_many :namespaces, inverse_of: :parent
|
has_many :namespaces, inverse_of: :parent
|
||||||
end
|
end
|
||||||
|
@ -134,6 +141,7 @@ module Gitlab
|
||||||
|
|
||||||
belongs_to :parent, class_name: 'Namespace', foreign_key: :namespace_id, inverse_of: 'projects'
|
belongs_to :parent, class_name: 'Namespace', foreign_key: :namespace_id, inverse_of: 'projects'
|
||||||
|
|
||||||
|
has_one :route, -> { where(source_type: 'Project') }, inverse_of: :source, foreign_key: :source_id
|
||||||
has_one :project_repository, inverse_of: :project
|
has_one :project_repository, inverse_of: :project
|
||||||
|
|
||||||
delegate :disk_path, to: :storage
|
delegate :disk_path, to: :storage
|
||||||
|
@ -194,6 +202,8 @@ module Gitlab
|
||||||
def project_repositories(start_id, stop_id)
|
def project_repositories(start_id, stop_id)
|
||||||
projects
|
projects
|
||||||
.without_project_repository
|
.without_project_repository
|
||||||
|
.includes(:route, parent: [:route]).references(:routes)
|
||||||
|
.includes(:parent).references(:namespaces)
|
||||||
.where(id: start_id..stop_id)
|
.where(id: start_id..stop_id)
|
||||||
.map { |project| build_attributes_for_project(project) }
|
.map { |project| build_attributes_for_project(project) }
|
||||||
.compact
|
.compact
|
||||||
|
|
|
@ -34,6 +34,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
|
||||||
let!(:project_hashed_storage_2) { create(:project, name: 'bar', path: 'bar', namespace: group, storage_version: 2) }
|
let!(:project_hashed_storage_2) { create(:project, name: 'bar', path: 'bar', namespace: group, storage_version: 2) }
|
||||||
let!(:project_legacy_storage_3) { create(:project, name: 'baz', path: 'baz', namespace: group, storage_version: 0) }
|
let!(:project_legacy_storage_3) { create(:project, name: 'baz', path: 'baz', namespace: group, storage_version: 0) }
|
||||||
let!(:project_legacy_storage_4) { create(:project, name: 'zoo', path: 'zoo', namespace: group, storage_version: nil) }
|
let!(:project_legacy_storage_4) { create(:project, name: 'zoo', path: 'zoo', namespace: group, storage_version: nil) }
|
||||||
|
let!(:project_legacy_storage_5) { create(:project, name: 'test', path: 'test', namespace: group, storage_version: nil) }
|
||||||
|
|
||||||
describe '.on_hashed_storage' do
|
describe '.on_hashed_storage' do
|
||||||
it 'finds projects with repository on hashed storage' do
|
it 'finds projects with repository on hashed storage' do
|
||||||
|
@ -47,7 +48,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
|
||||||
it 'finds projects with repository on legacy storage' do
|
it 'finds projects with repository on legacy storage' do
|
||||||
projects = described_class.on_legacy_storage.pluck(:id)
|
projects = described_class.on_legacy_storage.pluck(:id)
|
||||||
|
|
||||||
expect(projects).to match_array([project_legacy_storage_3.id, project_legacy_storage_4.id])
|
expect(projects).to match_array([project_legacy_storage_3.id, project_legacy_storage_4.id, project_legacy_storage_5.id])
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -58,7 +59,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
|
||||||
|
|
||||||
projects = described_class.without_project_repository.pluck(:id)
|
projects = described_class.without_project_repository.pluck(:id)
|
||||||
|
|
||||||
expect(projects).to contain_exactly(project_hashed_storage_2.id, project_legacy_storage_4.id)
|
expect(projects).to contain_exactly(project_hashed_storage_2.id, project_legacy_storage_4.id, project_legacy_storage_5.id)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -78,14 +79,23 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
|
||||||
expect(project.disk_path).to eq(project_legacy_storage_3.disk_path)
|
expect(project.disk_path).to eq(project_legacy_storage_3.disk_path)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'returns the correct disk_path using the route entry' do
|
||||||
|
project_legacy_storage_5.route.update(path: 'zoo/new-test')
|
||||||
|
project = described_class.find(project_legacy_storage_5.id)
|
||||||
|
|
||||||
|
expect(project.disk_path).to eq('zoo/new-test')
|
||||||
|
end
|
||||||
|
|
||||||
it 'raises OrphanedNamespaceError when any parent namespace does not exist' do
|
it 'raises OrphanedNamespaceError when any parent namespace does not exist' do
|
||||||
subgroup = create(:group, parent: group)
|
subgroup = create(:group, parent: group)
|
||||||
project_orphaned_namespace = create(:project, name: 'baz', path: 'baz', namespace: subgroup, storage_version: nil)
|
project_orphaned_namespace = create(:project, name: 'baz', path: 'baz', namespace: subgroup, storage_version: nil)
|
||||||
subgroup.update_column(:parent_id, Namespace.maximum(:id).succ)
|
subgroup.update_column(:parent_id, Namespace.maximum(:id).succ)
|
||||||
|
|
||||||
project = described_class.find(project_orphaned_namespace.id)
|
project = described_class.find(project_orphaned_namespace.id)
|
||||||
|
project.route.destroy
|
||||||
|
subgroup.route.destroy
|
||||||
|
|
||||||
expect { project.disk_path }
|
expect { project.reload.disk_path }
|
||||||
.to raise_error(Gitlab::BackgroundMigration::BackfillProjectRepositories::OrphanedNamespaceError)
|
.to raise_error(Gitlab::BackgroundMigration::BackfillProjectRepositories::OrphanedNamespaceError)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -32,11 +32,13 @@ shared_examples 'backfill migration for project repositories' do |storage|
|
||||||
|
|
||||||
it 'inserts rows in a single query' do
|
it 'inserts rows in a single query' do
|
||||||
projects.create!(name: 'foo', path: 'foo', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
|
projects.create!(name: 'foo', path: 'foo', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
|
||||||
|
group2 = namespaces.create!(name: 'gro', path: 'gro')
|
||||||
|
|
||||||
control_count = ActiveRecord::QueryRecorder.new { described_class.new.perform(1, projects.last.id) }
|
control_count = ActiveRecord::QueryRecorder.new { described_class.new.perform(1, projects.last.id) }
|
||||||
|
|
||||||
projects.create!(name: 'bar', path: 'bar', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
|
projects.create!(name: 'bar', path: 'bar', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
|
||||||
projects.create!(name: 'zoo', path: 'zoo', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
|
projects.create!(name: 'top', path: 'top', namespace_id: group.id, storage_version: storage_version, repository_storage: shard.name)
|
||||||
|
projects.create!(name: 'zoo', path: 'zoo', namespace_id: group2.id, storage_version: storage_version, repository_storage: shard.name)
|
||||||
|
|
||||||
expect { described_class.new.perform(1, projects.last.id) }.not_to exceed_query_limit(control_count)
|
expect { described_class.new.perform(1, projects.last.id) }.not_to exceed_query_limit(control_count)
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue