diff --git a/lib/cloud_controller/config_schemas/base/api_schema.rb b/lib/cloud_controller/config_schemas/base/api_schema.rb index f484145847a..0332eeafb93 100644 --- a/lib/cloud_controller/config_schemas/base/api_schema.rb +++ b/lib/cloud_controller/config_schemas/base/api_schema.rb @@ -211,6 +211,7 @@ class ApiSchema < VCAP::Config logcache: { host: String, port: Integer, + optional(:timeout_in_seconds) => Integer, }, optional(:logcache_tls) => { diff --git a/lib/cloud_controller/dependency_locator.rb b/lib/cloud_controller/dependency_locator.rb index 5c2ec46258b..608271b4a88 100644 --- a/lib/cloud_controller/dependency_locator.rb +++ b/lib/cloud_controller/dependency_locator.rb @@ -516,6 +516,7 @@ def build_logcache_client Logcache::Client.new( host: config.get(:logcache, :host), port: config.get(:logcache, :port), + timeout: config.get(:logcache, :timeout_in_seconds), client_ca_path: config.get(:logcache_tls, :ca_file), client_cert_path: config.get(:logcache_tls, :cert_file), client_key_path: config.get(:logcache_tls, :key_file), diff --git a/lib/logcache/client.rb b/lib/logcache/client.rb index 0ab0ca7f40e..c4c8f223e33 100644 --- a/lib/logcache/client.rb +++ b/lib/logcache/client.rb @@ -6,7 +6,9 @@ class Client MAX_LIMIT = 1000 DEFAULT_LIMIT = 100 - def initialize(host:, port:, client_ca_path:, client_cert_path:, client_key_path:, tls_subject_name:) + def initialize(host:, port:, timeout:, client_ca_path:, client_cert_path:, client_key_path:, tls_subject_name:) + timeout ||= 250 + if client_ca_path client_ca = IO.read(client_ca_path) client_key = IO.read(client_key_path) @@ -16,13 +18,13 @@ def initialize(host:, port:, client_ca_path:, client_cert_path:, client_key_path "#{host}:#{port}", GRPC::Core::ChannelCredentials.new(client_ca, client_key, client_cert), channel_args: { GRPC::Core::Channel::SSL_TARGET => tls_subject_name }, - timeout: 250 + timeout: timeout ) else @service = Logcache::V1::Egress::Stub.new( "#{host}:#{port}", :this_channel_is_insecure, - timeout: 250 + timeout: timeout ) end end diff --git a/spec/logcache/client_spec.rb b/spec/logcache/client_spec.rb index 24c7aaf29b7..fa0aaf43c0e 100644 --- a/spec/logcache/client_spec.rb +++ b/spec/logcache/client_spec.rb @@ -10,6 +10,7 @@ module Logcache let(:host) { 'doppler.service.cf.internal' } let(:port) { '8080' } + let(:timeout) { nil } let(:expected_request_options) { { 'headers' => { 'Authorization' => 'bearer oauth-token' } } } describe 'with TLS' do @@ -20,7 +21,7 @@ module Logcache let(:credentials) { instance_double(GRPC::Core::ChannelCredentials) } let(:channel_arg_hash) { { GRPC::Core::Channel::SSL_TARGET => tls_subject_name } } let(:client) do - Logcache::Client.new(host: host, port: port, client_ca_path: client_ca_path, + Logcache::Client.new(host: host, port: port, timeout: timeout, client_ca_path: client_ca_path, client_cert_path: client_cert_path, client_key_path: client_key_path, tls_subject_name: tls_subject_name) end let(:client_ca) { File.open(client_ca_path).read } @@ -76,7 +77,7 @@ module Logcache end let(:client) do - Logcache::Client.new(host: host, port: port, client_ca_path: client_ca_path, + Logcache::Client.new(host: host, port: port, timeout: timeout, client_ca_path: client_ca_path, client_cert_path: client_cert_path, client_key_path: client_key_path, tls_subject_name: tls_subject_name) end @@ -107,7 +108,7 @@ module Logcache end let(:client) do - Logcache::Client.new(host: host, port: port, client_ca_path: client_ca_path, + Logcache::Client.new(host: host, port: port, timeout: timeout, client_ca_path: client_ca_path, client_cert_path: client_cert_path, client_key_path: client_key_path, tls_subject_name: tls_subject_name) end @@ -150,17 +151,17 @@ module Logcache Logcache::Client.new( host: host, port: port, + timeout: timeout, client_ca_path: nil, client_cert_path: nil, client_key_path: nil, tls_subject_name: nil ) end + let(:instance_count) { 2 } + let!(:process) { VCAP::CloudController::ProcessModel.make(instances: instance_count) } describe '#container_metrics' do - let(:instance_count) { 2 } - let!(:process) { VCAP::CloudController::ProcessModel.make(instances: instance_count) } - before do expect(GRPC::Core::ChannelCredentials).not_to receive(:new) expect(Logcache::V1::Egress::Stub).to receive(:new). @@ -185,6 +186,20 @@ module Logcache expect(logcache_service).to have_received(:read).with(logcache_request) end end + + describe 'when logcache is unreachable' do + let(:host) { '192.0.2.1' } # don't use an unresolvable hostname, but some unused IP address (see RFC 5737) + let(:timeout) { 1 } + + it 'aborts the request after the given timeout and raises an error' do + start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC) + expect { + client.container_metrics(source_guid: process.guid, envelope_limit: 1000, start_time: 100, end_time: 101) + }.to raise_error(CloudController::Errors::ApiError, /Connection to Log Cache timed out/) + end_time = Process.clock_gettime(Process::CLOCK_MONOTONIC) + expect(end_time - start_time).to be_between(timeout, timeout + 1) + end + end end end end diff --git a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb index 1728e1d515e..b95b4d2494a 100644 --- a/spec/unit/lib/cloud_controller/dependency_locator_spec.rb +++ b/spec/unit/lib/cloud_controller/dependency_locator_spec.rb @@ -489,14 +489,16 @@ allow(Logcache::Client).to receive(:new).and_return(logcache_client) end - it 'returns the tc-decorated client without TLS' do + it 'returns the tc-decorated client without TLS and default timeout' do TestConfig.override( - logcache_tls: nil + logcache_tls: nil, + timeout_in_seconds: nil, ) expect(locator.traffic_controller_compatible_logcache_client).to be_an_instance_of(Logcache::TrafficControllerDecorator) expect(Logcache::Client).to have_received(:new).with( host: 'http://doppler.service.cf.internal', port: 8080, + timeout: nil, client_ca_path: nil, client_cert_path: nil, client_key_path: nil, @@ -504,11 +506,12 @@ ) end - it 'returns the tc-decorated client with TLS certificates' do + it 'returns the tc-decorated client with TLS certificates and custom timeout' do TestConfig.override( logcache: { host: 'some-logcache-host', port: 1234, + timeout_in_seconds: 10, }, logcache_tls: { ca_file: 'logcache-ca', @@ -521,6 +524,7 @@ expect(Logcache::Client).to have_received(:new).with( host: 'some-logcache-host', port: 1234, + timeout: 10, client_ca_path: 'logcache-ca', client_cert_path: 'logcache-client-ca', client_key_path: 'logcache-client-key',