Skip to content

Commit

Permalink
Merge pull request #3567 from sap-contributions/prevent_stuck_binding…
Browse files Browse the repository at this point in the history
…_operation_when_db_is_unavailable

Put binding and job creation in one transaction
  • Loading branch information
kathap authored Jan 2, 2024
2 parents e88659c + c843c32 commit 596ded9
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 24 deletions.
32 changes: 17 additions & 15 deletions app/controllers/v3/service_credential_bindings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -216,21 +216,23 @@ def get_service_instance!(service_instance_guid)

def create_app_binding(message, service_instance, app)
action = V3::ServiceCredentialBindingAppCreate.new(user_audit_info, message.audit_hash)
binding = action.precursor(
service_instance,
app: app,
volume_mount_services_enabled: volume_services_enabled?,
message: message
)
log_telemetry(binding)

case service_instance
when ManagedServiceInstance
pollable_job_guid = enqueue_bind_job(:credential, binding.guid, message)
head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}")
when UserProvidedServiceInstance
action.bind(binding)
render status: :created, json: Presenters::V3::ServiceCredentialBindingPresenter.new(binding).to_hash
VCAP::CloudController::ServiceBinding.db.transaction do
binding = action.precursor(
service_instance,
app: app,
volume_mount_services_enabled: volume_services_enabled?,
message: message
)
log_telemetry(binding)

case service_instance
when ManagedServiceInstance
pollable_job_guid = enqueue_bind_job(:credential, binding.guid, message)
head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}")
when UserProvidedServiceInstance
action.bind(binding)
render status: :created, json: Presenters::V3::ServiceCredentialBindingPresenter.new(binding).to_hash
end
end
end

Expand Down
20 changes: 11 additions & 9 deletions app/controllers/v3/service_route_bindings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,17 @@ def create

check_parameters_support(service_instance, message)
action = V3::ServiceRouteBindingCreate.new(user_audit_info, message.audit_hash)
precursor = action.precursor(service_instance, route, message:)

case service_instance
when ManagedServiceInstance
pollable_job_guid = enqueue_bind_job(precursor.guid, message)
head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}")
when UserProvidedServiceInstance
action.bind(precursor)
render status: :created, json: Presenters::V3::ServiceRouteBindingPresenter.new(precursor)
VCAP::CloudController::RouteBinding.db.transaction do
precursor = action.precursor(service_instance, route, message:)

case service_instance
when ManagedServiceInstance
pollable_job_guid = enqueue_bind_job(precursor.guid, message)
head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}")
when UserProvidedServiceInstance
action.bind(precursor)
render status: :created, json: Presenters::V3::ServiceRouteBindingPresenter.new(precursor)
end
end
rescue V3::ServiceRouteBindingCreate::UnprocessableCreate => e
unprocessable!(e.message)
Expand Down
17 changes: 17 additions & 0 deletions spec/request/service_credential_bindings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,23 @@ def check_filtered_bindings(*bindings)
api_call.call(space_dev_headers)
end
end

context 'database disconnect error during creation of enqueue bind job' do
before do
allow_any_instance_of(ServiceCredentialBindingsController).to receive(:enqueue_bind_job).and_raise(Sequel::DatabaseDisconnectError)
end

it 'rolls back the transaction' do
api_call.call(admin_headers)

expect(last_response).to have_status_code(503)
expect(parsed_response['errors']).to include(include({
'detail' => include('Database connection failure'),
'title' => 'CF-ServiceUnavailable',
'code' => 10_015
}))
end
end
end
end

Expand Down
17 changes: 17 additions & 0 deletions spec/request/service_route_bindings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,23 @@
}))
end
end

context 'database disconnect error during creation of enqueue bind job' do
before do
allow_any_instance_of(ServiceRouteBindingsController).to receive(:enqueue_bind_job).and_raise(Sequel::DatabaseDisconnectError)
end

it 'rolls back the transaction' do
api_call.call(admin_headers)

expect(last_response).to have_status_code(503)
expect(parsed_response['errors']).to include(include({
'detail' => include('Database connection failure'),
'title' => 'CF-ServiceUnavailable',
'code' => 10_015
}))
end
end
end

context 'user-provided service instance' do
Expand Down

0 comments on commit 596ded9

Please sign in to comment.