diff --git a/app/actions/app_delete.rb b/app/actions/app_delete.rb index 2f301712b11..1d7cd5a25b4 100644 --- a/app/actions/app_delete.rb +++ b/app/actions/app_delete.rb @@ -52,6 +52,8 @@ def delete(apps, record_event: true) end logger.info("Deleted app: #{app.guid}") end + + [] end def delete_without_event(apps) diff --git a/app/actions/organization_delete.rb b/app/actions/organization_delete.rb index 81ad6c4254c..76327654ed1 100644 --- a/app/actions/organization_delete.rb +++ b/app/actions/organization_delete.rb @@ -35,6 +35,8 @@ def delete(org_dataset) Repositories::OrganizationEventRepository.new.record_organization_delete_request(org, @user_audit_info, { recursive: true }) end end + + [] end def timeout_error(dataset) diff --git a/app/actions/v2/organization_delete.rb b/app/actions/v2/organization_delete.rb index c6c8b69c173..7d463fa17a1 100644 --- a/app/actions/v2/organization_delete.rb +++ b/app/actions/v2/organization_delete.rb @@ -21,6 +21,7 @@ def delete(org_dataset) org.destroy end end + [] end def timeout_error(dataset) diff --git a/app/jobs/delete_action_job.rb b/app/jobs/delete_action_job.rb index a6e3de0fdf2..2e0e4b33c0b 100644 --- a/app/jobs/delete_action_job.rb +++ b/app/jobs/delete_action_job.rb @@ -17,13 +17,23 @@ def perform logger.info("Deleting model class '#{model_class}' with guid '#{resource_guid}'") dataset = model_class.where(guid: resource_guid) - if delete_action_can_return_warnings? - errors, warnings = delete_action.delete(dataset) - else - errors = delete_action.delete(dataset) + errors = [] + + begin + if delete_action_can_return_warnings? + errors, warnings = delete_action.delete(dataset) + else + errors = delete_action.delete(dataset) + end + rescue StandardError => e + errors << e end - raise errors.first unless errors&.empty? + # Ignore errors if the target resource has already been deleted (e.g., by a parallel job) + quoted_table_name = model_class.db.quote_identifier(model_class.table_name) + errors.reject! { |err| err.is_a?(Sequel::NoExistingObject) && err.message.include?("DELETE FROM #{quoted_table_name}") } unless errors.frozen? + + raise errors.first unless errors.empty? warnings end diff --git a/app/models/runtime/route.rb b/app/models/runtime/route.rb index 7ec7a0b2264..201752f2494 100644 --- a/app/models/runtime/route.rb +++ b/app/models/runtime/route.rb @@ -263,6 +263,10 @@ def internal_route_vip_range def destroy_route_bindings errors = RouteBindingDelete.new.delete(route_binding_dataset) + + quoted_table_name = RouteBinding.db.quote_identifier(RouteBinding.table_name) + errors.reject! { |e| e.is_a?(Sequel::NoExistingObject) && e.message.include?("DELETE FROM #{quoted_table_name}") } + raise errors.first unless errors.empty? end diff --git a/spec/unit/jobs/delete_action_job_spec.rb b/spec/unit/jobs/delete_action_job_spec.rb index a77774be72d..070c74c42f8 100644 --- a/spec/unit/jobs/delete_action_job_spec.rb +++ b/spec/unit/jobs/delete_action_job_spec.rb @@ -2,6 +2,19 @@ module VCAP::CloudController module Jobs + RSpec.shared_examples 'a delete action handling external deletion' do + before do + allow_any_instance_of(resource.class).to receive(:destroy).and_wrap_original do |original_method, *args| + Sequel::Model.db.run("DELETE FROM #{resource.class.table_name} WHERE id = #{resource.id}") # Simulate external deletion + original_method.call(*args) + end + end + + it 'still attempts to delete the resource even if it was already deleted externally' do + expect { delete_job.perform }.not_to raise_error + end + end + RSpec.describe DeleteActionJob, job_context: :worker do let(:user) { User.make(admin: true) } let(:delete_action) { instance_double(SpaceDelete, delete: []) } @@ -127,6 +140,32 @@ module Jobs expect(job.resource_guid).to eq(space.guid) end end + + context 'when the resource is deleted externally before destroy' do + it_behaves_like 'a delete action handling external deletion' do + let(:resource) { PackageModel.make } + let(:delete_action) { PackageDelete.new(nil) } + let(:delete_job) { DeleteActionJob.new(PackageModel, resource.guid, delete_action) } + end + + it_behaves_like 'a delete action handling external deletion' do + let(:resource) { Space.make } + let(:delete_action) { SpaceDelete.new(nil, nil) } + let(:delete_job) { DeleteActionJob.new(Space, resource.guid, delete_action) } + end + + it_behaves_like 'a delete action handling external deletion' do + let(:resource) { Route.make } + let(:delete_action) { RouteDeleteAction.new(nil) } + let(:delete_job) { DeleteActionJob.new(Route, resource.guid, delete_action) } + end + + it_behaves_like 'a delete action handling external deletion' do + let(:resource) { User.make } + let(:delete_action) { UserDeleteAction.new } + let(:delete_job) { DeleteActionJob.new(User, resource.guid, delete_action) } + end + end end end end diff --git a/spec/unit/models/runtime/route_spec.rb b/spec/unit/models/runtime/route_spec.rb index 173d4cf1234..52424d31993 100644 --- a/spec/unit/models/runtime/route_spec.rb +++ b/spec/unit/models/runtime/route_spec.rb @@ -1284,6 +1284,23 @@ module VCAP::CloudController expect(process.reload.routes[0]).to eq route end end + + context 'when route_binding is deleted externally before destroy' do + before do + allow_any_instance_of(ServiceKeyDelete).to receive(:delete_service_binding).and_wrap_original do |original_method, *args| + service_binding = args.first + service_binding.destroy + original_method.call(*args) + end + end + + it 'does not raise a Sequel::NoExistingObject error' do + route_binding = RouteBinding.make + route = route_binding.route + stub_unbind(route_binding) + expect { route.destroy }.not_to raise_error + end + end end end