Fix state handling for POST /v3/spaces/:guid/actions/apply_manifest

app/actions/app_apply_manifest.rb

@@ -140,6 +140,7 @@ def create_service_bindings(manifest_service_bindings_message, app)
       manifest_service_bindings_message.manifest_service_bindings.each do |manifest_service_binding|
         service_instance = app.space.find_visible_service_instance_by_name(manifest_service_binding.name)
         service_instance_not_found!(manifest_service_binding.name) unless service_instance
+
         binding = ServiceBinding.first(service_instance: service_instance, app: app)
         next if binding&.create_succeeded?
 

app/actions/service_credential_binding_app_create.rb

@@ -56,6 +56,7 @@ def validate_service_instance!(app, service_instance, volume_mount_services_enab
           service_not_bindable! unless service_instance.service_plan.bindable?
           service_not_available! unless service_instance.service_plan.active?
           volume_mount_not_enabled! if service_instance.volume_service? && !volume_mount_services_enabled
+          service_instance_not_found! if service_instance.create_failed?
           operation_in_progress! if service_instance.operation_in_progress?
         end
       end
@@ -84,6 +85,10 @@ def operation_in_progress!
         raise UnprocessableCreate.new('There is an operation in progress for the service instance')
       end
 
+      def service_instance_not_found!
+        raise UnprocessableCreate.new('Service instance not found')
+      end
+
       def app_is_required!
         raise UnprocessableCreate.new('No app was specified')
       end

spec/unit/actions/app_apply_manifest_spec.rb

@@ -932,6 +932,151 @@ module VCAP::CloudController
                 end
               end
             end
+
+            context 'different service instance states' do
+              context 'when the last operation state is create in progress' do
+                before do
+                  service_instance.save_with_new_operation({}, { type: 'create', state: 'in progress' })
+                  allow(service_cred_binding_create).to receive(:precursor).and_raise(V3::ServiceCredentialBindingAppCreate::UnprocessableCreate,
+'There is an operation in progress for the service instance')
+                end
+
+                it 'fails with a service binding error' do
+                  expect {
+                    app_apply_manifest.apply(app.guid, message)
+                  }.to raise_error(AppApplyManifest::ServiceBindingError,
+                     "For service '#{service_instance.name}': There is an operation in progress for the service instance")
+                end
+              end
+
+              context 'when the last operation state is create succeeded' do
+                before do
+                  service_instance.save_with_new_operation({}, { type: 'create', state: 'succeeded' })
+                end
+
+                it 'creates the binding' do
+                  service_binding_1 = instance_double(ServiceBinding)
+                  service_binding_2 = instance_double(ServiceBinding)
+
+                  allow(service_cred_binding_create).to receive(:precursor).and_return(
+                    service_binding_1,
+                    service_binding_2,
+                    )
+
+                  app_apply_manifest.apply(app.guid, message)
+
+                  expect(service_cred_binding_create).to have_received(:bind).with(service_binding_1, parameters: nil)
+                  expect(service_cred_binding_create).to have_received(:bind).with(service_binding_2, parameters: { 'foo' => 'bar' })
+                end
+              end
+
+              context 'when the last operation state is create failed' do
+                before do
+                  service_instance.save_with_new_operation({}, { type: 'create', state: 'failed' })
+                  allow(service_cred_binding_create).to receive(:precursor).and_raise(V3::ServiceCredentialBindingAppCreate::UnprocessableCreate, 'Service instance not found')
+                end
+
+                it 'fails with a service binding error' do
+                  expect {
+                    app_apply_manifest.apply(app.guid, message)
+                  }.to raise_error(AppApplyManifest::ServiceBindingError,
+                     "For service '#{service_instance.name}': Service instance not found")
+                end
+              end
+
+              context 'when the last operation state is update in progress' do
+                before do
+                  service_instance.save_with_new_operation({}, { type: 'update', state: 'in progress' })
+                  allow(service_cred_binding_create).to receive(:precursor).and_raise(V3::ServiceCredentialBindingAppCreate::UnprocessableCreate,
+'There is an operation in progress for the service instance')
+                end
+
+                it 'fails with a service binding error' do
+                  expect {
+                    app_apply_manifest.apply(app.guid, message)
+                  }.to raise_error(AppApplyManifest::ServiceBindingError,
+                     "For service '#{service_instance.name}': There is an operation in progress for the service instance")
+                end
+              end
+
+              context 'when the last operation state is update succeeded' do
+                before do
+                  service_instance.save_with_new_operation({}, { type: 'update', state: 'succeeded' })
+                end
+
+                it 'creates the binding' do
+                  service_binding_1 = instance_double(ServiceBinding)
+                  service_binding_2 = instance_double(ServiceBinding)
+
+                  allow(service_cred_binding_create).to receive(:precursor).and_return(
+                    service_binding_1,
+                    service_binding_2,
+                    )
+
+                  app_apply_manifest.apply(app.guid, message)
+
+                  expect(service_cred_binding_create).to have_received(:bind).with(service_binding_1, parameters: nil)
+                  expect(service_cred_binding_create).to have_received(:bind).with(service_binding_2, parameters: { 'foo' => 'bar' })
+                end
+              end
+
+              context 'when the last operation state is update failed' do
+                before do
+                  service_instance.save_with_new_operation({}, { type: 'update', state: 'failed' })
+                end
+
+                it 'creates the binding' do
+                  service_binding_1 = instance_double(ServiceBinding)
+                  service_binding_2 = instance_double(ServiceBinding)
+
+                  allow(service_cred_binding_create).to receive(:precursor).and_return(
+                    service_binding_1,
+                    service_binding_2,
+                    )
+
+                  app_apply_manifest.apply(app.guid, message)
+
+                  expect(service_cred_binding_create).to have_received(:bind).with(service_binding_1, parameters: nil)
+                  expect(service_cred_binding_create).to have_received(:bind).with(service_binding_2, parameters: { 'foo' => 'bar' })
+                end
+              end
+
+              context 'when the last operation state is delete in progress' do
+                before do
+                  service_instance.save_with_new_operation({}, { type: 'delete', state: 'in progress' })
+                  allow(service_cred_binding_create).to receive(:precursor).and_raise(V3::ServiceCredentialBindingAppCreate::UnprocessableCreate,
+'There is an operation in progress for the service instance')
+                end
+
+                it 'fails with a service binding error' do
+                  expect {
+                    app_apply_manifest.apply(app.guid, message)
+                  }.to raise_error(AppApplyManifest::ServiceBindingError,
+                     "For service '#{service_instance.name}': There is an operation in progress for the service instance")
+                end
+              end
+
+              context 'when the last operation state is delete failed' do
+                before do
+                  service_instance.save_with_new_operation({}, { type: 'delete', state: 'failed' })
+                end
+
+                it 'creates the binding' do
+                  service_binding_1 = instance_double(ServiceBinding)
+                  service_binding_2 = instance_double(ServiceBinding)
+
+                  allow(service_cred_binding_create).to receive(:precursor).and_return(
+                    service_binding_1,
+                    service_binding_2,
+                    )
+
+                  app_apply_manifest.apply(app.guid, message)
+
+                  expect(service_cred_binding_create).to have_received(:bind).with(service_binding_1, parameters: nil)
+                  expect(service_cred_binding_create).to have_received(:bind).with(service_binding_2, parameters: { 'foo' => 'bar' })
+                end
+              end
+            end
           end
         end
 

spec/unit/actions/service_credential_binding_app_create_spec.rb

@@ -230,6 +230,19 @@ module V3
               end
             end
 
+            context "when the service instance is in state 'create failed'" do
+              it 'raises an error' do
+                service_instance.save_with_new_operation({}, { type: 'create', state: 'failed' })
+
+                expect {
+                  action.precursor(service_instance, app: app, volume_mount_services_enabled: false, message: message)
+                }.to raise_error(
+                  ServiceCredentialBindingAppCreate::UnprocessableCreate,
+                       'Service instance not found'
+                     )
+              end
+            end
+
             context 'when the service is a volume service and service volume mounting is enabled' do
               let(:service_instance) { ManagedServiceInstance.make(:volume_mount, **si_details) }