Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 4 additions & 17 deletions app/actions/app_delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
9 changes: 5 additions & 4 deletions app/actions/app_stop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!

Expand All @@ -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)
Expand All @@ -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
Expand Down
7 changes: 6 additions & 1 deletion app/actions/mixins/bindings_delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
21 changes: 11 additions & 10 deletions app/actions/service_instance_unshare.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand All @@ -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
36 changes: 22 additions & 14 deletions app/actions/v3/service_instance_delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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?
Expand All @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/runtime/apps_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
27 changes: 22 additions & 5 deletions app/controllers/v3/apps_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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'

Expand Down
40 changes: 34 additions & 6 deletions app/controllers/v3/service_instances_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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?

Expand Down
Loading
Loading