From 65840591cd8bdc81281357d062728be9309c5597 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 20 Jun 2018 10:21:59 +0200 Subject: Gitaly metrics check for read/writeability Prior to this change, health checks checked for writeability of the NFS shards. Given we're moving away from that, this patch extends the checks for Gitaly to check for read and writeability. Potentially some dashboards will break, as over time these metrics will no longer appear as Prometheus doesn't get the data anymore. Observability in the circuit breaker will be reduced, but its not expected to be turned on and the circuit breaker is being removed soon too. Closes https://gitlab.com/gitlab-org/gitaly/issues/1218 --- GITALY_SERVER_VERSION | 2 +- Gemfile | 2 +- Gemfile.lock | 4 +- Gemfile.rails5.lock | 4 +- app/controllers/health_controller.rb | 1 - app/services/metrics_service.rb | 3 +- .../unreleased/zj-gitaly-read-write-check.yml | 5 + lib/gitaly/server.rb | 18 +- lib/gitlab/health_checks/fs_shards_check.rb | 169 ----------------- lib/gitlab/health_checks/gitaly_check.rb | 14 +- spec/controllers/health_controller_spec.rb | 4 +- spec/controllers/metrics_controller_spec.rb | 7 + spec/lib/gitaly/server_spec.rb | 34 ++++ .../gitlab/health_checks/fs_shards_check_spec.rb | 200 --------------------- spec/lib/gitlab/health_checks/gitaly_check_spec.rb | 7 +- 15 files changed, 81 insertions(+), 393 deletions(-) create mode 100644 changelogs/unreleased/zj-gitaly-read-write-check.yml delete mode 100644 lib/gitlab/health_checks/fs_shards_check.rb delete mode 100644 spec/lib/gitlab/health_checks/fs_shards_check_spec.rb diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index a9a7f3fec01..3f667dbcd89 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.107.0 +0.108.0 diff --git a/Gemfile b/Gemfile index 93c6115eeec..283886b7187 100644 --- a/Gemfile +++ b/Gemfile @@ -418,7 +418,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.102.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.103.0', require: 'gitaly' gem 'grpc', '~> 1.11.0' # Locked until https://github.com/google/protobuf/issues/4210 is closed diff --git a/Gemfile.lock b/Gemfile.lock index 8281c1eff9a..13f9212c637 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -282,7 +282,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.102.0) + gitaly-proto (0.103.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1041,7 +1041,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.102.0) + gitaly-proto (~> 0.103.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 52388f17c7c..3161e4653ee 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -285,7 +285,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.102.0) + gitaly-proto (0.103.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1051,7 +1051,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.102.0) + gitaly-proto (~> 0.103.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index e54f372344d..3fedd5bfb29 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -8,7 +8,6 @@ class HealthController < ActionController::Base Gitlab::HealthChecks::Redis::CacheCheck, Gitlab::HealthChecks::Redis::QueuesCheck, Gitlab::HealthChecks::Redis::SharedStateCheck, - Gitlab::HealthChecks::FsShardsCheck, Gitlab::HealthChecks::GitalyCheck ].freeze diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index 236e9fe8c44..51ff9eff5e4 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -6,7 +6,8 @@ class MetricsService Gitlab::HealthChecks::Redis::RedisCheck, Gitlab::HealthChecks::Redis::CacheCheck, Gitlab::HealthChecks::Redis::QueuesCheck, - Gitlab::HealthChecks::Redis::SharedStateCheck + Gitlab::HealthChecks::Redis::SharedStateCheck, + Gitlab::HealthChecks::GitalyCheck ].freeze def prometheus_metrics_text diff --git a/changelogs/unreleased/zj-gitaly-read-write-check.yml b/changelogs/unreleased/zj-gitaly-read-write-check.yml new file mode 100644 index 00000000000..43951d20e8f --- /dev/null +++ b/changelogs/unreleased/zj-gitaly-read-write-check.yml @@ -0,0 +1,5 @@ +--- +title: Gitaly metrics check for read/writeability +merge_request: 20022 +author: +type: other diff --git a/lib/gitaly/server.rb b/lib/gitaly/server.rb index 605e93022e7..2760211fee8 100644 --- a/lib/gitaly/server.rb +++ b/lib/gitaly/server.rb @@ -22,6 +22,18 @@ module Gitaly server_version == Gitlab::GitalyClient.expected_server_version end + def read_writeable? + readable? && writeable? + end + + def readable? + storage_status&.readable + end + + def writeable? + storage_status&.writeable + end + def address Gitlab::GitalyClient.address(@storage) rescue RuntimeError => e @@ -30,13 +42,17 @@ module Gitaly private + def storage_status + @storage_status ||= info.storage_statuses.find { |s| s.storage_name == storage } + end + def info @info ||= begin Gitlab::GitalyClient::ServerService.new(@storage).info rescue GRPC::Unavailable, GRPC::GRPC::DeadlineExceeded # This will show the server as being out of date - Gitaly::ServerInfoResponse.new(git_version: '', server_version: '') + Gitaly::ServerInfoResponse.new(git_version: '', server_version: '', storage_statuses: []) end end end diff --git a/lib/gitlab/health_checks/fs_shards_check.rb b/lib/gitlab/health_checks/fs_shards_check.rb deleted file mode 100644 index 050fe7a5173..00000000000 --- a/lib/gitlab/health_checks/fs_shards_check.rb +++ /dev/null @@ -1,169 +0,0 @@ -module Gitlab - module HealthChecks - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1218 - class FsShardsCheck - extend BaseAbstractCheck - RANDOM_STRING = SecureRandom.hex(1000).freeze - COMMAND_TIMEOUT = '1'.freeze - TIMEOUT_EXECUTABLE = 'timeout'.freeze - - class << self - def readiness - repository_storages.map do |storage_name| - begin - if !storage_circuitbreaker_test(storage_name) - HealthChecks::Result.new(false, 'circuitbreaker tripped', shard: storage_name) - elsif !storage_stat_test(storage_name) - HealthChecks::Result.new(false, 'cannot stat storage', shard: storage_name) - else - with_temp_file(storage_name) do |tmp_file_path| - if !storage_write_test(tmp_file_path) - HealthChecks::Result.new(false, 'cannot write to storage', shard: storage_name) - elsif !storage_read_test(tmp_file_path) - HealthChecks::Result.new(false, 'cannot read from storage', shard: storage_name) - else - HealthChecks::Result.new(true, nil, shard: storage_name) - end - end - end - rescue RuntimeError => ex - message = "unexpected error #{ex} when checking storage #{storage_name}" - Rails.logger.error(message) - HealthChecks::Result.new(false, message, shard: storage_name) - end - end - end - - def metrics - repository_storages.flat_map do |storage_name| - [ - storage_stat_metrics(storage_name), - storage_write_metrics(storage_name), - storage_read_metrics(storage_name), - storage_circuitbreaker_metrics(storage_name) - ].flatten - end - end - - private - - def operation_metrics(ok_metric, latency_metric, **labels) - result, elapsed = yield - [ - metric(latency_metric, elapsed, **labels), - metric(ok_metric, result ? 1 : 0, **labels) - ] - rescue RuntimeError => ex - Rails.logger.error("unexpected error #{ex} when checking #{ok_metric}") - [metric(ok_metric, 0, **labels)] - end - - def repository_storages - storages_paths.keys - end - - def storages_paths - Gitlab.config.repositories.storages - end - - def exec_with_timeout(cmd_args, *args, &block) - Gitlab::Popen.popen([TIMEOUT_EXECUTABLE, COMMAND_TIMEOUT].concat(cmd_args), *args, &block) - end - - def with_temp_file(storage_name) - temp_file_path = Dir::Tmpname.create(%w(fs_shards_check +deleted), storage_path(storage_name)) { |path| path } - yield temp_file_path - ensure - delete_test_file(temp_file_path) - end - - def storage_path(storage_name) - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - storages_paths[storage_name]&.legacy_disk_path - end - end - - # All below test methods use shell commands to perform actions on storage volumes. - # In case a storage volume have connectivity problems causing pure Ruby IO operation to wait indefinitely, - # we can rely on shell commands to be terminated once `timeout` kills them. - # - # However we also fallback to pure Ruby file operations in case a specific shell command is missing - # so we are still able to perform healthchecks and gather metrics from such system. - - def delete_test_file(tmp_path) - _, status = exec_with_timeout(%W{ rm -f #{tmp_path} }) - status.zero? - rescue Errno::ENOENT - File.delete(tmp_path) rescue Errno::ENOENT - end - - def storage_stat_test(storage_name) - stat_path = File.join(storage_path(storage_name), '.') - begin - _, status = exec_with_timeout(%W{ stat #{stat_path} }) - status.zero? - rescue Errno::ENOENT - File.exist?(stat_path) && File::Stat.new(stat_path).readable? - end - end - - def storage_write_test(tmp_path) - _, status = exec_with_timeout(%W{ tee #{tmp_path} }) do |stdin| - stdin.write(RANDOM_STRING) - end - status.zero? - rescue Errno::ENOENT - written_bytes = File.write(tmp_path, RANDOM_STRING) rescue Errno::ENOENT - written_bytes == RANDOM_STRING.length - end - - def storage_read_test(tmp_path) - _, status = exec_with_timeout(%W{ diff #{tmp_path} - }) do |stdin| - stdin.write(RANDOM_STRING) - end - status.zero? - rescue Errno::ENOENT - file_contents = File.read(tmp_path) rescue Errno::ENOENT - file_contents == RANDOM_STRING - end - - def storage_circuitbreaker_test(storage_name) - Gitlab::Git::Storage::CircuitBreaker.build(storage_name).perform { "OK" } - rescue Gitlab::Git::Storage::Inaccessible - nil - end - - def storage_stat_metrics(storage_name) - operation_metrics(:filesystem_accessible, :filesystem_access_latency_seconds, shard: storage_name) do - with_timing { storage_stat_test(storage_name) } - end - end - - def storage_write_metrics(storage_name) - operation_metrics(:filesystem_writable, :filesystem_write_latency_seconds, shard: storage_name) do - with_temp_file(storage_name) do |tmp_file_path| - with_timing { storage_write_test(tmp_file_path) } - end - end - end - - def storage_read_metrics(storage_name) - operation_metrics(:filesystem_readable, :filesystem_read_latency_seconds, shard: storage_name) do - with_temp_file(storage_name) do |tmp_file_path| - storage_write_test(tmp_file_path) # writes data used by read test - with_timing { storage_read_test(tmp_file_path) } - end - end - end - - def storage_circuitbreaker_metrics(storage_name) - operation_metrics(:filesystem_circuitbreaker, - :filesystem_circuitbreaker_latency_seconds, - shard: storage_name) do - with_timing { storage_circuitbreaker_test(storage_name) } - end - end - end - end - end -end diff --git a/lib/gitlab/health_checks/gitaly_check.rb b/lib/gitlab/health_checks/gitaly_check.rb index 11416c002e3..1f623e0b6ec 100644 --- a/lib/gitlab/health_checks/gitaly_check.rb +++ b/lib/gitlab/health_checks/gitaly_check.rb @@ -13,14 +13,14 @@ module Gitlab end def metrics - repository_storages.flat_map do |storage_name| - result, elapsed = with_timing { check(storage_name) } - labels = { shard: storage_name } + Gitaly::Server.all.flat_map do |server| + result, elapsed = with_timing { server.read_writeable? } + labels = { shard: server.storage } [ - metric("#{metric_prefix}_success", successful?(result) ? 1 : 0, **labels), + metric("#{metric_prefix}_success", result ? 1 : 0, **labels), metric("#{metric_prefix}_latency_seconds", elapsed, **labels) - ].flatten + ] end end @@ -36,10 +36,6 @@ module Gitlab METRIC_PREFIX end - def successful?(result) - result[:success] - end - def repository_storages storages.keys end diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb index 542eddc2d16..d800ad7c187 100644 --- a/spec/controllers/health_controller_spec.rb +++ b/spec/controllers/health_controller_spec.rb @@ -69,8 +69,7 @@ describe HealthController do expect(json_response['cache_check']['status']).to eq('ok') expect(json_response['queues_check']['status']).to eq('ok') expect(json_response['shared_state_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['labels']['shard']).to eq('default') + expect(json_response['gitaly_check']['status']).to eq('ok') end end @@ -122,7 +121,6 @@ describe HealthController do expect(json_response['cache_check']['status']).to eq('ok') expect(json_response['queues_check']['status']).to eq('ok') expect(json_response['shared_state_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['status']).to eq('ok') end end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 9e8a37171ec..7376841fac8 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -59,6 +59,13 @@ describe MetricsController do expect(response.body).to match(/^redis_shared_state_ping_latency_seconds [0-9\.]+$/) end + it 'returns Gitaly metrics' do + get :index + + expect(response.body).to match(/^gitaly_health_check_success{shard="default"} 1$/) + expect(response.body).to match(/^gitaly_health_check_latency_seconds{shard="default"} [0-9\.]+$/) + end + context 'prometheus metrics are disabled' do before do allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(false) diff --git a/spec/lib/gitaly/server_spec.rb b/spec/lib/gitaly/server_spec.rb index ed5d56e91d4..09bf21b5946 100644 --- a/spec/lib/gitaly/server_spec.rb +++ b/spec/lib/gitaly/server_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitaly::Server do + let(:server) { described_class.new('default') } + describe '.all' do let(:storages) { Gitlab.config.repositories.storages } @@ -17,6 +19,38 @@ describe Gitaly::Server do it { is_expected.to respond_to(:up_to_date?) } it { is_expected.to respond_to(:address) } + describe 'readable?' do + context 'when the storage is readable' do + it 'returns true' do + expect(server).to be_readable + end + end + + context 'when the storage is not readable' do + let(:server) { described_class.new('broken') } + + it 'returns false' do + expect(server).not_to be_readable + end + end + end + + describe 'writeable?' do + context 'when the storage is writeable' do + it 'returns true' do + expect(server).to be_writeable + end + end + + context 'when the storage is not writeable' do + let(:server) { described_class.new('broken') } + + it 'returns false' do + expect(server).not_to be_writeable + end + end + end + describe 'request memoization' do context 'when requesting multiple properties', :request_store do it 'uses memoization for the info request' do diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb deleted file mode 100644 index 9dcf272d25e..00000000000 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ /dev/null @@ -1,200 +0,0 @@ -require 'spec_helper' - -describe Gitlab::HealthChecks::FsShardsCheck do - def command_exists?(command) - _, status = Gitlab::Popen.popen(%W{ #{command} 1 echo }) - status.zero? - rescue Errno::ENOENT - false - end - - def timeout_command - @timeout_command ||= - if command_exists?('timeout') - 'timeout' - elsif command_exists?('gtimeout') - 'gtimeout' - else - '' - end - end - - let(:metric_class) { Gitlab::HealthChecks::Metric } - let(:result_class) { Gitlab::HealthChecks::Result } - let(:repository_storages) { ['default'] } - let(:tmp_dir) { Dir.mktmpdir } - - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => tmp_dir) - }.with_indifferent_access - end - - before do - allow(described_class).to receive(:repository_storages) { repository_storages } - allow(described_class).to receive(:storages_paths) { storages_paths } - stub_const('Gitlab::HealthChecks::FsShardsCheck::TIMEOUT_EXECUTABLE', timeout_command) - end - - after do - FileUtils.remove_entry_secure(tmp_dir) if Dir.exist?(tmp_dir) - end - - shared_examples 'filesystem checks' do - describe '#readiness' do - subject { described_class.readiness } - - context 'storage has a tripped circuitbreaker', :broken_storage do - let(:repository_storages) { ['broken'] } - let(:storages_paths) do - Gitlab.config.repositories.storages - end - - it { is_expected.to include(result_class.new(false, 'circuitbreaker tripped', shard: 'broken')) } - end - - context 'storage points to not existing folder' do - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/this/path/doesnt/exist') - }.with_indifferent_access - end - - before do - allow(described_class).to receive(:storage_circuitbreaker_test) { true } - end - - it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: 'default')) } - end - - context 'storage points to directory that has both read and write rights' do - before do - FileUtils.chmod_R(0755, tmp_dir) - end - - it { is_expected.to include(result_class.new(true, nil, shard: 'default')) } - - it 'cleans up files used for testing' do - expect(described_class).to receive(:storage_write_test).with(any_args).and_call_original - - expect { subject }.not_to change(Dir.entries(tmp_dir), :count) - end - - context 'read test fails' do - before do - allow(described_class).to receive(:storage_read_test).with(any_args).and_return(false) - end - - it { is_expected.to include(result_class.new(false, 'cannot read from storage', shard: 'default')) } - end - - context 'write test fails' do - before do - allow(described_class).to receive(:storage_write_test).with(any_args).and_return(false) - end - - it { is_expected.to include(result_class.new(false, 'cannot write to storage', shard: 'default')) } - end - end - end - - describe '#metrics' do - context 'storage points to not existing folder' do - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/this/path/doesnt/exist') - }.with_indifferent_access - end - - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_circuitbreaker_latency_seconds, value: be >= 0)) - end - end - - context 'storage points to directory that has both read and write rights' do - before do - FileUtils.chmod_R(0755, tmp_dir) - end - - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_circuitbreaker_latency_seconds, value: be >= 0)) - end - - it 'cleans up files used for metrics' do - expect { described_class.metrics }.not_to change(Dir.entries(tmp_dir), :count) - end - end - end - end - - context 'when timeout kills fs checks' do - before do - stub_const('Gitlab::HealthChecks::FsShardsCheck::COMMAND_TIMEOUT', '1') - - allow(described_class).to receive(:exec_with_timeout).and_wrap_original { |m| m.call(%w(sleep 60)) } - FileUtils.chmod_R(0755, tmp_dir) - end - - describe '#readiness' do - subject { described_class.readiness } - - it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: 'default')) } - end - - describe '#metrics' do - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - end - end - end - - context 'when popen always finds required binaries' do - before do - allow(described_class).to receive(:exec_with_timeout).and_wrap_original do |method, *args, &block| - begin - method.call(*args, &block) - rescue RuntimeError, Errno::ENOENT - raise 'expected not to happen' - end - end - - stub_const('Gitlab::HealthChecks::FsShardsCheck::COMMAND_TIMEOUT', '10') - end - - it_behaves_like 'filesystem checks' - end - - context 'when popen never finds required binaries' do - before do - allow(Gitlab::Popen).to receive(:popen).and_raise(Errno::ENOENT) - end - - it_behaves_like 'filesystem checks' - end -end diff --git a/spec/lib/gitlab/health_checks/gitaly_check_spec.rb b/spec/lib/gitlab/health_checks/gitaly_check_spec.rb index 724beefff69..4912cd48761 100644 --- a/spec/lib/gitlab/health_checks/gitaly_check_spec.rb +++ b/spec/lib/gitlab/health_checks/gitaly_check_spec.rb @@ -30,13 +30,14 @@ describe Gitlab::HealthChecks::GitalyCheck do describe '#metrics' do subject { described_class.metrics } + let(:server) { double(storage: 'default', read_writeable?: up) } before do - expect(Gitlab::GitalyClient::HealthCheckService).to receive(:new).and_return(gitaly_check) + allow(Gitaly::Server).to receive(:new).and_return(server) end context 'Gitaly server is up' do - let(:gitaly_check) { double(check: { success: true }) } + let(:up) { true } it 'provides metrics' do expect(subject).to all(have_attributes(labels: { shard: 'default' })) @@ -46,7 +47,7 @@ describe Gitlab::HealthChecks::GitalyCheck do end context 'Gitaly server is down' do - let(:gitaly_check) { double(check: { success: false, message: 'Connection refused' }) } + let(:up) { false } it 'provides metrics' do expect(subject).to include(an_object_having_attributes(name: 'gitaly_health_check_success', value: 0)) -- cgit v1.2.1