Allow configuring the circuitbreaker through the API and UIbvl-circuitbreaker-improvements

8 files changed, 100 insertions, 25 deletions

diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 7bd34df5c95..1ee8911bb1a 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -108,6 +108,34 @@ module ApplicationSettingsHelper
     options_for_select(Sidekiq::Queue.all.map(&:name), @application_setting.sidekiq_throttling_queues)
   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_failure_wait_time_help_text
+    _("When access to a storage fails. GitLab will prevent access to the "\
+      "storage for the time specified here. This allows the filesystem to "\
+      "recover. Repositories on failing shards are temporarly unavailable")
+  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 visible_attributes
     [
       :admin_notification_email,
@@ -116,6 +144,10 @@ module ApplicationSettingsHelper
       :akismet_api_key,
       :akismet_enabled,
       :auto_devops_enabled,
+      :circuitbreaker_failure_count_threshold,
+      :circuitbreaker_failure_reset_time,
+      :circuitbreaker_failure_wait_time,
+      :circuitbreaker_storage_timeout,
       :clientside_sentry_dsn,
       :clientside_sentry_enabled,
       :container_registry_token_expire_delay,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index c0cc60d5ebf..a93b777a9bc 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -151,6 +151,13 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             numericality: { greater_than_or_equal_to: 0 }
 
+  validates :circuitbreaker_failure_count_threshold,
+            :circuitbreaker_failure_wait_time,
+            :circuitbreaker_failure_reset_time,
+            :circuitbreaker_storage_timeout,
+            presence: true,
+            numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+
   SUPPORTED_KEY_TYPES.each do |type|
     validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
   end
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index dbaed1d09fb..2b23af9212e 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -530,6 +530,32 @@
           = succeed "." do
             = link_to "repository storages documentation", help_page_path("administration/repository_storages")
 
+  %fieldset
+    %legend Git Storage Circuitbreaker settings
+    .form-group
+      = f.label :circuitbreaker_failure_count_threshold, _('Maximum git storage failures'), class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :circuitbreaker_failure_count_threshold, class: 'form-control'
+        .help-block
+          = circuitbreaker_failure_count_help_text
+    .form-group
+      = f.label :circuitbreaker_failure_wait_time, _('Seconds to wait after a storage failure'), class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :circuitbreaker_failure_wait_time, class: 'form-control'
+        .help-block
+          = circuitbreaker_failure_wait_time_help_text
+    .form-group
+      = f.label :circuitbreaker_failure_reset_time, _('Seconds before reseting failure information'), class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :circuitbreaker_failure_reset_time, class: 'form-control'
+        .help-block
+          = circuitbreaker_failure_reset_time_help_text
+    .form-group
+      = f.label :circuitbreaker_storage_timeout, _('Seconds to wait for a storage access attempt'), class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.number_field :circuitbreaker_storage_timeout, class: 'form-control'
+        .help-block
+          = circuitbreaker_storage_timeout_help_text
 
   %fieldset
     %legend Repository Checks
diff --git a/doc/administration/img/circuitbreaker_config.png b/doc/administration/img/circuitbreaker_config.png
new file mode 100644
index 00000000000..9250d38297c
--- /dev/null
+++ b/doc/administration/img/circuitbreaker_config.png
diff --git a/doc/administration/repository_storage_paths.md b/doc/administration/repository_storage_paths.md
index 86ce87b130d..efcabd69822 100644
--- a/doc/administration/repository_storage_paths.md
+++ b/doc/administration/repository_storage_paths.md
@@ -105,36 +105,26 @@ When GitLab detects access to the repositories storage fails repeatedly, it can
 gracefully prevent attempts to access the storage. This might be useful when
 the repositories are stored somewhere on the network.
 
-The configuration could look as follows:
+This can be configured from the admin interface:
 
-**For Omnibus installations**
-
-1. Edit `/etc/gitlab/gitlab.rb`:
-
-    ```ruby
-    git_data_dirs({
-      "default" => {
-        "path" => "/mnt/nfs-01/git-data"
-       }
-    })
-    ```
+![circuitbreaker configuration](img/circuitbreaker_config.png)
 
-1. Save the file and [reconfigure GitLab][reconfigure-gitlab] for the changes to take effect.
 
----
-
-**For installations from source**
+**Maximum git storage failures:** 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: `https://gitlab.example.com/admin/health_check` or using the
+[api](../api/repository_storage_health.md) to allow access to the storage again.
 
-1. Edit `config/gitlab.yml`:
+**Seconds to wait after a storage failure:** When access to a storage fails. GitLab
+will prevent access to the storage for the time specified here. This allows the
+filesystem to recover.
 
-    ```yaml
-    repositories:
-      storages: # You must have at least a `default` storage path.
-        default:
-          path: /home/git/repositories/
-    ```
+**Seconds before reseting failure information:** The time in seconds GitLab will
+keep failure information. When no failures occur during this time, information about the
+mount is reset.
 
-1. Save the file and [restart GitLab][restart-gitlab] for the changes to take effect.
+**Seconds to wait for a storage access attempt:** The time in seconds GitLab will
+try to access storage. After this time a timeout error will be raised.
 
 When storage failures occur, this will be visible in the admin interface like this:
 
diff --git a/doc/api/settings.md b/doc/api/settings.md
index be75e1b66ba..664f3ef7b77 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -69,6 +69,10 @@ PUT /application/settings
 | `after_sign_up_text`                     | string           | no                                            | Text shown to the user after signing up                                                                                                                                                                                                                                                                                                                                                                                                                   |
 | `akismet_api_key`                        | string           | no                                            | API key for akismet spam protection                                                                                                                                                                                                                                                                                                                                                                                                                       |
 | `akismet_enabled`                        | boolean          | no                                            | Enable or disable akismet spam protection                                                                                                                                                                                                                                                                                                                                                                                                                 |
+| `circuitbreaker_failure_count_threshold` | integer          | no                                            | The number of failures of after which GitLab will completely prevent access to the storage.                                                                                                                                                                                                                                                                                                                                                               |
+| `circuitbreaker_failure_reset_time`      | integer          | no                                            | Time in seconds GitLab will keep storage failure information. When no failures occur during this time, the failure information is reset.                                                                                                                                                                                                                                                                                                                  |
+| `circuitbreaker_failure_wait_time`       | integer          | no                                            | Time in seconds GitLab will block access to a failing storage to allow it to recover.                                                                                                                                                                                                                                                                                                                                                                     |
+| `circuitbreaker_storage_timeout`         | integer          | no                                            | Seconds to wait for a storage access attempt                                                                                                                                                                                                                                                                                                                                                                                                              |
 | `clientside_sentry_dsn`                  | string           | no                                            | Required if `clientside_sentry_dsn` is enabled                                                                                                                                                                                                                                                                                                                                                                                                            |
 | `clientside_sentry_enabled`              | boolean          | no                                            | Enable Sentry error reporting for the client side                                                                                                                                                                                                                                                                                                                                                                                                         |
 | `container_registry_token_expire_delay`  | integer          | no                                            | Container Registry token duration in minutes                                                                                                                                                                                                                                                                                                                                                                                                              |
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 78cacf9ff5d..cf192691507 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -114,6 +114,19 @@ describe ApplicationSetting do
       it { expect(setting.repository_storages).to eq(['default']) }
     end
 
+    context 'circuitbreaker settings' do
+      [:circuitbreaker_failure_count_threshold,
+       :circuitbreaker_failure_wait_time,
+       :circuitbreaker_failure_reset_time,
+       :circuitbreaker_storage_timeout].each do |field|
+        it "Validates #{field} as number" do
+          is_expected.to validate_numericality_of(field)
+                           .only_integer
+                           .is_greater_than_or_equal_to(0)
+        end
+      end
+    end
+
     context 'repository storages' do
       before do
         storages = {
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 0b9a4b5c3db..c24de58ee9d 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -23,6 +23,7 @@ describe API::Settings, 'Settings' do
       expect(json_response['dsa_key_restriction']).to eq(0)
       expect(json_response['ecdsa_key_restriction']).to eq(0)
       expect(json_response['ed25519_key_restriction']).to eq(0)
+      expect(json_response['circuitbreaker_failure_count_threshold']).not_to be_nil
     end
   end
 
@@ -52,7 +53,8 @@ describe API::Settings, 'Settings' do
           rsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE,
           dsa_key_restriction: 2048,
           ecdsa_key_restriction: 384,
-          ed25519_key_restriction: 256
+          ed25519_key_restriction: 256,
+          circuitbreaker_failure_wait_time: 2
 
         expect(response).to have_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
@@ -73,6 +75,7 @@ describe API::Settings, 'Settings' do
         expect(json_response['dsa_key_restriction']).to eq(2048)
         expect(json_response['ecdsa_key_restriction']).to eq(384)
         expect(json_response['ed25519_key_restriction']).to eq(256)
+        expect(json_response['circuitbreaker_failure_wait_time']).to eq(2)
       end
     end