Raise the error when repository storage move fails

Previously repository storage moves would absorb errors and only log
them. This has turned out to be difficult to diagnose since sidekiq
considers the jobs successful. Instead we should blowup.

Changelog: fixed

app/services/concerns/update_repository_storage_methods.rb

@@ -38,11 +38,7 @@ module UpdateRepositoryStorageMethods
   rescue StandardError => e
     repository_storage_move.do_fail!
 
-    Gitlab::ErrorTracking.track_exception(e, container_klass: container.class.to_s, container_path: container.full_path)
-
-    ServiceResponse.error(
-      message: s_("UpdateRepositoryStorage|Error moving repository storage for %{container_full_path} - %{message}") % { container_full_path: container.full_path, message: e.message }
-    )
+    Gitlab::ErrorTracking.track_and_raise_exception(e, container_klass: container.class.to_s, container_path: container.full_path)
   end
 
   private

ee/spec/services/groups/update_repository_storage_service_spec.rb

@@ -76,9 +76,10 @@ RSpec.describe Groups::UpdateRepositoryStorageService do
           .with(wiki.repository.raw)
           .and_raise(Gitlab::Git::CommandError)
 
-        result = subject.execute
+        expect do
+          subject.execute
+        end.to raise_error(Gitlab::Git::CommandError)
 
-        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
@@ -94,9 +95,10 @@ RSpec.describe Groups::UpdateRepositoryStorageService do
         expect(original_wiki_repository_double).to receive(:remove)
           .and_raise(Gitlab::Git::CommandError)
 
-        result = subject.execute
+        expect do
+          subject.execute
+        end.to raise_error(Gitlab::Git::CommandError)
 
-        expect(result).to be_error
         expect(repository_storage_move).to be_cleanup_failed
       end
     end
@@ -108,9 +110,10 @@ RSpec.describe Groups::UpdateRepositoryStorageService do
         expect(wiki_repository_double).to receive(:checksum)
           .and_return('not matching checksum')
 
-        result = subject.execute
+        expect do
+          subject.execute
+        end.to raise_error(UpdateRepositoryStorageMethods::Error, /Failed to verify wiki repository checksum from \w+ to not matching checksum/)
 
-        expect(result).to be_error
         expect(group).not_to be_repository_read_only
         expect(wiki.repository_storage).to eq('default')
       end

locale/gitlab.pot

@@ -34908,9 +34908,6 @@ msgstr ""
 msgid "UpdateProject|Project could not be updated!"
 msgstr ""
 
-msgid "UpdateRepositoryStorage|Error moving repository storage for %{container_full_path} - %{message}"
-msgstr ""
-
 msgid "UpdateRepositoryStorage|Failed to verify %{type} repository checksum from %{old} to %{new}"
 msgstr ""
 

spec/services/projects/update_repository_storage_service_spec.rb

@@ -83,9 +83,10 @@ RSpec.describe Projects::UpdateRepositoryStorageService do
             .with(project.repository.raw)
             .and_raise(Gitlab::Git::CommandError)
 
-          result = subject.execute
+          expect do
+            subject.execute
+          end.to raise_error(Gitlab::Git::CommandError)
 
-          expect(result).to be_error
           expect(project).not_to be_repository_read_only
           expect(project.repository_storage).to eq('default')
           expect(repository_storage_move).to be_failed
@@ -101,9 +102,10 @@ RSpec.describe Projects::UpdateRepositoryStorageService do
           expect(original_project_repository_double).to receive(:remove)
             .and_raise(Gitlab::Git::CommandError)
 
-          result = subject.execute
+          expect do
+            subject.execute
+          end.to raise_error(Gitlab::Git::CommandError)
 
-          expect(result).to be_error
           expect(repository_storage_move).to be_cleanup_failed
         end
       end
@@ -118,9 +120,10 @@ RSpec.describe Projects::UpdateRepositoryStorageService do
           expect(project_repository_double).to receive(:checksum)
             .and_return('not matching checksum')
 
-          result = subject.execute
+          expect do
+            subject.execute
+          end.to raise_error(UpdateRepositoryStorageMethods::Error, /Failed to verify project repository checksum/)
 
-          expect(result).to be_error
           expect(project).not_to be_repository_read_only
           expect(project.repository_storage).to eq('default')
         end

spec/services/snippets/update_repository_storage_service_spec.rb

@@ -75,9 +75,10 @@ RSpec.describe Snippets::UpdateRepositoryStorageService do
           .with(snippet.repository.raw)
           .and_raise(Gitlab::Git::CommandError)
 
-        result = subject.execute
+        expect do
+          subject.execute
+        end.to raise_error(Gitlab::Git::CommandError)
 
-        expect(result).to be_error
         expect(snippet).not_to be_repository_read_only
         expect(snippet.repository_storage).to eq('default')
         expect(repository_storage_move).to be_failed
@@ -93,9 +94,10 @@ RSpec.describe Snippets::UpdateRepositoryStorageService do
         expect(original_snippet_repository_double).to receive(:remove)
           .and_raise(Gitlab::Git::CommandError)
 
-        result = subject.execute
+        expect do
+          subject.execute
+        end.to raise_error(Gitlab::Git::CommandError)
 
-        expect(result).to be_error
         expect(repository_storage_move).to be_cleanup_failed
       end
     end
@@ -107,9 +109,10 @@ RSpec.describe Snippets::UpdateRepositoryStorageService do
         expect(snippet_repository_double).to receive(:checksum)
           .and_return('not matching checksum')
 
-        result = subject.execute
+        expect do
+          subject.execute
+        end.to raise_error(UpdateRepositoryStorageMethods::Error, /Failed to verify snippet repository checksum from \w+ to not matching checksum/)
 
-        expect(result).to be_error
         expect(snippet).not_to be_repository_read_only
         expect(snippet.repository_storage).to eq('default')
       end

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

@@ -123,9 +123,10 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
         .with(repository.raw)
         .and_raise(Gitlab::Git::CommandError)
 
-      result = subject.execute
+      expect do
+        subject.execute
+      end.to raise_error(Gitlab::Git::CommandError)
 
-      expect(result).to be_error
       expect(project).not_to be_repository_read_only
       expect(project.repository_storage).to eq('default')
       expect(repository_storage_move).to be_failed
@@ -149,9 +150,10 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
       expect(original_repository_double).to receive(:remove)
         .and_raise(Gitlab::Git::CommandError)
 
-      result = subject.execute
+      expect do
+        subject.execute
+      end.to raise_error(Gitlab::Git::CommandError)
 
-      expect(result).to be_error
       expect(repository_storage_move).to be_cleanup_failed
     end
   end
@@ -170,9 +172,10 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
       allow(repository_double).to receive(:checksum)
         .and_return('not matching checksum')
 
-      result = subject.execute
+      expect do
+        subject.execute
+      end.to raise_error(UpdateRepositoryStorageMethods::Error, /Failed to verify \w+ repository checksum from \w+ to not matching checksum/)
 
-      expect(result).to be_error
       expect(project).not_to be_repository_read_only
       expect(project.repository_storage).to eq('default')
     end