Read circuitbreaker settings from `Gitlab::CurrentSettings`

Instead of from the configuration file

11 files changed, 80 insertions, 106 deletions

diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 1069c7be5f0..4bfa5be0136 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -522,11 +522,6 @@ production: &base
         path: /home/git/repositories/
         gitaly_address: unix:/home/git/gitlab/tmp/sockets/private/gitaly.socket # TCP connections are supported too (e.g. tcp://host:port)
         # gitaly_token: 'special token' # Optional: override global gitaly.token for this storage.
-        failure_count_threshold: 10 # number of failures before stopping attempts
-        failure_wait_time: 30 # Seconds after an access failure before allowing access again
-        failure_reset_time: 1800 # Time in seconds to expire failures
-        storage_timeout: 30 # Time in seconds to wait before aborting a storage access attempt
-
 
   ## Backup settings
   backup:
@@ -659,9 +654,6 @@ test:
       default:
         path: tmp/tests/repositories/
         gitaly_address: unix:tmp/tests/gitaly/gitaly.socket
-        failure_count_threshold: 999999
-        failure_wait_time: 0
-        storage_timeout: 30
       broken:
         path: tmp/tests/non-existent-repositories
         gitaly_address: unix:tmp/tests/gitaly/gitaly.socket
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index a4b7c1a3919..d8d48e3ae6b 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -453,17 +453,6 @@ Settings.repositories.storages.each do |key, storage|
 
   # Expand relative paths
   storage['path'] = Settings.absolute(storage['path'])
-  # Set failure defaults
-  storage['failure_count_threshold'] ||= 10
-  storage['failure_wait_time'] ||= 30
-  storage['failure_reset_time'] ||= 1800
-  storage['storage_timeout'] ||= 5
-  # Set turn strings into numbers
-  storage['failure_count_threshold'] = storage['failure_count_threshold'].to_i
-  storage['failure_wait_time'] = storage['failure_wait_time'].to_i
-  storage['failure_reset_time'] = storage['failure_reset_time'].to_i
-  # We might want to have a timeout shorter than 1 second.
-  storage['storage_timeout'] = storage['storage_timeout'].to_f
 
   Settings.repositories.storages[key] = storage
 end
diff --git a/doc/administration/repository_storage_paths.md b/doc/administration/repository_storage_paths.md
index 624a908b3a3..86ce87b130d 100644
--- a/doc/administration/repository_storage_paths.md
+++ b/doc/administration/repository_storage_paths.md
@@ -114,11 +114,7 @@ The configuration could look as follows:
     ```ruby
     git_data_dirs({
       "default" => {
-        "path" => "/mnt/nfs-01/git-data",
-        "failure_count_threshold" => 10,
-        "failure_wait_time" => 30,
-        "failure_reset_time" => 1800,
-        "storage_timeout" => 5
+        "path" => "/mnt/nfs-01/git-data"
        }
     })
     ```
@@ -136,31 +132,10 @@ The configuration could look as follows:
       storages: # You must have at least a `default` storage path.
         default:
           path: /home/git/repositories/
