diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-10-17 10:43:05 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-10-17 10:43:05 +0000 |
commit | 4bbdab764d808a86e2e726b4a717a64da8655a10 (patch) | |
tree | 72adb48ca2f33581377de3baa8e9725c928aeda5 /spec | |
parent | 79e889122b9f1cb41eb75ee33e94e625a8c679e2 (diff) | |
parent | 38af7c1613e75561b405b15d6b8db1724da59ef6 (diff) | |
download | gitlab-ce-4bbdab764d808a86e2e726b4a717a64da8655a10.tar.gz |
Merge branch 'bvl-circuitbreaker-improvements' into 'master'
Make the circuitbreaker configurable at runtime
See merge request gitlab-org/gitlab-ce!14842
Diffstat (limited to 'spec')
-rw-r--r-- | spec/features/admin/admin_health_check_spec.rb | 4 | ||||
-rw-r--r-- | spec/initializers/settings_spec.rb | 20 | ||||
-rw-r--r-- | spec/lib/gitlab/git/storage/circuit_breaker_spec.rb | 54 | ||||
-rw-r--r-- | spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/application_setting_spec.rb | 13 | ||||
-rw-r--r-- | spec/requests/api/settings_spec.rb | 5 | ||||
-rw-r--r-- | spec/support/stub_configuration.rb | 4 |
7 files changed, 60 insertions, 44 deletions
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/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 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)) |