From ec4423665cacfe2f0675fb8b9b5bd8dd17a77dd7 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Fri, 13 Apr 2018 12:57:19 +0200 Subject: Gitlab::Shell works on shard name, not path Direct disk access is done through Gitaly now, so the legacy path was deprecated. This path was used in Gitlab::Shell however. This required the refactoring in this commit. Added is the removal of direct path access on the project model, as that lookup wasn't needed anymore is most cases. Closes https://gitlab.com/gitlab-org/gitaly/issues/1111 --- app/helpers/projects_helper.rb | 3 ++- app/models/concerns/storage/legacy_namespace.rb | 30 +++++++++++----------- app/models/project.rb | 10 +++----- app/models/project_wiki.rb | 2 +- app/models/repository.rb | 15 ++++++----- app/models/storage/hashed_project.rb | 4 +-- app/models/storage/legacy_project.rb | 8 +++--- app/services/projects/destroy_service.rb | 6 ++--- .../hashed_storage/migrate_repository_service.rb | 6 ++--- app/services/projects/transfer_service.rb | 2 +- app/workers/repository_fork_worker.rb | 4 ++- 11 files changed, 45 insertions(+), 45 deletions(-) (limited to 'app') diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index a64b2acdd77..801e624e1de 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -400,7 +400,8 @@ module ProjectsHelper exports_path = File.join(Settings.shared['path'], 'tmp/project_exports') filtered_message = message.strip.gsub(exports_path, "[REPO EXPORT PATH]") - filtered_message.gsub(project.repository_storage_path.chomp('/'), "[REPOS PATH]") + disk_path = Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path + filtered_message.gsub(disk_path.chomp('/'), "[REPOS PATH]") end def project_child_container_class(view_path) diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index f05e606995d..f66bdd529f1 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -45,25 +45,25 @@ module Storage # Hooks - # Save the storage paths before the projects are destroyed to use them on after destroy + # Save the storages before the projects are destroyed to use them on after destroy def prepare_for_destroy - old_repository_storage_paths + old_repository_storages end private def move_repositories - # Move the namespace directory in all storage paths used by member projects - repository_storage_paths.each do |repository_storage_path| + # Move the namespace directory in all storages used by member projects + repository_storages.each do |repository_storage| # Ensure old directory exists before moving it - gitlab_shell.add_namespace(repository_storage_path, full_path_was) + gitlab_shell.add_namespace(repository_storage, 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 + gitlab_shell.add_namespace(repository_storage, parent.full_path) if parent - unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path) + unless gitlab_shell.mv_namespace(repository_storage, full_path_was, full_path) - Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}" + Rails.logger.error "Exception moving path #{repository_storage} 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 @@ -72,33 +72,33 @@ module Storage end end - def old_repository_storage_paths - @old_repository_storage_paths ||= repository_storage_paths + def old_repository_storages + @old_repository_storage_paths ||= repository_storages end - def repository_storage_paths + def repository_storages # We need to get the storage paths for all the projects, even the ones that are # pending delete. Unscoping also get rids of the default order, which causes # problems with SELECT DISTINCT. Project.unscoped do - all_projects.select('distinct(repository_storage)').to_a.map(&:repository_storage_path) + all_projects.select('distinct(repository_storage)').to_a.map(&:repository_storage) end end def rm_dir # Remove the namespace directory in all storages paths used by member projects - old_repository_storage_paths.each do |repository_storage_path| + old_repository_storages.each do |repository_storage| # Move namespace directory into trash. # We will remove it later async new_path = "#{full_path}+#{id}+deleted" - if gitlab_shell.mv_namespace(repository_storage_path, full_path, new_path) + if gitlab_shell.mv_namespace(repository_storage, full_path, new_path) Gitlab::AppLogger.info %Q(Namespace directory "#{full_path}" moved to "#{new_path}") # Remove namespace directroy async with delay so # GitLab has time to remove all projects first run_after_commit do - GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage_path, new_path) + GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage, new_path) end end end diff --git a/app/models/project.rb b/app/models/project.rb index cec1e705aa8..1eee8fa266a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -512,10 +512,6 @@ class Project < ActiveRecord::Base repository.empty? end - def repository_storage_path - Gitlab.config.repositories.storages[repository_storage]&.legacy_disk_path - end - def team @team ||= ProjectTeam.new(self) end @@ -1106,7 +1102,7 @@ class Project < ActiveRecord::Base # Check if repository already exists on disk def check_repository_path_availability return true if skip_disk_validation - return false unless repository_storage_path + return false unless repository_storage expires_full_path_cache # we need to clear cache to validate renames correctly @@ -1907,14 +1903,14 @@ class Project < ActiveRecord::Base def check_repository_absence! return if skip_disk_validation - if repository_storage_path.blank? || repository_with_same_path_already_exists? + if repository_storage.blank? || repository_with_same_path_already_exists? errors.add(:base, 'There is already a repository with that name on disk') throw :abort end end def repository_with_same_path_already_exists? - gitlab_shell.exists?(repository_storage_path, "#{disk_path}.git") + gitlab_shell.exists?(repository_storage, "#{disk_path}.git") end # set last_activity_at to the same as created_at diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index b7e38ceb502..f799a0b4227 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -21,7 +21,7 @@ class ProjectWiki end delegate :empty?, to: :pages - delegate :repository_storage_path, :hashed_storage?, to: :project + delegate :repository_storage, :hashed_storage?, to: :project def path @project.path + '.wiki' diff --git a/app/models/repository.rb b/app/models/repository.rb index 5bdaa7f0720..6831305fb93 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -84,9 +84,14 @@ class Repository # Return absolute path to repository def path_to_repo - @path_to_repo ||= File.expand_path( - File.join(repository_storage_path, disk_path + '.git') - ) + @path_to_repo ||= + begin + storage = Gitlab.config.repositories.storages[@project.repository_storage] + + File.expand_path( + File.join(storage.legacy_disk_path, disk_path + '.git') + ) + end end def inspect @@ -915,10 +920,6 @@ class Repository raw_repository.fetch_ref(source_repository.raw_repository, source_ref: source_ref, target_ref: target_ref) end - def repository_storage_path - @project.repository_storage_path - end - def rebase(user, merge_request) raw.rebase(user, merge_request.id, branch: merge_request.source_branch, branch_sha: merge_request.source_branch_sha, diff --git a/app/models/storage/hashed_project.rb b/app/models/storage/hashed_project.rb index fae1b64961a..26b4b78ac64 100644 --- a/app/models/storage/hashed_project.rb +++ b/app/models/storage/hashed_project.rb @@ -1,7 +1,7 @@ module Storage class HashedProject attr_accessor :project - delegate :gitlab_shell, :repository_storage_path, to: :project + delegate :gitlab_shell, :repository_storage, to: :project ROOT_PATH_PREFIX = '@hashed'.freeze @@ -24,7 +24,7 @@ module Storage end def ensure_storage_path_exists - gitlab_shell.add_namespace(repository_storage_path, base_dir) + gitlab_shell.add_namespace(repository_storage, base_dir) end def rename_repo diff --git a/app/models/storage/legacy_project.rb b/app/models/storage/legacy_project.rb index 9d9e5e1d352..27cb388c702 100644 --- a/app/models/storage/legacy_project.rb +++ b/app/models/storage/legacy_project.rb @@ -1,7 +1,7 @@ module Storage class LegacyProject attr_accessor :project - delegate :namespace, :gitlab_shell, :repository_storage_path, to: :project + delegate :namespace, :gitlab_shell, :repository_storage, to: :project def initialize(project) @project = project @@ -24,18 +24,18 @@ module Storage def ensure_storage_path_exists return unless namespace - gitlab_shell.add_namespace(repository_storage_path, base_dir) + gitlab_shell.add_namespace(repository_storage, base_dir) end def rename_repo new_full_path = project.build_full_path - if gitlab_shell.mv_repository(repository_storage_path, project.full_path_was, new_full_path) + if gitlab_shell.mv_repository(repository_storage, project.full_path_was, new_full_path) # If repository moved successfully we need to send update instructions to users. # However we cannot allow rollback since we moved repository # So we basically we mute exceptions in next actions begin - gitlab_shell.mv_repository(repository_storage_path, "#{project.full_path_was}.wiki", "#{new_full_path}.wiki") + gitlab_shell.mv_repository(repository_storage, "#{project.full_path_was}.wiki", "#{new_full_path}.wiki") return true rescue => e Rails.logger.error "Exception renaming #{project.full_path_was} -> #{new_full_path}: #{e}" diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 44e869851ca..71c93660b4b 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -91,7 +91,7 @@ module Projects project.run_after_commit do # self is now project - GitlabShellWorker.perform_in(5.minutes, :remove_repository, self.repository_storage_path, new_path) + GitlabShellWorker.perform_in(5.minutes, :remove_repository, self.repository_storage, new_path) end else false @@ -100,9 +100,9 @@ module Projects def mv_repository(from_path, to_path) # There is a possibility project does not have repository or wiki - return true unless gitlab_shell.exists?(project.repository_storage_path, from_path + '.git') + return true unless gitlab_shell.exists?(project.repository_storage, from_path + '.git') - gitlab_shell.mv_repository(project.repository_storage_path, from_path, to_path) + gitlab_shell.mv_repository(project.repository_storage, from_path, to_path) end def attempt_rollback(project, message) diff --git a/app/services/projects/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb index 67178de75de..68c1af2396b 100644 --- a/app/services/projects/hashed_storage/migrate_repository_service.rb +++ b/app/services/projects/hashed_storage/migrate_repository_service.rb @@ -47,8 +47,8 @@ module Projects private def move_repository(from_name, to_name) - from_exists = gitlab_shell.exists?(project.repository_storage_path, "#{from_name}.git") - to_exists = gitlab_shell.exists?(project.repository_storage_path, "#{to_name}.git") + from_exists = gitlab_shell.exists?(project.repository_storage, "#{from_name}.git") + to_exists = gitlab_shell.exists?(project.repository_storage, "#{to_name}.git") # If we don't find the repository on either original or target we should log that as it could be an issue if the # project was not originally empty. @@ -60,7 +60,7 @@ module Projects return true end - gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name) + gitlab_shell.mv_repository(project.repository_storage, from_name, to_name) end def rollback_folder_move diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 5a23f0f0a62..61acdd58021 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -127,7 +127,7 @@ module Projects end def move_repo_folder(from_name, to_name) - gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name) + gitlab_shell.mv_repository(project.repository_storage, from_name, to_name) end def execute_system_hooks diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 51fad4faf36..08b1c3a7d7a 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -13,7 +13,9 @@ class RepositoryForkWorker # See https://gitlab.com/gitlab-org/gitaly/issues/1110 if args.empty? source_project = target_project.forked_from_project - return target_project.mark_import_as_failed('Source project cannot be found.') unless source_project + unless source_project + return target_project.mark_import_as_failed('Source project cannot be found.') + end fork_repository(target_project, source_project.repository_storage, source_project.disk_path) else -- cgit v1.2.3