Add async recursive delete for apps and service instances

Behind `temporary_enable_async_recursive_delete flag` (default off). Recursive
delete jobs re-enqueue instead of failing when sub-resources (service bindings)
are deleted asynchronously, waiting for them to settle and surfacing the
original broker error on failure. Adds root/sub job tracking via `root_job_guid`
on the jobs table. Foundation for future recursive deletes (org, space).

Author: johha

app/actions/app_delete.rb

@@ -13,23 +13,12 @@
 require 'actions/route_mapping_delete'
 require 'actions/staging_cancel'
 require 'actions/mixins/bindings_delete'
+require 'errors/sub_resource_error'
 
 module VCAP::CloudController
   class AppDelete
     include V3::BindingsDeleteMixin
 
-    class AsyncBindingDeletionsTriggered < StandardError; end
-
-    class SubResourceError < StandardError
-      def initialize(errors)
-        @errors = errors
-      end
-
-      def underlying_errors
-        @errors
-      end
-    end
-
     def initialize(user_audit_info)
       @user_audit_info = user_audit_info
     end
@@ -86,7 +75,7 @@ def delete_subresources(app)
 
     def delete_non_transactional_subresources(app)
       errors = delete_bindings(app.service_bindings, user_audit_info: @user_audit_info)
-      raise SubResourceError.new(errors) if errors.any?
+      SubResourceError.raise_from(errors)
     end
 
     def stagers
@@ -106,10 +95,8 @@ def logger
       @logger ||= Steno.logger('cc.action.app_delete')
     end
 
-    def unbinding_operation_in_progress!(binding)
-      raise AsyncBindingDeletionsTriggered.new(
-        "An operation for the service binding between app #{binding.app.name} and service instance #{binding.service_instance.name} is in progress."
-      )
+    def unbinding_in_progress_message(binding)
+      "An operation for the service binding between app #{binding.app.name} and service instance #{binding.service_instance.name} is in progress."
     end
   end
 end

app/actions/app_stop.rb

@@ -3,7 +3,7 @@ class AppStop
     class InvalidApp < StandardError; end
 
     class << self
-      def stop(app:, user_audit_info:, record_event: true)
+      def stop(app:, user_audit_info:, record_event: true, delete_triggered: false)
         app.db.transaction do
           app.lock!
 
@@ -13,7 +13,7 @@ def stop(app:, user_audit_info:, record_event: true)
             process.update(state: ProcessModel::STOPPED)
           end
 
-          record_audit_event(app, user_audit_info) if record_event
+          record_audit_event(app, user_audit_info, delete_triggered:) if record_event
         end
       rescue Sequel::ValidationFailed => e
         raise InvalidApp.new(e.message)
@@ -25,10 +25,11 @@ def stop_without_event(app)
 
       private
 
-      def record_audit_event(app, user_audit_info)
+      def record_audit_event(app, user_audit_info, delete_triggered: false)
         Repositories::AppEventRepository.new.record_app_stop(
           app,
-          user_audit_info
+          user_audit_info,
+          delete_triggered:
         )
       end
     end

app/actions/mixins/bindings_delete.rb

@@ -3,6 +3,7 @@
 require 'jobs/generic_enqueuer'
 require 'jobs/v3/delete_binding_job'
 require 'jobs/v3/delete_service_binding_job_factory'
+require 'errors/sub_resource_error'
 
 module VCAP::CloudController
   module V3
@@ -21,12 +22,16 @@ def delete_bindings(bindings, user_audit_info:)
           unless result[:finished]
             polling_job = DeleteBindingJob.new(type, binding.guid, user_audit_info:)
             Jobs::GenericEnqueuer.shared.enqueue_pollable(polling_job)
-            unbinding_operation_in_progress!(binding)
+            raise AsyncOperationInProgress.new(unbinding_in_progress_message(binding))
           end
         rescue StandardError => e
           errors << e
         end
       end
+
+      def unbinding_in_progress_message(binding)
+        "An operation for service binding #{binding.guid} is in progress."
+      end
     end
   end
 end

app/actions/service_instance_unshare.rb

@@ -1,5 +1,6 @@
 require 'actions/service_credential_binding_delete'
 require 'actions/mixins/bindings_delete'
