Update the database when moving repos between identical storages

When any two storages are configured identically it was previously
impossible to move projects from one to the other. This is because
before trying to move the repository we check if the storages are
identical to prevent dataloss. Instead of bailing out with an exception,
now we simply update the database. This allows admins to update all
projects before fixing the configuration error.

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
 
-    remove_old_paths
-    enqueue_housekeeping
+    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