v3(services): async delete route binding (cloudfoundry#1881)

* v3(services): async delete route binding

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

* Use named subject to improve test clarity

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

Author: blgm

app/actions/service_route_binding_delete.rb

@@ -5,47 +5,71 @@ class UnprocessableDelete < StandardError; end
 
       RequiresAsync = Class.new.freeze
       DeleteComplete = Class.new.freeze
-      DeleteInProgress = Struct.new(:operation).freeze
+      DeleteStarted = Struct.new(:operation).freeze
+      DeleteInProgress = Struct.new(:retry_after).freeze
 
       def initialize(service_event_repository)
         @service_event_repository = service_event_repository
       end
 
       def delete(binding, async_allowed:)
-        return RequiresAsync unless async_allowed || binding.service_instance.user_provided_instance?
+        return RequiresAsync.new unless async_allowed || binding.service_instance.user_provided_instance?
 
         operation_in_progress! if binding.service_instance.operation_in_progress?
 
         result = send_unbind_to_broker(binding)
-        unsupported! unless result == DeleteComplete
+        case result
+        when DeleteStarted
+          update_last_operation(binding, operation: result[:operation])
+        when DeleteComplete
+          perform_delete_actions(binding)
+        end
 
-        record_audit_event(binding)
-        binding.destroy
-        binding.notify_diego
-
-        DeleteComplete
+        result
       rescue => e
-        binding.save_with_new_operation({}, {
-          type: 'delete',
-          state: 'failed',
-          description: e.message,
-        })
+        update_last_operation(binding, state: 'failed', description: e.message)
 
         raise e
       end
 
+      def poll(binding)
+        client = VCAP::Services::ServiceClientProvider.provide(instance: binding.service_instance)
+        details = client.fetch_service_binding_last_operation(binding)
+        case details[:last_operation][:state]
+        when 'in progress'
+          update_last_operation(binding, description: details[:last_operation][:description])
+          DeleteInProgress.new(details[:retry_after])
+        when 'succeeded'
+          perform_delete_actions(binding)
+          DeleteComplete.new
+        when 'failed'
+          update_last_operation(binding, state: 'failed', description: details[:last_operation][:description])
+          DeleteComplete.new
+        end
+      rescue => e
+        update_last_operation(binding, description: e.message)
+
+        DeleteInProgress.new(nil)
+      end
+
       private
 
       attr_reader :service_event_repository
 
       def send_unbind_to_broker(binding)
         client = VCAP::Services::ServiceClientProvider.provide(instance: binding.service_instance)
-        details = client.unbind(binding, accepts_incomplete: false)
-        details[:async] ? DeleteInProgress.new(details[:operation]) : DeleteComplete
+        details = client.unbind(binding, nil, true)
+        details[:async] ? DeleteStarted.new(details[:operation]) : DeleteComplete.new
       rescue => err
         raise UnprocessableDelete.new("Service broker failed to delete service binding for instance #{binding.service_instance.name}: #{err.message}")
       end
 
+      def perform_delete_actions(binding)
+        record_audit_event(binding)
+        binding.destroy
+        binding.notify_diego
+      end
+
       def record_audit_event(binding)
         service_event_repository.record_service_instance_event(
           :unbind_route,
@@ -54,12 +78,17 @@ def record_audit_event(binding)
         )
       end
 
-      def operation_in_progress!
-        raise UnprocessableDelete.new('There is an operation in progress for the service instance')
+      def update_last_operation(binding, description: nil, state: 'in progress', operation: nil)
+        binding.save_with_new_operation({}, {
+          type: 'delete',
+          state: state,
+          description: description,
+          broker_provided_operation: operation
+        })
       end
 
-      def unsupported!
-        raise UnprocessableDelete.new('async unbind not supported yet')
+      def operation_in_progress!
+        raise UnprocessableDelete.new('There is an operation in progress for the service instance')
       end
     end
   end

app/controllers/v3/service_route_bindings_controller.rb

@@ -68,7 +68,7 @@ def destroy
     action = V3::ServiceRouteBindingDelete.new(service_event_repository)
     result = action.delete(route_binding, async_allowed: false)
 
-    if result == V3::ServiceRouteBindingDelete::RequiresAsync
+    if result.is_a? V3::ServiceRouteBindingDelete::RequiresAsync
       pollable_job_guid = enqueue_unbind_job(route_binding.guid)
       head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{pollable_job_guid}")
     else

app/jobs/v3/delete_route_binding_job.rb

@@ -4,9 +4,10 @@
 
 module VCAP::CloudController
   module V3
-    class DeleteRouteBindingJob < VCAP::CloudController::Jobs::CCJob
+    class DeleteRouteBindingJob < Jobs::ReoccurringJob
       def initialize(binding_guid, user_audit_info:)
         super()
+        @first_time = true
         @binding_guid = binding_guid
         @user_audit_info = user_audit_info
       end
@@ -41,7 +42,23 @@ def perform
 
         service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithUserActor.new(@user_audit_info)
         action = V3::ServiceRouteBindingDelete.new(service_event_repository)
-        action.delete(binding, async_allowed: true)
+        compute_maximum_duration
+
+        if @first_time
+          @first_time = false
+          delete_result = action.delete(binding, async_allowed: true)
+          return finish if delete_result.is_a? V3::ServiceRouteBindingDelete::DeleteComplete
+        end
+
+        polling_status = action.poll(binding)
+        case polling_status
+        when V3::ServiceRouteBindingDelete::DeleteComplete
+          finish
+        when V3::ServiceRouteBindingDelete::DeleteInProgress
+          unless polling_status.retry_after.nil?
+            self.polling_interval_seconds = polling_status.retry_after.to_i
+          end
+        end
       rescue => e
         raise CloudController::Errors::ApiError.new_from_details('UnableToPerform', 'unbind', e.message)
       end
@@ -52,6 +69,11 @@ def route_binding
         RouteBinding.first(guid: resource_guid)
       end
 
+      def compute_maximum_duration
+        max_poll_duration_on_plan = route_binding.service_instance.service_plan.try(:maximum_polling_duration)
+        self.maximum_duration_seconds = max_poll_duration_on_plan
+      end
+
       def gone!
         raise CloudController::Errors::ApiError.new_from_details('ResourceNotFound', "The binding could not be found: #{resource_guid}")
       end

spec/request/service_route_bindings_spec.rb

@@ -408,7 +408,22 @@
             end
           end
 
-          it_behaves_like 'create binding last operation response handling'
+          it_behaves_like 'binding last operation response handling', 'create'
+
+          context 'last operation response is 410 Gone' do
+            let(:last_operation_status_code) { 410 }
+            let(:last_operation_body) { {} }
+
+            it 'continues polling' do
+              execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+              binding.reload
+              expect(binding.last_operation.type).to eq('create')
+              expect(binding.last_operation.state).to eq('in progress')
+
+              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)
+            end
+          end
 
           context 'binding not retrievable' do
             let(:offering) { VCAP::CloudController::Service.make(bindings_retrievable: false, requires: ['route_forwarding']) }
