diff --git a/lib/gitlab/import_export/project/base_task.rb b/lib/gitlab/import_export/project/base_task.rb index 5c75eca2bed393f7fa1485f067617db2f8fe6b41..f6afbfcc50dcde949de1d3696175710389a74bf4 100644 --- a/lib/gitlab/import_export/project/base_task.rb +++ b/lib/gitlab/import_export/project/base_task.rb @@ -26,6 +26,22 @@ def measurement_options } end + def disable_upload_object_storage + overwrite_uploads_setting('enabled', false) do + yield + end + end + + def overwrite_uploads_setting(key, value) + old_value = Settings.uploads.object_store[key] + Settings.uploads.object_store[key] = value + + yield + + ensure + Settings.uploads.object_store[key] = old_value + end + def success(message) logger.info(message) diff --git a/lib/gitlab/import_export/project/export_task.rb b/lib/gitlab/import_export/project/export_task.rb index 22efd0cb8a3e4a0786154a94e2eb0a5420d84f3d..4919ace0742fe2534e92e571fb4fc9abd6f6e024 100644 --- a/lib/gitlab/import_export/project/export_task.rb +++ b/lib/gitlab/import_export/project/export_task.rb @@ -19,7 +19,11 @@ def export .execute(Gitlab::ImportExport::AfterExportStrategies::MoveFileStrategy.new(archive_path: file_path), measurement_options) end + return error(project.import_export_shared.errors.join(', ')) if project.import_export_shared.errors.any? + success('Done!') + rescue Gitlab::ImportExport::Error => e + error(e.message) end private @@ -32,8 +36,13 @@ def file_path_exists? def with_export with_request_store do - ::Gitlab::GitalyClient.allow_n_plus_1_calls do - yield + # We are disabling ObjectStorage for `export` + # since when direct upload is enabled, remote storage will be used + # and Gitlab::ImportExport::AfterExportStrategies::MoveFileStrategy will fail to copy exported archive + disable_upload_object_storage do + ::Gitlab::GitalyClient.allow_n_plus_1_calls do + yield + end end end end diff --git a/lib/gitlab/import_export/project/import_task.rb b/lib/gitlab/import_export/project/import_task.rb index 101cc5f5ab0a5241b9cd8ff526c759e5a1039356..20f7ac1eb18781a749bca243830fdee70decd315 100644 --- a/lib/gitlab/import_export/project/import_task.rb +++ b/lib/gitlab/import_export/project/import_task.rb @@ -67,24 +67,6 @@ def execute_sidekiq_job Sidekiq::Worker.drain_all end - def disable_upload_object_storage - overwrite_uploads_setting('background_upload', false) do - overwrite_uploads_setting('direct_upload', false) do - yield - end - end - end - - def overwrite_uploads_setting(key, value) - old_value = Settings.uploads.object_store[key] - Settings.uploads.object_store[key] = value - - yield - - ensure - Settings.uploads.object_store[key] = old_value - end - def full_path "#{namespace.full_path}/#{project_path}" end diff --git a/spec/lib/gitlab/import_export/project/export_task_spec.rb b/spec/lib/gitlab/import_export/project/export_task_spec.rb index 7c091c4363798c91da413cf973ac7e5f82c0bfa3..dc8eb54dc14573cc26a896ea0e56ab638be80d94 100644 --- a/spec/lib/gitlab/import_export/project/export_task_spec.rb +++ b/spec/lib/gitlab/import_export/project/export_task_spec.rb @@ -10,6 +10,7 @@ let(:file_path) { 'spec/fixtures/gitlab/import_export/test_project_export.tar.gz' } let(:project) { create(:project, creator: user, namespace: user.namespace) } let(:project_name) { project.name } + let(:rake_task) { described_class.new(task_params) } let(:task_params) do { @@ -21,7 +22,7 @@ } end - subject { described_class.new(task_params).export } + subject { rake_task.export } context 'when project is found' do let(:project) { create(:project, creator: user, namespace: user.namespace) } @@ -29,9 +30,13 @@ around do |example| example.run ensure - File.delete(file_path) + File.delete(file_path) if File.exist?(file_path) end + include_context 'rake task object storage shared context' + + it_behaves_like 'rake task with disabled object_storage', ::Projects::ImportExport::ExportService, :success + it 'performs project export successfully' do expect { subject }.to output(/Done!/).to_stdout @@ -39,8 +44,6 @@ expect(File).to exist(file_path) end - - it_behaves_like 'measurable' end context 'when project is not found' do @@ -66,4 +69,32 @@ expect(subject).to eq(false) end end + + context 'when after export strategy fails' do + before do + allow_next_instance_of(Gitlab::ImportExport::AfterExportStrategies::MoveFileStrategy) do |after_export_strategy| + allow(after_export_strategy).to receive(:strategy_execute).and_raise(Gitlab::ImportExport::AfterExportStrategies::BaseAfterExportStrategy::StrategyError) + end + end + + it 'error is logged' do + expect(rake_task).to receive(:error).and_call_original + + expect(subject).to eq(false) + end + end + + context 'when saving services fail' do + before do + allow_next_instance_of(::Projects::ImportExport::ExportService) do |service| + allow(service).to receive(:execute).and_raise(Gitlab::ImportExport::Error) + end + end + + it 'error is logged' do + expect(rake_task).to receive(:error).and_call_original + + expect(subject).to eq(false) + end + end end diff --git a/spec/lib/gitlab/import_export/project/import_task_spec.rb b/spec/lib/gitlab/import_export/project/import_task_spec.rb index 4f4fcd3ad8adf452cf7bb36b143e52e3780faf8b..d83cc173388e322b0058a5fb14b4fc1efe230fb2 100644 --- a/spec/lib/gitlab/import_export/project/import_task_spec.rb +++ b/spec/lib/gitlab/import_export/project/import_task_spec.rb @@ -8,7 +8,7 @@ let!(:user) { create(:user, username: username) } let(:measurement_enabled) { false } let(:project) { Project.find_by_full_path("#{namespace_path}/#{project_name}") } - let(:import_task) { described_class.new(task_params) } + let(:rake_task) { described_class.new(task_params) } let(:task_params) do { username: username, @@ -19,29 +19,16 @@ } end - before do - allow(Settings.uploads.object_store).to receive(:[]=).and_call_original - end - - around do |example| - old_direct_upload_setting = Settings.uploads.object_store['direct_upload'] - old_background_upload_setting = Settings.uploads.object_store['background_upload'] - - Settings.uploads.object_store['direct_upload'] = true - Settings.uploads.object_store['background_upload'] = true - - example.run - - Settings.uploads.object_store['direct_upload'] = old_direct_upload_setting - Settings.uploads.object_store['background_upload'] = old_background_upload_setting - end - - subject { import_task.import } + subject { rake_task.import } context 'when project import is valid' do let(:project_name) { 'import_rake_test_project' } let(:file_path) { 'spec/fixtures/gitlab/import_export/lightweight_project_export.tar.gz' } + include_context 'rake task object storage shared context' + + it_behaves_like 'rake task with disabled object_storage', ::Projects::GitlabProjectsImportService, :execute_sidekiq_job + it 'performs project import successfully' do expect { subject }.to output(/Done!/).to_stdout expect { subject }.not_to raise_error @@ -53,28 +40,6 @@ expect(project.import_state.status).to eq('finished') end - it 'disables direct & background upload only during project creation' do - expect_next_instance_of(Projects::GitlabProjectsImportService) do |service| - expect(service).to receive(:execute).and_wrap_original do |m| - expect(Settings.uploads.object_store['background_upload']).to eq(false) - expect(Settings.uploads.object_store['direct_upload']).to eq(false) - - m.call - end - end - - expect(import_task).to receive(:execute_sidekiq_job).and_wrap_original do |m| - expect(Settings.uploads.object_store['background_upload']).to eq(true) - expect(Settings.uploads.object_store['direct_upload']).to eq(true) - expect(Settings.uploads.object_store).not_to receive(:[]=).with('backgroud_upload', false) - expect(Settings.uploads.object_store).not_to receive(:[]=).with('direct_upload', false) - - m.call - end - - subject - end - it_behaves_like 'measurable' end diff --git a/spec/support/shared_contexts/lib/gitlab/import_export/project/rake_task_object_storage_shared_context.rb b/spec/support/shared_contexts/lib/gitlab/import_export/project/rake_task_object_storage_shared_context.rb new file mode 100644 index 0000000000000000000000000000000000000000..dc1a52e362987f87ef4f95db146d79d8ea791c19 --- /dev/null +++ b/spec/support/shared_contexts/lib/gitlab/import_export/project/rake_task_object_storage_shared_context.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +RSpec.shared_context 'rake task object storage shared context' do + before do + allow(Settings.uploads.object_store).to receive(:[]=).and_call_original + end + + around do |example| + old_object_store_setting = Settings.uploads.object_store['enabled'] + + Settings.uploads.object_store['enabled'] = true + + example.run + + Settings.uploads.object_store['enabled'] = old_object_store_setting + end +end diff --git a/spec/support/shared_examples/lib/gitlab/import_export/project/rake_task_object_storage_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/import_export/project/rake_task_object_storage_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..d6dc89a2c153a5db140dec7c09bc9e17a04cee0c --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/import_export/project/rake_task_object_storage_shared_examples.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'rake task with disabled object_storage' do |service_class, method| + it 'disables direct & background upload only for service call' do + expect_next_instance_of(service_class) do |service| + expect(service).to receive(:execute).and_wrap_original do |m| + expect(Settings.uploads.object_store['enabled']).to eq(false) + + m.call + end + end + + expect(rake_task).to receive(method).and_wrap_original do |m, *args| + expect(Settings.uploads.object_store['enabled']).to eq(true) + expect(Settings.uploads.object_store).not_to receive(:[]=).with('enabled', false) + + m.call(*args) + end + + subject + end +end