Commit 21136697 authored by George Koltsov's avatar George Koltsov

Update Project/Group Exporter temp folder cleanup

  - Do not clean up base temp directory of Project/Group
  export when export is completed, since there can be
  simultaneous concurrent exports running at the same time,
  which can fail if base directory has been cleaned up
parent 036539ce
...@@ -22,7 +22,7 @@ module Groups ...@@ -22,7 +22,7 @@ module Groups
save! save!
ensure ensure
remove_base_tmp_dir remove_archive_tmp_dir
end end
private private
...@@ -81,8 +81,8 @@ module Groups ...@@ -81,8 +81,8 @@ module Groups
Gitlab::ImportExport::Saver.new(exportable: @group, shared: @shared) Gitlab::ImportExport::Saver.new(exportable: @group, shared: @shared)
end end
def remove_base_tmp_dir def remove_archive_tmp_dir
FileUtils.rm_rf(shared.base_path) if shared&.base_path FileUtils.rm_rf(shared.archive_path) if shared&.archive_path
end end
def notify_error! def notify_error!
......
---
title: Update Project/Group Exporter temp folder cleanup
merge_request: 51969
author:
type: fixed
...@@ -31,7 +31,7 @@ module Gitlab ...@@ -31,7 +31,7 @@ module Gitlab
@shared.error(e) @shared.error(e)
false false
ensure ensure
remove_base_tmp_dir remove_archive_tmp_dir
end end
private private
...@@ -40,8 +40,8 @@ module Gitlab ...@@ -40,8 +40,8 @@ module Gitlab
tar_czf(archive: archive_file, dir: @shared.export_path) tar_czf(archive: archive_file, dir: @shared.export_path)
end end
def remove_base_tmp_dir def remove_archive_tmp_dir
FileUtils.rm_rf(@shared.base_path) FileUtils.rm_rf(@shared.archive_path)
end end
def archive_file def archive_file
......
...@@ -6,7 +6,8 @@ require 'fileutils' ...@@ -6,7 +6,8 @@ require 'fileutils'
RSpec.describe Gitlab::ImportExport::Saver do RSpec.describe Gitlab::ImportExport::Saver do
let!(:project) { create(:project, :public, name: 'project') } let!(:project) { create(:project, :public, name: 'project') }
let(:base_path) { "#{Dir.tmpdir}/project_tree_saver_spec" } let(:base_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
let(:export_path) { "#{base_path}/project_tree_saver_spec/export" } let(:archive_path) { "#{base_path}/archive" }
let(:export_path) { "#{archive_path}/export" }
let(:shared) { project.import_export_shared } let(:shared) { project.import_export_shared }
subject { described_class.new(exportable: project, shared: shared) } subject { described_class.new(exportable: project, shared: shared) }
...@@ -35,10 +36,13 @@ RSpec.describe Gitlab::ImportExport::Saver do ...@@ -35,10 +36,13 @@ RSpec.describe Gitlab::ImportExport::Saver do
.to match(%r[\/uploads\/-\/system\/import_export_upload\/export_file.*]) .to match(%r[\/uploads\/-\/system\/import_export_upload\/export_file.*])
end end
it 'removes tmp files' do it 'removes archive path and keeps base path untouched' do
allow(shared).to receive(:archive_path).and_return(archive_path)
subject.save subject.save
expect(FileUtils).to have_received(:rm_rf).with(base_path) expect(FileUtils).not_to have_received(:rm_rf).with(base_path)
expect(Dir.exist?(base_path)).to eq(false) expect(FileUtils).to have_received(:rm_rf).with(archive_path)
expect(Dir.exist?(archive_path)).to eq(false)
end end
end end
...@@ -134,7 +134,7 @@ RSpec.describe Groups::ImportExport::ExportService do ...@@ -134,7 +134,7 @@ RSpec.describe Groups::ImportExport::ExportService do
expect { service.execute }.to raise_error(Gitlab::ImportExport::Error) expect { service.execute }.to raise_error(Gitlab::ImportExport::Error)
expect(group.import_export_upload).to be_nil expect(group.import_export_upload).to be_nil
expect(Dir.exist?(shared.base_path)).to eq(false) expect(Dir.exist?(shared.archive_path)).to eq(false)
end end
it 'notifies the user about failed group export' do it 'notifies the user about failed group export' do
...@@ -159,7 +159,7 @@ RSpec.describe Groups::ImportExport::ExportService do ...@@ -159,7 +159,7 @@ RSpec.describe Groups::ImportExport::ExportService do
expect { service.execute }.to raise_error(Gitlab::ImportExport::Error) expect { service.execute }.to raise_error(Gitlab::ImportExport::Error)
expect(group.import_export_upload).to be_nil expect(group.import_export_upload).to be_nil
expect(Dir.exist?(shared.base_path)).to eq(false) expect(Dir.exist?(shared.archive_path)).to eq(false)
end end
it 'notifies logger' do it 'notifies logger' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment