diff options
author | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2019-03-01 16:51:49 +0000 |
---|---|---|
committer | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2019-03-01 16:51:49 +0000 |
commit | 3977421ed1b0cf75ab2a4bdf7c06d7b8c80489f0 (patch) | |
tree | 2c3316de8854c05719f030a63ca9295f71ae221f /spec/services | |
parent | 25c91fa452c61058f6bb119a95955406a36eb41f (diff) | |
parent | 3524a618d61a401b589bf6025cb50e042d532a4f (diff) | |
download | gitlab-ce-3977421ed1b0cf75ab2a4bdf7c06d7b8c80489f0.tar.gz |
Merge branch '53966-make-hashed-storage-migration-safer-and-more-inviting' into 'master'
Hashed Storage rollback mechanism
See merge request gitlab-org/gitlab-ce!23955
Diffstat (limited to 'spec/services')
5 files changed, 283 insertions, 3 deletions
diff --git a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb index 61dbb57ec08..639dd930618 100644 --- a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb @@ -70,10 +70,10 @@ describe Projects::HashedStorage::MigrateAttachmentsService do FileUtils.mkdir_p(base_path(hashed_storage)) end - it 'raises AttachmentMigrationError' do + it 'raises AttachmentCannotMoveError' do expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)) - expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentMigrationError) + expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError) end end end @@ -86,6 +86,8 @@ describe Projects::HashedStorage::MigrateAttachmentsService do context '#new_disk_path' do it 'returns new disk_path for project' do + service.execute + expect(service.new_disk_path).to eq(project.disk_path) end end diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 0772dc4b85b..e77e2198439 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -28,7 +28,17 @@ describe Projects::HashedStorage::MigrateRepositoryService do it 'fails when a git operation is in progress' do allow(project).to receive(:repo_reference_count) { 1 } - expect { service.execute }.to raise_error(Projects::HashedStorage::RepositoryMigrationError) + expect { service.execute }.to raise_error(Projects::HashedStorage::RepositoryInUseError) + end + end + + context 'when repository doesnt exist on disk' do + let(:project) { create(:project, :legacy_storage) } + + it 'skips the disk change but increase the version' do + service.execute + + expect(project.hashed_storage?(:repository)).to be_truthy end end diff --git a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb new file mode 100644 index 00000000000..6f4154d6011 --- /dev/null +++ b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::HashedStorage::RollbackAttachmentsService do + subject(:service) { described_class.new(project, logger: nil) } + + let(:project) { create(:project, :repository, skip_disk_validation: true) } + let(:legacy_storage) { Storage::LegacyProject.new(project) } + let(:hashed_storage) { Storage::HashedProject.new(project) } + + let!(:upload) { Upload.find_by(path: file_uploader.upload_path) } + let(:file_uploader) { build(:file_uploader, project: project) } + let(:old_disk_path) { File.join(base_path(hashed_storage), upload.path) } + let(:new_disk_path) { File.join(base_path(legacy_storage), upload.path) } + + context '#execute' do + context 'when succeeds' do + it 'moves attachments to legacy storage layout' do + expect(File.file?(old_disk_path)).to be_truthy + expect(File.file?(new_disk_path)).to be_falsey + expect(File.exist?(base_path(hashed_storage))).to be_truthy + expect(File.exist?(base_path(legacy_storage))).to be_falsey + expect(FileUtils).to receive(:mv).with(base_path(hashed_storage), base_path(legacy_storage)).and_call_original + + service.execute + + expect(File.exist?(base_path(legacy_storage))).to be_truthy + expect(File.exist?(base_path(hashed_storage))).to be_falsey + expect(File.file?(old_disk_path)).to be_falsey + expect(File.file?(new_disk_path)).to be_truthy + end + + it 'returns true' do + expect(service.execute).to be_truthy + end + + it 'sets skipped to false' do + service.execute + + expect(service.skipped?).to be_falsey + end + end + + context 'when original folder does not exist anymore' do + before do + FileUtils.rm_rf(base_path(hashed_storage)) + end + + it 'skips moving folders and go to next' do + expect(FileUtils).not_to receive(:mv).with(base_path(hashed_storage), base_path(legacy_storage)) + + service.execute + + expect(File.exist?(base_path(legacy_storage))).to be_falsey + expect(File.file?(new_disk_path)).to be_falsey + end + + it 'returns true' do + expect(service.execute).to be_truthy + end + + it 'sets skipped to true' do + service.execute + + expect(service.skipped?).to be_truthy + end + end + + context 'when target folder already exists' do + before do + FileUtils.mkdir_p(base_path(legacy_storage)) + end + + it 'raises AttachmentCannotMoveError' do + expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)) + + expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError) + end + end + end + + context '#old_disk_path' do + it 'returns old disk_path for project' do + expect(service.old_disk_path).to eq(project.disk_path) + end + end + + context '#new_disk_path' do + it 'returns new disk_path for project' do + service.execute + + expect(service.new_disk_path).to eq(project.full_path) + end + end + + def base_path(storage) + File.join(FileUploader.root, storage.disk_path) + end +end diff --git a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb new file mode 100644 index 00000000000..41927934501 --- /dev/null +++ b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis_shared_state do + include GitHelpers + + let(:gitlab_shell) { Gitlab::Shell.new } + let(:project) { create(:project, :repository, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } + let(:legacy_storage) { Storage::LegacyProject.new(project) } + let(:hashed_storage) { Storage::HashedProject.new(project) } + + subject(:service) { described_class.new(project, project.disk_path) } + + describe '#execute' do + let(:old_disk_path) { hashed_storage.disk_path } + let(:new_disk_path) { legacy_storage.disk_path } + + before do + allow(service).to receive(:gitlab_shell) { gitlab_shell } + end + + context 'repository lock' do + it 'tries to lock the repository' do + expect(service).to receive(:try_to_set_repository_read_only!) + + service.execute + end + + it 'fails when a git operation is in progress' do + allow(project).to receive(:repo_reference_count) { 1 } + + expect { service.execute }.to raise_error(Projects::HashedStorage::RepositoryInUseError) + end + end + + context 'when repository doesnt exist on disk' do + let(:project) { create(:project) } + + it 'skips the disk change but decrease the version' do + service.execute + + expect(project.legacy_storage?).to be_truthy + end + end + + context 'when succeeds' do + it 'renames project and wiki repositories' do + service.execute + + expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy + end + + it 'updates project to be legacy and not read-only' do + service.execute + + expect(project.legacy_storage?).to be_truthy + expect(project.repository_read_only).to be_falsey + end + + it 'move operation is called for both repositories' do + expect_move_repository(old_disk_path, new_disk_path) + expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") + + service.execute + end + + it 'writes project full path to .git/config' do + service.execute + + rugged_config = rugged_repo(project.repository).config['gitlab.fullpath'] + + expect(rugged_config).to eq project.full_path + end + end + + context 'when one move fails' do + it 'rolls repositories back to original name' do + allow(service).to receive(:move_repository).and_call_original + allow(service).to receive(:move_repository).with(old_disk_path, new_disk_path).once { false } # will disable first move only + + expect(service).to receive(:rollback_folder_move).and_call_original + + service.execute + + expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey + expect(project.repository_read_only?).to be_falsey + end + + context 'when rollback fails' do + before do + legacy_storage.ensure_storage_path_exists + gitlab_shell.mv_repository(project.repository_storage, old_disk_path, new_disk_path) + end + + it 'does not try to move nil repository over existing' do + expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage, old_disk_path, new_disk_path) + expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") + + service.execute + end + end + end + + def expect_move_repository(from_name, to_name) + expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage, from_name, to_name).and_call_original + end + end +end diff --git a/spec/services/projects/hashed_storage/rollback_service_spec.rb b/spec/services/projects/hashed_storage/rollback_service_spec.rb new file mode 100644 index 00000000000..427d1535559 --- /dev/null +++ b/spec/services/projects/hashed_storage/rollback_service_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::HashedStorage::RollbackService do + let(:project) { create(:project, :empty_repo, :wiki_repo) } + let(:logger) { double } + + subject(:service) { described_class.new(project, project.full_path, logger: logger) } + + describe '#execute' do + context 'attachments rollback' do + let(:attachments_service_class) { Projects::HashedStorage::RollbackAttachmentsService } + let(:attachments_service) { attachments_service_class.new(project, logger: logger) } + + it 'delegates rollback to Projects::HashedStorage::RollbackAttachmentsService' do + expect(attachments_service_class).to receive(:new) + .with(project, logger: logger) + .and_return(attachments_service) + expect(attachments_service).to receive(:execute) + + service.execute + end + + it 'does not delegate rollback if repository is in legacy storage already' do + project.storage_version = nil + expect(attachments_service_class).not_to receive(:new) + + service.execute + end + end + + context 'repository rollback' do + let(:repository_service_class) { Projects::HashedStorage::RollbackRepositoryService } + let(:repository_service) { repository_service_class.new(project, project.full_path, logger: logger) } + + it 'delegates rollback to RollbackRepositoryService' do + project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository] + + expect(repository_service_class).to receive(:new) + .with(project, project.full_path, logger: logger) + .and_return(repository_service) + expect(repository_service).to receive(:execute) + + service.execute + end + + it 'does not delegate rollback if repository is in legacy storage already' do + project.storage_version = nil + + expect(repository_service_class).not_to receive(:new) + + service.execute + end + end + end +end |