Commit 1dc5cbde authored by Dmitry Gruzd's avatar Dmitry Gruzd

Prevent calling ES at the admin/integrations page

We're removing a call to ES from admin/integrations page, instead
we'll be using controllers hooks to stop updating indexing setting
when we don't have ES index.
parent ee79fa62
...@@ -244,6 +244,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -244,6 +244,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
def render_update_error def render_update_error
action = valid_setting_panels.include?(action_name) ? action_name : :general action = valid_setting_panels.include?(action_name) ? action_name : :general
flash[:alert] = _('Application settings update failed')
render action render action
end end
......
...@@ -4,17 +4,21 @@ class Admin::ElasticsearchController < Admin::ApplicationController ...@@ -4,17 +4,21 @@ class Admin::ElasticsearchController < Admin::ApplicationController
before_action :check_elasticsearch_web_indexing_feature_flag! before_action :check_elasticsearch_web_indexing_feature_flag!
def check_elasticsearch_web_indexing_feature_flag! def check_elasticsearch_web_indexing_feature_flag!
render_404 unless Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && Gitlab::Elastic::Helper.index_exists? render_404 unless Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true)
end end
# POST # POST
# Scheduling indexing jobs # Scheduling indexing jobs
def enqueue_index def enqueue_index
::Elastic::IndexProjectsService.new.execute if Gitlab::Elastic::Helper.index_exists?
::Elastic::IndexProjectsService.new.execute
notice = _('Elasticsearch indexing started') notice = _('Elasticsearch indexing started')
queue_link = helpers.link_to(_('(check progress)'), sidekiq_path + '/queues/elastic_full_index') queue_link = helpers.link_to(_('(check progress)'), sidekiq_path + '/queues/elastic_full_index')
flash[:notice] = "#{notice} #{queue_link}".html_safe flash[:notice] = "#{notice} #{queue_link}".html_safe
else
flash[:warning] = _('Please create an index before enabling indexing')
end
redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings') redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
end end
......
...@@ -8,6 +8,8 @@ module EE ...@@ -8,6 +8,8 @@ module EE
override :execute override :execute
def execute def execute
return false if prevent_elasticsearch_indexing_update?
# Repository size limit comes as MB from the view # Repository size limit comes as MB from the view
limit = params.delete(:repository_size_limit) limit = params.delete(:repository_size_limit)
application_setting.repository_size_limit = ::Gitlab::Utils.try_megabytes_to_bytes(limit) if limit application_setting.repository_size_limit = ::Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
...@@ -38,6 +40,14 @@ module EE ...@@ -38,6 +40,14 @@ module EE
# Add new containers # Add new containers
new_container_ids.each { |id| klass.create!(klass.target_attr_name => id) } new_container_ids.each { |id| klass.create!(klass.target_attr_name => id) }
end end
private
def prevent_elasticsearch_indexing_update?
!application_setting.elasticsearch_indexing &&
params[:elasticsearch_indexing].to_i.positive? &&
!::Gitlab::Elastic::Helper.index_exists?
end
end end
end end
end end
...@@ -22,19 +22,16 @@ ...@@ -22,19 +22,16 @@
.sub-section .sub-section
.form-group .form-group
.form-check .form-check
- indexing_enabled = Gitlab::CurrentSettings.elasticsearch_indexing? = f.check_box :elasticsearch_indexing, class: 'form-check-input', data: { qa_selector: 'indexing_checkbox' }
- index_exists = Gitlab::Elastic::Helper.index_exists? rescue false
- disable_indexing_checkbox = !indexing_enabled && !index_exists # Disable this checkbox only when we haven't enabled indexing and we don't have an index
= f.check_box :elasticsearch_indexing, class: 'form-check-input', data: { qa_selector: 'indexing_checkbox' }, disabled: disable_indexing_checkbox
= f.label :elasticsearch_indexing, class: 'form-check-label' do = f.label :elasticsearch_indexing, class: 'form-check-label' do
Elasticsearch indexing Elasticsearch indexing
- unless index_exists - unless Gitlab::CurrentSettings.elasticsearch_indexing?
.form-text.text-warning .form-text.text-muted
= _('Please create an index before enabling indexing') = _('Please create an index before enabling indexing')
- if Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && indexing_enabled - if Feature.enabled?(:elasticsearch_web_indexing, default_enabled: true) && Gitlab::CurrentSettings.elasticsearch_indexing?
.form-text .form-text
= link_to _('Index all projects'), admin_elasticsearch_enqueue_index_path, = link_to _('Index all projects'), admin_elasticsearch_enqueue_index_path,
class: ['btn', 'btn-success', ('disabled' unless index_exists)], method: :post class: ['btn', 'btn-success'], method: :post
.form-group .form-group
.form-check .form-check
......
...@@ -32,7 +32,8 @@ describe Admin::ElasticsearchController do ...@@ -32,7 +32,8 @@ describe Admin::ElasticsearchController do
post :enqueue_index post :enqueue_index
expect(response).to have_gitlab_http_status(:not_found) expect(controller).to set_flash[:warning].to include('create an index before enabling indexing')
expect(response).to redirect_to integrations_admin_application_settings_path(anchor: 'js-elasticsearch-settings')
end end
end end
......
...@@ -34,6 +34,30 @@ describe ApplicationSettings::UpdateService do ...@@ -34,6 +34,30 @@ describe ApplicationSettings::UpdateService do
end end
end end
context 'index dependent' do
using RSpec::Parameterized::TableSyntax
where(:index_exists, :indexing_enabled, :input_value, :result) do
false | false | '1' | false
false | true | '1' | true
true | false | '1' | true
true | true | '1' | true
end
with_them do
before do
allow(Gitlab::Elastic::Helper).to(receive(:index_exists?)).and_return(index_exists)
allow(service.application_setting).to(receive(:elasticsearch_indexing)).and_return(indexing_enabled)
end
let(:opts) { { elasticsearch_indexing: input_value } }
it 'returns correct result' do
expect(service.execute).to be(result)
end
end
end
context 'repository_size_limit assignment as Bytes' do context 'repository_size_limit assignment as Bytes' do
let(:service) { described_class.new(setting, user, opts) } let(:service) { described_class.new(setting, user, opts) }
......
...@@ -12,53 +12,31 @@ describe 'admin/application_settings/_elasticsearch_form' do ...@@ -12,53 +12,31 @@ describe 'admin/application_settings/_elasticsearch_form' do
allow(view).to receive(:expanded) { true } allow(view).to receive(:expanded) { true }
end end
context 'es index dependent' do context 'es indexing' do
let(:application_setting) { build(:application_setting) }
let(:button_text) { 'Index all projects' }
before do before do
allow(Gitlab::CurrentSettings).to(receive(:elasticsearch_indexing?)).and_return(es_indexing) allow(Gitlab::CurrentSettings).to(receive(:elasticsearch_indexing?)).and_return(es_indexing)
allow(Gitlab::Elastic::Helper).to(receive(:index_exists?)).and_return(index_exists)
end end
let(:warning_msg) { 'create an index before enabling indexing' } context 'indexing is enabled' do
let(:button_text) { 'Index all projects' }
let(:application_setting) { build(:application_setting, elasticsearch_indexing: es_indexing) }
context 'when elasticsearch index does not exist with indexing enabled' do
let(:es_indexing) { true } let(:es_indexing) { true }
let(:index_exists) { false }
it 'shows a warning and disables a button' do it 'hides index button when indexing is disabled' do
render render
expect(rendered).to have_content(warning_msg) expect(rendered).to have_css('a.btn-success', text: button_text)
expect(rendered).to have_css('a.btn-success.disabled', text: button_text)
expect(rendered).to have_css('#application_setting_elasticsearch_indexing')
expect(rendered).not_to have_css('#application_setting_elasticsearch_indexing[disabled="disabled"]')
end end
end end
context 'when elasticsearch index does not exist with indexing disabled' do context 'indexing is disabled' do
let(:es_indexing) { false } let(:es_indexing) { false }
let(:index_exists) { false }
it 'shows a warning and disables a checkbox and hides an indexing button' do it 'shows index button when indexing is enabled' do
render render
expect(rendered).to have_content(warning_msg)
expect(rendered).not_to have_css('a.btn-success', text: button_text) expect(rendered).not_to have_css('a.btn-success', text: button_text)
expect(rendered).to have_css('#application_setting_elasticsearch_indexing[disabled="disabled"]')
end
end
context 'when elasticsearch index exists' do
let(:es_indexing) { true }
let(:index_exists) { true }
it 'shows non-disabled index button without a warning' do
render
expect(rendered).to have_css('a.btn-success', text: button_text)
expect(rendered).not_to have_css('a.btn-success.disabled', text: button_text)
expect(rendered).not_to have_content(warning_msg)
end end
end end
end end
......
...@@ -2066,6 +2066,9 @@ msgstr "" ...@@ -2066,6 +2066,9 @@ msgstr ""
msgid "Application settings saved successfully" msgid "Application settings saved successfully"
msgstr "" msgstr ""
msgid "Application settings update failed"
msgstr ""
msgid "Application uninstalled but failed to destroy: %{error_message}" msgid "Application uninstalled but failed to destroy: %{error_message}"
msgstr "" msgstr ""
......
...@@ -11,7 +11,6 @@ describe 'admin/application_settings/integrations.html.haml' do ...@@ -11,7 +11,6 @@ describe 'admin/application_settings/integrations.html.haml' do
before do before do
assign(:application_setting, app_settings) assign(:application_setting, app_settings)
allow(Gitlab::Sourcegraph).to receive(:feature_available?).and_return(sourcegraph_flag) allow(Gitlab::Sourcegraph).to receive(:feature_available?).and_return(sourcegraph_flag)
allow(License).to receive(:feature_available?).with(:elastic_search).and_return(false) if defined?(License)
end end
context 'when sourcegraph feature is enabled' do context 'when sourcegraph feature is enabled' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment