Skip to content

Commit

Permalink
Merge pull request #651 from cfibmers/staging_during_evac
Browse files Browse the repository at this point in the history
Handle when selected DEA stager returns a 503
  • Loading branch information
rizwanreza committed Jul 25, 2016
2 parents 5266e93 + 96103f1 commit 323b01e
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 11 deletions.
8 changes: 6 additions & 2 deletions lib/cloud_controller/dea/app_stager_task.rb
Expand Up @@ -65,8 +65,12 @@ def handle_http_response(response, &callback)
private

def stage_with_http(url, msg)
success = Dea::Client.stage(url, msg)
stage_with_nats(msg) unless success
status = Dea::Client.stage(url, msg)
stage_with_nats(msg) if status == 404
if status == 503
logger.info('staging.http.stager-evacuating', app_guid: @app.guid, status: status)
stage { @completion_callback }
end
rescue => e
@app.mark_as_failed_to_stage('StagingError')
logger.error e.message
Expand Down
3 changes: 1 addition & 2 deletions lib/cloud_controller/dea/client.rb
Expand Up @@ -47,8 +47,7 @@ def stage(url, msg)
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
return status if [202, 404, 503].include?(status)
rescue => e
raise ApiError.new_from_details('StagerError', "url: #{url}/v1/stage, error: #{e.message}")
end
Expand Down
16 changes: 13 additions & 3 deletions spec/unit/lib/cloud_controller/dea/app_stager_task_spec.rb
Expand Up @@ -95,15 +95,15 @@ def stage(&blk)
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).and_return(true)
expect(Dea::Client).to receive(:stage).with(dea_advertisement.url, staging_message).and_return(202)
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(Dea::Client).to receive(:stage).with(dea_advertisement.url, staging_message).and_return(404)
expect(message_bus).to receive(:publish).with('staging.my_stager.start', staging_task.staging_request)

stage
Expand All @@ -115,15 +115,25 @@ def stage(&blk)

before do
allow(staging_task).to receive(:logger).and_return(logger)
allow(Dea::Client).to receive(:stage).and_raise 'failure'
allow(logger).to receive(:info)
end

it 'marks app as failed and raises an error' do
allow(Dea::Client).to receive(:stage).and_raise 'failure'

expect(app).to receive(:mark_as_failed_to_stage).with('StagingError')
expect(logger).to receive(:error).with(/failure/)
expect { staging_task.stage }.to raise_error 'failure'
end

context 'when the dea chosen returns a 503' do
it 'retries to stage' do
expect(Dea::Client).to receive(:stage).and_return(503, 202)
expect(staging_task).to receive(:stage).twice.and_call_original

staging_task.stage
end
end
end
end

Expand Down
15 changes: 11 additions & 4 deletions spec/unit/lib/cloud_controller/dea/client_spec.rb
Expand Up @@ -104,18 +104,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)).to be true
expect(Dea::Client.stage(stager_url, staging_msg)).to be 202
end

context 'when an error occurs' do
context 'when we get a status of 404' do
it 'returns false' do
it 'returns 404' 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 false
expect(Dea::Client.stage(stager_url, staging_msg)).to eq 404
end
end

context 'when we get a status other than 202 or 404' do
context 'when we get a status other than 202, 404 or 503' 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 All @@ -128,6 +128,13 @@ def create_ad(id, url=nil)
expect { Dea::Client.stage(stager_url, staging_msg) }.to raise_error CloudController::Errors::ApiError, /#{stager_url}.*failed to connect/
end
end

context 'when we get a 503' do
it 'returns a 503' do
stub_request(:post, post_url).with(body: /#{staging_msg}/, headers: { 'Content-Type' => 'application/json' }).to_return(status: [503, 'Service Unavailable'])
expect(Dea::Client.stage(stager_url, staging_msg)).to eq 503
end
end
end

context 'when not configured for HTTP' do
Expand Down

0 comments on commit 323b01e

Please sign in to comment.