From ff0dfda44e6b0896c31ef7822227877c4c95e12c Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 9 Feb 2023 15:39:53 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-8-stable-ee --- lib/gitlab/gitaly_client/server_service.rb | 13 ------- lib/gitlab/health_checks/gitaly_check.rb | 26 +++----------- .../gitlab/gitaly_client/server_service_spec.rb | 42 ---------------------- spec/lib/gitlab/health_checks/gitaly_check_spec.rb | 22 ++---------- .../gitlab/health_checks/probes/collection_spec.rb | 4 --- spec/requests/health_controller_spec.rb | 4 --- .../repository_check/dispatch_worker_spec.rb | 4 --- 7 files changed, 7 insertions(+), 108 deletions(-) delete mode 100644 spec/lib/gitlab/gitaly_client/server_service_spec.rb diff --git a/lib/gitlab/gitaly_client/server_service.rb b/lib/gitlab/gitaly_client/server_service.rb index 48fd0e66354..36bda67c26e 100644 --- a/lib/gitlab/gitaly_client/server_service.rb +++ b/lib/gitlab/gitaly_client/server_service.rb @@ -26,19 +26,6 @@ module Gitlab storage_specific(disk_statistics) end - def readiness_check - request = Gitaly::ReadinessCheckRequest.new(timeout: GitalyClient.medium_timeout) - response = GitalyClient.call(@storage, :server_service, :readiness_check, request, timeout: GitalyClient.default_timeout) - - return { success: true } if response.ok_response - - failed_checks = response.failure_response.failed_checks.map do |failed_check| - ["#{failed_check.name}: #{failed_check.error_message}"] - end - - { success: false, message: failed_checks.join("\n") } - end - private def storage_specific(response) diff --git a/lib/gitlab/health_checks/gitaly_check.rb b/lib/gitlab/health_checks/gitaly_check.rb index 2bd8ea711b5..f5f142c251f 100644 --- a/lib/gitlab/health_checks/gitaly_check.rb +++ b/lib/gitlab/health_checks/gitaly_check.rb @@ -27,35 +27,17 @@ module Gitlab end def check(storage_name) - storage_healthy = healthy(storage_name) - unless storage_healthy[:success] - return HealthChecks::Result.new( - name, - storage_healthy[:success], - storage_healthy[:message], - shard: storage_name - ) - end + serv = Gitlab::GitalyClient::HealthCheckService.new(storage_name) + result = serv.check - storage_ready = ready(storage_name) HealthChecks::Result.new( name, - storage_ready[:success], - storage_ready[:message], + result[:success], + result[:message], shard: storage_name ) end - def healthy(storage_name) - serv = Gitlab::GitalyClient::HealthCheckService.new(storage_name) - serv.check - end - - def ready(storage_name) - serv = Gitlab::GitalyClient::ServerService.new(storage_name) - serv.readiness_check - end - private def metric_prefix diff --git a/spec/lib/gitlab/gitaly_client/server_service_spec.rb b/spec/lib/gitlab/gitaly_client/server_service_spec.rb deleted file mode 100644 index 615f2ce0c21..00000000000 --- a/spec/lib/gitlab/gitaly_client/server_service_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GitalyClient::ServerService do - let(:storage) { 'default' } - - describe '#readiness_check' do - before do - ::Gitlab::GitalyClient.clear_stubs! - end - - let(:request) do - Gitaly::ReadinessCheckRequest.new(timeout: 30) - end - - subject(:readiness_check) { described_class.new(storage).readiness_check } - - it 'returns a positive success if no failures happened' do - expect_next_instance_of(Gitaly::ServerService::Stub) do |service| - response = Gitaly::ReadinessCheckResponse.new(ok_response: Gitaly::ReadinessCheckResponse::Ok.new) - expect(service).to receive(:readiness_check).with(request, kind_of(Hash)).and_return(response) - end - - expect(readiness_check[:success]).to eq(true) - end - - it 'returns a negative success and a compiled message if at least one failure happened' do - failure1 = Gitaly::ReadinessCheckResponse::Failure::Response.new(name: '1', error_message: 'msg 1') - failure2 = Gitaly::ReadinessCheckResponse::Failure::Response.new(name: '2', error_message: 'msg 2') - failures = Gitaly::ReadinessCheckResponse::Failure.new(failed_checks: [failure1, failure2]) - response = Gitaly::ReadinessCheckResponse.new(failure_response: failures) - - expect_next_instance_of(Gitaly::ServerService::Stub) do |service| - expect(service).to receive(:readiness_check).with(request, kind_of(Hash)).and_return(response) - end - - expect(readiness_check[:success]).to eq(false) - expect(readiness_check[:message]).to eq("1: msg 1\n2: msg 2") - end - 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 948452c0b58..64c4e92f80b 100644 --- a/spec/lib/gitlab/health_checks/gitaly_check_spec.rb +++ b/spec/lib/gitlab/health_checks/gitaly_check_spec.rb @@ -14,36 +14,20 @@ RSpec.describe Gitlab::HealthChecks::GitalyCheck do subject { described_class.readiness } before do - expect(Gitlab::GitalyClient::HealthCheckService).to receive(:new).and_return(healthy_check) + expect(Gitlab::GitalyClient::HealthCheckService).to receive(:new).and_return(gitaly_check) end context 'Gitaly server is up' do - before do - expect(Gitlab::GitalyClient::ServerService).to receive(:new).and_return(ready_check) - end - - let(:healthy_check) { double(check: { success: true }) } - let(:ready_check) { double(readiness_check: { success: true }) } + let(:gitaly_check) { double(check: { success: true }) } it { is_expected.to eq([result_class.new('gitaly_check', true, nil, shard: 'default')]) } end context 'Gitaly server is down' do - let(:healthy_check) { double(check: { success: false, message: 'Connection refused' }) } + let(:gitaly_check) { double(check: { success: false, message: 'Connection refused' }) } it { is_expected.to eq([result_class.new('gitaly_check', false, 'Connection refused', shard: 'default')]) } end - - context 'Gitaly server is not ready' do - before do - expect(Gitlab::GitalyClient::ServerService).to receive(:new).and_return(ready_check) - end - - let(:healthy_check) { double(check: { success: true }) } - let(:ready_check) { double(readiness_check: { success: false, message: 'A readiness check has failed' }) } - - it { is_expected.to match_array([result_class.new('gitaly_check', false, 'A readiness check has failed', shard: 'default')]) } - end end describe '#metrics' do diff --git a/spec/lib/gitlab/health_checks/probes/collection_spec.rb b/spec/lib/gitlab/health_checks/probes/collection_spec.rb index f1791375cea..09e1a828aeb 100644 --- a/spec/lib/gitlab/health_checks/probes/collection_spec.rb +++ b/spec/lib/gitlab/health_checks/probes/collection_spec.rb @@ -18,10 +18,6 @@ RSpec.describe Gitlab::HealthChecks::Probes::Collection do end it 'responds with readiness checks data' do - expect_next_instance_of(Gitlab::GitalyClient::ServerService) do |service| - expect(service).to receive(:readiness_check).and_return({ success: true }) - end - expect(subject.http_status).to eq(200) expect(subject.json[:status]).to eq('ok') diff --git a/spec/requests/health_controller_spec.rb b/spec/requests/health_controller_spec.rb index 83ec1565095..639f6194af9 100644 --- a/spec/requests/health_controller_spec.rb +++ b/spec/requests/health_controller_spec.rb @@ -127,10 +127,6 @@ RSpec.describe HealthController, feature_category: :database do end it 'responds with readiness checks data' do - expect_next_instance_of(Gitlab::GitalyClient::ServerService) do |service| - expect(service).to receive(:readiness_check).and_return({ success: true }) - end - subject expect(json_response['db_check']).to contain_exactly({ 'status' => 'ok' }) diff --git a/spec/workers/repository_check/dispatch_worker_spec.rb b/spec/workers/repository_check/dispatch_worker_spec.rb index 146228c0852..829abc7d895 100644 --- a/spec/workers/repository_check/dispatch_worker_spec.rb +++ b/spec/workers/repository_check/dispatch_worker_spec.rb @@ -22,10 +22,6 @@ RSpec.describe RepositoryCheck::DispatchWorker do end it 'dispatches work to RepositoryCheck::BatchWorker' do - expect_next_instance_of(Gitlab::GitalyClient::ServerService) do |service| - expect(service).to receive(:readiness_check).and_return({ success: true }) - end - expect(RepositoryCheck::BatchWorker).to receive(:perform_async).at_least(:once) subject.perform -- cgit v1.2.1