Merge branch '300456-create-all-es-indexes-on-setting-url' into 'master'

Make sure all ES indexes are created when setting server URL

See merge request gitlab-org/gitlab!57253

ee/app/services/ee/application_settings/update_service.rb

@@ -77,9 +77,10 @@ module EE
         # The order of checks is important. We should not attempt to create a new index
         # unless elasticsearch_indexing is enabled
         return unless application_setting.elasticsearch_indexing
-        return if elasticsearch_helper.index_exists?
 
-        elasticsearch_helper.create_empty_index
+        elasticsearch_helper.create_empty_index(options: { skip_if_exists: true })
+        elasticsearch_helper.create_standalone_indices(options: { skip_if_exists: true })
+        elasticsearch_helper.create_migrations_index unless elasticsearch_helper.migrations_index_exists?
       rescue Faraday::Error => e
         log_error(e)
       end

ee/changelogs/unreleased/300456-create-all-es-indexes-on-setting-url.yml

+---
+title: Make sure all ES indexes are created when setting server URL
+merge_request: 57253
+author:
+type: fixed

ee/lib/gitlab/elastic/helper.rb

@@ -115,30 +115,7 @@ module Gitlab
           alias_name = proxy.index_name
           new_index_name = "#{alias_name}-#{Time.now.strftime("%Y%m%d-%H%M")}"
 
-          raise "Index under '#{new_index_name}' already exists" if index_exists?(index_name: new_index_name)
-
-          if with_alias
-            raise "Alias under '#{alias_name}' already exists" if alias_exists?(name: alias_name)
-          end
-
-          settings = proxy.settings.to_hash
-          settings = settings.merge(options[:settings]) if options[:settings]
-
-          mappings = proxy.mappings.to_hash
-          mappings = mappings.merge(options[:mappings]) if options[:mappings]
-
-          create_index_options = {
-            index: new_index_name,
-            body: {
-              settings: settings,
-              mappings: mappings
-            }
-          }.merge(additional_index_options)
-
-          client.indices.create create_index_options
-
-          client.indices.put_alias(name: alias_name, index: new_index_name) if with_alias
-
+          create_index(new_index_name, alias_name, with_alias, proxy.settings.to_hash, proxy.mappings.to_hash, options)
           indices[new_index_name] = alias_name
         end
       end
@@ -163,26 +140,7 @@ module Gitlab
       def create_empty_index(with_alias: true, options: {})
         new_index_name = options[:index_name] || "#{target_name}-#{Time.now.strftime("%Y%m%d-%H%M")}"
 
-        if with_alias ? index_exists? : index_exists?(index_name: new_index_name)
-          raise "Index under '#{with_alias ? target_name : new_index_name}' already exists, use `recreate_index` to recreate it."
-        end
-
-        settings = default_settings
-        settings.merge!(options[:settings]) if options[:settings]
-
-        mappings = default_mappings
-        mappings.merge!(options[:mappings]) if options[:mappings]
-
-        create_index_options = {
-          index: new_index_name,
-          body: {
-            settings: settings.to_hash,
-            mappings: mappings.to_hash
-          }
-        }.merge(additional_index_options)
-
-        client.indices.create create_index_options
-        client.indices.put_alias(name: target_name, index: new_index_name) if with_alias
+        create_index(new_index_name, target_name, with_alias, default_settings, default_mappings, options)
 
         {
           new_index_name => target_name
@@ -304,6 +262,34 @@ module Gitlab
 
       private
 
+      def create_index(index_name, alias_name, with_alias, settings, mappings, options)
+        if index_exists?(index_name: index_name)
+          return if options[:skip_if_exists]
+
+          raise "Index under '#{index_name}' already exists."
+        end
+
+        if with_alias && index_exists?(index_name: alias_name)
+          return if options[:skip_if_exists]
+
+          raise "Index or alias under '#{alias_name}' already exists."
+        end
+
+        settings.merge!(options[:settings]) if options[:settings]
+        mappings.merge!(options[:mappings]) if options[:mappings]
+
+        create_index_options = {
+          index: index_name,
+          body: {
+            settings: settings,
+            mappings: mappings
+          }
+        }.merge(additional_index_options)
+
+        client.indices.create create_index_options
+        client.indices.put_alias(name: alias_name, index: index_name) if with_alias
+      end
+
       def additional_index_options
         {}.tap do |options|
           # include_type_name defaults to false in ES7. This will ensure ES7

ee/spec/lib/ee/gitlab/elastic/helper_spec.rb

@@ -96,6 +96,12 @@ RSpec.describe Gitlab::Elastic::Helper, :request_store do
       expect { helper.create_standalone_indices }.to raise_error(/already exists/)
     end
 
+    it 'does not raise an exception with skip_if_exists option' do
+      @indices = helper.create_standalone_indices
+
+      expect { helper.create_standalone_indices(options: { skip_if_exists: true }) }.not_to raise_error
+    end
+
     it 'raises an exception when there is an existing index' do
       @indices = helper.create_standalone_indices(with_alias: false)
 
@@ -159,7 +165,11 @@ RSpec.describe Gitlab::Elastic::Helper, :request_store do
       include_context 'with an existing index and alias'
 
       it 'raises an error' do
-        expect { helper.create_empty_index }.to raise_error(RuntimeError)
+        expect { helper.create_empty_index }.to raise_error(/Index under '.+' already exists/)
+      end
+
+      it 'does not raise error with skip_if_exists option' do
+        expect { helper.create_empty_index(options: { skip_if_exists: true }) }.not_to raise_error
       end
     end
 
@@ -167,7 +177,7 @@ RSpec.describe Gitlab::Elastic::Helper, :request_store do
       include_context 'with a legacy index'
 
       it 'raises an error' do
-        expect { helper.create_empty_index }.to raise_error(RuntimeError)
+        expect { helper.create_empty_index }.to raise_error(/Index or alias under '.+' already exists/)
       end
     end
   end

ee/spec/services/application_settings/update_service_spec.rb

@@ -44,19 +44,12 @@ RSpec.describe ApplicationSettings::UpdateService do
       context 'index creation' do
         let(:opts) { { elasticsearch_indexing: true } }
 
-        context 'when index exists' do
-          it 'skips creating a new index' do
-            expect(helper).to(receive(:index_exists?)).and_return(true)
-            expect(helper).not_to(receive(:create_empty_index))
-
-            service.execute
-          end
-        end
-
         context 'when index does not exist' do
           it 'creates a new index' do
-            expect(helper).to(receive(:index_exists?)).and_return(false)
-            expect(helper).to(receive(:create_empty_index))
+            expect(helper).to receive(:create_empty_index).with(options: { skip_if_exists: true })
+            expect(helper).to receive(:create_standalone_indices).with(options: { skip_if_exists: true })
+            expect(helper).to receive(:migrations_index_exists?).and_return(false)
+            expect(helper).to receive(:create_migrations_index)
 
             service.execute
           end
@@ -65,7 +58,7 @@ RSpec.describe ApplicationSettings::UpdateService do
         context 'when ES service is not reachable' do
           it 'does not throw exception' do
             expect(helper).to receive(:index_exists?).and_raise(Faraday::ConnectionFailed, nil)
-            expect(helper).not_to receive(:create_empty_index)
+            expect(helper).not_to receive(:create_standalone_indices)
 
             expect { service.execute }.not_to raise_error
           end