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 +- ...161220141214_remove_dot_git_from_group_names.rb | 12 ++--- ...20161226122833_remove_dot_git_from_usernames.rb | 22 ++++----- lib/gitlab/bare_repository_import/importer.rb | 9 +++- .../v1/rename_namespaces.rb | 13 +++--- .../v1/rename_projects.rb | 2 +- lib/gitlab/shell.rb | 28 +++++------- lib/tasks/gitlab/check.rake | 5 +-- lib/tasks/gitlab/list_repos.rake | 5 +-- spec/controllers/profiles_controller_spec.rb | 4 +- spec/factories/projects.rb | 6 ++- spec/helpers/projects_helper_spec.rb | 6 +-- .../gitlab/bare_repository_import/importer_spec.rb | 21 +++++---- .../bare_repository_import/repository_spec.rb | 2 +- spec/lib/gitlab/git/repository_spec.rb | 2 +- .../lib/gitlab/import_export/wiki_restorer_spec.rb | 4 +- spec/lib/gitlab/shell_spec.rb | 52 +++++++++++----------- spec/models/namespace_spec.rb | 9 ++-- spec/models/project_spec.rb | 20 +++------ spec/models/project_wiki_spec.rb | 2 +- spec/services/groups/destroy_service_spec.rb | 16 +++---- spec/services/projects/create_service_spec.rb | 5 +-- spec/services/projects/destroy_service_spec.rb | 16 +++---- spec/services/projects/fork_service_spec.rb | 2 +- .../migrate_repository_service_spec.rb | 14 +++--- spec/services/projects/transfer_service_spec.rb | 6 +-- spec/services/projects/update_service_spec.rb | 2 +- spec/services/users/destroy_service_spec.rb | 4 +- spec/spec_helper.rb | 4 +- .../support/helpers/javascript_fixtures_helpers.rb | 2 +- spec/support/helpers/test_env.rb | 5 ++- spec/tasks/gitlab/backup_rake_spec.rb | 21 +++++---- 42 files changed, 205 insertions(+), 206 deletions(-) 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 diff --git a/db/migrate/20161220141214_remove_dot_git_from_group_names.rb b/db/migrate/20161220141214_remove_dot_git_from_group_names.rb index bddc234db25..17357b67ab7 100644 --- a/db/migrate/20161220141214_remove_dot_git_from_group_names.rb +++ b/db/migrate/20161220141214_remove_dot_git_from_group_names.rb @@ -59,17 +59,17 @@ class RemoveDotGitFromGroupNames < ActiveRecord::Migration end def move_namespace(group_id, path_was, path) - repository_storage_paths = select_all("SELECT distinct(repository_storage) FROM projects WHERE namespace_id = #{group_id}").map do |row| - Gitlab.config.repositories.storages[row['repository_storage']].legacy_disk_path + repository_storages = select_all("SELECT distinct(repository_storage) FROM projects WHERE namespace_id = #{group_id}").map do |row| + row['repository_storage'] end.compact # Move the namespace directory in all storages paths used by member projects - repository_storage_paths.each do |repository_storage_path| + repository_storages.each do |repository_storage| # Ensure old directory exists before moving it - gitlab_shell.add_namespace(repository_storage_path, path_was) + gitlab_shell.add_namespace(repository_storage, path_was) - unless gitlab_shell.mv_namespace(repository_storage_path, path_was, path) - Rails.logger.error "Exception moving path #{repository_storage_path} from #{path_was} to #{path}" + unless gitlab_shell.mv_namespace(repository_storage, path_was, path) + Rails.logger.error "Exception moving on shard #{repository_storage} from #{path_was} to #{path}" # if we cannot move namespace directory we should rollback # db changes in order to prevent out of sync between db and fs diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 7c28d934c29..8986cd8cb4b 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -53,8 +53,8 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration select_all("SELECT id, path FROM routes WHERE path = '#{quote_string(path)}'").present? end - def path_exists?(path, repository_storage_path) - repository_storage_path && gitlab_shell.exists?(repository_storage_path, path) + def path_exists?(shard, repository_storage_path) + repository_storage_path && gitlab_shell.exists?(shard, repository_storage_path) end # Accepts invalid path like test.git and returns test_git or @@ -70,8 +70,8 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration def check_routes(base, counter, path) route_exists = route_exists?(path) - Gitlab.config.repositories.storages.each_value do |storage| - if route_exists || path_exists?(path, storage.legacy_disk_path) + Gitlab.config.repositories.storages.each do |shard, storage| + if route_exists || path_exists?(shard, storage.legacy_disk_path) counter += 1 path = "#{base}#{counter}" @@ -83,17 +83,17 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration end def move_namespace(namespace_id, path_was, path) - repository_storage_paths = select_all("SELECT distinct(repository_storage) FROM projects WHERE namespace_id = #{namespace_id}").map do |row| - Gitlab.config.repositories.storages[row['repository_storage']].legacy_disk_path + repository_storages = select_all("SELECT distinct(repository_storage) FROM projects WHERE namespace_id = #{namespace_id}").map do |row| + row['repository_storage'] end.compact - # Move the namespace directory in all storages 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, path_was) + gitlab_shell.add_namespace(repository_storage, path_was) - unless gitlab_shell.mv_namespace(repository_storage_path, path_was, path) - Rails.logger.error "Exception moving path #{repository_storage_path} from #{path_was} to #{path}" + unless gitlab_shell.mv_namespace(repository_storage, path_was, path) + Rails.logger.error "Exception moving on shard #{repository_storage} from #{path_was} to #{path}" # if we cannot move namespace directory we should rollback # db changes in order to prevent out of sync between db and fs diff --git a/lib/gitlab/bare_repository_import/importer.rb b/lib/gitlab/bare_repository_import/importer.rb index 1a25138e7d6..4ca5a78e068 100644 --- a/lib/gitlab/bare_repository_import/importer.rb +++ b/lib/gitlab/bare_repository_import/importer.rb @@ -75,10 +75,11 @@ module Gitlab end def mv_repo(project) - FileUtils.mv(repo_path, File.join(project.repository_storage_path, project.disk_path + '.git')) + storage_path = storage_path_for_shard(project.repository_storage) + FileUtils.mv(repo_path, project.repository.path_to_repo) if bare_repo.wiki_exists? - FileUtils.mv(wiki_path, File.join(project.repository_storage_path, project.disk_path + '.wiki.git')) + FileUtils.mv(wiki_path, File.join(storage_path, project.disk_path + '.wiki.git')) end true @@ -88,6 +89,10 @@ module Gitlab false end + def storage_path_for_shard(shard) + Gitlab.config.repositories.storages[shard].legacy_disk_path + end + def find_or_create_groups return nil unless group_path.present? diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb index 05b86f32ce2..73971af6a74 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb @@ -62,21 +62,20 @@ module Gitlab end def move_repositories(namespace, old_full_path, new_full_path) - repo_paths_for_namespace(namespace).each do |repository_storage_path| + repo_shards_for_namespace(namespace).each do |repository_storage| # Ensure old directory exists before moving it - gitlab_shell.add_namespace(repository_storage_path, old_full_path) + gitlab_shell.add_namespace(repository_storage, old_full_path) - unless gitlab_shell.mv_namespace(repository_storage_path, old_full_path, new_full_path) - message = "Exception moving path #{repository_storage_path} \ - from #{old_full_path} to #{new_full_path}" + unless gitlab_shell.mv_namespace(repository_storage, old_full_path, new_full_path) + message = "Exception moving on shard #{repository_storage} from #{old_full_path} to #{new_full_path}" Rails.logger.error message end end end - def repo_paths_for_namespace(namespace) + def repo_shards_for_namespace(namespace) projects_for_namespace(namespace).distinct.select(:repository_storage) - .map(&:repository_storage_path) + .map(&:repository_storage) end def projects_for_namespace(namespace) diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb index 979225dd216..827aeb12a02 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb @@ -51,7 +51,7 @@ module Gitlab end def move_repository(project, old_path, new_path) - unless gitlab_shell.mv_repository(project.repository_storage_path, + unless gitlab_shell.mv_repository(project.repository_storage, old_path, new_path) Rails.logger.error "Error moving #{old_path} to #{new_path}" diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index ac4ac537a8a..156115f8a8f 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -65,11 +65,11 @@ module Gitlab # Init new repository # - # storage - project's storage name + # storage - the shard key # name - project disk path # # Ex. - # create_repository("/path/to/storage", "gitlab/gitlab-ci") + # create_repository("default", "gitlab/gitlab-ci") # def create_repository(storage, name) relative_path = name.dup @@ -291,13 +291,13 @@ module Gitlab # Add empty directory for storing repositories # # Ex. - # add_namespace("/path/to/storage", "gitlab") + # add_namespace("default", "gitlab") # def add_namespace(storage, name) Gitlab::GitalyClient.migrate(:add_namespace, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |enabled| if enabled - gitaly_namespace_client(storage).add(name) + Gitlab::GitalyClient::NamespaceService.new(storage).add(name) else path = full_path(storage, name) FileUtils.mkdir_p(path, mode: 0770) unless exists?(storage, name) @@ -313,13 +313,13 @@ module Gitlab # Every repository inside this directory will be removed too # # Ex. - # rm_namespace("/path/to/storage", "gitlab") + # rm_namespace("default", "gitlab") # def rm_namespace(storage, name) Gitlab::GitalyClient.migrate(:remove_namespace, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |enabled| if enabled - gitaly_namespace_client(storage).remove(name) + Gitlab::GitalyClient::NamespaceService.new(storage).remove(name) else FileUtils.rm_r(full_path(storage, name), force: true) end @@ -338,7 +338,8 @@ module Gitlab Gitlab::GitalyClient.migrate(:rename_namespace, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |enabled| if enabled - gitaly_namespace_client(storage).rename(old_name, new_name) + Gitlab::GitalyClient::NamespaceService.new(storage) + .rename(old_name, new_name) else break false if exists?(storage, new_name) || !exists?(storage, old_name) @@ -374,7 +375,8 @@ module Gitlab Gitlab::GitalyClient.migrate(:namespace_exists, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |enabled| if enabled - gitaly_namespace_client(storage).exists?(dir_name) + Gitlab::GitalyClient::NamespaceService.new(storage) + .exists?(dir_name) else File.exist?(full_path(storage, dir_name)) end @@ -398,7 +400,7 @@ module Gitlab def full_path(storage, dir_name) raise ArgumentError.new("Directory name can't be blank") if dir_name.blank? - File.join(storage, dir_name) + File.join(Gitlab.config.repositories.storages[storage].legacy_disk_path, dir_name) end def gitlab_shell_projects_path @@ -475,14 +477,6 @@ module Gitlab Bundler.with_original_env { Popen.popen(cmd, nil, vars) } end - def gitaly_namespace_client(storage_path) - storage, _value = Gitlab.config.repositories.storages.find do |storage, value| - value.legacy_disk_path == storage_path - end - - Gitlab::GitalyClient::NamespaceService.new(storage) - end - def git_timeout Gitlab.config.gitlab_shell.git_timeout end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index abef8cd2bcc..c04dae7446f 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -427,10 +427,7 @@ namespace :gitlab do user = User.find_by(username: username) if user repo_dirs = user.authorized_projects.map do |p| - File.join( - p.repository_storage_path, - "#{p.disk_path}.git" - ) + p.repository.path_to_repo end repo_dirs.each { |repo_dir| check_repo_integrity(repo_dir) } diff --git a/lib/tasks/gitlab/list_repos.rake b/lib/tasks/gitlab/list_repos.rake index d7f28691098..b854c34a8e5 100644 --- a/lib/tasks/gitlab/list_repos.rake +++ b/lib/tasks/gitlab/list_repos.rake @@ -10,9 +10,8 @@ namespace :gitlab do end scope.find_each do |project| - base = File.join(project.repository_storage_path, project.disk_path) - puts base + '.git' - puts base + '.wiki.git' + puts project.repository.path_to_repo + puts project.wiki.repository.path_to_repo end end end diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index de6ef919221..c621eb69171 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -125,7 +125,7 @@ describe ProfilesController, :request_store do user.reload expect(response.status).to eq(302) - expect(gitlab_shell.exists?(project.repository_storage_path, "#{new_username}/#{project.path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{new_username}/#{project.path}.git")).to be_truthy end end @@ -143,7 +143,7 @@ describe ProfilesController, :request_store do user.reload expect(response.status).to eq(302) - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy expect(before_disk_path).to eq(project.disk_path) end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 1ae6152a1f0..fe0266991cd 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -147,7 +147,8 @@ FactoryBot.define do # We delete hooks so that gitlab-shell will not try to authenticate with # an API that isn't running - FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.disk_path}.git", 'hooks')) + project.gitlab_shell.rm_directory(project.repository_storage, + File.join("#{project.disk_path}.git", 'hooks')) end end @@ -165,7 +166,8 @@ FactoryBot.define do after(:create) do |project| raise "Failed to create repository!" unless project.create_repository - FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.disk_path}.git", 'refs')) + project.gitlab_shell.rm_directory(project.repository_storage, + File.join("#{project.disk_path}.git", 'refs')) end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 46c55da24f8..8fcb175416f 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -274,16 +274,16 @@ describe ProjectsHelper do end end - describe '#sanitized_import_error' do + describe '#sanitizerepo_repo_path' do let(:project) { create(:project, :repository) } + let(:storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path } before do - allow(project).to receive(:repository_storage_path).and_return('/base/repo/path') allow(Settings.shared).to receive(:[]).with('path').and_return('/base/repo/export/path') end it 'removes the repo path' do - repo = '/base/repo/path/namespace/test.git' + repo = "#{storage_path}/namespace/test.git" import_error = "Could not clone #{repo}\n" expect(sanitize_repo_path(project, import_error)).to eq('Could not clone [REPOS PATH]/namespace/test.git') diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index eb4b9d8b12f..5c8a19a53bc 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -4,6 +4,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do let!(:admin) { create(:admin) } let!(:base_dir) { Dir.mktmpdir + '/' } let(:bare_repository) { Gitlab::BareRepositoryImport::Repository.new(base_dir, File.join(base_dir, "#{project_path}.git")) } + let(:gitlab_shell) { Gitlab::Shell.new } subject(:importer) { described_class.new(admin, bare_repository) } @@ -84,12 +85,14 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do importer.create_project_if_needed project = Project.find_by_full_path(project_path) - repo_path = File.join(project.repository_storage_path, project.disk_path + '.git') + repo_path = "#{project.disk_path}.git" hook_path = File.join(repo_path, 'hooks') - expect(File).to exist(repo_path) - expect(File.symlink?(hook_path)).to be true - expect(File.readlink(hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) + expect(gitlab_shell.exists?(project.repository_storage, repo_path)).to be(true) + expect(gitlab_shell.exists?(project.repository_storage, hook_path)).to be(true) + + full_hook_path = File.join(project.repository.path_to_repo, 'hooks') + expect(File.readlink(full_hook_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) end context 'hashed storage enabled' do @@ -144,8 +147,8 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do project = Project.find_by_full_path("#{admin.full_path}/#{project_path}") - expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.git')) - expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.wiki.git')) + expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.git')).to be(true) + expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true) end it 'moves an existing project to the correct path' do @@ -155,7 +158,9 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do project = build(:project, :legacy_storage, :repository) original_commit_count = project.repository.commit_count - bare_repo = Gitlab::BareRepositoryImport::Repository.new(project.repository_storage_path, project.repository.path) + legacy_path = Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path + + bare_repo = Gitlab::BareRepositoryImport::Repository.new(legacy_path, project.repository.path) gitlab_importer = described_class.new(admin, bare_repo) expect(gitlab_importer).to receive(:create_project).and_call_original @@ -183,7 +188,7 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do project = Project.find_by_full_path(project_path) - expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.wiki.git')) + expect(gitlab_shell.exists?(project.repository_storage, project.disk_path + '.wiki.git')).to be(true) end end diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 0dc3705825d..1504826c7a5 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -67,7 +67,7 @@ describe ::Gitlab::BareRepositoryImport::Repository do end after do - gitlab_shell.remove_repository(root_path, hashed_path) + gitlab_shell.remove_repository(repository_storage, hashed_path) end subject { described_class.new(root_path, repo_path) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 5acf40ea5ce..da1a6229ccf 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -689,7 +689,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end after do - Gitlab::Shell.new.remove_repository(storage_path, 'my_project') + Gitlab::Shell.new.remove_repository('default', 'my_project') end shared_examples 'repository mirror fecthing' do diff --git a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb index 5c01ee0ebb8..f99f198da33 100644 --- a/spec/lib/gitlab/import_export/wiki_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/wiki_restorer_spec.rb @@ -24,8 +24,8 @@ describe Gitlab::ImportExport::WikiRestorer do after do FileUtils.rm_rf(export_path) - Gitlab::Shell.new.remove_repository(project_with_wiki.wiki.repository_storage_path, project_with_wiki.wiki.disk_path) - Gitlab::Shell.new.remove_repository(project.wiki.repository_storage_path, project.wiki.disk_path) + Gitlab::Shell.new.remove_repository(project_with_wiki.wiki.repository_storage, project_with_wiki.wiki.disk_path) + Gitlab::Shell.new.remove_repository(project.wiki.repository_storage, project.wiki.disk_path) end it 'restores the wiki repo successfully' do diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index 7f579df1c36..bf6ee4b0b59 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -447,18 +447,18 @@ describe Gitlab::Shell do let(:disk_path) { "#{project.disk_path}.git" } it 'returns true when the command succeeds' do - expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(true) + expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(true) - expect(gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path)).to be(true) + expect(gitlab_shell.remove_repository(project.repository_storage, project.disk_path)).to be(true) - expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(false) + expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(false) end it 'keeps the namespace directory' do - gitlab_shell.remove_repository(project.repository_storage_path, project.disk_path) + gitlab_shell.remove_repository(project.repository_storage, project.disk_path) - expect(gitlab_shell.exists?(project.repository_storage_path, disk_path)).to be(false) - expect(gitlab_shell.exists?(project.repository_storage_path, project.disk_path.gsub(project.name, ''))).to be(true) + expect(gitlab_shell.exists?(project.repository_storage, disk_path)).to be(false) + expect(gitlab_shell.exists?(project.repository_storage, project.disk_path.gsub(project.name, ''))).to be(true) end end @@ -469,18 +469,18 @@ describe Gitlab::Shell do old_path = project2.disk_path new_path = "project/new_path" - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{old_path}.git")).to be(true) - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{new_path}.git")).to be(false) + expect(gitlab_shell.exists?(project2.repository_storage, "#{old_path}.git")).to be(true) + expect(gitlab_shell.exists?(project2.repository_storage, "#{new_path}.git")).to be(false) - expect(gitlab_shell.mv_repository(project2.repository_storage_path, old_path, new_path)).to be_truthy + expect(gitlab_shell.mv_repository(project2.repository_storage, old_path, new_path)).to be_truthy - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{old_path}.git")).to be(false) - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{new_path}.git")).to be(true) + expect(gitlab_shell.exists?(project2.repository_storage, "#{old_path}.git")).to be(false) + expect(gitlab_shell.exists?(project2.repository_storage, "#{new_path}.git")).to be(true) end it 'returns false when the command fails' do - expect(gitlab_shell.mv_repository(project2.repository_storage_path, project2.disk_path, '')).to be_falsy - expect(gitlab_shell.exists?(project2.repository_storage_path, "#{project2.disk_path}.git")).to be(true) + expect(gitlab_shell.mv_repository(project2.repository_storage, project2.disk_path, '')).to be_falsy + expect(gitlab_shell.exists?(project2.repository_storage, "#{project2.disk_path}.git")).to be(true) end end @@ -679,48 +679,48 @@ describe Gitlab::Shell do describe 'namespace actions' do subject { described_class.new } - let(:storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path } + let(:storage) { Gitlab.config.repositories.storages.keys.first } describe '#add_namespace' do it 'creates a namespace' do - subject.add_namespace(storage_path, "mepmep") + subject.add_namespace(storage, "mepmep") - expect(subject.exists?(storage_path, "mepmep")).to be(true) + expect(subject.exists?(storage, "mepmep")).to be(true) end end describe '#exists?' do context 'when the namespace does not exist' do it 'returns false' do - expect(subject.exists?(storage_path, "non-existing")).to be(false) + expect(subject.exists?(storage, "non-existing")).to be(false) end end context 'when the namespace exists' do it 'returns true' do - subject.add_namespace(storage_path, "mepmep") + subject.add_namespace(storage, "mepmep") - expect(subject.exists?(storage_path, "mepmep")).to be(true) + expect(subject.exists?(storage, "mepmep")).to be(true) end end end describe '#remove' do it 'removes the namespace' do - subject.add_namespace(storage_path, "mepmep") - subject.rm_namespace(storage_path, "mepmep") + subject.add_namespace(storage, "mepmep") + subject.rm_namespace(storage, "mepmep") - expect(subject.exists?(storage_path, "mepmep")).to be(false) + expect(subject.exists?(storage, "mepmep")).to be(false) end end describe '#mv_namespace' do it 'renames the namespace' do - subject.add_namespace(storage_path, "mepmep") - subject.mv_namespace(storage_path, "mepmep", "2mep") + subject.add_namespace(storage, "mepmep") + subject.mv_namespace(storage, "mepmep", "2mep") - expect(subject.exists?(storage_path, "mepmep")).to be(false) - expect(subject.exists?(storage_path, "2mep")).to be(true) + expect(subject.exists?(storage, "mepmep")).to be(false) + expect(subject.exists?(storage, "2mep")).to be(true) end end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 62e95a622eb..506057dce87 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -5,6 +5,7 @@ describe Namespace do let!(:namespace) { create(:namespace) } let(:gitlab_shell) { Gitlab::Shell.new } + let(:repository_storage) { 'default' } describe 'associations' do it { is_expected.to have_many :projects } @@ -201,7 +202,7 @@ describe Namespace do it "moves dir if path changed" do namespace.update_attributes(path: namespace.full_path + '_new') - expect(gitlab_shell.exists?(project.repository_storage_path, "#{namespace.path}/#{project.path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{namespace.path}/#{project.path}.git")).to be_truthy end context 'with subgroups', :nested_groups do @@ -281,7 +282,7 @@ describe Namespace do namespace.update_attributes(path: namespace.full_path + '_new') expect(before_disk_path).to eq(project.disk_path) - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_truthy end end @@ -322,7 +323,7 @@ describe Namespace do end it 'schedules the namespace for deletion' do - expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage_path, deleted_path) + expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path) namespace.destroy end @@ -344,7 +345,7 @@ describe Namespace do end it 'schedules the namespace for deletion' do - expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage_path, deleted_path) + expect(GitlabShellWorker).to receive(:perform_in).with(5.minutes, :rm_namespace, repository_storage, deleted_path) child.destroy end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 4002722e358..5bef164f458 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -440,14 +440,6 @@ describe Project do end end - describe '#repository_storage_path' do - let(:project) { create(:project) } - - it 'returns the repository storage path' do - expect(Dir.exist?(project.repository_storage_path)).to be(true) - end - end - it 'returns valid url to repo' do project = described_class.new(path: 'somewhere') expect(project.url_to_repo).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + 'somewhere.git') @@ -1099,7 +1091,7 @@ describe Project do end context 'repository storage by default' do - let(:project) { create(:project) } + let(:project) { build(:project) } before do storages = { @@ -1452,7 +1444,7 @@ describe Project do .and_return(false) allow(shell).to receive(:create_repository) - .with(project.repository_storage_path, project.disk_path) + .with(project.repository_storage, project.disk_path) .and_return(true) expect(project).to receive(:create_repository).with(force: true) @@ -2673,7 +2665,7 @@ describe Project do describe '#ensure_storage_path_exists' do it 'delegates to gitlab_shell to ensure namespace is created' do - expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, project.base_dir) + expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage, project.base_dir) project.ensure_storage_path_exists end @@ -2712,12 +2704,12 @@ describe Project do expect(gitlab_shell).to receive(:mv_repository) .ordered - .with(project.repository_storage_path, "#{project.namespace.full_path}/foo", "#{project.full_path}") + .with(project.repository_storage, "#{project.namespace.full_path}/foo", "#{project.full_path}") .and_return(true) expect(gitlab_shell).to receive(:mv_repository) .ordered - .with(project.repository_storage_path, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") + .with(project.repository_storage, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") .and_return(true) expect_any_instance_of(SystemHooksService) @@ -2866,7 +2858,7 @@ describe Project do it 'delegates to gitlab_shell to ensure namespace is created' do allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, hashed_prefix) + expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage, hashed_prefix) project.ensure_storage_path_exists end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 4e83f4353cf..cbe7d111fcd 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -11,7 +11,7 @@ describe ProjectWiki do subject { project_wiki } it { is_expected.to delegate_method(:empty?).to :pages } - it { is_expected.to delegate_method(:repository_storage_path).to :project } + it { is_expected.to delegate_method(:repository_storage).to :project } it { is_expected.to delegate_method(:hashed_storage?).to :project } describe "#full_path" do diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index e8216abb08b..a9baccd061a 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -53,8 +53,8 @@ describe Groups::DestroyService do end it 'verifies that paths have been deleted' do - expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey - expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, group.path)).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey end end end @@ -71,13 +71,13 @@ describe Groups::DestroyService do after do # Clean up stale directories - gitlab_shell.rm_namespace(project.repository_storage_path, group.path) - gitlab_shell.rm_namespace(project.repository_storage_path, remove_path) + gitlab_shell.rm_namespace(project.repository_storage, group.path) + gitlab_shell.rm_namespace(project.repository_storage, remove_path) end it 'verifies original paths and projects still exist' do - expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_truthy - expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, group.path)).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey expect(Project.unscoped.count).to eq(1) expect(Group.unscoped.count).to eq(2) end @@ -144,7 +144,7 @@ describe Groups::DestroyService do let!(:project) { create(:project, :legacy_storage, :empty_repo, namespace: group) } it 'removes repository' do - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey end end @@ -152,7 +152,7 @@ describe Groups::DestroyService do let!(:project) { create(:project, :empty_repo, namespace: group) } it 'removes repository' do - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey end end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index e35f0f6337a..a8f003b1073 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -171,7 +171,6 @@ describe Projects::CreateService, '#execute' do context 'when another repository already exists on disk' do let(:repository_storage) { 'default' } - let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path } let(:opts) do { @@ -186,7 +185,7 @@ describe Projects::CreateService, '#execute' do end after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + gitlab_shell.remove_repository(repository_storage, "#{user.namespace.full_path}/existing") end it 'does not allow to create a project when path matches existing repository on disk' do @@ -222,7 +221,7 @@ describe Projects::CreateService, '#execute' do end after do - gitlab_shell.remove_repository(repository_storage_path, hashed_path) + gitlab_shell.remove_repository(repository_storage, hashed_path) end it 'does not allow to create a project when path matches existing repository on disk' do diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index a66e3c5e995..b2c52214f48 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -18,8 +18,8 @@ describe Projects::DestroyService do it 'deletes the project' do expect(Project.unscoped.all).not_to include(project) - expect(project.gitlab_shell.exists?(project.repository_storage_path, path + '.git')).to be_falsey - expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path + '.git')).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, path + '.git')).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, remove_path + '.git')).to be_falsey end end @@ -252,21 +252,21 @@ describe Projects::DestroyService do let(:path) { project.disk_path + '.git' } before do - expect(project.gitlab_shell.exists?(project.repository_storage_path, path)).to be_truthy - expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_truthy + expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey # Dont run sidekiq to check if renamed repository exists Sidekiq::Testing.fake! { destroy_project(project, user, {}) } - expect(project.gitlab_shell.exists?(project.repository_storage_path, path)).to be_falsey - expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_truthy + expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_truthy end it 'restores the repositories' do Sidekiq::Testing.fake! { described_class.new(project, user).attempt_repositories_rollback } - expect(project.gitlab_shell.exists?(project.repository_storage_path, path)).to be_truthy - expect(project.gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey + expect(project.gitlab_shell.exists?(project.repository_storage, path)).to be_truthy + expect(project.gitlab_shell.exists?(project.repository_storage, remove_path)).to be_falsey end end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 0f7c46367d0..a93f6f1ddc2 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -112,7 +112,7 @@ describe Projects::ForkService do end after do - gitlab_shell.remove_repository(repository_storage_path, "#{@to_user.namespace.full_path}/#{@from_project.path}") + gitlab_shell.remove_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") end it 'does not allow creation' do diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 747bd4529a0..7dca81eb59e 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -16,8 +16,8 @@ describe Projects::HashedStorage::MigrateRepositoryService do it 'renames project and wiki repositories' do service.execute - expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_truthy - expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{hashed_storage.disk_path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy end it 'updates project to be hashed and not read-only' do @@ -52,8 +52,8 @@ describe Projects::HashedStorage::MigrateRepositoryService do service.execute - expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_falsey - expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{hashed_storage.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey expect(project.repository_read_only?).to be_falsey end @@ -63,11 +63,11 @@ describe Projects::HashedStorage::MigrateRepositoryService do before do hashed_storage.ensure_storage_path_exists - gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name) + gitlab_shell.mv_repository(project.repository_storage, from_name, to_name) end it 'does not try to move nil repository over hashed' do - expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name) + expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage, from_name, to_name) expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki") service.execute @@ -76,7 +76,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do end def expect_move_repository(from_name, to_name) - expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name).and_call_original + expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage, from_name, to_name).and_call_original end end end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index ff9b2372a35..3e6483d7e28 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -84,7 +84,7 @@ describe Projects::TransferService do end def project_path(project) - File.join(project.repository_storage_path, "#{project.disk_path}.git") + project.repository.path_to_repo end def current_path @@ -94,7 +94,7 @@ describe Projects::TransferService do it 'rolls back repo location' do attempt_project_transfer - expect(Dir.exist?(original_path)).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be(true) expect(original_path).to eq current_path end @@ -165,7 +165,7 @@ describe Projects::TransferService do end after do - gitlab_shell.remove_repository(repository_storage_path, "#{group.full_path}/#{project.path}") + gitlab_shell.remove_repository(repository_storage, "#{group.full_path}/#{project.path}") end it { expect(@result).to eq false } diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index f48d466d263..3e6073b9861 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -200,7 +200,7 @@ describe Projects::UpdateService do end after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + gitlab_shell.remove_repository(repository_storage, "#{user.namespace.full_path}/existing") end it 'does not allow renaming when new path matches existing repository on disk' do diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 11c75ddfcf8..76f1e625fda 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -176,7 +176,7 @@ describe Users::DestroyService do let!(:project) { create(:project, :empty_repo, :legacy_storage, namespace: user.namespace) } it 'removes repository' do - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey end end @@ -184,7 +184,7 @@ describe Users::DestroyService do let!(:project) { create(:project, :empty_repo, namespace: user.namespace) } it 'removes repository' do - expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 53045815a6a..cc61cd7d838 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -113,10 +113,10 @@ RSpec.configure do |config| m.call(*args) shard_name, repository_relative_path = args - shard_path = Gitlab.config.repositories.storages.fetch(shard_name).legacy_disk_path # We can't leave the hooks in place after a fork, as those would fail in tests # The "internal" API is not available - FileUtils.rm_rf(File.join(shard_path, repository_relative_path, 'hooks')) + Gitlab::Shell.new.rm_directory(shard_name, + File.join(repository_relative_path, 'hooks')) end # Enable all features by default for testing diff --git a/spec/support/helpers/javascript_fixtures_helpers.rb b/spec/support/helpers/javascript_fixtures_helpers.rb index 2197bc9d853..086a345dca8 100644 --- a/spec/support/helpers/javascript_fixtures_helpers.rb +++ b/spec/support/helpers/javascript_fixtures_helpers.rb @@ -31,7 +31,7 @@ module JavaScriptFixturesHelpers end def remove_repository(project) - Gitlab::Shell.new.remove_repository(project.repository_storage_path, project.disk_path) + Gitlab::Shell.new.remove_repository(project.repository_storage, project.disk_path) end private diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index d87f265cdf0..1dad39fdab3 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -218,7 +218,8 @@ module TestEnv end def copy_repo(project, bare_repo:, refs:) - target_repo_path = File.expand_path(project.repository_storage_path + "/#{project.disk_path}.git") + target_repo_path = File.expand_path(repos_path + "/#{project.disk_path}.git") + FileUtils.mkdir_p(target_repo_path) FileUtils.cp_r("#{File.expand_path(bare_repo)}/.", target_repo_path) FileUtils.chmod_R 0755, target_repo_path @@ -226,7 +227,7 @@ module TestEnv end def repos_path - Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path + @repos_path ||= Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path end def backup_path diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 0d24782f317..a2e5642a72c 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -195,15 +195,12 @@ describe 'gitlab:app namespace rake task' do end context 'multiple repository storages' do - let(:storage_default) do - Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/default_storage')) - end let(:test_second_storage) do Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/custom_storage')) end let(:storages) do { - 'default' => storage_default, + 'default' => Gitlab.config.repositories.storages.default, 'test_second_storage' => test_second_storage } end @@ -215,8 +212,7 @@ describe 'gitlab:app namespace rake task' do before do # We only need a backup of the repositories for this test stub_env('SKIP', 'db,uploads,builds,artifacts,lfs,registry') - FileUtils.mkdir(Settings.absolute('tmp/tests/default_storage')) - FileUtils.mkdir(Settings.absolute('tmp/tests/custom_storage')) + allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) # Avoid asking gitaly about the root ref (which will fail beacuse of the @@ -225,14 +221,23 @@ describe 'gitlab:app namespace rake task' do end after do - FileUtils.rm_rf(Settings.absolute('tmp/tests/default_storage')) FileUtils.rm_rf(Settings.absolute('tmp/tests/custom_storage')) end it 'includes repositories in all repository storages' do - project_a = create(:project, :repository, repository_storage: 'default') + project_a = create(:project, :repository) project_b = create(:project, :repository, repository_storage: 'test_second_storage') + b_storage_dir = File.join(Settings.absolute('tmp/tests/custom_storage'), File.dirname(project_b.disk_path)) + + FileUtils.mkdir_p(b_storage_dir) + + # Even when overriding the storage, we have to move it there, so it exists + FileUtils.mv( + File.join(Settings.absolute(storages['default'].legacy_disk_path), project_b.repository.disk_path + '.git'), + Rails.root.join(storages['test_second_storage'].legacy_disk_path, project_b.repository.disk_path + '.git') + ) + expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout tar_contents, exit_status = Gitlab::Popen.popen( -- cgit v1.2.3