@@ -930,6 +945,7 @@
             {
               service_id: service_instance.service_plan.service.unique_id,
               plan_id: service_instance.service_plan.unique_id,
+              accepts_incomplete: true,
             }
           end
 
@@ -967,6 +983,95 @@
             end
           end
 
+          context 'when the unbind responds asynchronously' do
+            let(:broker_status_code) { 202 }
+            let(:operation) { Sham.guid }
+            let(:broker_response) { { operation: operation } }
+            let(:broker_binding_last_operation_url) { "#{broker_base_url}/v2/service_instances/#{service_instance.guid}/service_bindings/#{binding.guid}/last_operation" }
+            let(:last_operation_status_code) { 200 }
+            let(:description) { Sham.description }
+            let(:state) { 'in progress' }
+            let(:last_operation_body) do
+              {
+                description: description,
+                state: state,
+              }
+            end
+
+            before do
+              stub_request(:get, broker_binding_last_operation_url).
+                with(query: hash_including({
+                  operation: operation
+                })).
+                to_return(status: last_operation_status_code, body: last_operation_body.to_json, headers: {})
+            end
+
+            it 'polls the last operation endpoint' do
+              execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+              expect(
+                a_request(:get, broker_binding_last_operation_url).
+                  with(query: {
+                    operation: operation,
+                    service_id: service_instance.service_plan.service.unique_id,
+                    plan_id: service_instance.service_plan.unique_id,
+                  })
+              ).to have_been_made.once
+            end
+
+            it 'updates the binding and job' do
+              execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+              binding.reload
+              expect(binding.last_operation.type).to eq('delete')
+              expect(binding.last_operation.state).to eq(state)
+              expect(binding.last_operation.description).to eq(description)
+
+              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)
+            end
+
+            it 'enqueues the next fetch last operation job' do
+              execute_all_jobs(expected_successes: 1, expected_failures: 0)
+              expect(Delayed::Job.count).to eq(1)
+            end
+
+            context 'last operation response is 200 OK and indicates success' do
+              let(:state) { 'succeeded' }
+              let(:last_operation_status_code) { 200 }
+
+              it 'removes the binding' do
+                execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+                expect(VCAP::CloudController::RouteBinding.all).to be_empty
+              end
+
+              it 'completes the job' do
+                execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+                expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE)
+              end
+            end
+
+            context 'last operation response is 410 Gone' do
+              let(:last_operation_status_code) { 410 }
+              let(:last_operation_body) { {} }
+
+              it 'removes the binding' do
+                execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+                expect(VCAP::CloudController::RouteBinding.all).to be_empty
+              end
+
+              it 'completes the job' do
+                execute_all_jobs(expected_successes: 1, expected_failures: 0)
+
+                expect(job.state).to eq(VCAP::CloudController::PollableJobModel::COMPLETE_STATE)
+              end
+            end
+
+            it_behaves_like 'binding last operation response handling', 'delete'
+          end
+
           context 'when the broker returns a failure' do
             let(:broker_status_code) { 418 }
             let(:broker_response) { 'nope' }

