v3(services): orphan mitigation when delete fails

Co-authored-by: Brian Butz <bbutz@pivotal.io>

Author: Derik Evangelista

app/jobs/v3/delete_service_instance_job.rb

@@ -2,13 +2,18 @@
 
 module VCAP::CloudController
   module V3
+    class DeprovisionBadResponse < StandardError
+    end
+
     class DeleteServiceInstanceJob < ServiceInstanceAsyncJob
       def initialize(guid, audit_info)
         super(guid, audit_info)
       end
 
       def send_broker_request(client)
         client.deprovision(service_instance, { accepts_incomplete: true })
+      rescue VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerBadResponse => err
+        raise DeprovisionBadResponse.new(err.message)
       end
 
       def operation_succeeded
@@ -30,6 +35,30 @@ def operation_type
       def gone!
         finish
       end
+
+      def restart_on_failure?
+        true
+      end
+
+      def restart_job(msg)
+        super
+        logger.info("could not complete the operation: #{msg}. Triggering orphan mitigation")
+      end
+
+      def fail!(err)
+        case err
+        when DeprovisionBadResponse
+          trigger_orphan_mitigation(err)
+        else
+          super
+        end
+      end
+
+      private
+
+      def trigger_orphan_mitigation(err)
+        restart_job(err.message)
+      end
     end
   end
 end

app/jobs/v3/service_instance_async_job.rb

@@ -2,7 +2,11 @@
 
 module VCAP::CloudController
   module V3
+    class LastOperationStateFailed < StandardError
+    end
+
     class ServiceInstanceAsyncJob < VCAP::CloudController::Jobs::ReoccurringJob
+      MAX_RETRIES = 3
       attr_reader :warnings
 
       def initialize(guid, audit_info)
@@ -12,15 +16,13 @@ def initialize(guid, audit_info)
         @warnings = []
         @request_attr = {}
         @first_time = true
+        @attempts = 0
       end
 
       def perform
         gone! && return if service_instance.blank?
-        last_operation_type = service_instance.last_operation&.type
 
-        if service_instance.operation_in_progress? && last_operation_type != operation_type
-          aborted!(last_operation_type)
-        end
+        raise_if_cannot_proceed!
 
         client = VCAP::Services::ServiceClientProvider.provide({ instance: service_instance })
 
@@ -42,13 +44,13 @@ def perform
             record_event(si, @request_attr)
             finish
           end
+        rescue LastOperationStateFailed => err
+          fail_and_raise!(err.message) unless restart_on_failure?
+
+          restart_job(err.message || 'no error description returned by the broker')
         rescue => err
           fail!(err)
         end
-
-        if service_instance.present? && service_instance.last_operation.state == 'failed'
-          operation_failed!(service_instance.last_operation.description)
-        end
       end
 
       def handle_timeout
@@ -80,6 +82,10 @@ def display_name
         "service_instance.#{operation_type}"
       end
 
+      def restart_on_failure?
+        false
+      end
+
       private
 
       attr_reader :service_instance_guid
@@ -93,6 +99,21 @@ def execute_request(client)
         )
       end
 
+      def raise_if_cannot_proceed!
+        last_operation_type = service_instance.last_operation&.type
+
+        if service_instance.operation_in_progress? && last_operation_type != operation_type
+          aborted!(last_operation_type)
+        end
+      end
+
+      def restart_job(msg)
+        @attempts += 1
+        fail_and_raise!(msg) unless @attempts < MAX_RETRIES
+
+        @first_time = true
+      end
+
       def operation_completed?
         service_instance.last_operation.state == 'succeeded' && service_instance.last_operation.type == operation_type
       end
@@ -101,6 +122,13 @@ def fetch_last_operation(client)
         last_operation_result = client.fetch_service_instance_last_operation(service_instance)
         self.polling_interval_seconds = last_operation_result[:retry_after] if last_operation_result[:retry_after]
 
+        operation_failed!(last_operation_result.dig(:last_operation)[:description]) if last_operation_result[:http_status_code] == HTTP::Status::BAD_REQUEST
+
+        lo = last_operation_result[:last_operation]
+        if lo[:state] == 'failed'
+          raise LastOperationStateFailed.new(lo[:description])
+        end
+
         service_instance.save_and_update_operation(
           last_operation: last_operation_result[:last_operation].slice(:state, :description)
         )
@@ -139,15 +167,23 @@ def operation_succeeded
         nil
       end
 
-      def fail!(e)
+      def fail_last_operation(msg)
         unless service_instance.blank?
           service_instance.save_with_new_operation({}, {
             type: operation_type,
             state: 'failed',
-            description: e.message,
+            description: msg,
           })
         end
+      end
+
+      def fail_and_raise!(msg)
+        fail_last_operation(msg)
+        operation_failed!(msg)
+      end
 
+      def fail!(e)
+        fail_last_operation(e.message)
         raise e
       end
 
@@ -168,6 +204,10 @@ def volume_services_disabled?
       def route_services_disabled?
         !VCAP::CloudController::Config.config.get(:route_services_enabled)
       end
+
+      def logger
+        Steno.logger('cc-background')
+      end
     end
   end
 end

lib/services/service_brokers/v2/response_parser.rb

@@ -718,7 +718,8 @@ def validate(method:, uri:, code:, response:)
               'last_operation' => {
                 'state' => 'failed',
                 'description' => description
-              }
+              },
+              'http_status_code' => 400
             }
           end
         end