Merge branch '299057-fj-add-update-repository-storage-service' into 'master'

Create Groups::UpdateRepositoryStorageService service

See merge request gitlab-org/gitlab!52687

ee/app/models/ee/group.rb

@@ -462,6 +462,10 @@ module EE
       reference_counter(type: ::Gitlab::GlRepository::WIKI).value > 0
     end
 
+    def repository_storage
+      group_wiki_repository&.shard_name || ::Repository.pick_storage_shard
+    end
+
     private
 
     def custom_project_templates_group_allowed

ee/app/models/group_wiki.rb

@@ -23,9 +23,7 @@ class GroupWiki < Wiki
 
   override :repository_storage
   def repository_storage
-    strong_memoize(:repository_storage) do
-      container.group_wiki_repository&.shard_name || Repository.pick_storage_shard
-    end
+    container.repository_storage
   end
 
   override :hashed_storage?

ee/app/services/groups/update_repository_storage_service.rb

+# frozen_string_literal: true
+
+module Groups
+  class UpdateRepositoryStorageService
+    include UpdateRepositoryStorageMethods
+
+    delegate :group, to: :repository_storage_move
+
+    private
+
+    def track_repository(destination_storage_name)
+      if group.wiki_repository_exists?
+        group.wiki.track_wiki_repository(destination_storage_name)
+      end
+    end
+
+    def mirror_repositories
+      if group.wiki_repository_exists?
+        mirror_repository(type: Gitlab::GlRepository::WIKI)
+      end
+    end
+
+    def remove_old_paths
+      if group.wiki_repository_exists?
+        Gitlab::Git::Repository.new(
+          source_storage_name,
+          "#{group.wiki.disk_path}.git",
+          nil,
+          nil
+        ).remove
+      end
+    end
+  end
+end

ee/spec/models/group_spec.rb

@@ -1264,6 +1264,25 @@ RSpec.describe Group do
     end
   end
 
+  describe '#repository_storage', :aggregated_failures do
+    context 'when wiki does not have a tracked repository storage' do
+      it 'returns the default shard' do
+        expect(::Repository).to receive(:pick_storage_shard).and_call_original
+        expect(subject.repository_storage).to eq('default')
+      end
+    end
+
+    context 'when wiki has a tracked repository storage' do
+      it 'returns the persisted shard' do
+        group.wiki.create_wiki_repository
+
+        expect(group.group_wiki_repository).to receive(:shard_name).and_return('foo')
+
+        expect(group.repository_storage).to eq('foo')
+      end
+    end
+  end
+
   it_behaves_like 'can move repository storage' do
     let_it_be(:container) { create(:group, :wiki_repo) }
 

ee/spec/models/group_wiki_spec.rb

@@ -73,34 +73,10 @@ RSpec.describe GroupWiki do
     end
 
     describe '#repository_storage' do
-      context 'when a tracking entry does not exist' do
-        let(:wiki_container) { wiki_container_without_repo }
-
-        it 'returns the default shard' do
-          expect(subject.repository_storage).to eq('default')
-        end
-
-        context 'when multiple shards are configured' do
-          let(:shards) { (1..).each }
-
-          before do
-            # Force pick_storage_shard to always return a different value
-            allow(Repository).to receive(:pick_storage_shard) { "storage-#{shards.next}" }
-          end
+      it 'gets the repository storage from the container' do
+        expect(wiki.container).to receive(:repository_storage).and_return('foo')
 
-          it 'always returns the same shard when called repeatedly' do
-            shard = subject.repository_storage
-
-            expect(subject.repository_storage).to eq(shard)
-          end
-        end
-      end
-
-      context 'when a tracking entry exists' do
-        it 'returns the persisted shard if the repository is tracked' do
-          expect(wiki_container.group_wiki_repository).to receive(:shard_name).and_return('foo')
-          expect(subject.repository_storage).to eq('foo')
-        end
+        expect(subject.repository_storage).to eq 'foo'
       end
     end
 

