diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2018-10-09 07:59:42 +0200 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2018-10-10 09:08:18 +0200 |
commit | 30b4ce940d28804e0b38ea9ea4f89793d41392db (patch) | |
tree | ecbf29b27a726867d260521dc799214a4cd6d4c4 /app | |
parent | 550f55745a3be5f86bafaf25b3bcc90beba8e2ac (diff) | |
download | gitlab-ce-30b4ce940d28804e0b38ea9ea4f89793d41392db.tar.gz |
Remove Git circuit breaker
Was introduced in the time that GitLab still used NFS, which is not
required anymore in most cases. By removing this, the API it calls will
return empty responses. This interface has to be removed in the next
major release, expected to be 12.0.
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/admin/health_check_controller.rb | 7 | ||||
-rw-r--r-- | app/controllers/application_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/health_controller.rb | 11 | ||||
-rw-r--r-- | app/helpers/application_settings_helper.rb | 36 | ||||
-rw-r--r-- | app/helpers/storage_health_helper.rb | 34 | ||||
-rw-r--r-- | app/models/application_setting.rb | 18 | ||||
-rw-r--r-- | app/views/admin/application_settings/_repository_storage.html.haml | 27 | ||||
-rw-r--r-- | app/views/admin/application_settings/repository.html.haml | 2 | ||||
-rw-r--r-- | app/views/admin/health_check/_failing_storages.html.haml | 15 | ||||
-rw-r--r-- | app/views/admin/health_check/show.html.haml | 3 |
10 files changed, 11 insertions, 144 deletions
diff --git a/app/controllers/admin/health_check_controller.rb b/app/controllers/admin/health_check_controller.rb index 44864f9c7d0..25cc241e5b0 100644 --- a/app/controllers/admin/health_check_controller.rb +++ b/app/controllers/admin/health_check_controller.rb @@ -3,12 +3,5 @@ class Admin::HealthCheckController < Admin::ApplicationController def show @errors = HealthCheck::Utils.process_checks(['standard']) - @failing_storage_statuses = Gitlab::Git::Storage::Health.for_failing_storages - end - - def reset_storage_health - Gitlab::Git::Storage::FailureInfo.reset_all! - redirect_to admin_health_check_path, - notice: _('Git storage health information has been reset') end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ec45e2813c5..bbeaeb7694e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -66,7 +66,7 @@ class ApplicationController < ActionController::Base head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window end - rescue_from Gitlab::Git::Storage::Inaccessible, GRPC::Unavailable, Gitlab::Git::CommandError do |exception| + rescue_from GRPC::Unavailable, Gitlab::Git::CommandError do |exception| log_exception(exception) headers['Retry-After'] = exception.retry_after if exception.respond_to?(:retry_after) diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index ab4bc911e17..dc9a52f8da5 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class HealthController < ActionController::Base - protect_from_forgery with: :exception, except: :storage_check, prepend: true + protect_from_forgery with: :exception, prepend: true include RequiresWhitelistedMonitoringClient CHECKS = [ @@ -25,15 +25,6 @@ class HealthController < ActionController::Base render_check_results(results) end - def storage_check - results = Gitlab::Git::Storage::Checker.check_all - - render json: { - check_interval: Gitlab::CurrentSettings.current_application_settings.circuitbreaker_check_interval, - results: results - } - end - private def render_check_results(results) diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 15cbfeea609..d6753e46165 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -108,37 +108,6 @@ module ApplicationSettingsHelper options_for_select(options, selected) end - def circuitbreaker_failure_count_help_text - health_link = link_to(s_('AdminHealthPageLink|health page'), admin_health_check_path) - api_link = link_to(s_('CircuitBreakerApiLink|circuitbreaker api'), help_page_path("api/repository_storage_health")) - message = _("The number of failures of after which GitLab will completely "\ - "prevent access to the storage. The number of failures can be "\ - "reset in the admin interface: %{link_to_health_page} or using "\ - "the %{api_documentation_link}.") - message = message % { link_to_health_page: health_link, api_documentation_link: api_link } - - message.html_safe - end - - def circuitbreaker_access_retries_help_text - _('The number of attempts GitLab will make to access a storage.') - end - - def circuitbreaker_failure_reset_time_help_text - _("The time in seconds GitLab will keep failure information. When no "\ - "failures occur during this time, information about the mount is reset.") - end - - def circuitbreaker_storage_timeout_help_text - _("The time in seconds GitLab will try to access storage. After this time a "\ - "timeout error will be raised.") - end - - def circuitbreaker_check_interval_help_text - _("The time in seconds between storage checks. When a previous check did "\ - "complete yet, GitLab will skip a check.") - end - def visible_attributes [ :admin_notification_email, @@ -150,11 +119,6 @@ module ApplicationSettingsHelper :authorized_keys_enabled, :auto_devops_enabled, :auto_devops_domain, - :circuitbreaker_access_retries, - :circuitbreaker_check_interval, - :circuitbreaker_failure_count_threshold, - :circuitbreaker_failure_reset_time, - :circuitbreaker_storage_timeout, :clientside_sentry_dsn, :clientside_sentry_enabled, :container_registry_token_expire_delay, diff --git a/app/helpers/storage_health_helper.rb b/app/helpers/storage_health_helper.rb deleted file mode 100644 index 182e8e6641b..00000000000 --- a/app/helpers/storage_health_helper.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module StorageHealthHelper - def failing_storage_health_message(storage_health) - storage_name = content_tag(:strong, h(storage_health.storage_name)) - host_names = h(storage_health.failing_on_hosts.to_sentence) - translation_params = { storage_name: storage_name, - host_names: host_names, - failed_attempts: storage_health.total_failures } - - translation = n_('%{storage_name}: failed storage access attempt on host:', - '%{storage_name}: %{failed_attempts} failed storage access attempts:', - storage_health.total_failures) % translation_params - - translation.html_safe - end - - def message_for_circuit_breaker(circuit_breaker) - maximum_failures = circuit_breaker.failure_count_threshold - current_failures = circuit_breaker.failure_count - - translation_params = { number_of_failures: current_failures, - maximum_failures: maximum_failures } - - if circuit_breaker.circuit_broken? - s_("%{number_of_failures} of %{maximum_failures} failures. GitLab will not "\ - "retry automatically. Reset storage information when the problem is "\ - "resolved.") % translation_params - else - _("%{number_of_failures} of %{maximum_failures} failures. GitLab will "\ - "allow access on the next attempt.") % translation_params - end - end -end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 65a2f760f93..23131af1b7d 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -4,6 +4,7 @@ class ApplicationSetting < ActiveRecord::Base include CacheableAttributes include CacheMarkdownField include TokenAuthenticatable + include IgnorableColumn add_authentication_token_field :runners_registration_token add_authentication_token_field :health_check_access_token @@ -27,6 +28,12 @@ class ApplicationSetting < ActiveRecord::Base serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize + ignore_column :circuitbreaker_failure_count_threshold + ignore_column :circuitbreaker_failure_reset_time + ignore_column :circuitbreaker_storage_timeout + ignore_column :circuitbreaker_access_retries + ignore_column :circuitbreaker_check_interval + cache_markdown_field :sign_in_text cache_markdown_field :help_page_text cache_markdown_field :shared_runners_text, pipeline: :plain_markdown @@ -150,17 +157,6 @@ class ApplicationSetting < ActiveRecord::Base presence: true, numericality: { greater_than_or_equal_to: 0 } - validates :circuitbreaker_failure_count_threshold, - :circuitbreaker_failure_reset_time, - :circuitbreaker_storage_timeout, - :circuitbreaker_check_interval, - presence: true, - numericality: { only_integer: true, greater_than_or_equal_to: 0 } - - validates :circuitbreaker_access_retries, - presence: true, - numericality: { only_integer: true, greater_than_or_equal_to: 1 } - validates :gitaly_timeout_default, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 } diff --git a/app/views/admin/application_settings/_repository_storage.html.haml b/app/views/admin/application_settings/_repository_storage.html.haml index 908b30cc3ce..c6c29ed1f21 100644 --- a/app/views/admin/application_settings/_repository_storage.html.haml +++ b/app/views/admin/application_settings/_repository_storage.html.haml @@ -20,32 +20,5 @@ Manage repository storage paths. Learn more in the = succeed "." do = link_to "repository storages documentation", help_page_path("administration/repository_storage_paths") - .sub-section - %h4 Circuit breaker - .form-group - = f.label :circuitbreaker_check_interval, _('Check interval'), class: 'label-bold' - = f.number_field :circuitbreaker_check_interval, class: 'form-control' - .form-text.text-muted - = circuitbreaker_check_interval_help_text - .form-group - = f.label :circuitbreaker_access_retries, _('Number of access attempts'), class: 'label-bold' - = f.number_field :circuitbreaker_access_retries, class: 'form-control' - .form-text.text-muted - = circuitbreaker_access_retries_help_text - .form-group - = f.label :circuitbreaker_storage_timeout, _('Seconds to wait for a storage access attempt'), class: 'label-bold' - = f.number_field :circuitbreaker_storage_timeout, class: 'form-control' - .form-text.text-muted - = circuitbreaker_storage_timeout_help_text - .form-group - = f.label :circuitbreaker_failure_count_threshold, _('Maximum git storage failures'), class: 'label-bold' - = f.number_field :circuitbreaker_failure_count_threshold, class: 'form-control' - .form-text.text-muted - = circuitbreaker_failure_count_help_text - .form-group - = f.label :circuitbreaker_failure_reset_time, _('Seconds before reseting failure information'), class: 'label-bold' - = f.number_field :circuitbreaker_failure_reset_time, class: 'form-control' - .form-text.text-muted - = circuitbreaker_failure_reset_time_help_text = f.submit 'Save changes', class: "btn btn-success qa-save-changes-button" diff --git a/app/views/admin/application_settings/repository.html.haml b/app/views/admin/application_settings/repository.html.haml index be13138a764..b50a0dd5a18 100644 --- a/app/views/admin/application_settings/repository.html.haml +++ b/app/views/admin/application_settings/repository.html.haml @@ -20,7 +20,7 @@ %button.btn.btn-default.js-settings-toggle{ type: 'button' } = expanded_by_default? ? _('Collapse') : _('Expand') %p - = _('Configure storage path and circuit breaker settings.') + = _('Configure storage path settings.') .settings-content = render 'repository_storage' diff --git a/app/views/admin/health_check/_failing_storages.html.haml b/app/views/admin/health_check/_failing_storages.html.haml deleted file mode 100644 index 6830201538d..00000000000 --- a/app/views/admin/health_check/_failing_storages.html.haml +++ /dev/null @@ -1,15 +0,0 @@ -- if failing_storages.any? - = _('There are problems accessing Git storage: ') - %ul - - failing_storages.each do |storage_health| - %li - = failing_storage_health_message(storage_health) - %ul - - storage_health.failing_circuit_breakers.each do |circuit_breaker| - %li - #{circuit_breaker.hostname}: #{message_for_circuit_breaker(circuit_breaker)} - - = _("Access to failing storages has been temporarily disabled to allow the mount to recover. Reset storage information after the issue has been resolved to allow access again.") - .prepend-top-10 - = button_to _("Reset git storage health information"), reset_storage_health_admin_health_check_path, - method: :post, class: 'btn btn-default' diff --git a/app/views/admin/health_check/show.html.haml b/app/views/admin/health_check/show.html.haml index d51ac854b04..0f5e97e288a 100644 --- a/app/views/admin/health_check/show.html.haml +++ b/app/views/admin/health_check/show.html.haml @@ -1,6 +1,6 @@ - @no_container = true - page_title _('Health Check') -- no_errors = @errors.blank? && @failing_storage_statuses.blank? +- no_errors = @errors.blank? %div{ class: container_class } %h3.page-title= page_title @@ -39,4 +39,3 @@ #{ s_('HealthCheck|No Health Problems Detected') } - else = @errors - = render partial: 'failing_storages', object: @failing_storage_statuses |