Clear hard failure when updating mirror via UI

There was a regression wherein updating a mirror that hard failed
wasn't updating even when the update is triggered via web UI.

It was because the `StartPullMirroringService` was reseting the
retry count before calling `ProjectImportState#force_import_jobi!`.
As a result, the method will return and do nothing because it
detects that the mirror is due to be updated and it's no longer
considered as a hard failure.

To fix it, we remove the need to reset the retry count before
`#force_import_job!` is called. The retry count will be reset in
`#force_import_job!` once it deemed that the mirror can be force
updated.

Now, we only reset the retry count when updating the next
execution timestamp in the said service.

ee/app/services/start_pull_mirroring_service.rb

@@ -6,15 +6,12 @@ class StartPullMirroringService < BaseService
   def execute
     import_state = project.import_state
 
-    if import_state.hard_failed?
-      return error('Mirroring for the project is on pause', 403) if params[:pause_on_hard_failure]
-
-      import_state.reset_retry_count
-    end
+    return error('Mirroring for the project is on pause', 403) if params[:pause_on_hard_failure] && import_state.hard_failed?
 
     if update_now?(import_state)
       import_state.force_import_job!
     else
+      import_state.reset_retry_count if import_state.hard_failed?
       import_state.update(next_execution_timestamp: INTERVAL.since(import_state.last_update_at))
     end
 

ee/changelogs/unreleased/221321-clear-hard-failure-on-force-update.yml

+---
+title: Clear hard failure when updating mirror via UI
+merge_request: 34614
+author:
+type: fixed

ee/spec/services/start_pull_mirroring_service_spec.rb

@@ -3,9 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe StartPullMirroringService do
-  let(:project) { create(:project) }
-  let(:import_state) { create(:import_state, project: project) }
+  let(:project) { create(:project, :mirror, :repository) }
   let(:user) { create(:user) }
+  let(:import_state) { project.import_state }
 
   subject { described_class.new(project, user, pause_on_hard_failure: pause_on_hard_failure) }
 
@@ -59,6 +59,17 @@ RSpec.describe StartPullMirroringService do
     end
   end
 
+  shared_examples_for 'pull mirroring has not started' do |status|
+    it 'does not start pull mirroring' do
+      expect { execute }.to not_change { UpdateAllMirrorsWorker.jobs.size }
+      expect(execute[:status]).to eq(status)
+    end
+  end
+
+  before do
+    import_state.update(next_execution_timestamp: 1.minute.from_now)
+  end
+
   context 'when pause_on_hard_failure is false' do
     let(:pause_on_hard_failure) { false }
 
@@ -72,6 +83,14 @@ RSpec.describe StartPullMirroringService do
       it 'resets the import state retry_count' do
         expect { execute }.to change { import_state.retry_count }.to(0)
       end
+
+      context 'when mirror is due to be updated' do
+        before do
+          import_state.update(next_execution_timestamp: 1.minute.ago)
+        end
+
+        it_behaves_like 'pull mirroring has started'
+      end
     end
 
     context 'when does not reach the max retry limit yet' do
@@ -81,6 +100,14 @@ RSpec.describe StartPullMirroringService do
 
       it_behaves_like 'pull mirroring has started'
       it_behaves_like 'retry count did not reset'
+
+      context 'when mirror is due to be updated' do
+        before do
+          import_state.update(next_execution_timestamp: 1.minute.ago)
+        end
+
+        it_behaves_like 'pull mirroring has not started', :success
+      end
     end
   end
 
@@ -93,11 +120,7 @@ RSpec.describe StartPullMirroringService do
       end
 
       it_behaves_like 'retry count did not reset'
-
-      it 'does not start pull mirroring' do
-        expect { execute }.to not_change { UpdateAllMirrorsWorker.jobs.size }
-        expect(execute[:status]).to eq(:error)
-      end
+      it_behaves_like 'pull mirroring has not started', :error
     end
 
     context 'when does not reach the max retry limit yet' do
@@ -107,6 +130,14 @@ RSpec.describe StartPullMirroringService do
 
       it_behaves_like 'pull mirroring has started'
       it_behaves_like 'retry count did not reset'
+
+      context 'when mirror is due to be updated' do
+        before do
+          import_state.update(next_execution_timestamp: 1.minute.ago)
+        end
+
+        it_behaves_like 'pull mirroring has not started', :success
+      end
     end
   end