Call RenameRepository instead of RenameNamespace

call RenameRepository RPC instead of RenameNamespace in gitlab_shell.
This is a step towards deprecating all RenameNamespace calls.

GITALY_SERVER_VERSION

-1.62.0
+1.63.0

Gemfile

@@ -428,7 +428,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 1.58.0'
+gem 'gitaly', '~> 1.63.0'
 
 gem 'grpc', '~> 1.19.0'
 

Gemfile.lock

@@ -332,7 +332,7 @@ GEM
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
     git (1.5.0)
-    gitaly (1.58.0)
+    gitaly (1.63.0)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab-labkit (0.5.2)
@@ -1126,7 +1126,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 1.58.0)
+  gitaly (~> 1.63.0)
   github-markup (~> 1.7.0)
   gitlab-labkit (~> 0.5)
   gitlab-markup (~> 1.7.0)

app/models/concerns/storage/legacy_namespace.rb

@@ -54,25 +54,19 @@ module Storage
     private
 
     def move_repositories
-      # 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, full_path_before_last_save)
+      all_projects.each do |project|
+        next unless project.legacy_storage?
 
-        # Ensure new directory exists before moving it (if there's a parent)
-        gitlab_shell.add_namespace(repository_storage, parent.full_path) if parent
-
-        unless gitlab_shell.mv_namespace(repository_storage, full_path_before_last_save, full_path)
-
-          Rails.logger.error "Exception moving path #{repository_storage} from #{full_path_before_last_save} to #{full_path}" # rubocop:disable Gitlab/RailsLogger
-
-          # if we cannot move namespace directory we should rollback
-          # db changes in order to prevent out of sync between db and fs
+        unless gitlab_shell.mv_repository(project.repository_storage, full_repo_path_before_last_save(project), project.disk_path )
           raise Gitlab::UpdatePathError.new('namespace directory cannot be moved')
         end
       end
     end
 
+    def full_repo_path_before_last_save(project)
+      project.disk_path.gsub(full_path, full_path_before_last_save)
+    end
+
     def old_repository_storages
       @old_repository_storage_paths ||= repository_storages
     end

app/models/storage/legacy_project.rb

@@ -38,7 +38,7 @@ module Storage
         # 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, "#{old_full_path}.wiki", "#{new_full_path}.wiki")
+          gitlab_shell.mv_wiki(repository_storage, old_full_path, new_full_path) if project.wiki.exists?
           return true
         rescue => e
           Rails.logger.error "Exception renaming #{old_full_path} -> #{new_full_path}: #{e}" # rubocop:disable Gitlab/RailsLogger

app/services/projects/hashed_storage/base_repository_service.rb

@@ -29,20 +29,12 @@ module Projects
       # rubocop: disable CodeReuse/ActiveRecord
       def move_repository(from_name, to_name)
         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.
-        if !from_exists && !to_exists
-          logger.warn "Can't find a repository on either source or target paths for #{project.full_path} (ID=#{project.id}) ..."
 
-          # We return true so we still reflect the change in the database.
-          # Next time the repository is (re)created it will be under the new storage layout
-          return true
-        elsif !from_exists
-          # Repository have been moved already.
-          return true
-        end
+        # Repository have been moved already.
+        return true unless from_exists
 
         gitlab_shell.mv_repository(project.repository_storage, from_name, to_name)
       end

app/services/projects/hashed_storage/migrate_repository_service.rb

@@ -8,7 +8,6 @@ module Projects
 
         @old_storage_version = project.storage_version
         project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository]
-        project.ensure_storage_path_exists
 
         @new_disk_path = project.disk_path
 

changelogs/unreleased/jc-legacy-storage-to-hashed-storage.yml

+---
+title: Call RenameRepository instead of RenameNamespace
+merge_request: 32684
+author:
+type: deprecated

lib/gitlab/git/repository.rb

@@ -131,6 +131,12 @@ module Gitlab
         end
       end
 
+      def rename(new_relative_path)
+        wrapped_gitaly_errors do
+          gitaly_repository_client.rename(new_relative_path)
+        end
+      end
+
       def expire_has_local_branches_cache
         clear_memoization(:has_local_branches)
       end

lib/gitlab/gitaly_client/repository_service.rb

@@ -346,6 +346,12 @@ module Gitlab
         GitalyClient.call(@storage, :object_pool_service, :disconnect_git_alternates, request)
       end
 