+require 'errors/sub_resource_error'
 
 module VCAP::CloudController
   class ServiceInstanceUnshare
@@ -8,11 +9,15 @@ class ServiceInstanceUnshare
     class Error < ::StandardError
     end
 
-    def unshare(service_instance, target_space, user_audit_info)
+    def unshare(service_instance, target_space, user_audit_info, fail_if_in_progress: true)
       errors = delete_bindings_in_target_space!(service_instance, target_space, user_audit_info)
       if errors.any?
-        error!("Unshare of service instance failed because one or more bindings could not be deleted.\n\n " \
-               "#{errors.map { |err| "\t#{err.message}" }.join("\n\n")}")
+        if fail_if_in_progress
+          raise Error.new("Unshare of service instance failed because one or more bindings could not be deleted.\n\n " \
+                          "#{errors.map { |err| "\t#{err.message}" }.join("\n\n")}")
+        else
+          SubResourceError.raise_from(errors)
+        end
       end
 
       service_instance.remove_shared_space(target_space)
@@ -24,19 +29,15 @@ def unshare(service_instance, target_space, user_audit_info)
 
     private
 
-    def error!(message)
-      raise Error.new(message)
-    end
-
     def delete_bindings_in_target_space!(service_instance, target_space, user_audit_info)
       active_bindings = ServiceBinding.where(service_instance_guid: service_instance.guid)
       bindings_in_target_space = active_bindings.all.select { |b| b.app.space_guid == target_space.guid }
       delete_bindings(bindings_in_target_space, user_audit_info:)
     end
 
-    def unbinding_operation_in_progress!(binding)
-      raise Error.new("The binding between an application and service instance #{binding.service_instance.name} " \
-                      "in space #{binding.app.space.name} is being deleted asynchronously.")
+    def unbinding_in_progress_message(binding)
+      "The binding between an application and service instance #{binding.service_instance.name} " \
+        "in space #{binding.app.space.name} is being deleted asynchronously."
     end
   end
 end

app/actions/v3/service_instance_delete.rb

@@ -4,6 +4,7 @@
 require 'actions/service_instance_unshare'
 require 'cloud_controller/errors/api_error'
 require 'actions/mixins/bindings_delete'
+require 'errors/sub_resource_error'
 
 module VCAP::CloudController
   module V3
@@ -13,9 +14,6 @@ class ServiceInstanceDelete
       class DeleteFailed < StandardError
       end
 
-      class UnbindingOperatationInProgress < StandardError
-      end
-
       DeleteStatus = Struct.new(:finished, :operation).freeze
       DeleteStarted = ->(operation) { DeleteStatus.new(false, operation) }
       DeleteComplete = DeleteStatus.new(true, nil).freeze
@@ -24,9 +22,10 @@ class UnbindingOperatationInProgress < StandardError
       PollingFinished = PollingStatus.new(true, nil).freeze
       ContinuePolling = ->(retry_after) { PollingStatus.new(false, retry_after) }
 
-      def initialize(service_instance, event_repo)
+      def initialize(service_instance, event_repo, fail_if_in_progress: true)
         @service_instance = service_instance
         @service_event_repository = event_repo
+        @fail_if_in_progress = fail_if_in_progress
       end
 
       def blocking_operation_in_progress?
@@ -38,7 +37,11 @@ def delete
         operation_in_progress! if blocking_operation_in_progress?
 
         errors = remove_associations
-        raise errors.first if errors.any?
+        if errors.any?
+          raise errors.first if @fail_if_in_progress # Single-shot callers fail on the first error
+
+          SubResourceError.raise_from(errors)
+        end
 
         result = send_deprovison_to_broker
         if result[:finished]
@@ -49,6 +52,11 @@ def delete
         end
 
         result
+      rescue SubResourceError => e
+        raise if !@fail_if_in_progress && e.any_in_progress? # re-raise SubResourceError so that root job continues to run
+
+        update_last_operation_with_failure(e.message) unless service_instance.operation_in_progress?
+        raise e
       rescue StandardError => e
         update_last_operation_with_failure(e.message) unless service_instance.operation_in_progress?
         raise e
@@ -154,7 +162,9 @@ def unshare_all_spaces
 
         unshare_action = ServiceInstanceUnshare.new
         space_guids.each_with_object([]) do |space_guid, errors|
-          unshare_action.unshare(service_instance, Space.first(guid: space_guid), service_event_repository.user_audit_info)
+          unshare_action.unshare(service_instance, Space.first(guid: space_guid), service_event_repository.user_audit_info, fail_if_in_progress: @fail_if_in_progress)
+        rescue SubResourceError => e
+          errors.concat(e.underlying_errors)
         rescue StandardError => e
           errors << e
         end
@@ -182,14 +192,12 @@ def operation_in_progress!
         raise CloudController::Errors::ApiError.new_from_details('AsyncServiceInstanceOperationInProgress', service_instance.name)
       end
 
-      def unbinding_operation_in_progress!(binding)
-        raise UnbindingOperatationInProgress.new(
-          if binding.is_a?(VCAP::CloudController::ServiceBinding)
-            "An operation for the service binding between app #{binding.app.name} and service instance #{service_instance.name} is in progress."
-          else
-            "An operation for a service binding of service instance #{service_instance.name} is in progress."
-          end
-        )
+      def unbinding_in_progress_message(binding)
+        if binding.is_a?(VCAP::CloudController::ServiceBinding)
+          "An operation for the service binding between app #{binding.app.name} and service instance #{service_instance.name} is in progress."
+        else
+          "An operation for a service binding of service instance #{service_instance.name} is in progress."
+        end
       end
 
       def delete_failed!(message)

app/controllers/runtime/apps_controller.rb

@@ -6,6 +6,7 @@
 require 'actions/v2/route_mapping_create'
 require 'models/helpers/process_types'
 require 'controllers/runtime/mixins/find_process_through_app'
+require 'errors/sub_resource_error'
 
 module VCAP::CloudController
   class AppsController < RestController::ModelController
@@ -148,7 +149,7 @@ def delete(guid)
         AppDelete.new(UserAuditInfo.from_context(SecurityContext)).delete_without_event([process.app])
       rescue Sequel::NoExistingObject
         raise self.class.not_found_exception(guid, AppModel)
-      rescue AppDelete::SubResourceError => e
+      rescue SubResourceError => e
         error_message = e.underlying_errors.map { |err| "\t" + err.message }.join("\n")
         raise CloudController::Errors::ApiError.new_from_details('AppRecursiveDeleteFailed', process.app.name, error_message)
       end

app/controllers/v3/apps_controller.rb

@@ -5,11 +5,13 @@
 require 'actions/app_update'
 require 'actions/app_patch_environment_variables'
 require 'actions/app_delete'
+require 'errors/sub_resource_error'
 require 'actions/app_restart'
 require 'actions/app_apply_manifest'
 require 'actions/app_start'
 require 'actions/app_stop'
 require 'actions/app_assign_droplet'
+require 'jobs/v3/recursive_delete_app_job'
 require 'decorators/include_space_decorator'
 require 'decorators/include_organization_decorator'
 require 'decorators/include_space_organization_decorator'
@@ -154,12 +156,23 @@ def destroy
     unauthorized! unless permission_queryer.can_write_to_active_space?(space.id)
     require_writable_space!(space)
 
-    delete_action = AppDelete.new(user_audit_info)
-    deletion_job  = VCAP::CloudController::Jobs::DeleteActionJob.new(AppModel, app.guid, delete_action)
+    if async_recursive_delete_enabled?
+      job = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_or_find_active_pollable(
+        resource_model: AppModel,
+        resource_guid: app.guid,
+        operation: 'app.delete'
+      ) { VCAP::CloudController::V3::RecursiveDeleteAppJob.new(app.guid, user_audit_info) }
 
-    job = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(deletion_job) do |pollable_job|
-      DeleteAppErrorTranslatorJob.new(pollable_job)
+      app_not_found! unless job
+    else
+      delete_action = AppDelete.new(user_audit_info)
+      deletion_job  = VCAP::CloudController::Jobs::DeleteActionJob.new(AppModel, app.guid, delete_action)
+
+      job = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(deletion_job) do |pollable_job|
+        DeleteAppErrorTranslatorJob.new(pollable_job)
+      end
     end
+
     VCAP::AppLogEmitter.emit(app.guid, "Enqueued job to delete app with guid #{app.guid}")
     head HTTP::ACCEPTED, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job.guid}")
   end
@@ -372,7 +385,7 @@ class DeleteAppErrorTranslatorJob < VCAP::CloudController::Jobs::ErrorTranslator
     include V3ErrorsHelper
 
     def translate_error(e)
-      if e.instance_of?(VCAP::CloudController::AppDelete::SubResourceError)
+      if e.instance_of?(VCAP::CloudController::SubResourceError)
         underlying_errors = e.underlying_errors.map { |err| unprocessable(err.message) }
         e = CloudController::Errors::CompoundError.new(underlying_errors)
       end
@@ -382,6 +395,10 @@ def translate_error(e)
 
   private
 
+  def async_recursive_delete_enabled?
+    !!VCAP::CloudController::Config.config.get(:temporary_enable_async_recursive_delete, :apps)
+  end
+
   def handle_order_by_presented_value(page_results)
     return unless page_results.try(:pagination_options).try(:order_by) == 'desired_state'
 

app/controllers/v3/service_instances_controller.rb

@@ -29,6 +29,7 @@
 require 'decorators/field_service_instance_plan_decorator'
 require 'jobs/v3/create_service_instance_job'
 require 'jobs/v3/update_service_instance_job'
+require 'jobs/v3/recursive_delete_service_instance_job'
 
 class ServiceInstancesV3Controller < ApplicationController
   include ServicePermissions
@@ -115,13 +116,33 @@ def destroy
     end
 
     delete_action = V3::ServiceInstanceDelete.new(service_instance, service_event_repository)
-    operation_in_progress! if delete_action.blocking_operation_in_progress?
 
     case service_instance
     when VCAP::CloudController::ManagedServiceInstance
-      job_guid = enqueue_delete_job(service_instance)
-      head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job_guid}")
+      if async_recursive_delete_enabled?
+        # Idempotent retry: return the active delete job instead of 422-ing on its delete-in-progress last_operation.
+        active_delete = PollableJobModel.find_active_delete(resource_guid: service_instance.guid, operation: 'service_instance.delete')
+        operation_in_progress! if active_delete.nil? && delete_action.blocking_operation_in_progress?
+
+        job = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_or_find_active_pollable(
+          resource_model: ManagedServiceInstance,
+          resource_guid: service_instance.guid,
+          operation: 'service_instance.delete'
+        ) do |locked_instance|
+          log_service_instance_deletion(locked_instance)
+          V3::RecursiveDeleteServiceInstanceJob.new(locked_instance.guid, user_audit_info)
+        end
+
+        service_instance_not_found! unless job
+
+        head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job.guid}")
+      else
+        operation_in_progress! if delete_action.blocking_operation_in_progress?
+        job_guid = enqueue_delete_job(service_instance)
+        head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job_guid}")
+      end
     when VCAP::CloudController::UserProvidedServiceInstance
