Fix halted migrations unpausing

During the issues migration rollout we noticed that for halted migration
Elastic::MigrationWorker has been unpausing indexing each run, which
could be dangerous and not expected by instance admins. This change
fixes this problem by introducing halted_indexing_unpaused flag,
which is flipped only once after the migration is halted.

ee/app/workers/elastic/migration_worker.rb

@@ -88,9 +88,12 @@ module Elastic
     def unpause_indexing!(migration)
       return unless migration.pause_indexing?
       return unless migration.load_state[:pause_indexing]
+      return if migration.load_state[:halted_indexing_unpaused]
 
       logger.info 'MigrationWorker: unpausing indexing'
       Gitlab::CurrentSettings.update!(elasticsearch_pause_indexing: false)
+
+      migration.save_state!(halted_indexing_unpaused: true) if migration.halted?
     end
 
     def helper

ee/changelogs/unreleased/fix-halted-migrations-unpausing.yml

+---
+title: Fix halted migrations unpausing
+merge_request: 51326
+author:
+type: fixed

ee/lib/elastic/migration.rb

@@ -36,7 +36,8 @@ module Elastic
     def fail_migration_halt_error!(retry_attempt: 0)
       set_migration_state(
         retry_attempt: retry_attempt,
-        halted: true
+        halted: true,
+        halted_indexing_unpaused: false
       )
     end
 

ee/spec/elastic/migrate/20201123123400_migrate_issues_to_separate_index_spec.rb

@@ -80,7 +80,7 @@ RSpec.describe MigrateIssuesToSeparateIndex, :elastic, :sidekiq_inline do
 
           migration.migrate
 
-          expect(migration.migration_state).to match(slice: 0, max_slices: 2, retry_attempt: 30, halted: true)
+          expect(migration.migration_state).to match(slice: 0, max_slices: 2, retry_attempt: 30, halted: true, halted_indexing_unpaused: false)
           expect(migration).not_to receive(:process_response)
         end
       end

ee/spec/workers/elastic/migration_worker_spec.rb

@@ -45,38 +45,30 @@ RSpec.describe Elastic::MigrationWorker, :elastic do
       end
 
       context 'migration is halted' do
-        before do
-          allow(Gitlab::CurrentSettings).to receive(:elasticsearch_pause_indexing?).and_return(true)
-          allow(subject).to receive(:current_migration).and_return(migration)
-          allow(migration).to receive(:pause_indexing?).and_return(true)
-          allow(migration).to receive(:halted?).and_return(true)
-        end
-
-        it 'skips execution' do
-          expect(migration).not_to receive(:migrate)
+        using RSpec::Parameterized::TableSyntax
 
-          subject.perform
+        where(:pause_indexing, :halted_indexing_unpaused, :unpause) do
+          false | false | false
+          false | true  | false
+          true  | false | true
+          true  | true  | false
         end
 
-        context 'pause indexing is not allowed' do
+        with_them do
           before do
-            migration.save_state!(pause_indexing: false)
+            allow(Gitlab::CurrentSettings).to receive(:elasticsearch_pause_indexing?).and_return(true)
+            allow(migration).to receive(:pause_indexing?).and_return(true)
+            migration.save_state!(halted: true, pause_indexing: pause_indexing, halted_indexing_unpaused: halted_indexing_unpaused)
           end
 
-          it 'does not unpauses indexing' do
+          it 'unpauses indexing' do
+            if unpause
+              expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: false)
+            else
               expect(Gitlab::CurrentSettings).not_to receive(:update!)
-
-            subject.perform
-          end
             end
 
-        context 'pause indexing is allowed' do
-          before do
-            migration.save_state!(pause_indexing: true)
-          end
-
-          it 'unpauses indexing' do
-            expect(Gitlab::CurrentSettings).to receive(:update!).with(elasticsearch_pause_indexing: false)
+            expect(migration).not_to receive(:migrate)
 
             subject.perform
           end