+      def rename(relative_path)
+        request = Gitaly::RenameRepositoryRequest.new(repository: @gitaly_repo, relative_path: relative_path)
+
+        GitalyClient.call(@storage, :repository_service, :rename_repository, request, timeout: GitalyClient.fast_timeout)
+      end
+
       private
 
       def search_results_from_response(gitaly_response)

lib/gitlab/shell.rb

@@ -117,16 +117,26 @@ module Gitlab
     # mv_namespace. Given the underlying implementation is a move action,
     # indescriminate of what the folders might be.
     #
-    # storage - project's storage path
+    # storage - project's storage name
     # path - project disk path
     # new_path - new project disk path
     #
     # Ex.
-    #   mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new")
+    #   mv_repository("default", "gitlab/gitlab-ci", "randx/gitlab-ci-new")
     def mv_repository(storage, path, new_path)
       return false if path.empty? || new_path.empty?
 
-      !!mv_directory(storage, "#{path}.git", "#{new_path}.git")
+      Gitlab::Git::Repository.new(storage, "#{path}.git", nil, nil).rename("#{new_path}.git")
+    rescue => e
+      Gitlab::Sentry.track_acceptable_exception(e, extra: { path: path, new_path: new_path, storage: storage })
+
+      false
+    end
+
+    def mv_wiki(storage, path, new_path)
+      return false if path.empty? || new_path.empty?
+
+      Gitlab::Git::Repository.new(storage, "#{path}.wiki.git", nil, nil).rename("#{new_path}.wiki.git")
     end
 
     # Fork repository to new path

spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb

@@ -21,6 +21,8 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
     it 'searches using nested paths' do
       namespace = create(:namespace, path: 'hello')
       project = create(:project, :legacy_storage, path: 'THE-path', namespace: namespace)
+      project.create_repository
+      project.create_wiki
 
       result_ids = described_class.new(['Hello/the-path'], migration)
                      .projects_for_paths.map(&:id)
@@ -30,6 +32,9 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
 
     it 'includes the correct projects' do
       project = create(:project, :legacy_storage, path: 'THE-path')
+      project.create_repository
+      project.create_wiki
+
       _other_project = create(:project, :legacy_storage)
 
       result_ids = subject.projects_for_paths.map(&:id)
@@ -41,6 +46,13 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
   describe '#rename_projects' do
     let!(:projects) { create_list(:project, 2, :legacy_storage, path: 'the-path') }
 
+    before do
+      projects.each do |p|
+        p.create_repository
+        p.create_wiki
+      end
+    end
+
     it 'renames each project' do
       expect(subject).to receive(:rename_project).twice
 
@@ -56,6 +68,10 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
   end
 
   describe '#rename_project' do
+    before do
+      project.create_repository
+      project.create_wiki
+    end
     it 'renames path & route for the project' do
       expect(subject).to receive(:rename_path_for_routable)
                            .with(project)
@@ -81,6 +97,11 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
   end
 
   describe '#move_project_folders' do
+    before do
+      project.create_repository
+      project.create_wiki
+    end
+
     it 'moves the wiki & the repo' do
       expect(subject).to receive(:move_repository)
                            .with(project, 'known-parent/the-path.wiki', 'known-parent/the-path0.wiki')
@@ -125,6 +146,11 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
     let(:known_parent) { create(:namespace, path: 'known-parent') }
     let(:project) { create(:project, :repository, :legacy_storage, path: 'the-path', namespace: known_parent) }
 
+    before do
+      project.create_repository
+      project.create_wiki
+    end
+
     it 'moves the repository for a project' do
       expected_path = File.join(TestEnv.repos_path, 'known-parent', 'new-repo.git')
 
@@ -135,6 +161,11 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
   end
 
   describe '#revert_renames', :redis do
+    before do
+      project.create_repository
+      project.create_wiki
+    end
+
     it 'renames the routes back to the previous values' do
       subject.rename_project(project)
 
@@ -152,7 +183,6 @@ describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :de
     end
 
     it 'moves the repositories back to their original place' do
-      project.create_repository
       subject.rename_project(project)
 
       expected_path = File.join(TestEnv.repos_path, 'known-parent', 'the-path.git')

spec/models/namespace_spec.rb

@@ -362,9 +362,9 @@ describe Namespace do
       it 'updates project full path in .git/config' do
         parent.update(path: 'mygroup_new')
 
-        expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}"
-        expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}"
-        expect(project_rugged(legacy_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}"
+        expect(project_rugged(project_in_parent_group.reload).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}"
+        expect(project_rugged(hashed_project_in_subgroup.reload).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}"
+        expect(project_rugged(legacy_project_in_subgroup.reload).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}"
       end
 
       it 'updates the project storage location' do