+      operation_in_progress! if delete_action.blocking_operation_in_progress?
       delete_action.delete
       head :no_content
     end
@@ -391,9 +412,7 @@ def fetch_writable_service_instance(guid)
     service_instance
   end
 
-  def enqueue_delete_job(service_instance)
-    delete_job = V3::DeleteServiceInstanceJob.new(service_instance.guid, user_audit_info)
-
+  def log_service_instance_deletion(service_instance)
     plan = service_instance.service_plan
     service = plan.service
     broker = service.service_broker
@@ -404,11 +423,20 @@ def enqueue_delete_job(service_instance)
       "from service offering '#{service.label}' " \
       "provided by broker '#{broker.name}'."
     )
+  end
 
+  # Legacy enqueue path used when the async-recursive-delete rollout flag is off. Preserves main's behaviour.
+  def enqueue_delete_job(service_instance)
+    log_service_instance_deletion(service_instance)
+    delete_job = V3::DeleteServiceInstanceJob.new(service_instance.guid, user_audit_info)
     pollable_job = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(delete_job)
     pollable_job.guid
   end
 
+  def async_recursive_delete_enabled?
+    !!VCAP::CloudController::Config.config.get(:temporary_enable_async_recursive_delete, :service_instances)
+  end
+
   def unreadable_error_message(service_instance_name, unreadable_space_guids)
     return unless unreadable_space_guids.any?