diff --git a/Gemfile b/Gemfile index d659bf7ccdc..e25358c3197 100644 --- a/Gemfile +++ b/Gemfile @@ -21,6 +21,7 @@ gem 'membrane', '~> 1.0' gem 'mime-types', '~> 3.4' gem 'multi_json' gem 'multipart-parser' +gem 'net-http-persistent' gem 'net-ssh' gem 'netaddr', '>= 2.0.4' gem 'newrelic_rpm' diff --git a/Gemfile.lock b/Gemfile.lock index bc624add998..b71495fc456 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -120,6 +120,7 @@ GEM simplecov (<= 0.13) coderay (1.1.3) concurrent-ruby (1.2.0) + connection_pool (2.3.0) cookiejar (0.3.3) crack (0.4.5) rexml @@ -326,6 +327,8 @@ GEM mustermann (3.0.0) ruby2_keywords (~> 0.0.1) mysql2 (0.5.5) + net-http-persistent (4.0.1) + connection_pool (~> 2.2) net-ssh (7.0.1) netaddr (2.0.6) netrc (0.11.0) @@ -584,6 +587,7 @@ DEPENDENCIES multi_json multipart-parser mysql2 (~> 0.5.5) + net-http-persistent net-ssh netaddr (>= 2.0.4) newrelic_rpm diff --git a/lib/cloud_controller/deployment_updater/dispatcher.rb b/lib/cloud_controller/deployment_updater/dispatcher.rb index 486eba09c9a..70de260ab94 100644 --- a/lib/cloud_controller/deployment_updater/dispatcher.rb +++ b/lib/cloud_controller/deployment_updater/dispatcher.rb @@ -15,7 +15,7 @@ def dispatch deployments_to_cancel = DeploymentModel.where(state: DeploymentModel::CANCELING_STATE).all begin - workpool = WorkPool.new(50) + workpool = WorkPool.new logger.info("scaling #{deployments_to_scale.size} deployments") deployments_to_scale.each do |deployment| diff --git a/lib/cloud_controller/diego/processes_sync.rb b/lib/cloud_controller/diego/processes_sync.rb index 1d2959156c1..86119bcdfb9 100644 --- a/lib/cloud_controller/diego/processes_sync.rb +++ b/lib/cloud_controller/diego/processes_sync.rb @@ -12,7 +12,7 @@ class BBSFetchError < Error def initialize(config:, statsd_updater: VCAP::CloudController::Metrics::StatsdUpdater.new) @config = config - @workpool = WorkPool.new(50, store_exceptions: true) + @workpool = WorkPool.new(store_exceptions: true) @statsd_updater = statsd_updater end diff --git a/lib/cloud_controller/diego/reporters/instances_reporter.rb b/lib/cloud_controller/diego/reporters/instances_reporter.rb index 810852c6e90..b16eabf76bf 100644 --- a/lib/cloud_controller/diego/reporters/instances_reporter.rb +++ b/lib/cloud_controller/diego/reporters/instances_reporter.rb @@ -14,7 +14,7 @@ def initialize(bbs_instances_client) end def self.singleton_workpool - @singleton_workpool ||= WorkPool.new(50) + @singleton_workpool ||= WorkPool.new end def all_instances_for_app(process) diff --git a/lib/cloud_controller/diego/tasks_sync.rb b/lib/cloud_controller/diego/tasks_sync.rb index 95c7bb1ff45..3e32fb0be4a 100644 --- a/lib/cloud_controller/diego/tasks_sync.rb +++ b/lib/cloud_controller/diego/tasks_sync.rb @@ -10,7 +10,7 @@ class BBSFetchError < Error def initialize(config:) @config = config - @workpool = WorkPool.new(50, store_exceptions: true) + @workpool = WorkPool.new(store_exceptions: true) end def sync diff --git a/lib/diego/client.rb b/lib/diego/client.rb index 2325db95805..3fc7b043a55 100644 --- a/lib/diego/client.rb +++ b/lib/diego/client.rb @@ -1,16 +1,16 @@ require 'diego/bbs/bbs' require 'diego/errors' require 'diego/routes' +require 'net/http/persistent' +require 'uri' module Diego class Client - PROTOBUF_HEADER = { 'Content-Type'.freeze => 'application/x-protobuf'.freeze }.freeze - def initialize(url:, ca_cert_file:, client_cert_file:, client_key_file:, connect_timeout:, send_timeout:, receive_timeout:) ENV['PB_IGNORE_DEPRECATIONS'] ||= 'true' - @client = build_client( - url, + @bbs_url = URI(url) + @http_client = new_http_client( ca_cert_file, client_cert_file, client_key_file, @@ -20,157 +20,121 @@ def initialize(url:, ca_cert_file:, client_cert_file:, client_key_file:, end def ping - response = with_request_error_handling do - client.post(Routes::PING) - end + req = post_request(path: Routes::PING) + response = request_with_error_handling(req) - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::PingResponse) end def upsert_domain(domain:, ttl:) - request = protobuf_encode!({ domain: domain, ttl: ttl.to_i }, Bbs::Models::UpsertDomainRequest) - - response = with_request_error_handling do - client.post(Routes::UPSERT_DOMAIN, request, PROTOBUF_HEADER) - end + req = post_request(body: protobuf_encode!({ domain: domain, ttl: ttl.to_i }, Bbs::Models::UpsertDomainRequest), path: Routes::UPSERT_DOMAIN) + response = request_with_error_handling(req) - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::UpsertDomainResponse) end def desire_task(task_definition:, domain:, task_guid:) - request = protobuf_encode!({ task_definition: task_definition, domain: domain, task_guid: task_guid }, Bbs::Models::DesireTaskRequest) + req = post_request(body: protobuf_encode!({ task_definition: task_definition, domain: domain, task_guid: task_guid }, Bbs::Models::DesireTaskRequest), +path: Routes::DESIRE_TASK) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::DESIRE_TASK, request, PROTOBUF_HEADER) - end - - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::TaskLifecycleResponse) end def task_by_guid(task_guid) - request = protobuf_encode!({ task_guid: task_guid }, Bbs::Models::TaskByGuidRequest) - - response = with_request_error_handling do - client.post(Routes::TASK_BY_GUID, request, PROTOBUF_HEADER) - end + req = post_request(body: protobuf_encode!({ task_guid: task_guid }, Bbs::Models::TaskByGuidRequest), path: Routes::TASK_BY_GUID) + response = request_with_error_handling(req) - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::TaskResponse) end def tasks(domain: '', cell_id: '') - request = protobuf_encode!({ domain: domain, cell_id: cell_id }, Bbs::Models::TasksRequest) + req = post_request(body: protobuf_encode!({ domain: domain, cell_id: cell_id }, Bbs::Models::TasksRequest), path: Routes::LIST_TASKS) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::LIST_TASKS, request, PROTOBUF_HEADER) - end - - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::TasksResponse) end def cancel_task(task_guid) - request = protobuf_encode!({ task_guid: task_guid }, Bbs::Models::TaskGuidRequest) - - response = with_request_error_handling do - client.post(Routes::CANCEL_TASK, request, PROTOBUF_HEADER) - end + req = post_request(body: protobuf_encode!({ task_guid: task_guid }, Bbs::Models::TaskGuidRequest), path: Routes::CANCEL_TASK) + response = request_with_error_handling(req) - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::TaskLifecycleResponse) end def desire_lrp(lrp) - request = protobuf_encode!({ desired_lrp: lrp }, Bbs::Models::DesireLRPRequest) + req = post_request(body: protobuf_encode!({ desired_lrp: lrp }, Bbs::Models::DesireLRPRequest), path: Routes::DESIRE_LRP) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::DESIRE_LRP, request, PROTOBUF_HEADER) - end - - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::DesiredLRPLifecycleResponse) end def desired_lrp_by_process_guid(process_guid) - request = protobuf_encode!({ process_guid: process_guid }, Bbs::Models::DesiredLRPByProcessGuidRequest) - - response = with_request_error_handling do - client.post(Routes::DESIRED_LRP_BY_PROCESS_GUID, request, PROTOBUF_HEADER) - end + req = post_request(body: protobuf_encode!({ process_guid: process_guid }, Bbs::Models::DesiredLRPByProcessGuidRequest), path: Routes::DESIRED_LRP_BY_PROCESS_GUID) + response = request_with_error_handling(req) - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::DesiredLRPResponse) end def update_desired_lrp(process_guid, lrp_update) - request = protobuf_encode!({ process_guid: process_guid, update: lrp_update }, Bbs::Models::UpdateDesiredLRPRequest) + req = post_request(body: protobuf_encode!({ process_guid: process_guid, update: lrp_update }, Bbs::Models::UpdateDesiredLRPRequest), path: Routes::UPDATE_DESIRED_LRP) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::UPDATE_DESIRED_LRP, request, PROTOBUF_HEADER) - end - - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::DesiredLRPLifecycleResponse) end def remove_desired_lrp(process_guid) - request = protobuf_encode!({ process_guid: process_guid }, Bbs::Models::RemoveDesiredLRPRequest) - - response = with_request_error_handling do - client.post(Routes::REMOVE_DESIRED_LRP, request, PROTOBUF_HEADER) - end + req = post_request(body: protobuf_encode!({ process_guid: process_guid }, Bbs::Models::RemoveDesiredLRPRequest), path: Routes::REMOVE_DESIRED_LRP) + response = request_with_error_handling(req) - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::DesiredLRPLifecycleResponse) end def retire_actual_lrp(actual_lrp_key) - request = protobuf_encode!({ actual_lrp_key: actual_lrp_key }, Bbs::Models::RetireActualLRPRequest) + req = post_request(body: protobuf_encode!({ actual_lrp_key: actual_lrp_key }, Bbs::Models::RetireActualLRPRequest), path: Routes::RETIRE_ACTUAL_LRP) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::RETIRE_ACTUAL_LRP, request, PROTOBUF_HEADER) - end - - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::ActualLRPLifecycleResponse) end def desired_lrp_scheduling_infos(domain) - request = protobuf_encode!({ domain: domain }, Bbs::Models::DesiredLRPsRequest) - - response = with_request_error_handling do - client.post(Routes::DESIRED_LRP_SCHEDULING_INFOS, request, PROTOBUF_HEADER) - end + req = post_request(body: protobuf_encode!({ domain: domain }, Bbs::Models::DesiredLRPsRequest), path: Routes::DESIRED_LRP_SCHEDULING_INFOS) + response = request_with_error_handling(req) - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::DesiredLRPSchedulingInfosResponse) end def actual_lrps_by_process_guid(process_guid) - request = protobuf_encode!({ process_guid: process_guid }, Bbs::Models::ActualLRPsRequest) + req = post_request(body: protobuf_encode!({ process_guid: process_guid }, Bbs::Models::ActualLRPsRequest), path: Routes::ACTUAL_LRPS) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::ACTUAL_LRPS, request, PROTOBUF_HEADER) - end - - validate_status!(response: response, statuses: [200]) + validate_status!(response) protobuf_decode!(response.body, Bbs::Models::ActualLRPsResponse) end - def with_request_error_handling(&blk) - tries ||= 3 - yield + def request_with_error_handling(req) + attempt ||= 1 + http_client.request(bbs_url + req.path, req) rescue => e - retry unless (tries -= 1).zero? + retry unless (attempt += 1) > 3 raise RequestError.new(e.message) end private - attr_reader :client + attr_reader :http_client, :bbs_url def protobuf_encode!(hash, protobuf_message_class) # See below link to understand proto3 message encoding @@ -180,8 +144,15 @@ def protobuf_encode!(hash, protobuf_message_class) raise EncodeError.new(e.message) end - def validate_status!(response:, statuses:) - raise ResponseError.new("failed with status: #{response.status}, body: #{response.body}") unless statuses.include?(response.status) + def post_request(body: nil, path:) + req = Net::HTTP::Post.new(path) + req.body = body if body + req['Content-Type'.freeze] = 'application/x-protobuf'.freeze + req + end + + def validate_status!(response) + raise ResponseError.new("failed with status: #{response.code}, body: #{response.body}") unless response.code == '200' end def protobuf_decode!(message, protobuf_decoder) @@ -190,14 +161,16 @@ def protobuf_decode!(message, protobuf_decoder) raise DecodeError.new(e.message) end - def build_client(url, ca_cert_file, client_cert_file, client_key_file, - connect_timeout, send_timeout, receive_timeout) - client = HTTPClient.new(base_url: url) - client.connect_timeout = connect_timeout - client.send_timeout = send_timeout - client.receive_timeout = receive_timeout - client.ssl_config.set_client_cert_file(client_cert_file, client_key_file) - client.ssl_config.set_trust_ca(ca_cert_file) + def new_http_client(ca_cert_file, client_cert_file, client_key_file, + connect_timeout, send_timeout, receive_timeout) + client = Net::HTTP::Persistent.new(pool_size: WorkPool::SIZE) + client.verify_mode = OpenSSL::SSL::VERIFY_PEER + client.private_key = OpenSSL::PKey::RSA.new(File.read(client_key_file)) + client.certificate = OpenSSL::X509::Certificate.new(File.read(client_cert_file)) + client.ca_file = ca_cert_file + client.open_timeout = connect_timeout + client.read_timeout = receive_timeout + client.write_timeout = send_timeout client end end diff --git a/lib/utils/workpool.rb b/lib/utils/workpool.rb index dec67bf2096..6100df16c3b 100644 --- a/lib/utils/workpool.rb +++ b/lib/utils/workpool.rb @@ -1,7 +1,9 @@ class WorkPool + SIZE = 50 + attr_reader :exceptions, :threads - def initialize(size, store_exceptions: false) + def initialize(size: SIZE, store_exceptions: false) @size = size @store_exceptions = store_exceptions diff --git a/spec/diego/client_spec.rb b/spec/diego/client_spec.rb index ffacaad0aa2..5e6ca8da3f6 100644 --- a/spec/diego/client_spec.rb +++ b/spec/diego/client_spec.rb @@ -3,14 +3,17 @@ module Diego RSpec.describe Client do - let(:bbs_url) { 'https://bbs.example.com:4443' } + let(:bbs_host) { 'bbs.example.com' } + let(:bbs_port) { 4443 } + let(:bbs_url) { "https://#{bbs_host}:#{bbs_port}" } let(:ca_cert_file) { File.join(Paths::FIXTURES, 'certs/bbs_ca.crt') } let(:client_cert_file) { File.join(Paths::FIXTURES, 'certs/bbs_client.crt') } let(:client_key_file) { File.join(Paths::FIXTURES, 'certs/bbs_client.key') } + let(:timeout) { 10 } subject(:client) do Client.new(url: bbs_url, ca_cert_file: ca_cert_file, client_cert_file: client_cert_file, client_key_file: client_key_file, - connect_timeout: 10, send_timeout: 10, receive_timeout: 10) + connect_timeout: timeout, send_timeout: timeout, receive_timeout: timeout) end describe 'configuration' do @@ -26,12 +29,12 @@ module Diego let(:response_status) { 200 } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/ping').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/ping").to_return(status: response_status, body: response_body) end it 'returns a ping response' do expect(client.ping.available).to be_truthy - expect(a_request(:post, 'https://bbs.example.com:4443/v1/ping')).to have_been_made + expect(a_request(:post, "#{bbs_url}/v1/ping")).to have_been_made.once end context 'when it does not return successfully' do @@ -45,11 +48,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/ping').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/ping").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.ping }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/ping")).to have_been_made.times(3) end end @@ -69,7 +73,7 @@ module Diego let(:ttl) { VCAP::CloudController::Diego::APP_LRP_DOMAIN_TTL } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/domains/upsert').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/domains/upsert").to_return(status: response_status, body: response_body) end it 'returns a domain lifecycle response' do @@ -78,10 +82,10 @@ module Diego response = client.upsert_domain(domain: domain, ttl: ttl) expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/domains/upsert').with( + expect(a_request(:post, "#{bbs_url}/v1/domains/upsert").with( body: Bbs::Models::UpsertDomainRequest.encode(expected_domain_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -95,11 +99,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/domains/upsert').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/domains/upsert").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.upsert_domain(domain: domain, ttl: ttl) }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/domains/upsert")).to have_been_made.times(3) end end @@ -124,7 +129,7 @@ module Diego let(:task_definition) { Bbs::Models::TaskDefinition.new } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/tasks/desire.r2').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/tasks/desire.r2").to_return(status: response_status, body: response_body) end it 'returns a task lifecycle response' do @@ -133,10 +138,10 @@ module Diego response = client.desire_task(task_definition: task_definition, task_guid: 'task_guid', domain: 'domain') expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/tasks/desire.r2').with( + expect(a_request(:post, "#{bbs_url}/v1/tasks/desire.r2").with( body: Bbs::Models::DesireTaskRequest.encode(expected_task_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -152,11 +157,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/tasks/desire.r2').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/tasks/desire.r2").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.desire_task(task_definition: task_definition, task_guid: 'task_guid', domain: 'domain') }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/tasks/desire.r2")).to have_been_made.times(3) end end @@ -180,7 +186,7 @@ module Diego let(:response_status) { 200 } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/tasks/list.r2').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/tasks/list.r2").to_return(status: response_status, body: response_body) end it 'returns a tasks response' do @@ -189,10 +195,10 @@ module Diego expected_request = Bbs::Models::TasksRequest.new expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/tasks/list.r2').with( + expect(a_request(:post, "#{bbs_url}/v1/tasks/list.r2").with( body: Bbs::Models::TasksRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end describe 'filtering' do @@ -202,10 +208,10 @@ module Diego expected_request = Bbs::Models::TasksRequest.new(domain: 'some-domain') expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/tasks/list.r2').with( + expect(a_request(:post, "#{bbs_url}/v1/tasks/list.r2").with( body: Bbs::Models::TasksRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end it 'filters by cell_id' do @@ -214,10 +220,10 @@ module Diego expected_request = Bbs::Models::TasksRequest.new(cell_id: 'cell-id-thing') expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/tasks/list.r2').with( + expect(a_request(:post, "#{bbs_url}/v1/tasks/list.r2").with( body: Bbs::Models::TasksRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end end @@ -232,11 +238,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/tasks/list.r2').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/tasks/list.r2").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.tasks }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/tasks/list.r2")).to have_been_made.times(3) end end @@ -260,7 +267,7 @@ module Diego let(:response_status) { 200 } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/tasks/get_by_task_guid.r2').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/tasks/get_by_task_guid.r2").to_return(status: response_status, body: response_body) end it 'returns a task response' do @@ -269,10 +276,10 @@ module Diego expected_request = Bbs::Models::TaskByGuidRequest.new(task_guid: 'some-guid') expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/tasks/get_by_task_guid.r2').with( + expect(a_request(:post, "#{bbs_url}/v1/tasks/get_by_task_guid.r2").with( body: Bbs::Models::TaskByGuidRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -286,11 +293,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/tasks/get_by_task_guid.r2').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/tasks/get_by_task_guid.r2").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.task_by_guid('some-guid') }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/tasks/get_by_task_guid.r2")).to have_been_made.times(3) end end @@ -314,7 +322,7 @@ module Diego let(:response_status) { 200 } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/tasks/cancel').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/tasks/cancel").to_return(status: response_status, body: response_body) end it 'returns a task lifecycle response' do @@ -323,10 +331,10 @@ module Diego response = client.cancel_task('some-guid') expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/tasks/cancel').with( + expect(a_request(:post, "#{bbs_url}/v1/tasks/cancel").with( body: Bbs::Models::TaskGuidRequest.encode(expected_cancel_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -340,11 +348,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/tasks/cancel').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/tasks/cancel").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.cancel_task('some-guid') }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/tasks/cancel")).to have_been_made.times(3) end end @@ -369,7 +378,7 @@ module Diego let(:lrp) { ::Diego::Bbs::Models::DesiredLRP.new } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/desire.r2').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/desired_lrp/desire.r2").to_return(status: response_status, body: response_body) end it 'returns a Desired LRP Lifecycle Response' do @@ -378,10 +387,10 @@ module Diego response = client.desire_lrp(lrp) expect(response).to be_a(Bbs::Models::DesiredLRPLifecycleResponse) expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/desire.r2').with( + expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/desire.r2").with( body: Bbs::Models::DesireLRPRequest.encode(expected_desire_lrp_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -395,11 +404,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/desire.r2').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/desired_lrp/desire.r2").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.desire_lrp(lrp) }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/desire.r2")).to have_been_made.times(3) end end @@ -426,7 +436,7 @@ module Diego let(:process_guid) { 'process-guid' } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrps/get_by_process_guid.r2').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/desired_lrps/get_by_process_guid.r2").to_return(status: response_status, body: response_body) end it 'returns a Desired LRP Response' do @@ -436,10 +446,10 @@ module Diego expect(response).to be_a(Bbs::Models::DesiredLRPResponse) expect(response.error).to be_nil expect(response.desired_lrp).to eq(lrp) - expect(a_request(:post, 'https://bbs.example.com:4443/v1/desired_lrps/get_by_process_guid.r2').with( + expect(a_request(:post, "#{bbs_url}/v1/desired_lrps/get_by_process_guid.r2").with( body: Bbs::Models::DesiredLRPByProcessGuidRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -455,11 +465,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrps/get_by_process_guid.r2').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/desired_lrps/get_by_process_guid.r2").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.desired_lrp_by_process_guid(process_guid) }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/desired_lrps/get_by_process_guid.r2")).to have_been_made.times(3) end end @@ -484,7 +495,7 @@ module Diego let(:response_status) { 200 } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/remove').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/desired_lrp/remove").to_return(status: response_status, body: response_body) end it 'returns a Desired LRP Lifecycle Response' do @@ -493,10 +504,10 @@ module Diego response = client.remove_desired_lrp(process_guid) expect(response).to be_a(Bbs::Models::DesiredLRPLifecycleResponse) expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/remove').with( + expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/remove").with( body: Bbs::Models::RemoveDesiredLRPRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -510,11 +521,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/remove').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/desired_lrp/remove").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.remove_desired_lrp(process_guid) }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/remove")).to have_been_made.times(3) end end @@ -539,7 +551,7 @@ module Diego let(:response_status) { 200 } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/actual_lrps/retire').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/actual_lrps/retire").to_return(status: response_status, body: response_body) end it 'returns an Actual LRP Lifecycle Response' do @@ -548,10 +560,10 @@ module Diego response = client.retire_actual_lrp(actual_lrp_key) expect(response).to be_a(Bbs::Models::ActualLRPLifecycleResponse) expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/actual_lrps/retire').with( + expect(a_request(:post, "#{bbs_url}/v1/actual_lrps/retire").with( body: Bbs::Models::RetireActualLRPRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -567,11 +579,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/actual_lrps/retire').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/actual_lrps/retire").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.retire_actual_lrp(actual_lrp_key) }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/actual_lrps/retire")).to have_been_made.times(3) end end @@ -590,25 +603,6 @@ module Diego end end - describe '#with_request_error_handling' do - it 'retries' do - tries = 0 - - client.with_request_error_handling do - tries += 1 - raise 'error' if tries < 2 - end - - expect(tries).to be > 1 - end - - it 'raises an error after all retries fail' do - expect { - client.with_request_error_handling { raise 'error' } - }.to raise_error(RequestError) - end - end - describe '#update_desired_lrp' do let(:process_guid) { 'process-guid' } let(:lrp_update) { ::Diego::Bbs::Models::DesiredLRPUpdate.new(instances: 3) } @@ -617,7 +611,7 @@ module Diego let(:response_status) { 200 } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/update').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/desired_lrp/update").to_return(status: response_status, body: response_body) end it 'returns a Desired LRP Lifecycle Response' do @@ -626,10 +620,10 @@ module Diego response = client.update_desired_lrp(process_guid, lrp_update) expect(response).to be_a(Bbs::Models::DesiredLRPLifecycleResponse) expect(response.error).to be_nil - expect(a_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/update').with( + expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/update").with( body: Bbs::Models::UpdateDesiredLRPRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -643,11 +637,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp/update').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/desired_lrp/update").to_raise(StandardError.new('error message')) end it 'raises' do expect { client.update_desired_lrp(process_guid, lrp_update) }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/desired_lrp/update")).to have_been_made.times(3) end end @@ -677,7 +672,7 @@ module Diego let(:actual_lrps) { [::Diego::Bbs::Models::ActualLRP.new] } let(:response_status) { 200 } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/actual_lrps/list').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/actual_lrps/list").to_return(status: response_status, body: response_body) end it 'returns a LRP instances response' do @@ -687,10 +682,10 @@ module Diego expect(response).to be_a(Bbs::Models::ActualLRPsResponse) expect(response.error).to be_nil expect(response.actual_lrps).to eq(actual_lrps) - expect(a_request(:post, 'https://bbs.example.com:4443/v1/actual_lrps/list').with( + expect(a_request(:post, "#{bbs_url}/v1/actual_lrps/list").with( body: Bbs::Models::ActualLRPsRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end end @@ -704,7 +699,7 @@ module Diego let(:domain) { 'domain' } before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp_scheduling_infos/list').to_return(status: response_status, body: response_body) + stub_request(:post, "#{bbs_url}/v1/desired_lrp_scheduling_infos/list").to_return(status: response_status, body: response_body) end it 'returns a Desired LRP Scheduling Infos Response' do @@ -714,10 +709,10 @@ module Diego expect(response).to be_a(Bbs::Models::DesiredLRPSchedulingInfosResponse) expect(response.error).to be_nil expect(response.desired_lrp_scheduling_infos).to eq(scheduling_infos) - expect(a_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp_scheduling_infos/list').with( + expect(a_request(:post, "#{bbs_url}/v1/desired_lrp_scheduling_infos/list").with( body: Bbs::Models::DesiredLRPsRequest.encode(expected_request).to_s, headers: { 'Content-Type' => 'application/x-protobuf' } - )).to have_been_made + )).to have_been_made.once end context 'when it does not return successfully' do @@ -733,13 +728,12 @@ module Diego context 'when it fails to make the request' do before do - stub_request(:post, 'https://bbs.example.com:4443/v1/desired_lrp_scheduling_infos/list').to_raise(StandardError.new('error message')) + stub_request(:post, "#{bbs_url}/v1/desired_lrp_scheduling_infos/list").to_raise(StandardError.new('error message')) end it 'raises' do - expect { - client.desired_lrp_scheduling_infos(domain) - }.to raise_error(RequestError, /error message/) + expect { client.desired_lrp_scheduling_infos(domain) }.to raise_error(RequestError, /error message/) + expect(a_request(:post, "#{bbs_url}/v1/desired_lrp_scheduling_infos/list")).to have_been_made.times(3) end end @@ -758,22 +752,45 @@ module Diego end end - describe '#with_request_error_handling' do - it 'retries' do - tries = 0 + RSpec::Matchers.define :ping_request do + match { |actual| actual.instance_of?(Net::HTTP::Post) && actual.path == '/v1/ping' } + end - client.with_request_error_handling do - tries += 1 - raise 'error' if tries < 2 + describe 'dns failover' do + context 'bbs on first IP address is unresponsive' do + let(:bbs_host) { 'fake.bbs' } # in WebMock allow-list, see spec/spec_helper.rb + let(:unresponsive_bbs) { Addrinfo.tcp('1.2.3.4', bbs_port) } + let(:responsive_bbs) { Addrinfo.tcp('5.6.7.8', bbs_port) } + let(:responsive_socket) do + double('socket', + :setsockopt => nil, + :sync_close= => nil, + :connect_nonblock => nil, + :post_connection_check => nil, + :ssl_version => nil, + :cipher => [], + :close => nil) + end + let(:ping_response) do + r = Net::HTTPOK.new('1.1', '200', 'OK') + r.body = Bbs::Models::PingResponse.encode(Bbs::Models::PingResponse.new(available: true)).to_s + r end - expect(tries).to be > 1 - end + before do + allow(Addrinfo).to receive(:getaddrinfo).with(bbs_host, bbs_port, any_args).and_return([unresponsive_bbs, responsive_bbs]) + allow(unresponsive_bbs).to receive(:connect).with(timeout: timeout).and_raise(Errno::ETIMEDOUT, 'user specified timeout') + allow(responsive_bbs).to receive(:connect).with(timeout: timeout).and_return(responsive_socket) + allow(OpenSSL::SSL::SSLSocket).to receive(:new).with(responsive_socket, anything).and_return(responsive_socket) + allow_any_instance_of(Net::HTTP).to receive(:transport_request).with(ping_request).and_return(ping_response) + end - it 'raises an error after all retries fail' do - expect { - client.with_request_error_handling { raise 'error' } - }.to raise_error(RequestError) + it 'automatically triggers a failover after timeout' do + expect(client.ping.available).to be_truthy + + expect(unresponsive_bbs).to have_received(:connect).once + expect(responsive_bbs).to have_received(:connect).once + end end end end diff --git a/spec/request/processes_spec.rb b/spec/request/processes_spec.rb index ccfebd85dfe..7b004657ced 100644 --- a/spec/request/processes_spec.rb +++ b/spec/request/processes_spec.rb @@ -11,7 +11,7 @@ let(:user) { VCAP::CloudController::User.make } let(:admin_header) { admin_headers_for(user) } let(:user_name) { 'ProcHudson' } - let(:build_client) { instance_double(HTTPClient, post: nil) } + let(:http_client) { instance_double(Net::HTTP::Persistent, request: nil) } let(:metadata) do { labels: { @@ -22,7 +22,7 @@ } end before do - allow_any_instance_of(::Diego::Client).to receive(:build_client).and_return(build_client) + allow_any_instance_of(::Diego::Client).to receive(:new_http_client).and_return(http_client) end describe 'GET /v3/processes' do diff --git a/spec/request/v2/apps_spec.rb b/spec/request/v2/apps_spec.rb index b5962d78a0f..65f0429c831 100644 --- a/spec/request/v2/apps_spec.rb +++ b/spec/request/v2/apps_spec.rb @@ -4,13 +4,13 @@ RSpec.describe 'Apps' do let(:user) { VCAP::CloudController::User.make } let(:space) { VCAP::CloudController::Space.make } - let(:build_client) { instance_double(HTTPClient, post: nil) } + let(:http_client) { instance_double(Net::HTTP::Persistent, request: nil) } before do space.organization.add_user(user) space.add_developer(user) TestConfig.override(kubernetes: {}) - allow_any_instance_of(::Diego::Client).to receive(:build_client).and_return(build_client) + allow_any_instance_of(::Diego::Client).to receive(:new_http_client).and_return(http_client) end describe 'GET /v2/apps' do diff --git a/spec/request/v2/spaces_spec.rb b/spec/request/v2/spaces_spec.rb index 713af2d6554..34d54602469 100644 --- a/spec/request/v2/spaces_spec.rb +++ b/spec/request/v2/spaces_spec.rb @@ -258,12 +258,12 @@ let(:maintenance_info) { { version: '1.0.0', desciption: 'this is description about the maintenance' } } let!(:service_plan) { VCAP::CloudController::ServicePlan.make(maintenance_info: maintenance_info) } let!(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, service_plan: service_plan, maintenance_info: maintenance_info) } - let(:build_client) { instance_double(HTTPClient, post: nil) } + let(:http_client) { instance_double(Net::HTTP::Persistent, request: nil) } before do space.organization.add_user(user) space.add_developer(user) - allow_any_instance_of(::Diego::Client).to receive(:build_client).and_return(build_client) + allow_any_instance_of(::Diego::Client).to receive(:new_http_client).and_return(http_client) end it 'returns the space summary' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d189acf7c0a..3612cdb21f5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -127,14 +127,14 @@ rspec_config.include SpaceRestrictedResponseGenerators - rspec_config.before(:all) { WebMock.disable_net_connect!(allow: 'codeclimate.com') } + rspec_config.before(:all) { WebMock.disable_net_connect!(allow: ['codeclimate.com', 'fake.bbs']) } rspec_config.before(:all, type: :integration) do WebMock.allow_net_connect! @uaa_server = FakeUAAServer.new(6789) @uaa_server.start end rspec_config.after(:all, type: :integration) do - WebMock.disable_net_connect!(allow: 'codeclimate.com') + WebMock.disable_net_connect!(allow: ['codeclimate.com', 'fake.bbs']) @uaa_server.stop end diff --git a/spec/unit/lib/utils/workpool_spec.rb b/spec/unit/lib/utils/workpool_spec.rb index a640b329280..3173be7a67b 100644 --- a/spec/unit/lib/utils/workpool_spec.rb +++ b/spec/unit/lib/utils/workpool_spec.rb @@ -3,7 +3,7 @@ RSpec.describe WorkPool do describe '#drain' do it 'runs blocks passed in' do - wp = WorkPool.new(1) + wp = WorkPool.new(size: 1) ran = false wp.submit do ran = true @@ -13,7 +13,7 @@ end it 'propagates arguments passed in to the inner block' do - wp = WorkPool.new(1) + wp = WorkPool.new(size: 1) args = [] wp.submit(1, 2, 3) do |a, b, c| args = [a, b, c] @@ -23,7 +23,7 @@ end it 'parallelizes up to "size" threads' do - wp = WorkPool.new(5) + wp = WorkPool.new(size: 5) require 'benchmark' time = Benchmark.realtime do @@ -38,7 +38,7 @@ end it 'finishes running work before draining' do - wp = WorkPool.new(1) + wp = WorkPool.new(size: 1) ran = false wp.submit do sleep 0.1 @@ -52,7 +52,7 @@ context 'when a queued block raises an exception' do context 'when store_exceptions is true' do it 'exposes the exception after draining' do - wp = WorkPool.new(1, store_exceptions: true) + wp = WorkPool.new(size: 1, store_exceptions: true) exception = RuntimeError.new('Boom') wp.submit do raise exception @@ -64,7 +64,7 @@ it 'processes other work in the queue' do ran = false - wp = WorkPool.new(1, store_exceptions: true) + wp = WorkPool.new(size: 1, store_exceptions: true) wp.submit do raise 'Boom' end @@ -77,7 +77,7 @@ end it 'accumulates all exceptions raised' do - wp = WorkPool.new(1, store_exceptions: true) + wp = WorkPool.new(size: 1, store_exceptions: true) 3.times do wp.submit do @@ -93,7 +93,7 @@ context 'when store_exceptions is false' do it 'processes other work in the queue' do ran = false - wp = WorkPool.new(1) + wp = WorkPool.new(size: 1) wp.submit do raise 'Boom' end @@ -106,7 +106,7 @@ end it 'does NOT accumulate exceptions' do - wp = WorkPool.new(1) + wp = WorkPool.new(size: 1) 3.times do wp.submit do @@ -125,7 +125,7 @@ context 'when there are dead threads in the pool' do it 'replaces the dead threads with new ones' do thread_count = 2 - wp = WorkPool.new(thread_count) + wp = WorkPool.new(size: thread_count) wp.submit do 1 + 1 end @@ -152,7 +152,7 @@ context 'when all of the threads in the pool are healthy' do it 'does not modify the healthy threads' do thread_count = 2 - wp = WorkPool.new(thread_count) + wp = WorkPool.new(size: thread_count) wp.submit do 1 + 1 end