v3(services): deleting msi synchronously (cloudfoundry#1729)

[finishes #171726168](https://www.pivotaltracker.com/story/show/171726168)

Co-authored-by: George Blue <gblue@pivotal.io>
Co-authored-by: Brian Butz <butzopower@gmail.com>

Author: 3 people

app/actions/v3/service_instance_delete.rb

@@ -1,9 +1,11 @@
+require 'jobs/v3/delete_service_instance_job'
+
 module VCAP::CloudController
   module V3
     class ServiceInstanceDelete
       class AssociationNotEmptyError < StandardError; end
+
       class InstanceSharedError < StandardError; end
-      class NotImplementedError < StandardError; end
 
       def initialize(event_repo)
         @service_event_repository = event_repo
@@ -14,20 +16,38 @@ def delete(service_instance)
 
         cannot_delete_shared_instances! if service_instance.shared?
 
-        case service_instance
-        when ManagedServiceInstance
-          raise NotImplementedError
-        end
+        lock = DeleterLock.new(service_instance)
+        lock.lock!
+
+        job = case service_instance
+              when ManagedServiceInstance
+                asynchronous_destroy(service_instance)
+              when UserProvidedServiceInstance
+                synchronous_destroy(service_instance, lock)
+              end
 
-        service_instance.db.transaction do
-          service_instance.lock!
-          service_instance.destroy
-          service_event_repository.record_user_provided_service_instance_event(:delete, service_instance, {})
-        end
+        job
       end
 
       private
 
+      def synchronous_destroy(service_instance, lock)
+        lock.unlock_and_destroy!
+        service_event_repository.record_user_provided_service_instance_event(:delete, service_instance, {})
+        nil
+      end
+
+      def asynchronous_destroy(service_instance)
+        delete_job = V3::DeleteServiceInstanceJob.new(
+          service_instance.guid,
+          :deprovision,
+          service_event_repository.user_audit_info)
+
+        pollable_job = Jobs::Enqueuer.new(delete_job, queue: Jobs::Queues.generic).enqueue_pollable
+
+        pollable_job.guid
+      end
+
       def association_not_empty!
         raise AssociationNotEmptyError
       end

app/controllers/v3/service_instances_controller.rb

@@ -109,16 +109,18 @@ def destroy
 
     unauthorized! unless can_write_space?(service_instance.space)
 
-    service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithUserActor.new(user_audit_info)
-    V3::ServiceInstanceDelete.new(service_event_repository).delete(service_instance)
+    service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository.new(user_audit_info)
+    job_guid = V3::ServiceInstanceDelete.new(service_event_repository).delete(service_instance)
 
-    head :no_content
+    if job_guid.blank?
+      head :no_content
+    else
+      head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job_guid}")
+    end
   rescue V3::ServiceInstanceDelete::AssociationNotEmptyError
     associations_not_empty!
   rescue V3::ServiceInstanceDelete::InstanceSharedError
     cannot_delete_shared_instances!(service_instance.name)
-  rescue V3::ServiceInstanceDelete::NotImplementedError
-    head :not_implemented
   end
 
   def share_service_instance

app/jobs/v3/delete_service_instance_job.rb

@@ -0,0 +1,93 @@
+require 'jobs/reoccurring_job'
+
+module VCAP::CloudController
+  module V3
+    class DeleteServiceInstanceJob < VCAP::CloudController::Jobs::ReoccurringJob
+      attr_reader :warnings
+
+      def initialize(guid, operation, audit_info)
+        super()
+        @service_instance_guid = guid
+        @operation = operation
+        @client_arguments = { accepts_incomplete: false }
+        @user_audit_info = audit_info
+        @warnings = []
+      end
+
+      def perform
+        client = VCAP::Services::ServiceClientProvider.provide({ instance: service_instance })
+
+        execute_request(client)
+
+        finish
+      end
+
+      def job_name_in_configuration
+        "service_instance_#{operation_type}"
+      end
+
+      def max_attempts
+        1
+      end
+
+      def resource_type
+        'service_instances'
+      end
+
+      def resource_guid
+        service_instance_guid
+      end
+
+      def display_name
+        "service_instance.#{operation_type}"
+      end
+
+      private
+
+      attr_reader :service_instance_guid
+
+      def service_instance
+        ManagedServiceInstance.first(guid: @service_instance_guid)
+      end
+
+      def execute_request(client)
+        broker_response = client.public_send(
+          @operation,
+          service_instance,
+          @client_arguments
+        )
+
+        if broker_response.dig(:last_operation, :state) == 'succeeded'
+          ServiceInstance.db.transaction do
+            si = service_instance
+            service_instance.lock!
+            service_instance.last_operation&.destroy
+            service_instance.destroy
+            record_event(si, nil)
+          end
+        end
+      rescue => e
+        service_instance.save_with_new_operation({}, {
+          type: operation_type,
+          state: 'failed',
+          description: e.message,
+        })
+        raise e
+      end
+
+      def record_event(service_instance, request_attrs)
+        Repositories::ServiceEventRepository.new(@user_audit_info).
+          record_service_instance_event(:delete, service_instance, request_attrs)
+      end
+
+      def operation_type
+        case @operation
+        when :deprovision
+          'delete'
+        else
+          ''
+        end
+      end
+    end
+  end
+end

spec/request/service_instances_spec.rb

@@ -2366,6 +2366,99 @@ def check_filtered_instances(*instances)
 
     context 'managed service instance' do
       let!(:instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space) }
