Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit de96396

Browse files
FelisiaMblgm
andauthored
V3 delete recursively user provided services (cloudfoundry#1996)
* WIP Co-authored-by: Felisia Martini <fmartini@pivotal.io> * v3(services) Recursively delete user provided service instances Co-Authored-By: George Blue <blgm@users.noreply.github.com> Co-authored-by: George Blue <gblue@pivotal.io> Co-authored-by: George Blue <blgm@users.noreply.github.com>
1 parent 5a44009 commit de96396

4 files changed

Lines changed: 106 additions & 4 deletions

File tree

app/actions/v3/service_instance_delete.rb

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
require 'jobs/v3/delete_service_instance_job'
2-
require 'actions/services/locks/deleter_lock'
2+
require 'actions/service_route_binding_delete'
3+
require 'actions/service_credential_binding_delete'
34
require 'cloud_controller/errors/api_error'
45

56
module VCAP::CloudController
@@ -30,6 +31,11 @@ def initialize(service_instance, event_repo)
3031
def delete
3132
operation_in_progress! if service_instance.operation_in_progress? && service_instance.last_operation.type != 'create'
3233

34+
if service_instance.is_a?(UserProvidedServiceInstance)
35+
errors = remove_bindings
36+
raise errors.first if errors.any?
37+
end
38+
3339
result = send_deprovison_to_broker
3440
if result[:finished]
3541
destroy
@@ -117,6 +123,25 @@ def destroy
117123
end
118124
end
119125

126+
def remove_bindings
127+
errors = []
128+
route_bindings_action = ServiceRouteBindingDelete.new(service_event_repository.user_audit_info)
129+
RouteBinding.where(service_instance: service_instance).each do |route_binding|
130+
route_bindings_action.delete(route_binding)
131+
rescue => e
132+
errors << e
133+
end
134+
135+
service_bindings_action = ServiceCredentialBindingDelete.new(:credential, service_event_repository.user_audit_info)
136+
service_instance.service_bindings.each do |service_binding|
137+
service_bindings_action.delete(service_binding)
138+
rescue => e
139+
errors << e
140+
end
141+
142+
return errors
143+
end
144+
120145
def update_last_operation_with_operation_id(operation_id)
121146
service_instance.save_with_new_operation(
122147
{},

app/controllers/v3/service_instances_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,10 @@ def destroy
113113
end
114114

115115
delete_action = V3::ServiceInstanceDelete.new(service_instance, service_event_repository)
116-
delete_action.delete_checks
117116

118117
case service_instance
119118
when VCAP::CloudController::ManagedServiceInstance
119+
delete_action.delete_checks
120120
job_guid = enqueue_delete_job(service_instance)
121121
head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job_guid}")
122122
when VCAP::CloudController::UserProvidedServiceInstance

spec/request/service_instances_spec.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2588,10 +2588,16 @@ def check_filtered_instances(*instances)
25882588
expect(VCAP::CloudController::ServiceInstanceAnnotationModel.where(service_instance: instance).all).to be_empty
25892589
end
25902590

2591-
it 'fails to delete when there are bindings' do
2591+
it 'deletes any related bindings' do
2592+
VCAP::CloudController::RouteBinding.make(service_instance: instance)
25922593
VCAP::CloudController::ServiceBinding.make(service_instance: instance)
2594+
25932595
api_call.call(admin_headers)
2594-
expect(last_response).to have_status_code(422)
2596+
expect(last_response).to have_status_code(204)
2597+
2598+
expect(VCAP::CloudController::ServiceInstance.all).to be_empty
2599+
expect(VCAP::CloudController::RouteBinding.all).to be_empty
2600+
expect(VCAP::CloudController::ServiceBinding.all).to be_empty
25952601
end
25962602

25972603
context 'with purge' do

spec/unit/actions/v3/service_instance_delete_spec.rb

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,77 @@ module V3
7979
result = action.delete
8080
expect(result[:finished]).to be_truthy
8181
end
82+
83+
context 'when there are bindings' do
84+
let(:delete_route_binding_action) do
85+
double(ServiceRouteBindingDelete).tap do |d|
86+
allow(d).to receive(:delete), &:destroy
87+
end
88+
end
89+
let(:delete_service_binding_action) do
90+
double(ServiceCredentialBindingDelete).tap do |d|
91+
allow(d).to receive(:delete), &:destroy
92+
end
93+
end
94+
95+
let!(:route_binding_1) { RouteBinding.make(service_instance: service_instance) }
96+
let!(:route_binding_2) { RouteBinding.make(service_instance: service_instance) }
97+
let!(:route_binding_3) { RouteBinding.make(service_instance: service_instance) }
98+
let!(:service_binding_1) { ServiceBinding.make(service_instance: service_instance) }
99+
let!(:service_binding_2) { ServiceBinding.make(service_instance: service_instance) }
100+
let!(:service_binding_3) { ServiceBinding.make(service_instance: service_instance) }
101+
102+
before do
103+
allow(ServiceRouteBindingDelete).to receive(:new).and_return(delete_route_binding_action)
104+
allow(ServiceCredentialBindingDelete).to receive(:new).and_return(delete_service_binding_action)
105+
end
106+
107+
it 'unbinds all the bindings' do
108+
action.delete
109+
110+
expect(ServiceRouteBindingDelete).to have_received(:new).with(event_repository.user_audit_info)
111+
expect(delete_route_binding_action).to have_received(:delete).with(route_binding_1)
112+
expect(delete_route_binding_action).to have_received(:delete).with(route_binding_2)
113+
expect(delete_route_binding_action).to have_received(:delete).with(route_binding_3)
114+
115+
expect(ServiceCredentialBindingDelete).to have_received(:new).with(:credential, event_repository.user_audit_info)
116+
expect(delete_service_binding_action).to have_received(:delete).with(service_binding_1)
117+
expect(delete_service_binding_action).to have_received(:delete).with(service_binding_2)
118+
expect(delete_service_binding_action).to have_received(:delete).with(service_binding_3)
119+
end
120+
121+
context 'when deleting bindings raises' do
122+
let(:delete_route_binding_action) do
123+
double(ServiceRouteBindingDelete).tap do |d|
124+
allow(d).to receive(:delete) do |binding|
125+
raise StandardError.new('boom-route') if binding == route_binding_2
126+
127+
binding.destroy
128+
end
129+
end
130+
end
131+
132+
let(:delete_service_binding_action) do
133+
double(ServiceCredentialBindingDelete).tap do |d|
134+
allow(d).to receive(:delete) do |binding|
135+
raise StandardError.new('boom-credential') if binding == service_binding_2
136+
137+
binding.destroy
138+
end
139+
end
140+
end
141+
142+
it 'attempts to remove other route bindings' do
143+
expect {
144+
action.delete
145+
}.to raise_error(StandardError, 'boom-route')
146+
147+
expect(ServiceInstance.all).to contain_exactly(service_instance)
148+
expect(RouteBinding.all).to contain_exactly(route_binding_2)
149+
expect(ServiceBinding.all).to contain_exactly(service_binding_2)
150+
end
151+
end
152+
end
82153
end
83154

84155
context 'managed service instances' do

0 commit comments

Comments
 (0)