Merge branch 'update_db_on_move_when_identical' into 'master'

Update the database when moving repos between identical storages

See merge request gitlab-org/gitlab!52743

app/services/concerns/update_repository_storage_methods.rb

 # frozen_string_literal: true
 
 module UpdateRepositoryStorageMethods
+  include Gitlab::Utils::StrongMemoize
+
   Error = Class.new(StandardError)
-  SameFilesystemError = Class.new(Error)
 
   attr_reader :repository_storage_move
   delegate :container, :source_storage_name, :destination_storage_name, to: :repository_storage_move
@@ -18,9 +19,7 @@ module UpdateRepositoryStorageMethods
       repository_storage_move.start!
     end
 
-    raise SameFilesystemError if same_filesystem?(source_storage_name, destination_storage_name)
-
-    mirror_repositories
+    mirror_repositories unless same_filesystem?
 
     repository_storage_move.transaction do
       repository_storage_move.finish_replication!
@@ -28,8 +27,10 @@ module UpdateRepositoryStorageMethods
       track_repository(destination_storage_name)
     end
 
+    unless same_filesystem?
       remove_old_paths
       enqueue_housekeeping
+    end
 
     repository_storage_move.finish_cleanup!
 
@@ -80,8 +81,10 @@ module UpdateRepositoryStorageMethods
     end
   end
 
-  def same_filesystem?(old_storage, new_storage)
-    Gitlab::GitalyClient.filesystem_id(old_storage) == Gitlab::GitalyClient.filesystem_id(new_storage)
+  def same_filesystem?
+    strong_memoize(:same_filesystem) do
+      Gitlab::GitalyClient.filesystem_id(source_storage_name) == Gitlab::GitalyClient.filesystem_id(destination_storage_name)
+    end
   end
 
   def remove_old_paths

changelogs/unreleased/update_db_on_move_when_identical.yml

+---
+title: Update the database when moving repos between identical storages
+merge_request: 52743
+author:
+type: fixed

ee/spec/services/groups/update_repository_storage_service_spec.rb

@@ -55,13 +55,18 @@ RSpec.describe Groups::UpdateRepositoryStorageService do
     end
 
     context 'when the filesystems are the same' do
-      let(:destination) { wiki.repository_storage }
+      before do
+        expect(Gitlab::GitalyClient).to receive(:filesystem_id).twice.and_return(SecureRandom.uuid)
+      end
 
-      it 'bails out and does nothing' do
+      it 'updates the database without trying to move the repostory', :aggregate_failures do
         result = subject.execute
+        group.reload
 
-        expect(result).to be_error
-        expect(result.message).to match(/SameFilesystemError/)
+        expect(result).to be_success
+        expect(group).not_to be_repository_read_only
+        expect(wiki.repository_storage).to eq(destination)
+        expect(group.group_wiki_repository.shard_name).to eq(destination)
       end
     end
 

spec/services/projects/update_repository_storage_service_spec.rb

@@ -59,13 +59,18 @@ RSpec.describe Projects::UpdateRepositoryStorageService do
       end
 
       context 'when the filesystems are the same' do
-        let(:destination) { project.repository_storage }
+        before do
+          expect(Gitlab::GitalyClient).to receive(:filesystem_id).twice.and_return(SecureRandom.uuid)
+        end
 
-        it 'bails out and does nothing' do
+        it 'updates the database without trying to move the repostory', :aggregate_failures do
           result = subject.execute
+          project.reload
 
-          expect(result).to be_error
-          expect(result.message).to match(/SameFilesystemError/)
+          expect(result).to be_success
+          expect(project).not_to be_repository_read_only
+          expect(project.repository_storage).to eq('test_second_storage')
+          expect(project.project_repository.shard_name).to eq('test_second_storage')
         end
       end
 

spec/services/snippets/update_repository_storage_service_spec.rb

@@ -54,13 +54,18 @@ RSpec.describe Snippets::UpdateRepositoryStorageService do
     end
 
     context 'when the filesystems are the same' do
-      let(:destination) { snippet.repository_storage }
+      before do
+        expect(Gitlab::GitalyClient).to receive(:filesystem_id).twice.and_return(SecureRandom.uuid)
+      end
 
-      it 'bails out and does nothing' do
+      it 'updates the database without trying to move the repostory', :aggregate_failures do
         result = subject.execute
+        snippet.reload
 
-        expect(result).to be_error
-        expect(result.message).to match(/SameFilesystemError/)
+        expect(result).to be_success
+        expect(snippet).not_to be_repository_read_only
+        expect(snippet.repository_storage).to eq(destination)
+        expect(snippet.snippet_repository.shard_name).to eq(destination)
       end
     end
 

spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb

@@ -95,13 +95,18 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
   end
 
   context 'when the filesystems are the same' do
-    let(:destination) { project.repository_storage }
+    before do
+      expect(Gitlab::GitalyClient).to receive(:filesystem_id).twice.and_return(SecureRandom.uuid)
+    end
 
-    it 'bails out and does nothing' do
+    it 'updates the database without trying to move the repostory', :aggregate_failures do
       result = subject.execute
+      project.reload
 
-      expect(result).to be_error
-      expect(result.message).to match(/SameFilesystemError/)
+      expect(result).to be_success
+      expect(project).not_to be_repository_read_only
+      expect(project.repository_storage).to eq('test_second_storage')
+      expect(project.project_repository.shard_name).to eq('test_second_storage')
     end
   end