Merge branch...

Merge branch '329273-gitlab-should-only-block-unpausing-elasticsearch-indexing-if-there-is-migration-running-that' into 'master'

Only block pausing indexing if running migration requires it

See merge request gitlab-org/gitlab!65281

ee/app/models/elastic/migration_record.rb

@@ -68,6 +68,10 @@ module Elastic
       []
     end
 
+    def running?
+      started? && !halted? && !completed?
+    end
+
     private
 
     def timestamps(completed:)

ee/app/views/admin/application_settings/_elasticsearch_form.html.haml

@@ -49,16 +49,17 @@
             .gl-card-body
               .form-group
                 .form-check
-                - pending_migrations = elasticsearch_available && Elastic::DataMigrationService.pending_migrations? && Gitlab::CurrentSettings.elasticsearch_pause_indexing?
-                - disable_checkbox = !Gitlab::CurrentSettings.elasticsearch_indexing? || pending_migrations || @last_elasticsearch_reindexing_task&.in_progress?
+                - first_pending_migration = Elastic::DataMigrationService.pending_migrations.first if elasticsearch_available
+                - pending_migration_running_and_pauses_indexing = first_pending_migration&.running? && first_pending_migration&.pause_indexing?
+                - disable_checkbox = !Gitlab::CurrentSettings.elasticsearch_indexing? || pending_migration_running_and_pauses_indexing || @last_elasticsearch_reindexing_task&.in_progress?
                 = f.check_box :elasticsearch_pause_indexing, class: 'form-check-input', data: { qa_selector: 'pause_checkbox' }, disabled: disable_checkbox
                 = f.label :elasticsearch_pause_indexing, class: 'form-check-label' do
                   = _('Pause Elasticsearch indexing')
                 .form-text.gl-text-gray-600.gl-mt-0
                   = _('Changes are still tracked. Useful for cluster/index migrations.')
-                - if pending_migrations
+                - if pending_migration_running_and_pauses_indexing
                   .form-text.text-warning
-                    = _('There are pending advanced search migrations. Indexing must remain paused until the migrations are completed.')
+                    = _('There are pending advanced search migrations which require indexing to be paused. Indexing must remain paused until the migrations are completed.')
 
             .form-group
               .form-check

ee/spec/models/elastic/migration_record_spec.rb

@@ -88,4 +88,28 @@ RSpec.describe Elastic::MigrationRecord, :elastic do
       expect(described_class.load_versions(completed: false)).to eq([])
     end
   end
+
+  describe '#running?' do
+    using RSpec::Parameterized::TableSyntax
+
+    before do
+      allow(record).to receive(:halted?).and_return(halted)
+      allow(record).to receive(:started?).and_return(started)
+      allow(record).to receive(:completed?).and_return(completed)
+    end
+
+    where(:started, :halted, :completed, :expected) do
+      false | false | false | false
+      true  | false | false | true
+      true  | true  | false | false
+      true  | true  | true  | false
+      true  | false | true  | false
+    end
+
+    with_them do
+      it 'returns the expected result' do
+        expect(record.running?).to eq(expected)
+      end
+    end
+  end
 end

ee/spec/views/admin/application_settings/_elasticsearch_form.html.haml_spec.rb

@@ -44,13 +44,34 @@ RSpec.describe 'admin/application_settings/_elasticsearch_form' do
       end
 
       context 'pending migrations' do
+        using RSpec::Parameterized::TableSyntax
+
         let(:pending_migrations) { true }
-        let(:pause_indexing) { true }
+        let(:migration) { Elastic::DataMigrationService.migrations.first }
 
-        it 'renders a disabled pause checkbox' do
-          render
+        before do
+          allow(Elastic::DataMigrationService).to receive(:pending_migrations).and_return([migration])
+          allow(migration).to receive(:running?).and_return(running)
+          allow(migration).to receive(:pause_indexing?).and_return(pause_indexing)
+        end
+
+        where(:running, :pause_indexing, :disabled) do
+          false | false | false
+          false | true  | false
+          true  | false | false
+          true  | true  | true
+        end
+
+        with_them do
+          it 'renders pause checkbox with disabled set appropriately' do
+            render
 
-          expect(rendered).to have_css('input[id=application_setting_elasticsearch_pause_indexing][disabled="disabled"]')
+            if disabled
+              expect(rendered).to have_css('input[id=application_setting_elasticsearch_pause_indexing][disabled="disabled"]')
+            else
+              expect(rendered).not_to have_css('input[id=application_setting_elasticsearch_pause_indexing][disabled="disabled"]')
+            end
+          end
         end
       end
     end

locale/gitlab.pot

@@ -32817,7 +32817,7 @@ msgstr ""
 msgid "There are no variables yet."
 msgstr ""
 
-msgid "There are pending advanced search migrations. Indexing must remain paused until the migrations are completed."
+msgid "There are pending advanced search migrations which require indexing to be paused. Indexing must remain paused until the migrations are completed."
 msgstr ""
 
 msgid "There are running deployments on the environment. Please retry later."