From c843c328c6bc133b8eb107b0793fd5ed72ffbb5f Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 21 Dec 2023 09:32:20 +0100 Subject: [PATCH] Put binding and job creation in one transaction When the DB becomes unavailable after the service binding operation was stored in the DB, but before the delayed job was stored, the operation will be stuck in state 'create initial'. The user cannot delete the binding nor retrigger the creation. An admin has to set the operation to state failed manually in the DB. This fixes it in that way, that both the operation and the delayed job get created in one DB transaction. --- .../service_credential_bindings_controller.rb | 32 ++++++++++--------- .../v3/service_route_bindings_controller.rb | 20 ++++++------ .../service_credential_bindings_spec.rb | 17 ++++++++++ spec/request/service_route_bindings_spec.rb | 17 ++++++++++ 4 files changed, 62 insertions(+), 24 deletions(-) diff --git a/app/controllers/v3/service_credential_bindings_controller.rb b/app/controllers/v3/service_credential_bindings_controller.rb index 3010706ddab..859b42e087a 100644 --- a/app/controllers/v3/service_credential_bindings_controller.rb +++ b/app/controllers/v3/service_credential_bindings_controller.rb @@ -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 diff --git a/app/controllers/v3/service_route_bindings_controller.rb b/app/controllers/v3/service_route_bindings_controller.rb index 800a0aaedae..ca013b45f56 100644 --- a/app/controllers/v3/service_route_bindings_controller.rb +++ b/app/controllers/v3/service_route_bindings_controller.rb @@ -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) diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index 905dc15f4af..a6396e310db 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -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 diff --git a/spec/request/service_route_bindings_spec.rb b/spec/request/service_route_bindings_spec.rb index 3023220bd57..f9f008d4a13 100644 --- a/spec/request/service_route_bindings_spec.rb +++ b/spec/request/service_route_bindings_spec.rb @@ -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