spec/support/shared_examples/request/last_operation_response_handling.rb

@@ -1,9 +1,10 @@
-RSpec.shared_examples 'create binding last operation response handling' do
+RSpec.shared_examples 'binding last operation response handling' do |type|
   context 'valid http codes' do
     valid_responses = [
       { code: 400, body: { description: 'helpful message' }, expected_description: 'helpful message' },
       { code: 200, body: { state: 'failed', description: 'something went wrong' }, expected_description: 'something went wrong' }
     ]
+
     valid_responses.each do |response|
       context "last operation response is #{response[:code]}" do
         let(:state) { response[:state] }
@@ -13,7 +14,8 @@
         it 'updates the binding and job' do
           execute_all_jobs(expected_successes: 1, expected_failures: 0)
 
-          expect(binding.last_operation.type).to eq('create')
+          binding.reload
+          expect(binding.last_operation.type).to eq(type)
           expect(binding.last_operation.state).to eq('failed')
           expect(binding.last_operation.description).to eq(response[:body][:description])
 
@@ -24,17 +26,18 @@
   end
 
   context 'invalid http codes' do
-    [404, 410, 500].each do |code|
+    [404, 500].each do |code|
       context "last operation response is #{code}" do
         let(:last_operation_status_code) { code }
         let(:last_operation_body) { 'something awful' }
 
         it 'continues polling' do
           execute_all_jobs(expected_successes: 1, expected_failures: 0)
 
-          expect(binding.last_operation.type).to eq('create')
+          binding.reload
+          expect(binding.last_operation.type).to eq(type)
           expect(binding.last_operation.state).to eq('in progress')
-          expect(binding.last_operation.description).to include("Status Code: #{code}") unless code == 410
+          expect(binding.last_operation.description).to include("Status Code: #{code}")
 
           expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)
         end
@@ -53,7 +56,8 @@
     it 'continues polling' do
       execute_all_jobs(expected_successes: 1, expected_failures: 0)
 
-      expect(binding.last_operation.type).to eq('create')
+      binding.reload
+      expect(binding.last_operation.type).to eq(type)
       expect(binding.last_operation.state).to eq('in progress')
 
       expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)
@@ -72,7 +76,8 @@
       it 'continues polling' do
         execute_all_jobs(expected_successes: 1, expected_failures: 0)
 
-        expect(binding.last_operation.type).to eq('create')
+        binding.reload
+        expect(binding.last_operation.type).to eq(type)
         expect(binding.last_operation.state).to eq('in progress')
 
         expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)