diff --git a/lib/diego/client.rb b/lib/diego/client.rb index 2325db95805..747be4e363c 100644 --- a/lib/diego/client.rb +++ b/lib/diego/client.rb @@ -1,16 +1,17 @@ require 'diego/bbs/bbs' require 'diego/errors' require 'diego/routes' +require 'uri' +require 'resolv' +require 'net/http' 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 +21,136 @@ 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) - - response = with_request_error_handling do - client.post(Routes::DESIRE_TASK, request, PROTOBUF_HEADER) - end + 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) - 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) + req = post_request(body: protobuf_encode!({ task_guid: task_guid }, Bbs::Models::TaskByGuidRequest), path: Routes::TASK_BY_GUID) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::TASK_BY_GUID, request, PROTOBUF_HEADER) - end - - 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) - - response = with_request_error_handling do - client.post(Routes::LIST_TASKS, request, PROTOBUF_HEADER) - end + 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) - 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) - - response = with_request_error_handling do - client.post(Routes::UPDATE_DESIRED_LRP, request, PROTOBUF_HEADER) - end + 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) - 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) + req = post_request(body: protobuf_encode!({ process_guid: process_guid }, Bbs::Models::RemoveDesiredLRPRequest), path: Routes::REMOVE_DESIRED_LRP) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::REMOVE_DESIRED_LRP, request, PROTOBUF_HEADER) - end - - 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) - - response = with_request_error_handling do - client.post(Routes::RETIRE_ACTUAL_LRP, request, PROTOBUF_HEADER) - end + 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) - 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) + req = post_request(body: protobuf_encode!({ domain: domain }, Bbs::Models::DesiredLRPsRequest), path: Routes::DESIRED_LRP_SCHEDULING_INFOS) + response = request_with_error_handling(req) - response = with_request_error_handling do - client.post(Routes::DESIRED_LRP_SCHEDULING_INFOS, request, PROTOBUF_HEADER) - end - - 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) - - response = with_request_error_handling do - client.post(Routes::ACTUAL_LRPS, request, PROTOBUF_HEADER) - end + req = post_request(body: protobuf_encode!({ process_guid: process_guid }, Bbs::Models::ActualLRPsRequest), path: Routes::ACTUAL_LRPS) + response = request_with_error_handling(req) - 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.ipaddr = bbs_ip # tell the HTTP client which exact IP to target + http_client.request(req) + rescue Resolv::ResolvError, Resolv::ResolvTimeout => e + raise DnsResolutionError.new("dns resolution failed for #{bbs_url.host}: #{e.message}") rescue => e - retry unless (tries -= 1).zero? + eliminated_ip = ips_remaining.shift + logger.debug("attempt #{attempt} of 3: failed to reach the active bbs server on #{eliminated_ip}, removing from list") + retry unless ips_remaining.empty? && (attempt += 1) > 3 raise RequestError.new(e.message) end + def bbs_ip + self.ips_remaining = bbs_ip_addresses if ips_remaining.nil? || ips_remaining.empty? + ips_remaining.first + end + private - attr_reader :client + attr_reader :http_client, :bbs_url + attr_accessor :ips_remaining + + def logger + @logger ||= Steno.logger('cc.diego.client') + end def protobuf_encode!(hash, protobuf_message_class) # See below link to understand proto3 message encoding @@ -180,8 +160,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 +177,21 @@ 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, + def bbs_ip_addresses + Resolv.getaddresses(bbs_url.host).dup + end + + def new_http_client(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) + client = Net::HTTP.new(bbs_url.host, bbs_url.port) + client.use_ssl = true + client.verify_mode = OpenSSL::SSL::VERIFY_PEER + client.key = OpenSSL::PKey::RSA.new(File.read(client_key_file)) + client.cert = 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/diego/errors.rb b/lib/diego/errors.rb index cad25251d71..1275a7e0405 100644 --- a/lib/diego/errors.rb +++ b/lib/diego/errors.rb @@ -13,4 +13,7 @@ class DecodeError < Error class EncodeError < Error end + + class DnsResolutionError < Error + end end diff --git a/spec/diego/client_spec.rb b/spec/diego/client_spec.rb index ffacaad0aa2..fa2b94dc46e 100644 --- a/spec/diego/client_spec.rb +++ b/spec/diego/client_spec.rb @@ -3,15 +3,25 @@ module Diego RSpec.describe Client do - let(:bbs_url) { 'https://bbs.example.com:4443' } + let(:bbs_domain) { 'bbs.example.com' } + let(:bbs_port) { '4443' } + let(:bbs_uri) { "https://#{bbs_domain}:#{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(:bbs_ip_1) { '1.2.3.4' } + let(:bbs_ip_2) { '5.6.7.8' } + let(:logger) { instance_double(Steno::Logger) } 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, + Client.new(url: bbs_uri, 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) end + before do + allow(Resolv).to receive(:getaddresses).with(bbs_domain).and_return([bbs_ip_1, bbs_ip_2]) + allow(Steno).to receive(:logger).and_return(logger) + allow(logger).to receive(:debug) + end describe 'configuration' do it "should set ENV['PB_IGNORE_DEPRECATIONS'] to true" do @@ -26,12 +36,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, "https://#{bbs_domain}:#{bbs_port}/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, "https://#{bbs_domain}:#{bbs_port}/v1/ping")).to have_been_made.once end context 'when it does not return successfully' do @@ -45,7 +55,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/ping").to_raise(StandardError.new('error message')) end it 'raises' do @@ -69,7 +79,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, "https://#{bbs_domain}:#{bbs_port}/v1/domains/upsert").to_return(status: response_status, body: response_body) end it 'returns a domain lifecycle response' do @@ -78,10 +88,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +105,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/domains/upsert").to_raise(StandardError.new('error message')) end it 'raises' do @@ -124,7 +134,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, "https://#{bbs_domain}:#{bbs_port}/v1/tasks/desire.r2").to_return(status: response_status, body: response_body) end it 'returns a task lifecycle response' do @@ -133,10 +143,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +162,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/tasks/desire.r2").to_raise(StandardError.new('error message')) end it 'raises' do @@ -180,7 +190,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, "https://#{bbs_domain}:#{bbs_port}/v1/tasks/list.r2").to_return(status: response_status, body: response_body) end it 'returns a tasks response' do @@ -189,10 +199,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, "https://#{bbs_domain}:#{bbs_port}/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 +212,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, "https://#{bbs_domain}:#{bbs_port}/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 +224,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +242,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/tasks/list.r2").to_raise(StandardError.new('error message')) end it 'raises' do @@ -260,7 +270,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, "https://#{bbs_domain}:#{bbs_port}/v1/tasks/get_by_task_guid.r2").to_return(status: response_status, body: response_body) end it 'returns a task response' do @@ -269,10 +279,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +296,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/tasks/get_by_task_guid.r2").to_raise(StandardError.new('error message')) end it 'raises' do @@ -314,7 +324,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, "https://#{bbs_domain}:#{bbs_port}/v1/tasks/cancel").to_return(status: response_status, body: response_body) end it 'returns a task lifecycle response' do @@ -323,10 +333,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +350,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/tasks/cancel").to_raise(StandardError.new('error message')) end it 'raises' do @@ -369,7 +379,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, "https://#{bbs_domain}:#{bbs_port}/v1/desired_lrp/desire.r2").to_return(status: response_status, body: response_body) end it 'returns a Desired LRP Lifecycle Response' do @@ -378,10 +388,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +405,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/desired_lrp/desire.r2").to_raise(StandardError.new('error message')) end it 'raises' do @@ -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, "https://#{bbs_domain}:#{bbs_port}/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, "https://#{bbs_domain}:#{bbs_port}/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,7 +465,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/desired_lrps/get_by_process_guid.r2").to_raise(StandardError.new('error message')) end it 'raises' do @@ -484,7 +494,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, "https://#{bbs_domain}:#{bbs_port}/v1/desired_lrp/remove").to_return(status: response_status, body: response_body) end it 'returns a Desired LRP Lifecycle Response' do @@ -493,10 +503,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +520,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/desired_lrp/remove").to_raise(StandardError.new('error message')) end it 'raises' do @@ -539,7 +549,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, "https://#{bbs_domain}:#{bbs_port}/v1/actual_lrps/retire").to_return(status: response_status, body: response_body) end it 'returns an Actual LRP Lifecycle Response' do @@ -548,10 +558,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +577,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/actual_lrps/retire").to_raise(StandardError.new('error message')) end it 'raises' do @@ -590,25 +600,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 +608,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, "https://#{bbs_domain}:#{bbs_port}/v1/desired_lrp/update").to_return(status: response_status, body: response_body) end it 'returns a Desired LRP Lifecycle Response' do @@ -626,10 +617,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +634,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/desired_lrp/update").to_raise(StandardError.new('error message')) end it 'raises' do @@ -677,7 +668,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, "https://#{bbs_domain}:#{bbs_port}/v1/actual_lrps/list").to_return(status: response_status, body: response_body) end it 'returns a LRP instances response' do @@ -687,10 +678,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, "https://#{bbs_domain}:#{bbs_port}/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 +695,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, "https://#{bbs_domain}:#{bbs_port}/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 +705,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, "https://#{bbs_domain}:#{bbs_port}/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,7 +724,7 @@ 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, "https://#{bbs_domain}:#{bbs_port}/v1/desired_lrp_scheduling_infos/list").to_raise(StandardError.new('error message')) end it 'raises' do @@ -758,22 +749,92 @@ module Diego end end - describe '#with_request_error_handling' do - it 'retries' do - tries = 0 + describe '#request_with_error_handling' do + let(:http_client) { Net::HTTP.new(bbs_domain, bbs_port) } + + before do + allow(http_client).to receive(:ipaddr=).with(bbs_ip_1) + allow(http_client).to receive(:ipaddr=).with(bbs_ip_2) + allow(Net::HTTP).to receive(:new).and_return(http_client) + end + + context 'when all requests fail' do + before do + stub_request(:post, "https://#{bbs_domain}:#{bbs_port}/fake_path").to_raise(StandardError.new('error message')) + end - client.with_request_error_handling do - tries += 1 - raise 'error' if tries < 2 + it 'makes three attempts for each IP before raising an error' do + expect { + client.request_with_error_handling(Net::HTTP::Post.new('/fake_path')) + }.to raise_error(RequestError) + expect(a_request(:post, "https://#{bbs_domain}:#{bbs_port}/fake_path")).to have_been_made.times(6) + expect(http_client).to have_received(:ipaddr=).with(bbs_ip_1).exactly(3).times + expect(http_client).to have_received(:ipaddr=).with(bbs_ip_2).exactly(3).times end + end - expect(tries).to be > 1 + context 'when the first request succeeds' do + before do + stub_request(:post, "https://#{bbs_domain}:#{bbs_port}/fake_path").to_return(status: 200) + allow_any_instance_of(Net::HTTP).to receive(:ipaddr=).with(bbs_ip_1) + end + + it 'makes one attempt for the first IP' do + client.request_with_error_handling(Net::HTTP::Post.new('/fake_path')) + expect(a_request(:post, "https://#{bbs_domain}:#{bbs_port}/fake_path")).to have_been_made.once + expect(http_client).to have_received(:ipaddr=).with(bbs_ip_1).once + end end - it 'raises an error after all retries fail' do - expect { - client.with_request_error_handling { raise 'error' } - }.to raise_error(RequestError) + context 'when the first four requests fail and the fifth succeeds' do + before do + stub_request(:post, "https://#{bbs_domain}:#{bbs_port}/fake_path"). + to_raise(StandardError.new('error message')).times(4).then. + to_return(status: 200) + end + + it 'makes five attempts' do + client.request_with_error_handling(Net::HTTP::Post.new('/fake_path')) + expect(a_request(:post, "https://#{bbs_domain}:#{bbs_port}/fake_path")).to have_been_made.times(5) + expect(http_client).to have_received(:ipaddr=).with(bbs_ip_1).exactly(3).times + expect(http_client).to have_received(:ipaddr=).with(bbs_ip_2).twice + end + end + + context 'when DNS resolution times out' do + before do + allow(Resolv).to receive(:getaddresses).and_raise(Resolv::ResolvTimeout.new('soz, you timed out')) + end + + it 'raises an error' do + expect { client.request_with_error_handling(Net::HTTP::Post.new('/fake_path')) }. + to raise_error(DnsResolutionError, /dns resolution failed for #{bbs_domain}: soz, you timed out/) + end + end + + context 'when we reach the DNS server but the host fails to resolve' do + before do + allow(Resolv).to receive(:getaddresses).and_raise(Resolv::ResolvError.new('your domain sucks')) + end + + it 'raises an error' do + expect { client.request_with_error_handling(Net::HTTP::Post.new('/fake_path')) }. + to raise_error(DnsResolutionError, /dns resolution failed for #{bbs_domain}: your domain sucks/) + end + end + + context 'logging' do + before do + stub_request(:post, "https://#{bbs_domain}:#{bbs_port}/fake_path"). + to_raise(StandardError.new('error message')).then. + to_return(status: 200) + allow(http_client).to receive(:ipaddr=).and_call_original + end + + it 'emits a debug log after each failed request' do + client.request_with_error_handling(Net::HTTP::Post.new('/fake_path')) + expect(logger).to have_received(:debug).with(/attempt 1 of 3: failed to reach the active bbs server on #{bbs_ip_1}, removing from list/).once + end end end end diff --git a/spec/request/processes_spec.rb b/spec/request/processes_spec.rb index 7a766fce183..2ea5df213c7 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(:build_client) { instance_double(Net::HTTP, request: nil) } let(:metadata) do { labels: { @@ -22,7 +22,9 @@ } end before do - allow_any_instance_of(::Diego::Client).to receive(:build_client).and_return(build_client) + allow(build_client).to receive(:ipaddr=).and_return('1.2.3.4') + allow(::Resolv).to receive(:getaddresses).and_return(['1.2.3.4']) + allow_any_instance_of(::Diego::Client).to receive(:new_http_client).and_return(build_client) end describe 'GET /v3/processes' do diff --git a/spec/request/v2/apps_spec.rb b/spec/request/v2/apps_spec.rb index 2c706fe63a1..7a052fede89 100644 --- a/spec/request/v2/apps_spec.rb +++ b/spec/request/v2/apps_spec.rb @@ -4,13 +4,15 @@ 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(:build_client) { instance_double(Net::HTTP, 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(build_client).to receive(:ipaddr=).and_return('1.2.3.4') + allow(::Resolv).to receive(:getaddresses).and_return(['1.2.3.4']) + allow_any_instance_of(::Diego::Client).to receive(:new_http_client).and_return(build_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..3d63cf212bd 100644 --- a/spec/request/v2/spaces_spec.rb +++ b/spec/request/v2/spaces_spec.rb @@ -258,12 +258,14 @@ 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(:build_client) { instance_double(Net::HTTP, 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(build_client).to receive(:ipaddr=).and_return('1.2.3.4') + allow(::Resolv).to receive(:getaddresses).and_return(['1.2.3.4']) + allow_any_instance_of(::Diego::Client).to receive(:new_http_client).and_return(build_client) end it 'returns the space summary' do