-          failure_count_threshold: 10 # number of failures before stopping attempts
-          failure_wait_time: 30 # Seconds after last access failure before trying again
-          failure_reset_time: 1800 # Time in seconds to expire failures
-          storage_timeout: 5 # Time in seconds to wait before aborting a storage access attempt
     ```
 
 1. Save the file and [restart GitLab][restart-gitlab] for the changes to take effect.
 
-
-**`failure_count_threshold`:** 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.
-
-**`failure_wait_time`:** When access to a storage fails. GitLab will prevent
-access to the storage for the time specified here. This allows the filesystem to
-recover without.
-
-**`failure_reset_time`:** The time in seconds GitLab will keep failure
-information. When no failures occur during this time, information about the
-mount is reset.
-
-**`storage_timeout`:** 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:
 
 ![failing storage](img/failing_storage.png)
diff --git a/lib/gitlab/git/storage/circuit_breaker.rb b/lib/gitlab/git/storage/circuit_breaker.rb
index 1eaa2d83fb6..4e2d261f461 100644
--- a/lib/gitlab/git/storage/circuit_breaker.rb
+++ b/lib/gitlab/git/storage/circuit_breaker.rb
@@ -2,15 +2,13 @@ module Gitlab
   module Git
     module Storage
       class CircuitBreaker
+        include CircuitBreakerSettings
+
         FailureInfo = Struct.new(:last_failure, :failure_count)
 
         attr_reader :storage,
                     :hostname,
-                    :storage_path,
-                    :failure_count_threshold,
-                    :failure_wait_time,
-                    :failure_reset_time,
-                    :storage_timeout
+                    :storage_path
 
         delegate :last_failure, :failure_count, to: :failure_info
 
@@ -53,10 +51,6 @@ module Gitlab
 
           config = Gitlab.config.repositories.storages[@storage]
           @storage_path = config['path']
-          @failure_count_threshold = config['failure_count_threshold']
-          @failure_wait_time = config['failure_wait_time']
-          @failure_reset_time = config['failure_reset_time']
-          @storage_timeout = config['storage_timeout']
         end
 
         def perform
diff --git a/lib/gitlab/git/storage/circuit_breaker_settings.rb b/lib/gitlab/git/storage/circuit_breaker_settings.rb
new file mode 100644
index 00000000000..d2313fe7c1b
--- /dev/null
+++ b/lib/gitlab/git/storage/circuit_breaker_settings.rb
@@ -0,0 +1,29 @@
+module Gitlab
+  module Git
+    module Storage
+      module CircuitBreakerSettings
+        def failure_count_threshold
+          application_settings.circuitbreaker_failure_count_threshold
+        end
+
+        def failure_wait_time
+          application_settings.circuitbreaker_failure_wait_time
+        end
+
+        def failure_reset_time
+          application_settings.circuitbreaker_failure_reset_time
+        end
+
+        def storage_timeout
+          application_settings.circuitbreaker_storage_timeout
+        end
+
+        private
+
+        def application_settings
+          Gitlab::CurrentSettings.current_application_settings
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/git/storage/null_circuit_breaker.rb b/lib/gitlab/git/storage/null_circuit_breaker.rb
index 297c043d054..60c6791a7e4 100644
--- a/lib/gitlab/git/storage/null_circuit_breaker.rb
+++ b/lib/gitlab/git/storage/null_circuit_breaker.rb
@@ -2,15 +2,14 @@ module Gitlab
   module Git
     module Storage
       class NullCircuitBreaker
+        include CircuitBreakerSettings
+
         # These will have actual values
         attr_reader :storage,
                     :hostname
 
         # These will always have nil values
-        attr_reader :storage_path,
-                    :failure_wait_time,
-                    :failure_reset_time,
-                    :storage_timeout
+        attr_reader :storage_path
 
         def initialize(storage, hostname, error: nil)
           @storage = storage
@@ -26,16 +25,12 @@ module Gitlab
           !!@error
         end
 
-        def failure_count_threshold
-          1
-        end
-
         def last_failure
           circuit_broken? ? Time.now : nil
         end
 
         def failure_count
-          circuit_broken? ? 1 : 0
+          circuit_broken? ? failure_count_threshold : 0
         end
 
         def failure_info
diff --git a/spec/features/admin/admin_health_check_spec.rb b/spec/features/admin/admin_health_check_spec.rb
index 09e6965849a..4430fc15501 100644
--- a/spec/features/admin/admin_health_check_spec.rb
+++ b/spec/features/admin/admin_health_check_spec.rb
@@ -65,9 +65,11 @@ feature "Admin Health Check", :feature, :broken_storage do
 
     it 'shows storage failure information' do
       hostname = Gitlab::Environment.hostname
+      maximum_failures = Gitlab::CurrentSettings.current_application_settings
+                           .circuitbreaker_failure_count_threshold
 
       expect(page).to have_content('broken: failed storage access attempt on host:')
-      expect(page).to have_content("#{hostname}: 1 of 10 failures.")
+      expect(page).to have_content("#{hostname}: 1 of #{maximum_failures} failures.")
     end
 
     it 'allows resetting storage failures' do
diff --git a/spec/initializers/settings_spec.rb b/spec/initializers/settings_spec.rb
index 9a974e70e8c..a11824d0ac5 100644
--- a/spec/initializers/settings_spec.rb
+++ b/spec/initializers/settings_spec.rb
@@ -18,26 +18,6 @@ describe Settings do
     end
   end
 
-  describe '#repositories' do
-    it 'assigns the default failure attributes' do
-      repository_settings = Gitlab.config.repositories.storages['broken']
-
-      expect(repository_settings['failure_count_threshold']).to eq(10)
-      expect(repository_settings['failure_wait_time']).to eq(30)
-      expect(repository_settings['failure_reset_time']).to eq(1800)
-      expect(repository_settings['storage_timeout']).to eq(5)
-    end
-
-    it 'can be accessed with dot syntax all the way down' do
-      expect(Gitlab.config.repositories.storages.broken.failure_count_threshold).to eq(10)
-    end
-
-    it 'can be accessed in a very specific way that breaks without reassigning each element with Settingslogic' do
-      storage_settings = Gitlab.config.repositories.storages['broken']
-      expect(storage_settings.failure_count_threshold).to eq(10)
-    end
-  end
-
   describe '#host_without_www' do
     context 'URL with protocol' do
       it 'returns the host' do
diff --git a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb
index 98cf7966dad..68eb93eb9f9 100644
--- a/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb
+++ b/spec/lib/gitlab/git/storage/circuit_breaker_spec.rb
@@ -10,18 +10,10 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
     # Override test-settings for the circuitbreaker with something more realistic
     # for these specs.
     stub_storage_settings('default' => {
-                            'path' => TestEnv.repos_path,
-                            'failure_count_threshold' => 10,
-                            'failure_wait_time' => 30,
-                            'failure_reset_time' => 1800,
-                            'storage_timeout' => 5
+                            'path' => TestEnv.repos_path
                           },
                           'broken' => {
-                            'path' => 'tmp/tests/non-existent-repositories',
-                            'failure_count_threshold' => 10,
-                            'failure_wait_time' => 30,
-                            'failure_reset_time' => 1800,
-                            'storage_timeout' => 5
+                            'path' => 'tmp/tests/non-existent-repositories'
                           },
                           'nopath' => { 'path' => nil }
                          )
@@ -75,10 +67,39 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
       expect(circuit_breaker.hostname).to eq(hostname)
       expect(circuit_breaker.storage).to eq('default')
       expect(circuit_breaker.storage_path).to eq(TestEnv.repos_path)
-      expect(circuit_breaker.failure_count_threshold).to eq(10)
-      expect(circuit_breaker.failure_wait_time).to eq(30)
-      expect(circuit_breaker.failure_reset_time).to eq(1800)
-      expect(circuit_breaker.storage_timeout).to eq(5)
+    end
+  end
+
+  context 'circuitbreaker settings' do
+    before do
+      stub_application_setting(circuitbreaker_failure_count_threshold: 0,
+                               circuitbreaker_failure_wait_time: 1,
+                               circuitbreaker_failure_reset_time: 2,
+                               circuitbreaker_storage_timeout: 3)
+    end
+
+    describe '#failure_count_threshold' do
+      it 'reads the value from settings' do
+        expect(circuit_breaker.failure_count_threshold).to eq(0)
+      end
+    end
+
+    describe '#failure_wait_time' do
+      it 'reads the value from settings' do
+        expect(circuit_breaker.failure_wait_time).to eq(1)
+      end
+    end
+
+    describe '#failure_reset_time' do
+      it 'reads the value from settings' do
+        expect(circuit_breaker.failure_reset_time).to eq(2)
+      end
+    end
+
+    describe '#storage_timeout' do
+      it 'reads the value from settings' do
+        expect(circuit_breaker.storage_timeout).to eq(3)
+      end
     end
   end
 
@@ -151,10 +172,7 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
 
     context 'the `failure_wait_time` is set to 0' do
       before do
-        stub_storage_settings('default' => {
-                                'failure_wait_time' => 0,
-                                'path' => TestEnv.repos_path
-                              })
+        stub_application_setting(circuitbreaker_failure_wait_time: 0)
       end
 
       it 'is working even when there is a recent failure' do
diff --git a/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb b/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb
index 0e645008c88..7ee6d2f3709 100644
--- a/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb
+++ b/spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb
@@ -54,6 +54,10 @@ describe Gitlab::Git::Storage::NullCircuitBreaker do
   end
 
   describe '#failure_count_threshold' do
+    before do
+      stub_application_setting(circuitbreaker_failure_count_threshold: 1)
+    end
+
     it { expect(breaker.failure_count_threshold).to eq(1) }
   end
 
diff --git a/spec/support/stub_configuration.rb b/spec/support/stub_configuration.rb
index 2dfb4d4a07f..4d448a55978 100644
--- a/spec/support/stub_configuration.rb
+++ b/spec/support/stub_configuration.rb
@@ -43,10 +43,6 @@ module StubConfiguration
     messages['default'] ||= Gitlab.config.repositories.storages.default
     messages.each do |storage_name, storage_settings|
       storage_settings['path'] = TestEnv.repos_path unless storage_settings.key?('path')
-      storage_settings['failure_count_threshold'] ||= 10
-      storage_settings['failure_wait_time'] ||= 30
-      storage_settings['failure_reset_time'] ||= 1800
-      storage_settings['storage_timeout'] ||= 5
     end
 
     allow(Gitlab.config.repositories).to receive(:storages).and_return(Settingslogic.new(messages))