From 6a0bbb5aa58e37a0ad8c3817c4e809143adce1be Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 2 Aug 2016 11:32:28 +0200 Subject: using shared path for project import uploads and refactored gitlab remove export worker --- CHANGELOG | 3 + .../import/gitlab_projects_controller.rb | 7 ++- app/models/project.rb | 5 -- app/services/import_export_clean_up_service.rb | 24 ++++++++ .../repository_archive_clean_up_service.rb | 4 +- app/workers/gitlab_remove_project_export_worker.rb | 9 --- .../import_export_project_cleanup_worker.rb | 9 +++ config/initializers/1_settings.rb | 6 +- lib/gitlab/import_export.rb | 4 ++ .../import_export_clean_up_service_spec.rb | 64 ++++++++++++++++++++++ 10 files changed, 113 insertions(+), 22 deletions(-) create mode 100644 app/services/import_export_clean_up_service.rb delete mode 100644 app/workers/gitlab_remove_project_export_worker.rb create mode 100644 app/workers/import_export_project_cleanup_worker.rb create mode 100644 spec/services/import_export_clean_up_service_spec.rb diff --git a/CHANGELOG b/CHANGELOG index c099c63ce86..dd3ee006593 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -47,6 +47,9 @@ v 8.11.0 (unreleased) - Fix RequestProfiler::Middleware error when code is reloaded in development - Catch what warden might throw when profiling requests to re-throw it +v 8.10.4 (unreleased) + - Fix Import/Export project import not working in HA mode + v 8.10.3 - Fix Import/Export issue importing milestones and labels not associated properly. !5426 - Fix timing problems running imports on production. !5523 diff --git a/app/controllers/import/gitlab_projects_controller.rb b/app/controllers/import/gitlab_projects_controller.rb index 30df1fb2fec..3ec173abcdb 100644 --- a/app/controllers/import/gitlab_projects_controller.rb +++ b/app/controllers/import/gitlab_projects_controller.rb @@ -12,13 +12,14 @@ class Import::GitlabProjectsController < Import::BaseController return redirect_back_or_default(options: { alert: "You need to upload a GitLab project export archive." }) end - imported_file = project_params[:file].path + "-import" + import_upload_path = Gitlab::ImportExport.import_upload_path(filename: project_params[:file].original_filename) - FileUtils.copy_entry(project_params[:file].path, imported_file) + FileUtils.mkdir_p(File.dirname(import_upload_path)) + FileUtils.copy_entry(project_params[:file].path, import_upload_path) @project = Gitlab::ImportExport::ProjectCreator.new(project_params[:namespace_id], current_user, - File.expand_path(imported_file), + import_upload_path, project_params[:path]).execute if @project.saved? diff --git a/app/models/project.rb b/app/models/project.rb index 83b848ded8b..a18aef09acd 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -378,11 +378,6 @@ class Project < ActiveRecord::Base joins(join_body).reorder('join_note_counts.amount DESC') end - - # Deletes gitlab project export files older than 24 hours - def remove_gitlab_exports! - Gitlab::Popen.popen(%W(find #{Gitlab::ImportExport.storage_path} -not -path #{Gitlab::ImportExport.storage_path} -mmin +1440 -delete)) - end end def repository_storage_path diff --git a/app/services/import_export_clean_up_service.rb b/app/services/import_export_clean_up_service.rb new file mode 100644 index 00000000000..6442406d77e --- /dev/null +++ b/app/services/import_export_clean_up_service.rb @@ -0,0 +1,24 @@ +class ImportExportCleanUpService + LAST_MODIFIED_TIME_IN_MINUTES = 1440 + + attr_reader :mmin, :path + + def initialize(mmin = LAST_MODIFIED_TIME_IN_MINUTES) + @mmin = mmin + @path = Gitlab::ImportExport.storage_path + end + + def execute + Gitlab::Metrics.measure(:import_export_clean_up) do + return unless File.directory?(path) + + clean_up_export_files + end + end + + private + + def clean_up_export_files + Gitlab::Popen.popen(%W(find #{path} -not -path #{path} -mmin +#{mmin} -delete)) + end +end diff --git a/app/services/repository_archive_clean_up_service.rb b/app/services/repository_archive_clean_up_service.rb index 0b56b09738d..aa84d36a206 100644 --- a/app/services/repository_archive_clean_up_service.rb +++ b/app/services/repository_archive_clean_up_service.rb @@ -1,6 +1,8 @@ class RepositoryArchiveCleanUpService LAST_MODIFIED_TIME_IN_MINUTES = 120 + attr_reader :mmin, :path + def initialize(mmin = LAST_MODIFIED_TIME_IN_MINUTES) @mmin = mmin @path = Gitlab.config.gitlab.repository_downloads_path @@ -17,8 +19,6 @@ class RepositoryArchiveCleanUpService private - attr_reader :mmin, :path - def clean_up_old_archives run(%W(find #{path} -not -path #{path} -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -maxdepth 2 -mmin +#{mmin} -delete)) end diff --git a/app/workers/gitlab_remove_project_export_worker.rb b/app/workers/gitlab_remove_project_export_worker.rb deleted file mode 100644 index 1d91897d520..00000000000 --- a/app/workers/gitlab_remove_project_export_worker.rb +++ /dev/null @@ -1,9 +0,0 @@ -class GitlabRemoveProjectExportWorker - include Sidekiq::Worker - - sidekiq_options queue: :default - - def perform - Project.remove_gitlab_exports! - end -end diff --git a/app/workers/import_export_project_cleanup_worker.rb b/app/workers/import_export_project_cleanup_worker.rb new file mode 100644 index 00000000000..72e3a9ae734 --- /dev/null +++ b/app/workers/import_export_project_cleanup_worker.rb @@ -0,0 +1,9 @@ +class ImportExportProjectCleanupWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform + ImportExportCleanUpService.new.execute + end +end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 49130f37b31..deac3b0f0f9 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -287,9 +287,9 @@ Settings.cron_jobs['admin_email_worker']['job_class'] = 'AdminEmailWorker' Settings.cron_jobs['repository_archive_cache_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['repository_archive_cache_worker']['cron'] ||= '0 * * * *' Settings.cron_jobs['repository_archive_cache_worker']['job_class'] = 'RepositoryArchiveCacheWorker' -Settings.cron_jobs['gitlab_remove_project_export_worker'] ||= Settingslogic.new({}) -Settings.cron_jobs['gitlab_remove_project_export_worker']['cron'] ||= '0 * * * *' -Settings.cron_jobs['gitlab_remove_project_export_worker']['job_class'] = 'GitlabRemoveProjectExportWorker' +Settings.cron_jobs['import_export_project_cleanup_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['import_export_project_cleanup_worker']['cron'] ||= '0 * * * *' +Settings.cron_jobs['import_export_project_cleanup_worker']['job_class'] = 'ImportExportProjectCleanupWorker' Settings.cron_jobs['requests_profiles_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['requests_profiles_worker']['cron'] ||= '0 0 * * *' Settings.cron_jobs['requests_profiles_worker']['job_class'] = 'RequestsProfilesWorker' diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb index 48b2c43ac21..bb562bdcd2c 100644 --- a/lib/gitlab/import_export.rb +++ b/lib/gitlab/import_export.rb @@ -13,6 +13,10 @@ module Gitlab File.join(Settings.shared['path'], 'tmp/project_exports') end + def import_upload_path(filename:) + File.join(storage_path, 'uploads', filename) + end + def project_filename "project.json" end diff --git a/spec/services/import_export_clean_up_service_spec.rb b/spec/services/import_export_clean_up_service_spec.rb new file mode 100644 index 00000000000..130b9fc4b82 --- /dev/null +++ b/spec/services/import_export_clean_up_service_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe ImportExportCleanUpService, services: true do + describe '#execute' do + let(:service) { described_class.new } + + let(:tmp_import_export_folder) { 'tmp/project_exports' } + + context 'when the import/export directory does not exist' do + it 'does not remove any archives' do + path = '/invalid/path/' + stub_repository_downloads_path(path) + + expect(File).to receive(:directory?).with(path + tmp_import_export_folder).and_return(false).at_least(:once) + expect(service).not_to receive(:clean_up_export_files) + + service.execute + end + end + + context 'when the import/export directory exists' do + it 'removes old files' do + in_directory_with_files(mtime: 2.days.ago) do |dir, files| + service.execute + + files.each { |file| expect(File.exist?(file)).to eq false } + expect(File.directory?(dir)).to eq false + end + end + + it 'does not remove new files' do + in_directory_with_files(mtime: 2.hours.ago) do |dir, files| + service.execute + + files.each { |file| expect(File.exist?(file)).to eq true } + expect(File.directory?(dir)).to eq true + end + end + end + + def in_directory_with_files(mtime:) + Dir.mktmpdir do |tmpdir| + stub_repository_downloads_path(tmpdir) + dir = File.join(tmpdir, tmp_import_export_folder, 'subfolder') + FileUtils.mkdir_p(dir) + + files = FileUtils.touch(file_list(dir) + [dir], mtime: mtime) + + yield(dir, files) + end + end + + def stub_repository_downloads_path(path) + new_shared_settings = Settings.shared.merge('path' => path) + allow(Settings).to receive(:shared).and_return(new_shared_settings) + end + + def file_list(dir) + Array.new(5) do |num| + File.join(dir, "random-#{num}.tar.gz") + end + end + end +end -- cgit v1.2.1 From 9772cd893eba9493037da572df39a3bfb992af59 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 4 Aug 2016 13:39:38 +0200 Subject: fix spec --- spec/services/import_export_clean_up_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/import_export_clean_up_service_spec.rb b/spec/services/import_export_clean_up_service_spec.rb index 130b9fc4b82..81b1d327696 100644 --- a/spec/services/import_export_clean_up_service_spec.rb +++ b/spec/services/import_export_clean_up_service_spec.rb @@ -44,7 +44,7 @@ describe ImportExportCleanUpService, services: true do dir = File.join(tmpdir, tmp_import_export_folder, 'subfolder') FileUtils.mkdir_p(dir) - files = FileUtils.touch(file_list(dir) + [dir], mtime: mtime) + files = FileUtils.touch(file_list(dir) + [dir], mtime: mtime.to_time) yield(dir, files) end -- cgit v1.2.1