Orphan mitigation for instances/bindings in state "create failed" (#3988)

* Orphan mitigation for instances/bindings in state "create failed"

It might be that a service brokr that responds with create failed on a
service instance/binding creation keeps
some meta data about the resource. The OSBAPI spec recommends that the
platform performs orphan mitigation in such a case. Adding orphan
mitigation to instance and binding creations, if they are in state
create failed.

Author: kathap

app/actions/service_credential_binding_app_create.rb

@@ -32,7 +32,10 @@ def precursor(service_instance, message:, app: nil, volume_mount_services_enable
 
         ServiceBinding.new.tap do |b|
           ServiceBinding.db.transaction do
-            binding.destroy if binding
+            if binding
+              binding.destroy
+              VCAP::Services::ServiceBrokers::V2::OrphanMitigator.new.cleanup_failed_bind(binding)
+            end
             b.save_with_attributes_and_new_operation(
               binding_details,
               CREATE_INITIAL_OPERATION

app/actions/service_credential_binding_key_create.rb

@@ -27,7 +27,10 @@ def precursor(service_instance, message:)
 
         ServiceKey.new.tap do |b|
           ServiceKey.db.transaction do
-            key.destroy if key
+            if key
+              key.destroy
+              VCAP::Services::ServiceBrokers::V2::OrphanMitigator.new.cleanup_failed_bind(key)
+            end
             b.save_with_attributes_and_new_operation(
               binding_details,
               CREATE_INITIAL_OPERATION

app/actions/service_route_binding_create.rb

@@ -26,7 +26,10 @@ def precursor(service_instance, route, message:)
 
         RouteBinding.new.tap do |b|
           RouteBinding.db.transaction do
-            binding.destroy if binding
+            if binding
+              binding.destroy
+              VCAP::Services::ServiceBrokers::V2::OrphanMitigator.new.cleanup_failed_bind(binding)
+            end
             b.save_with_attributes_and_new_operation(
               binding_details,
               CREATE_INITIAL_OPERATION

app/actions/v3/service_instance_create_managed.rb

@@ -1,4 +1,5 @@
 require 'services/service_brokers/service_client_provider'
+require 'services/service_brokers/v2/orphan_mitigator'
 require 'actions/mixins/service_instance_create'
 require 'actions/metadata_update'
 
@@ -39,7 +40,10 @@ def precursor(message:, service_plan:)
 
         ManagedServiceInstance.new.tap do |i|
           ManagedServiceInstance.db.transaction do
-            instance.destroy if instance&.create_failed?
+            if instance&.create_failed?
+              instance.destroy
+              VCAP::Services::ServiceBrokers::V2::OrphanMitigator.new.cleanup_failed_provision(instance)
+            end
             i.save_with_new_operation(attr, last_operation)
             MetadataUpdate.update(i, message)
           end

spec/unit/actions/service_credential_binding_app_create_spec.rb

@@ -79,8 +79,12 @@ module V3
             end
 
             context "when the last binding operation is in 'create failed' state" do
+              let(:fake_orphan_mitigator) { instance_double(VCAP::Services::ServiceBrokers::V2::OrphanMitigator) }
+
               before do
                 binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'failed' })
+                allow(VCAP::Services::ServiceBrokers::V2::OrphanMitigator).to receive(:new).and_return(fake_orphan_mitigator)
+                allow(fake_orphan_mitigator).to receive(:cleanup_failed_bind).with(binding)
               end
 
               it 'deletes the existing binding and creates a new one' do
@@ -89,6 +93,7 @@ module V3
                 expect(b.guid).not_to eq(binding.guid)
                 expect(b).to be_create_in_progress
                 expect { binding.reload }.to raise_error Sequel::NoExistingObject
+                expect(fake_orphan_mitigator).to have_received(:cleanup_failed_bind).with(binding)
               end
             end
 

spec/unit/actions/service_credential_binding_key_create_spec.rb

@@ -67,8 +67,12 @@ module V3
             end
 
             context "when the last key operation is in 'create failed' state" do
+              let(:fake_orphan_mitigator) { instance_double(VCAP::Services::ServiceBrokers::V2::OrphanMitigator) }
+
               before do
                 binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'failed' })
+                allow(VCAP::Services::ServiceBrokers::V2::OrphanMitigator).to receive(:new).and_return(fake_orphan_mitigator)
+                allow(fake_orphan_mitigator).to receive(:cleanup_failed_bind).with(binding)
               end
 
               it 'deletes the existing key and creates a new one' do
@@ -77,6 +81,7 @@ module V3
                 expect(b.guid).not_to eq(binding.guid)
                 expect(b).to be_create_in_progress
                 expect { binding.reload }.to raise_error Sequel::NoExistingObject
+                expect(fake_orphan_mitigator).to have_received(:cleanup_failed_bind).with(binding)
               end
             end
 

spec/unit/actions/service_route_binding_create_spec.rb

@@ -119,8 +119,12 @@ module V3
             end
 
             context "when the last service route binding operation is in 'create failed' state" do
+              let(:fake_orphan_mitigator) { instance_double(VCAP::Services::ServiceBrokers::V2::OrphanMitigator) }
+
               before do
                 binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'failed' })
+                allow(VCAP::Services::ServiceBrokers::V2::OrphanMitigator).to receive(:new).and_return(fake_orphan_mitigator)
+                allow(fake_orphan_mitigator).to receive(:cleanup_failed_bind).with(binding)
               end
 
               it 'deletes the existing binding and creates a new one' do
@@ -129,6 +133,7 @@ module V3
                 expect(b.guid).not_to eq(binding.guid)
                 expect(b).to be_create_in_progress
                 expect { binding.reload }.to raise_error Sequel::NoExistingObject
+                expect(fake_orphan_mitigator).to have_received(:cleanup_failed_bind).with(binding)
               end
             end
 

spec/unit/actions/v3/service_instance_create_managed_spec.rb

@@ -123,14 +123,19 @@ module V3
           end
 
           context "when the last operation is in state 'create failed'" do
+            let(:fake_orphan_mitigator) { instance_double(VCAP::Services::ServiceBrokers::V2::OrphanMitigator) }
+
             before do
               instance.save_with_new_operation({}, { type: 'create', state: 'failed' })
+              allow(VCAP::Services::ServiceBrokers::V2::OrphanMitigator).to receive(:new).and_return(fake_orphan_mitigator)
+              allow(fake_orphan_mitigator).to receive(:cleanup_failed_provision).with(instance)
             end
 
             it 'deletes the existing service instance and creates a new one' do
               service_instance = action.precursor(message:, service_plan:)
 
               expect(service_instance.guid).not_to eq(instance.guid)
+              expect(fake_orphan_mitigator).to have_received(:cleanup_failed_provision).with(instance)
               expect(service_instance.last_operation.type).to eq('create')
               expect(service_instance.last_operation.state).to eq('initial')
             end