Merge branch 'avoid-mirroring-pending-delete-projects' into 'master'

Avoid scheduling mirrors for pending_delete projects

See merge request gitlab-org/gitlab!50820

ee/app/models/ee/project_import_state.rb

@@ -9,7 +9,7 @@ module EE
       BACKOFF_PERIOD = 24.seconds
       JITTER = 6.seconds
 
-      delegate :mirror?, to: :project
+      delegate :mirror?, :mirror_with_content?, :archived, :pending_delete, to: :project
 
       before_validation :set_next_execution_to_now, on: :create
 
@@ -78,7 +78,7 @@ module EE
     override :in_progress?
     def in_progress?
       # If we're importing while we do have a repository, we're simply updating the mirror.
-      super && !project.mirror_with_content?
+      super && !mirror_with_content?
     end
 
     def mirror_waiting_duration
@@ -94,13 +94,12 @@ module EE
     end
 
     def updating_mirror?
-      (scheduled? || started?) && project.mirror_with_content?
+      (scheduled? || started?) && mirror_with_content?
     end
 
     def mirror_update_due?
-      return false unless project.mirror_with_content?
+      return false unless project_eligible_for_mirroring?
       return false unless next_execution_timestamp?
-      return false if project.archived?
       return false if hard_failed?
       return false if updating_mirror?
 
@@ -172,6 +171,10 @@ module EE
 
     private
 
+    def project_eligible_for_mirroring?
+      mirror_with_content? && !archived && !pending_delete
+    end
+
     def state_updated?
       mirror? && last_update_at
     end

ee/app/workers/update_all_mirrors_worker.rb

@@ -105,6 +105,7 @@ class UpdateAllMirrorsWorker # rubocop:disable Scalability/IdempotentWorker
   def pull_mirrors_batch(freeze_at:, batch_size:, offset_at: nil)
     relation = Project
       .non_archived
+      .without_deleted
       .mirrors_to_sync(freeze_at)
       .reorder('import_state.next_execution_timestamp')
       .limit(batch_size)

ee/changelogs/unreleased/avoid-mirroring-pending-delete-projects.yml

+---
+title: Avoid scheduling mirrors for pending_delete projects
+merge_request: 50820
+author:
+type: changed

ee/spec/models/project_import_state_spec.rb

@@ -282,7 +282,7 @@ RSpec.describe ProjectImportState, type: :model do
       end
 
       before do
-        import_state.project.update!(archived: true)
+        import_state.project.update_column(:archived, true)
       end
 
       it 'returns false' do
@@ -290,6 +290,22 @@ RSpec.describe ProjectImportState, type: :model do
       end
     end
 
+    context 'when the project pending_delete' do
+      let(:import_state) do
+        create(:import_state,
+               :finished,
+               :mirror,
+               :repository,
+               next_execution_timestamp: Time.current - 2.minutes)
+      end
+
+      it 'returns false' do
+        import_state.project.update_column(:pending_delete, true)
+
+        expect(import_state.mirror_update_due?).to be false
+      end
+    end
+
     context 'when mirror has no content' do
       it 'returns false' do
         import_state = create(:import_state, :finished, :mirror)

ee/spec/workers/update_all_mirrors_worker_spec.rb

@@ -141,7 +141,7 @@ RSpec.describe UpdateAllMirrorsWorker do
     context 'when the instance is licensed' do
       def scheduled_mirror(at:)
         project = create(:project, :mirror)
-        project.import_state.update!(next_execution_timestamp: at)
+        project.import_state.update_column(:next_execution_timestamp, at)
         project
       end
 
@@ -165,8 +165,8 @@ RSpec.describe UpdateAllMirrorsWorker do
 
         create(:gitlab_subscription, (licensed ? :bronze : :free), namespace: namespace.root_ancestor)
 
-        project.import_state.update!(next_execution_timestamp: at)
-        project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) unless public
+        project.import_state.update_column(:next_execution_timestamp, at)
+        project.update_column(:visibility_level, Gitlab::VisibilityLevel::PRIVATE) unless public
         project
       end
 
@@ -232,7 +232,17 @@ RSpec.describe UpdateAllMirrorsWorker do
           end
 
           it "does not schedule a mirror of an archived project" do
-            licensed_project1.update!(archived: true)
+            licensed_project1.update_column(:archived, true)
+
+            schedule_mirrors!(capacity: 4)
+
+            expect_import_scheduled(licensed_project2, public_project)
+            expect_import_not_scheduled(licensed_project1)
+            expect_import_not_scheduled(*unlicensed_projects)
+          end
+
+          it "does not schedule a mirror of an pending_delete project" do
+            licensed_project1.update_column(:pending_delete, true)
 
             schedule_mirrors!(capacity: 4)