Merge commit '5b08d59f07fc53c1e34819fac20352119d5343e6' into object-storage-ee-to-ce-backport

7 files changed, 76 insertions, 44 deletions

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 b333e162909..26574df8bb5 100644
--- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb
+++ b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Gitlab::HealthChecks::FsShardsCheck do
   def command_exists?(command)
     _, status = Gitlab::Popen.popen(%W{ #{command} 1 echo })
-    status == 0
+    status.zero?
   rescue Errno::ENOENT
     false
   end
@@ -44,6 +44,15 @@ describe Gitlab::HealthChecks::FsShardsCheck do
     describe '#readiness' do
       subject { described_class.readiness }
 
+      context 'storage has a tripped circuitbreaker', broken_storage: true 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
           {
@@ -51,6 +60,10 @@ describe Gitlab::HealthChecks::FsShardsCheck do
           }.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
 
@@ -64,9 +77,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
         it 'cleans up files used for testing' do
           expect(described_class).to receive(:storage_write_test).with(any_args).and_call_original
 
-          subject
-
-          expect(Dir.entries(tmp_dir).count).to eq(2)
+          expect { subject }.not_to change(Dir.entries(tmp_dir), :count)
         end
 
         context 'read test fails' do
@@ -88,8 +99,6 @@ describe Gitlab::HealthChecks::FsShardsCheck do
     end
 
     describe '#metrics' do
-      subject { described_class.metrics }
-
       context 'storage points to not existing folder' do
         let(:storages_paths) do
           {
@@ -104,14 +113,16 @@ describe Gitlab::HealthChecks::FsShardsCheck do
         end
 
         it 'provides metrics' do
-          expect(subject).to all(have_attributes(labels: { shard: :default }))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 0))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 0))
-
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency, value: be >= 0))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency, value: be >= 0))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency, value: be >= 0))
+          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
 
@@ -121,15 +132,20 @@ describe Gitlab::HealthChecks::FsShardsCheck do
         end
 
         it 'provides metrics' do
-          expect(subject).to all(have_attributes(labels: { shard: :default }))
-
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_accessible, value: 1))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 1))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 1))
+          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
 
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency, value: be >= 0))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency, value: be >= 0))
-          expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency, value: be >= 0))
+        it 'cleans up files used for metrics' do
+          expect { described_class.metrics }.not_to change(Dir.entries(tmp_dir), :count)
         end
       end
     end
@@ -150,18 +166,16 @@ describe Gitlab::HealthChecks::FsShardsCheck do
     end
 
     describe '#metrics' do
-      subject { described_class.metrics }
-
       it 'provides metrics' do
-        expect(subject).to all(have_attributes(labels: { shard: :default }))
-
-        expect(subject).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0))
-        expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 0))
-        expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 0))
-
-        expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency, value: be >= 0))
-        expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency, value: be >= 0))
-        expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency, value: be >= 0))
+        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
diff --git a/spec/lib/gitlab/health_checks/redis/cache_check_spec.rb b/spec/lib/gitlab/health_checks/redis/cache_check_spec.rb
new file mode 100644
index 00000000000..3693f52b51b
--- /dev/null
+++ b/spec/lib/gitlab/health_checks/redis/cache_check_spec.rb
@@ -0,0 +1,6 @@
+require 'spec_helper'
+require_relative '../simple_check_shared'
+
+describe Gitlab::HealthChecks::Redis::CacheCheck do
+  include_examples 'simple_check', 'redis_cache_ping', 'RedisCache', 'PONG'
+end
diff --git a/spec/lib/gitlab/health_checks/redis/queues_check_spec.rb b/spec/lib/gitlab/health_checks/redis/queues_check_spec.rb
new file mode 100644
index 00000000000..c69443d205d
--- /dev/null
+++ b/spec/lib/gitlab/health_checks/redis/queues_check_spec.rb
@@ -0,0 +1,6 @@
+require 'spec_helper'
+require_relative '../simple_check_shared'
+
+describe Gitlab::HealthChecks::Redis::QueuesCheck do
+  include_examples 'simple_check', 'redis_queues_ping', 'RedisQueues', 'PONG'
+end
diff --git a/spec/lib/gitlab/health_checks/redis/redis_check_spec.rb b/spec/lib/gitlab/health_checks/redis/redis_check_spec.rb
new file mode 100644
index 00000000000..03afc1cd761
--- /dev/null
+++ b/spec/lib/gitlab/health_checks/redis/redis_check_spec.rb
@@ -0,0 +1,6 @@
+require 'spec_helper'
+require_relative '../simple_check_shared'
+
+describe Gitlab::HealthChecks::Redis::RedisCheck do
+  include_examples 'simple_check', 'redis_ping', 'Redis', 'PONG'
+end
diff --git a/spec/lib/gitlab/health_checks/redis/shared_state_check_spec.rb b/spec/lib/gitlab/health_checks/redis/shared_state_check_spec.rb
new file mode 100644
index 00000000000..b72e152bbe2
--- /dev/null
+++ b/spec/lib/gitlab/health_checks/redis/shared_state_check_spec.rb
@@ -0,0 +1,6 @@
+require 'spec_helper'
+require_relative '../simple_check_shared'
+
+describe Gitlab::HealthChecks::Redis::SharedStateCheck do
+  include_examples 'simple_check', 'redis_shared_state_ping', 'RedisSharedState', 'PONG'
+end
diff --git a/spec/lib/gitlab/health_checks/redis_check_spec.rb b/spec/lib/gitlab/health_checks/redis_check_spec.rb
deleted file mode 100644
index 734cdcb893e..00000000000
--- a/spec/lib/gitlab/health_checks/redis_check_spec.rb
+++ /dev/null
@@ -1,6 +0,0 @@
-require 'spec_helper'
-require_relative './simple_check_shared'
-
-describe Gitlab::HealthChecks::RedisCheck do
-  include_examples 'simple_check', 'redis_ping', 'Redis', 'PONG'
-end
diff --git a/spec/lib/gitlab/health_checks/simple_check_shared.rb b/spec/lib/gitlab/health_checks/simple_check_shared.rb
index 3f871d66034..e2643458aca 100644
--- a/spec/lib/gitlab/health_checks/simple_check_shared.rb
+++ b/spec/lib/gitlab/health_checks/simple_check_shared.rb
@@ -8,7 +8,7 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result|
 
       it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 1)) }
       it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 0)) }
-      it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be >= 0)) }
+      it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency_seconds", value: be >= 0)) }
     end
 
     context 'Check is misbehaving' do
@@ -18,7 +18,7 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result|
 
       it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 0)) }
       it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 0)) }
-      it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be >= 0)) }
+      it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency_seconds", value: be >= 0)) }
     end
 
     context 'Check is timeouting' do
@@ -28,7 +28,7 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result|
 
       it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 0)) }
       it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 1)) }
-      it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be >= 0)) }
+      it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency_seconds", value: be >= 0)) }
     end
   end
 
@@ -47,7 +47,7 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result|
         allow(described_class).to receive(:check).and_return 'error!'
       end
 
-      it { is_expected.to have_attributes(success: false, message: "unexpected #{check_name} check result: error!") }
+      it { is_expected.to have_attributes(success: false, message: "unexpected #{described_class.human_name} check result: error!") }
     end
 
     context 'Check is timeouting' do
@@ -55,7 +55,7 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result|
         allow(described_class).to receive(:check ).and_return Timeout::Error.new
       end
 
-      it { is_expected.to have_attributes(success: false, message: "#{check_name} check timed out") }
+      it { is_expected.to have_attributes(success: false, message: "#{described_class.human_name} check timed out") }
     end
   end