+      let(:broker_status_code) { 200 }
+      let(:broker_response) { {} }
+
+      before do
+        stub_request(:delete, "#{instance.service_broker.broker_url}/v2/service_instances/#{instance.guid}").
+          with(query: {
+            # 'accepts_incomplete' => false,
+            'service_id' => instance.service.broker_provided_id,
+            'plan_id' => instance.service_plan.broker_provided_id
+          }).
+          to_return(status: broker_status_code, body: broker_response.to_json, headers: {})
+      end
+
+      it 'sets the service instance last operation to delete in progress' do
+        api_call.call(admin_headers)
+        expect(last_response).to have_status_code(HTTP::Status::ACCEPTED)
+
+        instance.reload
+
+        expect(instance.last_operation).to_not be_nil
+        expect(instance.last_operation.type).to eq('delete')
+        expect(instance.last_operation.state).to eq('in progress')
+      end
+
+      it 'responds with job resource' do
+        api_call.call(admin_headers)
+        expect(last_response).to have_status_code(202)
+
+        job = VCAP::CloudController::PollableJobModel.last
+        expect(last_response.headers['Location']).to end_with("/v3/jobs/#{job.guid}")
+
+        expect(job.state).to eq(VCAP::CloudController::PollableJobModel::PROCESSING_STATE)
+        expect(job.operation).to eq('service_instance.delete')
+        expect(job.resource_guid).to eq(instance.guid)
+        expect(job.resource_type).to eq('service_instances')
+      end
+
+      describe 'the pollable job' do
+        it 'sends a delete request with the right arguments to the service broker' do
+          api_call.call(admin_headers)
+
+          execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+          expect(
+            a_request(:delete, "#{instance.service_broker.broker_url}/v2/service_instances/#{instance.guid}").
+              with(query: {
+                'service_id' => instance.service.broker_provided_id,
+                'plan_id' => instance.service_plan.broker_provided_id
+              })
+          ).to have_been_made.once
+        end
+
+        context 'when the service broker responds synchronously' do
+          context 'with success' do
+            it 'removes the service instance' do
+              api_call.call(admin_headers)
+              execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+              expect(VCAP::CloudController::ServiceInstance.first(guid: instance.guid)).to be_nil
+            end
+
+            it 'completes the job' do
+              api_call.call(admin_headers)
+              execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+              job = VCAP::CloudController::PollableJobModel.last
+              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE)
+            end
+          end
+
+          context 'with an error' do
+            let(:broker_status_code) { 500 }
+
+            it 'marks the service instance as delete failed' do
+              api_call.call(admin_headers)
+              execute_all_jobs(expected_successes: 0, expected_failures: 1)
+              instance.reload
+
+              expect(instance.last_operation).to_not be_nil
+              expect(instance.last_operation.type).to eq('delete')
+              expect(instance.last_operation.state).to eq('failed')
+            end
+
+            it 'completes with failure' do
+              api_call.call(admin_headers)
+              execute_all_jobs(expected_successes: 0, expected_failures: 1)
+
+              job = VCAP::CloudController::PollableJobModel.last
+              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::FAILED_STATE)
+            end
+          end
+        end
+      end
 
       context 'when it is shared' do
         let(:other_space) { VCAP::CloudController::Space.make }