diff options
| -rw-r--r-- | lib/gitlab/health_checks/fs_shards_check.rb | 15 | ||||
| -rw-r--r-- | spec/lib/gitlab/health_checks/fs_shards_check_spec.rb | 42 | ||||
| -rw-r--r-- | spec/support/timeout_helper.rb | 23 |
3 files changed, 73 insertions, 7 deletions
diff --git a/lib/gitlab/health_checks/fs_shards_check.rb b/lib/gitlab/health_checks/fs_shards_check.rb index df962d203b7..90612af3d63 100644 --- a/lib/gitlab/health_checks/fs_shards_check.rb +++ b/lib/gitlab/health_checks/fs_shards_check.rb @@ -42,6 +42,7 @@ module Gitlab private RANDOM_STRING = SecureRandom.hex(1000).freeze + COMMAND_TIMEOUT = 1.second def operation_metrics(ok_metric, latency_metric, operation, **labels) with_timing operation do |result, elapsed| @@ -64,7 +65,11 @@ module Gitlab end def with_timeout(args) - %w{timeout 1}.concat(args) + %W{timeout #{COMMAND_TIMEOUT.to_i}}.concat(args) + end + + def exec_with_timeout(cmd_args, *args, &block) + Gitlab::Popen.popen(with_timeout(cmd_args), *args, &block) end def tmp_file_path(storage_name) @@ -78,7 +83,7 @@ module Gitlab def storage_stat_test(storage_name) stat_path = File.join(path(storage_name), '.') begin - _, status = Gitlab::Popen.popen(with_timeout(%W{ stat #{stat_path} })) + _, status = exec_with_timeout(%W{ stat #{stat_path} }) status == 0 rescue Errno::ENOENT File.exist?(stat_path) && File::Stat.new(stat_path).readable? @@ -86,7 +91,7 @@ module Gitlab end def storage_write_test(tmp_path) - _, status = Gitlab::Popen.popen(with_timeout(%W{ tee #{tmp_path} })) do |stdin| + _, status = exec_with_timeout(%W{ tee #{tmp_path} }) do |stdin| stdin.write(RANDOM_STRING) end status == 0 @@ -96,7 +101,7 @@ module Gitlab end def storage_read_test(tmp_path) - _, status = Gitlab::Popen.popen(with_timeout(%W{ diff #{tmp_path} - })) do |stdin| + _, status = exec_with_timeout(%W{ diff #{tmp_path} - }) do |stdin| stdin.write(RANDOM_STRING) end status == 0 @@ -106,7 +111,7 @@ module Gitlab end def delete_test_file(tmp_path) - _, status = Gitlab::Popen.popen(with_timeout(%W{ rm -f #{tmp_path} })) + _, status = exec_with_timeout(%W{ rm -f #{tmp_path} }) status == 0 rescue Errno::ENOENT File.delete(tmp_path) rescue Errno::ENOENT diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb index 45ccd3d6459..289c93ecde7 100644 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::HealthChecks::FsShardsCheck do + include TimeoutHelper + let(:metric_class) { Gitlab::HealthChecks::Metric } let(:result_class) { Gitlab::HealthChecks::Result } let(:repository_storages) { [:default] } @@ -103,15 +105,51 @@ describe Gitlab::HealthChecks::FsShardsCheck do end end + context 'when timeout kills fs checks' do + let(:timeout_seconds) { 1.to_s } + + before do + skip 'timeout or gtimeout not available' unless any_timeout_command_exists? + + allow(described_class).to receive(:with_timeout) { [timeout_command, timeout_seconds].concat(%w{ sleep 2 }) } + 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 + subject { described_class.metrics } + + it { is_expected.to include(metric_class.new(:filesystem_accessible, 0, shard: :default)) } + it { is_expected.to include(metric_class.new(:filesystem_readable, 0, shard: :default)) } + it { is_expected.to include(metric_class.new(:filesystem_writable, 0, shard: :default)) } + + it { is_expected.to include(have_attributes(name: :filesystem_access_latency, value: be >= 0, labels: { shard: :default })) } + it { is_expected.to include(have_attributes(name: :filesystem_read_latency, value: be >= 0, labels: { shard: :default })) } + it { is_expected.to include(have_attributes(name: :filesystem_write_latency, value: be >= 0, labels: { shard: :default })) } + end + end + context 'when popen always finds required binaries' do + let(:timeout_seconds) { 30.to_s } before do - allow(Gitlab::Popen).to receive(:popen).and_wrap_original do |method, *args, &block| + skip 'timeout or gtimeout not available' unless any_timeout_command_exists? + + allow(described_class).to receive(:exec_with_timeout).and_wrap_original do |method, *args, &block| begin method.call(*args, &block) - rescue RuntimeError + rescue RuntimeError, Errno::ENOENT raise 'expected not to happen' end end + + allow(described_class).to receive(:with_timeout) do |args, &block| + [timeout_command, timeout_seconds].concat(args) + end end it_behaves_like 'filesystem checks' diff --git a/spec/support/timeout_helper.rb b/spec/support/timeout_helper.rb new file mode 100644 index 00000000000..8b1c14ad2d5 --- /dev/null +++ b/spec/support/timeout_helper.rb @@ -0,0 +1,23 @@ +module TimeoutHelper + def command_exists?(command) + _, status = Gitlab::Popen.popen(%W{ #{command} 1 echo }) + status == 0 + rescue Errno::ENOENT + false + end + + def any_timeout_command_exists? + command_exists?('timeout') || command_exists?('gtimeout') + end + + def timeout_command + @timeout_command ||= + if command_exists?('timeout') + 'timeout' + elsif command_exists?('gtimeout') + 'gtimeout' + else + '' + end + end +end |