Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit 80176ff

Browse files
Derik Evangelistablgm
andcommitted
v3: deprovision SI when LO returs state failed
[#172330022](https://www.pivotaltracker.com/story/show/172330022) Co-authored-by: George Blue <gblue@pivotal.io> Co-authored-by: Derik Evangelista <devangelista@pivotal.io> Co-authored-by: George Blue <gblue@pivotal.io>
1 parent 08e27ca commit 80176ff

5 files changed

Lines changed: 49 additions & 4 deletions

File tree

docs/v3/source/includes/experimental_resources/service_instances/_create.md.erb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,11 @@ Content-Type: application/json
9999
```
100100

101101
This endpoint creates a new service instance. Service instances can be of type `managed` or `user-provided`, and
102-
the required parameters are different for each type.
102+
the required parameters are different for each type. User provided service instances do not require interactions with
103+
service brokers.
104+
105+
If failures occur when creating managed service instances, the API might execute orphan mitigation steps
106+
accordingly to cases outlined in the [OSBAPI specification](https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#orphan-mitigation)
103107

104108
#### Definition
105109
`POST /v3/service_instances`

lib/services/service_brokers/v2/client.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,10 @@ def fetch_service_instance_last_operation(instance)
263263
}
264264
}
265265

266+
if result[:last_operation][:state] == 'failed'
267+
@orphan_mitigator.cleanup_failed_provision(@attrs, instance)
268+
end
269+
266270
result[:last_operation][:description] = last_operation_hash['description'] if last_operation_hash['description']
267271
result[:retry_after] = response[HttpResponse::HEADER_RETRY_AFTER] if response[HttpResponse::HEADER_RETRY_AFTER]
268272
result.merge(parsed_response.symbolize_keys)

spec/request/service_instances_spec.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1136,7 +1136,7 @@ def check_filtered_instances(*instances)
11361136
expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)
11371137

11381138
Timecop.freeze(Time.now + 1.hour) do
1139-
execute_all_jobs(expected_successes: 0, expected_failures: 1)
1139+
execute_all_jobs(expected_successes: 0, expected_failures: 1, jobs_to_execute: 1)
11401140
end
11411141
end
11421142

@@ -1149,6 +1149,12 @@ def check_filtered_instances(*instances)
11491149
expect(instance.last_operation.type).to eq('create')
11501150
expect(instance.last_operation.state).to eq('failed')
11511151
end
1152+
1153+
it 'fires an orphan mitigation job' do
1154+
jobs = Delayed::Job.where(failed_at: nil).all
1155+
expect(jobs).to have(1).jobs
1156+
expect(jobs.first).to be_a_fully_wrapped_job_of(VCAP::CloudController::Jobs::Services::DeleteOrphanedInstance)
1157+
end
11521158
end
11531159
end
11541160
end

spec/support/background_job_helpers.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
module BackgroundJobHelpers
22
include VCAP::CloudController
33

4-
def execute_all_jobs(expected_successes:, expected_failures:)
5-
successes, failures = Delayed::Worker.new.work_off
4+
def execute_all_jobs(expected_successes:, expected_failures:, jobs_to_execute: 100)
5+
successes, failures = Delayed::Worker.new.work_off(jobs_to_execute)
66
failure_message = "Expected #{expected_successes} successful and #{expected_failures} failed jobs, got #{successes} successful and #{failures} failed jobs."
77
fail_summaries = Delayed::Job.exclude(failed_at: nil).map { |j| "Handler: #{j.handler}, LastError: #{j.last_error}" }
88
if fail_summaries.count > 0

spec/unit/lib/services/service_brokers/v2/client_spec.rb

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,37 @@ module VCAP::Services::ServiceBrokers::V2
436436
end
437437
end
438438

439+
context 'when the broker returns 200 (ok)' do
440+
let(:code) { 200 }
441+
let(:message) { 'OK' }
442+
443+
context 'when the state is `failed`' do
444+
let(:response_data) do
445+
{
446+
state: 'failed'
447+
}
448+
end
449+
450+
it 'performs orphan mitigation' do
451+
client.fetch_service_instance_last_operation(instance)
452+
expect(orphan_mitigator).to have_received(:cleanup_failed_provision).with(client_attrs, instance)
453+
end
454+
end
455+
456+
context 'when the state is something else' do
457+
let(:response_data) do
458+
{
459+
state: 'succeeded'
460+
}
461+
end
462+
463+
it 'does not perform orphan mitigation' do
464+
client.fetch_service_instance_last_operation(instance)
465+
expect(orphan_mitigator).not_to have_received(:cleanup_failed_provision)
466+
end
467+
end
468+
end
469+
439470
context 'when the broker returns 410 (gone)' do
440471
let(:code) { 410 }
441472
let(:message) { 'GONE' }

0 commit comments

Comments
 (0)