diff options
author | Robert Speicher <robert@gitlab.com> | 2018-03-14 23:08:52 +0000 |
---|---|---|
committer | Ian Baum <ibaum@gitlab.com> | 2018-03-15 13:19:05 -0500 |
commit | 9015d9ff79e6b886835a870e8bf61936cb93002c (patch) | |
tree | bb68ff21d22d1e6ceed10cdb5340d77995d4b193 | |
parent | e5944b812d53b145f915c6a532a44f79e8ad7f1c (diff) | |
download | gitlab-ce-9015d9ff79e6b886835a870e8bf61936cb93002c.tar.gz |
Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master'
Fix moving local, unhashed upload or pages directories during group transfer
Closes #43993
See merge request gitlab-org/gitlab-ce!17658
-rw-r--r-- | app/models/concerns/storage/legacy_namespace.rb | 51 | ||||
-rw-r--r-- | changelogs/unreleased/mk-fix-move-upload-files-on-group-transfer.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/project_transfer.rb | 16 | ||||
-rw-r--r-- | spec/lib/gitlab/project_transfer_spec.rb | 59 | ||||
-rw-r--r-- | spec/models/namespace_spec.rb | 113 |
5 files changed, 214 insertions, 30 deletions
diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index 67a988addbe..f05e606995d 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -7,29 +7,24 @@ module Storage raise Gitlab::UpdatePathError.new('Namespace cannot be moved, because at least one project has tags in container registry') end - expires_full_path_cache - - # Move the namespace directory in all storage paths used by member projects - repository_storage_paths.each do |repository_storage_path| - # Ensure old directory exists before moving it - gitlab_shell.add_namespace(repository_storage_path, full_path_was) - - # Ensure new directory exists before moving it (if there's a parent) - gitlab_shell.add_namespace(repository_storage_path, parent.full_path) if parent + parent_was = if parent_changed? && parent_id_was.present? + Namespace.find(parent_id_was) # raise NotFound early if needed + end - unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path) + expires_full_path_cache - Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}" + move_repositories - # if we cannot move namespace directory we should rollback - # db changes in order to prevent out of sync between db and fs - raise Gitlab::UpdatePathError.new('namespace directory cannot be moved') - end + if parent_changed? + former_parent_full_path = parent_was&.full_path + parent_full_path = parent&.full_path + Gitlab::UploadsTransfer.new.move_namespace(path, former_parent_full_path, parent_full_path) + Gitlab::PagesTransfer.new.move_namespace(path, former_parent_full_path, parent_full_path) + else + Gitlab::UploadsTransfer.new.rename_namespace(full_path_was, full_path) + Gitlab::PagesTransfer.new.rename_namespace(full_path_was, full_path) end - Gitlab::UploadsTransfer.new.rename_namespace(full_path_was, full_path) - Gitlab::PagesTransfer.new.rename_namespace(full_path_was, full_path) - remove_exports! # If repositories moved successfully we need to @@ -57,6 +52,26 @@ module Storage private + def move_repositories + # Move the namespace directory in all storage paths used by member projects + repository_storage_paths.each do |repository_storage_path| + # Ensure old directory exists before moving it + gitlab_shell.add_namespace(repository_storage_path, full_path_was) + + # Ensure new directory exists before moving it (if there's a parent) + gitlab_shell.add_namespace(repository_storage_path, parent.full_path) if parent + + unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path) + + Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}" + + # if we cannot move namespace directory we should rollback + # db changes in order to prevent out of sync between db and fs + raise Gitlab::UpdatePathError.new('namespace directory cannot be moved') + end + end + end + def old_repository_storage_paths @old_repository_storage_paths ||= repository_storage_paths end diff --git a/changelogs/unreleased/mk-fix-move-upload-files-on-group-transfer.yml b/changelogs/unreleased/mk-fix-move-upload-files-on-group-transfer.yml new file mode 100644 index 00000000000..ba366b81600 --- /dev/null +++ b/changelogs/unreleased/mk-fix-move-upload-files-on-group-transfer.yml @@ -0,0 +1,5 @@ +--- +title: Fix missing uploads after group transfer +merge_request: 17658 +author: +type: fixed diff --git a/lib/gitlab/project_transfer.rb b/lib/gitlab/project_transfer.rb index 1bba0b78e2f..690c38737c0 100644 --- a/lib/gitlab/project_transfer.rb +++ b/lib/gitlab/project_transfer.rb @@ -1,13 +1,19 @@ module Gitlab + # This class is used to move local, unhashed files owned by projects to their new location class ProjectTransfer - def move_project(project_path, namespace_path_was, namespace_path) - new_namespace_folder = File.join(root_dir, namespace_path) - FileUtils.mkdir_p(new_namespace_folder) unless Dir.exist?(new_namespace_folder) - from = File.join(root_dir, namespace_path_was, project_path) - to = File.join(root_dir, namespace_path, project_path) + # nil parent_path (or parent_path_was) represents a root namespace + def move_namespace(path, parent_path_was, parent_path) + parent_path_was ||= '' + parent_path ||= '' + new_parent_folder = File.join(root_dir, parent_path) + FileUtils.mkdir_p(new_parent_folder) + from = File.join(root_dir, parent_path_was, path) + to = File.join(root_dir, parent_path, path) move(from, to, "") end + alias_method :move_project, :move_namespace + def rename_project(path_was, path, namespace_path) base_dir = File.join(root_dir, namespace_path) move(path_was, path, base_dir) diff --git a/spec/lib/gitlab/project_transfer_spec.rb b/spec/lib/gitlab/project_transfer_spec.rb index 10c5fb148cd..0b9b1f537b5 100644 --- a/spec/lib/gitlab/project_transfer_spec.rb +++ b/spec/lib/gitlab/project_transfer_spec.rb @@ -21,30 +21,77 @@ describe Gitlab::ProjectTransfer do describe '#move_project' do it "moves project upload to another namespace" do - FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path)) + path_to_be_moved = File.join(@root_dir, @namespace_path_was, @project_path) + expected_path = File.join(@root_dir, @namespace_path, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + @project_transfer.move_project(@project_path, @namespace_path_was, @namespace_path) - expected_path = File.join(@root_dir, @namespace_path, @project_path) expect(Dir.exist?(expected_path)).to be_truthy end end + describe '#move_namespace' do + context 'when moving namespace from root into another namespace' do + it "moves namespace projects' upload" do + child_namespace = 'test_child_namespace' + path_to_be_moved = File.join(@root_dir, child_namespace, @project_path) + expected_path = File.join(@root_dir, @namespace_path, child_namespace, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + + @project_transfer.move_namespace(child_namespace, nil, @namespace_path) + + expect(Dir.exist?(expected_path)).to be_truthy + end + end + + context 'when moving namespace from one parent to another' do + it "moves namespace projects' upload" do + child_namespace = 'test_child_namespace' + path_to_be_moved = File.join(@root_dir, @namespace_path_was, child_namespace, @project_path) + expected_path = File.join(@root_dir, @namespace_path, child_namespace, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + + @project_transfer.move_namespace(child_namespace, @namespace_path_was, @namespace_path) + + expect(Dir.exist?(expected_path)).to be_truthy + end + end + + context 'when moving namespace from having a parent to root' do + it "moves namespace projects' upload" do + child_namespace = 'test_child_namespace' + path_to_be_moved = File.join(@root_dir, @namespace_path_was, child_namespace, @project_path) + expected_path = File.join(@root_dir, child_namespace, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + + @project_transfer.move_namespace(child_namespace, @namespace_path_was, nil) + + expect(Dir.exist?(expected_path)).to be_truthy + end + end + end + describe '#rename_project' do it "renames project" do - FileUtils.mkdir_p(File.join(@root_dir, @namespace_path, @project_path_was)) + path_to_be_moved = File.join(@root_dir, @namespace_path, @project_path_was) + expected_path = File.join(@root_dir, @namespace_path, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + @project_transfer.rename_project(@project_path_was, @project_path, @namespace_path) - expected_path = File.join(@root_dir, @namespace_path, @project_path) expect(Dir.exist?(expected_path)).to be_truthy end end describe '#rename_namespace' do it "renames namespace" do - FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path)) + path_to_be_moved = File.join(@root_dir, @namespace_path_was, @project_path) + expected_path = File.join(@root_dir, @namespace_path, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + @project_transfer.rename_namespace(@namespace_path_was, @namespace_path) - expected_path = File.join(@root_dir, @namespace_path, @project_path) expect(Dir.exist?(expected_path)).to be_truthy end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 191b60e4383..16033c9052a 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -194,11 +194,60 @@ describe Namespace do allow(namespace).to receive(:path).and_return('new_path') end +<<<<<<< HEAD it 'raises an error about not movable project' do expect { namespace.move_dir }.to raise_error(/Namespace cannot be moved/) end end +||||||| parent of 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' + context 'with subgroups' do + let(:parent) { create(:group, name: 'parent', path: 'parent') } + let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } + let!(:project) { create(:project_empty_repo, :legacy_storage, path: 'the-project', namespace: child, skip_disk_validation: true) } + let(:uploads_dir) { FileUploader.root } + let(:pages_dir) { File.join(TestEnv.pages_path) } + + before do + FileUtils.mkdir_p(File.join(uploads_dir, project.full_path)) + FileUtils.mkdir_p(File.join(pages_dir, project.full_path)) + end + + context 'renaming child' do + it 'correctly moves the repository, uploads and pages' do + expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git') + expected_upload_path = File.join(uploads_dir, 'parent', 'renamed', 'the-project') + expected_pages_path = File.join(pages_dir, 'parent', 'renamed', 'the-project') +======= + context 'with subgroups', :nested_groups do + let(:parent) { create(:group, name: 'parent', path: 'parent') } + let(:new_parent) { create(:group, name: 'new_parent', path: 'new_parent') } + let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } + let!(:project) { create(:project_empty_repo, :legacy_storage, path: 'the-project', namespace: child, skip_disk_validation: true) } + let(:uploads_dir) { FileUploader.root } + let(:pages_dir) { File.join(TestEnv.pages_path) } + + def expect_project_directories_at(namespace_path) + expected_repository_path = File.join(TestEnv.repos_path, namespace_path, 'the-project.git') + expected_upload_path = File.join(uploads_dir, namespace_path, 'the-project') + expected_pages_path = File.join(pages_dir, namespace_path, 'the-project') + + expect(File.directory?(expected_repository_path)).to be_truthy + expect(File.directory?(expected_upload_path)).to be_truthy + expect(File.directory?(expected_pages_path)).to be_truthy + end + + before do + FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project.full_path}.git")) + FileUtils.mkdir_p(File.join(uploads_dir, project.full_path)) + FileUtils.mkdir_p(File.join(pages_dir, project.full_path)) + end + + context 'renaming child' do + it 'correctly moves the repository, uploads and pages' do + child.update!(path: 'renamed') +>>>>>>> 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' +<<<<<<< HEAD context 'with subgroups' do let(:parent) { create(:group, name: 'parent', path: 'parent') } let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } @@ -210,18 +259,81 @@ describe Namespace do FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project')) FileUtils.mkdir_p(File.join(pages_dir, 'parent', 'child', 'the-project')) end +||||||| parent of 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' + child.update_attributes!(path: 'renamed') + expect(File.directory?(expected_repository_path)).to be(true) + expect(File.directory?(expected_upload_path)).to be(true) + expect(File.directory?(expected_pages_path)).to be(true) + end + end +======= + expect_project_directories_at('parent/renamed') + end + end +>>>>>>> 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' + +<<<<<<< HEAD context 'renaming child' do it 'correctly moves the repository, uploads and pages' do expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git') expected_upload_path = File.join(uploads_dir, 'parent', 'renamed', 'the-project') expected_pages_path = File.join(pages_dir, 'parent', 'renamed', 'the-project') +||||||| parent of 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' + context 'renaming parent' do + it 'correctly moves the repository, uploads and pages' do + expected_repository_path = File.join(TestEnv.repos_path, 'renamed', 'child', 'the-project.git') + expected_upload_path = File.join(uploads_dir, 'renamed', 'child', 'the-project') + expected_pages_path = File.join(pages_dir, 'renamed', 'child', 'the-project') +======= + context 'renaming parent' do + it 'correctly moves the repository, uploads and pages' do + parent.update!(path: 'renamed') + + expect_project_directories_at('renamed/child') + end + end + + context 'moving from one parent to another' do + it 'correctly moves the repository, uploads and pages' do + child.update!(parent: new_parent) +>>>>>>> 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' +<<<<<<< HEAD child.update_attributes!(path: 'renamed') +||||||| parent of 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' + parent.update_attributes!(path: 'renamed') +======= + expect_project_directories_at('new_parent/child') + end + end + + context 'moving from having a parent to root' do + it 'correctly moves the repository, uploads and pages' do + child.update!(parent: nil) + expect_project_directories_at('child') + end + end + + context 'moving from root to having a parent' do + it 'correctly moves the repository, uploads and pages' do + parent.update!(parent: new_parent) +>>>>>>> 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' + +<<<<<<< HEAD expect(File.directory?(expected_repository_path)).to be(true) expect(File.directory?(expected_upload_path)).to be(true) expect(File.directory?(expected_pages_path)).to be(true) +||||||| parent of 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' + expect(File.directory?(expected_repository_path)).to be(true) + expect(File.directory?(expected_upload_path)).to be(true) + expect(File.directory?(expected_pages_path)).to be(true) + end +======= + expect_project_directories_at('new_parent/parent/child') + end +>>>>>>> 3fab8dac18... Merge branch 'mk/fix-move-upload-files-on-group-transfer' into 'master' end end @@ -481,7 +593,6 @@ describe Namespace do end end - # Note: Group transfers are not yet implemented context 'when a group is transferred into a root group' do context 'when the root group "Share with group lock" is enabled' do let(:root_group) { create(:group, share_with_group_lock: true) } |