Merge branch 'fj-61834-log-lfs-error-pull-mirror' into 'master'

Avoid failing pull mirroring if LFS import fails

See merge request gitlab-org/gitlab-ee!13133

ee/app/services/projects/update_mirror_service.rb

@@ -98,7 +98,11 @@ module Projects
     def update_lfs_objects
       result = Projects::LfsPointers::LfsImportService.new(project).execute
 
-      raise UpdateError, result[:message] if result[:status] == :error
+      if result[:status] == :error
+        log_error(result[:message])
+        # Uncomment once https://gitlab.com/gitlab-org/gitlab-ce/issues/61834 is closed
+        # raise UpdateError, result[:message]
+      end
     end
 
     def handle_diverged_branch(upstream, local, branch_name, errors)
@@ -127,5 +131,21 @@ module Projects
     def skip_branch?(name)
       project.only_mirror_protected_branches && !ProtectedBranch.protected?(project, name)
     end
+
+    def service_logger
+      @service_logger ||= Gitlab::UpdateMirrorServiceJsonLogger.build
+    end
+
+    def base_payload
+      {
+        user_id: current_user.id,
+        project_id: project.id,
+        import_url: project.safe_import_url
+      }
+    end
+
+    def log_error(error_message)
+      service_logger.error(base_payload.merge(error_message: error_message))
+    end
   end
 end

ee/changelogs/unreleased/fj-61834-log-lfs-error-pull-mirror.yml

+---
+title: Avoid failing pull mirroring if LFS import fails
+merge_request: 13133
+author:
+type: changed

ee/lib/gitlab/update_mirror_service_json_logger.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class UpdateMirrorServiceJsonLogger < Gitlab::JsonLogger
+    def self.file_name_noext
+      'update_mirror_service_json'
+    end
+  end
+end

ee/spec/services/projects/update_mirror_service_spec.rb

@@ -295,13 +295,33 @@ describe Projects::UpdateMirrorService do
         end
 
         context 'when Lfs import fails' do
-          it 'the mirror operation fails' do
-            expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: 'error message')
+          let(:error_message) { 'error_message' }
 
+          before do
+            expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: error_message)
+          end
+
+          # Uncomment once https://gitlab.com/gitlab-org/gitlab-ce/issues/61834 is closed
+          # it 'fails mirror operation' do
+          #   expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: 'error message')
+
+          #   result = subject.execute
+
+          #   expect(result[:status]).to eq :error
+          #   expect(result[:message]).to eq 'error message'
+          # end
+
+          # Remove once https://gitlab.com/gitlab-org/gitlab-ce/issues/61834 is closed
+          it 'does not fail mirror operation' do
             result = subject.execute
 
-            expect(result[:status]).to eq :error
-            expect(result[:message]).to eq 'error message'
+            expect(result[:status]).to eq :success
+          end
+
+          it 'logs the error' do
+            expect_any_instance_of(Gitlab::UpdateMirrorServiceJsonLogger).to receive(:error).with(hash_including(error_message: error_message))
+
+            subject.execute
           end
         end
       end