ee/spec/services/groups/update_repository_storage_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::UpdateRepositoryStorageService do
+  include Gitlab::ShellAdapter
+
+  subject { described_class.new(repository_storage_move) }
+
+  describe "#execute" do
+    let_it_be_with_reload(:group) { create(:group, :wiki_repo) }
+
+    let(:wiki) { group.wiki }
+    let(:checksum) { wiki.repository.checksum }
+    let(:destination) { 'test_second_storage' }
+    let(:repository_storage_move_state) { :scheduled }
+    let(:repository_storage_move) { create(:group_repository_storage_move, repository_storage_move_state, container: group, destination_storage_name: destination) }
+    let(:wiki_repository_double) { double(:repository) }
+    let(:original_wiki_repository_double) { double(:repository) }
+
+    before do
+      allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage])
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+      allow(Gitlab::GitalyClient).to receive(:filesystem_id).with(destination).and_return(SecureRandom.uuid)
+      allow(Gitlab::Git::Repository).to receive(:new).and_call_original
+      allow(Gitlab::Git::Repository).to receive(:new)
+        .with(destination, wiki.repository.raw.relative_path, wiki.repository.gl_repository, wiki.repository.full_path)
+        .and_return(wiki_repository_double)
+      allow(Gitlab::Git::Repository).to receive(:new)
+        .with('default', wiki.repository.raw.relative_path, nil, nil)
+        .and_return(original_wiki_repository_double)
+    end
+
+    context 'when the move succeeds' do
+      it 'moves the repository to the new storage and unmarks the repository as read only', :aggregate_failures do
+        old_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          wiki.repository.path_to_repo
+        end
+
+        expect(wiki_repository_double).to receive(:replicate)
+          .with(wiki.repository.raw)
+        expect(wiki_repository_double).to receive(:checksum)
+          .and_return(checksum)
+        expect(original_wiki_repository_double).to receive(:remove)
+
+        result = subject.execute
+        group.reload
+
+        expect(result).to be_success
+        expect(group).not_to be_repository_read_only
+        expect(wiki.repository_storage).to eq(destination)
+        expect(gitlab_shell.repository_exists?('default', old_path)).to be(false)
+        expect(group.group_wiki_repository.shard_name).to eq(destination)
+      end
+    end
+
+    context 'when the filesystems are the same' do
+      let(:destination) { wiki.repository_storage }
+
+      it 'bails out and does nothing' do
+        result = subject.execute
+
+        expect(result).to be_error
+        expect(result.message).to match(/SameFilesystemError/)
+      end
+    end
+
+    context 'when the move fails' do
+      it 'unmarks the repository as read-only without updating the repository storage' do
+        expect(wiki_repository_double).to receive(:replicate)
+          .with(wiki.repository.raw)
+          .and_raise(Gitlab::Git::CommandError)
+
+        result = subject.execute
+
+        expect(result).to be_error
+        expect(group.reload).not_to be_repository_read_only
+        expect(wiki.repository_storage).to eq('default')
+        expect(repository_storage_move).to be_failed
+      end
+    end
+
+    context 'when the cleanup fails' do
+      it 'sets the correct state' do
+        expect(wiki_repository_double).to receive(:replicate)
+          .with(wiki.repository.raw)
+        expect(wiki_repository_double).to receive(:checksum)
+          .and_return(checksum)
+        expect(original_wiki_repository_double).to receive(:remove)
+          .and_raise(Gitlab::Git::CommandError)
+
+        result = subject.execute
+
+        expect(result).to be_error
+        expect(repository_storage_move).to be_cleanup_failed
+      end
+    end
+
+    context 'when the checksum does not match' do
+      it 'unmarks the repository as read-only without updating the repository storage' do
+        expect(wiki_repository_double).to receive(:replicate)
+          .with(wiki.repository.raw)
+        expect(wiki_repository_double).to receive(:checksum)
+          .and_return('not matching checksum')
+
+        result = subject.execute
+
+        expect(result).to be_error
+        expect(group).not_to be_repository_read_only
+        expect(wiki.repository_storage).to eq('default')
+      end
+    end
+
+    context 'when the repository move is finished' do
+      let(:repository_storage_move_state) { :finished }
+
+      it 'is idempotent' do
+        expect do
+          result = subject.execute
+
+          expect(result).to be_success
+        end.not_to change(repository_storage_move, :state)
+      end
+    end
+
+    context 'when the repository move is failed' do
+      let(:repository_storage_move_state) { :failed }
+
+      it 'is idempotent' do
+        expect do
+          result = subject.execute
+
+          expect(result).to be_success
+        end.not_to change(repository_storage_move, :state)
+      end
+    end
+  end
+end