Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallback to NATs if staging over http isn't there #591

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/cloud_controller/dea/app_stager_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ def handle_http_response(response)
private

def stage_with_http(url, msg)
Dea::Client.stage(url, msg)
success = Dea::Client.stage(url, msg)
stage_with_nats(msg) unless success
rescue => e
@app.mark_as_failed_to_stage('StagingError')
logger.error e.message
Expand Down
7 changes: 6 additions & 1 deletion lib/cloud_controller/dea/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,17 @@ def enabled?
def stage(url, msg)
raise ApiError.new_from_details('StagerError', 'Client not HTTP enabled') unless enabled?

status = 999
begin
response = @http_client.post("#{url}/v1/stage", header: { 'Content-Type' => 'application/json' }, body: MultiJson.dump(msg))
status = response.status
return true if status == 202
return false if status == 404
rescue => e
raise ApiError.new_from_details('StagerError', "url: #{url}/v1/stage, error: #{e.message}")
end
raise ApiError.new_from_details('StagerError', "received #{response.status} from #{url}/v1/stage") unless response.status == 202

raise ApiError.new_from_details('StagerError', "received #{status} from #{url}/v1/stage")
end

def start(app, options={})
Expand Down
33 changes: 22 additions & 11 deletions spec/unit/lib/cloud_controller/dea/app_stager_task_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ module VCAP::CloudController
}
end

def stage(&blk)
stub_schedule_sync do
@before_staging_completion.call if @before_staging_completion
message_bus.respond_to_request("staging.#{stager_id}.start", first_reply_json)
end

response = staging_task.stage(&blk)
response
end

before do
expect(app.staged?).to be false

Expand All @@ -85,10 +95,21 @@ module VCAP::CloudController
expect(app).to receive(:update).with({ package_state: 'PENDING', staging_task_id: staging_task.task_id })
expect(message_bus).to receive(:publish).with('staging.stop', { app_id: app.guid })
expect(dea_pool).to receive(:reserve_app_memory).with(stager_id, app.memory)
expect(Dea::Client).to receive(:stage).with(dea_advertisement.url, staging_message)
expect(Dea::Client).to receive(:stage).with(dea_advertisement.url, staging_message).and_return(true)
expect(message_bus).not_to receive(:publish).with('staging.my_stager.start', staging_task.staging_request)
staging_task.stage
end

context 'when staging is not supported' do
it 'failsover to NATs' do
expect(message_bus).to receive(:publish).with('staging.stop', { app_id: app.guid })
expect(Dea::Client).to receive(:stage).with(dea_advertisement.url, staging_message).and_return(false)
expect(message_bus).to receive(:publish).with('staging.my_stager.start', staging_task.staging_request)

stage
end
end

context 'when an error occurs' do
let(:logger) { double(Steno) }

Expand Down Expand Up @@ -441,16 +462,6 @@ module VCAP::CloudController

describe 'staging' do
describe 'receiving the first response from the stager (the staging setup completion message)' do
def stage(&blk)
stub_schedule_sync do
@before_staging_completion.call if @before_staging_completion
message_bus.respond_to_request("staging.#{stager_id}.start", first_reply_json)
end

response = staging_task.stage(&blk)
response
end

context 'it sets up the app' do
before do
allow(app).to receive(:update).and_call_original
Expand Down
11 changes: 9 additions & 2 deletions spec/unit/lib/cloud_controller/dea/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,18 @@ def create_ad(id, url=nil)

it 'sends a stage message to the dea' do
stub_request(:post, post_url).with(body: /#{staging_msg}/, headers: { 'Content-Type' => 'application/json' }).to_return(status: [202, 'Accepted'])
expect { Dea::Client.stage(stager_url, staging_msg) }.not_to raise_error
expect(Dea::Client.stage(stager_url, staging_msg)).to be_truthy
end

context 'when an error occurs' do
context 'when we get a status other than 202' do
context 'when we get a status of 404' do
it 'returns false' do
stub_request(:post, post_url).with(body: /#{staging_msg}/, headers: { 'Content-Type' => 'application/json' }).to_return(status: [404, 'Not Found'])
expect(Dea::Client.stage(stager_url, staging_msg)).to be_falsey
end
end

context 'when we get a status other than 202 or 404' do
it 'raises an error' do
stub_request(:post, post_url).with(body: /#{staging_msg}/, headers: { 'Content-Type' => 'application/json' }).to_return(status: [500, 'Internal Server Error'])
expect { Dea::Client.stage(stager_url, staging_msg) }.to raise_error CloudController::Errors::ApiError, /received 500 from #{post_url}/
Expand Down