diff options
3 files changed, 137 insertions, 9 deletions
diff --git a/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb b/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb index ba62aee453c..a3b5324cc1d 100644 --- a/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb +++ b/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb @@ -12,7 +12,7 @@ module Gitlab class HashedProject attr_accessor :project - ROOT_PATH_PREFIX = '@hashed'.freeze + ROOT_PATH_PREFIX = '@hashed' def initialize(project) @project = project @@ -42,7 +42,7 @@ module Gitlab end # Concern used by Project and Namespace to determine the full - # route the the project + # route to the project module Routable extend ActiveSupport::Concern @@ -128,22 +128,65 @@ module Gitlab end end - # Class to add the fullpath to the git repo config - class Up + # Base class for Up and Down migration classes + class BackfillFullpathMigration + RETRY_DELAY = 15.minutes + MAX_RETRIES = 2 + + # Base class for retrying one project + class BaseRetryOne + def perform(project_id, retry_count) + project = Project.find(project_id) + + migration_class.new.safe_perform_one(project, retry_count) if project + end + end + def perform(start_id, end_id) Project.where(id: start_id..end_id).each do |project| - project.add_fullpath_config + safe_perform_one(project) end end + + def safe_perform_one(project, retry_count = 0) + perform_one(project) + rescue GRPC::NotFound, GRPC::InvalidArgument + nil + rescue GRPC::BadStatus + schedule_retry(project, retry_count + 1) if retry_count < MAX_RETRIES + end + + def schedule_retry(project, retry_count) + BackgroundMigrationWorker.perform_in(RETRY_DELAY, self.class::RetryOne.name, [project.id, retry_count]) + end + end + + # Class to add the fullpath to the git repo config + class Up < BackfillFullpathMigration + # Class used to retry + class RetryOne < BaseRetryOne + def migration_class + Up + end + end + + def perform_one(project) + project.add_fullpath_config + end end # Class to rollback adding the fullpath to the git repo config - class Down - def perform(start_id, end_id) - Project.where(id: start_id..end_id).each do |project| - project.remove_fullpath_config + class Down < BackfillFullpathMigration + # Class used to retry + class RetryOne < BaseRetryOne + def migration_class + Down end end + + def perform_one(project) + project.remove_fullpath_config + end end end end diff --git a/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb new file mode 100644 index 00000000000..b7c79027186 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig, :migration, schema: 20181010133639 do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:group) { namespaces.create!(name: 'foo', path: 'foo') } + let(:subgroup) { namespaces.create!(name: 'bar', path: 'bar', parent_id: group.id) } + + describe described_class::Storage::HashedProject do + let(:project) { double(id: 555) } + subject(:project_storage) { described_class.new(project) } + + it 'has the correct disk_path' do + expect(project_storage.disk_path).to eq('@hashed/91/a7/91a73fd806ab2c005c13b4dc19130a884e909dea3f72d46e30266fe1a1f588d8') + end + end + + describe described_class::Storage::LegacyProject do + let(:project) { double(full_path: 'this/is/the/full/path') } + subject(:project_storage) { described_class.new(project) } + + it 'has the correct disk_path' do + expect(project_storage.disk_path).to eq('this/is/the/full/path') + end + end + + describe described_class::Project do + let(:project_record) { projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') } + subject(:project) { described_class.find(project_record.id) } + + describe '#full_path' do + it 'returns path containing all parent namespaces' do + expect(project.full_path).to eq('foo/bar/baz') + end + end + end + + describe described_class::Up do + describe '#perform' do + subject(:migrate) { described_class.new.perform(projects.minimum(:id), projects.maximum(:id)) } + + it 'asks the gitaly client to set config' do + projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') + projects.create!(namespace_id: subgroup.id, name: 'buzz', path: 'buzz', storage_version: 1) + + expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| + expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/baz') + end + + expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service| + expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/buzz') + end + + migrate + end + end + end + + describe described_class::Down do + describe '#perform' do + subject(:migrate) { described_class.new.perform(projects.minimum(:id), projects.maximum(:id)) } + + it 'asks the gitaly client to set config' do + projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') + + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService) + .to receive(:delete_config).with(['gitlab.fullpath']) + + migrate + end + end + end +end diff --git a/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb b/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb index b5e770e6b01..ced6b09dfc0 100644 --- a/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb +++ b/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb @@ -27,6 +27,16 @@ describe BackfillStoreProjectFullPathInRepo, :migration do migration.up end + it 'retries in case of failure' do + repository_service = spy(:repository_service) + + allow(Gitlab::GitalyClient::RepositoryService).to receive(:new).and_return(repository_service) + allow(repository_service).to receive(:set_config).and_raise(GRPC::BadStatus, 'Retry me') + expect(repository_service).to receive(:set_config).exactly(3).times + + migration.up + end + context 'legacy storage' do it 'finds the repository at the correct location' do Project.find(project.id).create_repository |