cloudfoundry/cloud_controller_ng

Add none recursive service instance deletion parameter

beyhan opened this issue · 5 comments

beyhan commented

Issue

The service instance deletion API in V3 deletes a service instance recursively. The default behaviour of the V2 APIs is none recursive and there is a parameter called recursive which can be used to have a recursive deletion. The recursive deletion for service instances is challenging because they could have bindings which also get deleted but apps using the bindings will still have a reference to the binding and will fail by trying to access the service via it.

Steps to Reproduce

  1. Create a service of any type
  2. Deploy the spring music app and bind it to the service from step 1)
  3. Execute cf delete-service <my-service-instance>
  4. Check e.g. by calling https://<my-app-host>/appinfo that the app is still bound to the service

Expected result

With the current approach in CF to require a restage for service binding updates I don't think that this could be solved but we could provide a none recursive alternative.

Possible Fix

In case a CF API client wants to implement a none recursive deletion it will be useful to provide a parameter to disable the recursive deletion.

Gerg commented

Interestingly, it looks like the v6 CLI doesn't even make the delete request if you still have bindings:

❯ cf6 ds my-service -v

Really delete the service my-service?> y
Deleting service my-service in org org / space space as admin...

REQUEST: [2023-12-01T11:22:50-08:00]
GET /v2/spaces/7fe52db0-d740-4911-8197-17b45d39aebd/service_instances?return_user_provided_service_instances=true&q=name%3Amy-service&inline-relations-depth=1 HTTP/1.1
...


RESPONSE: [2023-12-01T11:22:50-08:00]
HTTP/1.1 200 OK
...

FAILED
Cannot delete service instance. Service keys, bindings, and shares must first be deleted.
  • 👀1
beyhan commented

yes, I noticed it also when I was investigating this and was surprised. This improvement will require less efforts on CF API clients side and will provide an "atomic" none recursive deletion. So, it is nice to have.

Gerg commented

I checked, and the v7 CLI has the same behavior as v6:

❯ cf7 delete-service my-service

Really delete the service my-service?> y
Deleting service my-service in org org / space space as admin...
FAILED
Cannot delete service instance. Service keys, bindings, and shares must first be deleted.
  • 👀1
Gerg commented

I dug up some stories from when the v8 command was originally introduced:

Based on those, it looks like the current v8 behavior was intentional. Not saying that this is the correct behavior that we want to continue with, but it wasn't an oversight.

  • 👍1
  • 👀1
stephanme commented

cloudfoundry/cli#1839 explicitly requested recursive service instance deletion (i.e. including all service bindings). This is now the standard behaviour of CF API v3 and cf8.

The rational was that cf delete-service now behaves consistent with cf delete-org and cf delete-space. But there is a difference: org/space/app deletion deletes recursively only dependent resources that are owned by the org/space/app. In contrast, the service bindings (to apps) are not owned by the service instance but by the app. I.e. the recursive deletion of a service instance breaks 'outside' resources which is not good in my eyes.
Service keys may be considered to be owned by the service instance.

A possible solution might be to change cf8 delete-service back to the cf6/cf7 behaviour, i.e. delete the service instance only if there no keys, bindings and shares. A --recursive flag could be added to support the current cf8 behaviour e.g. for CI.
This could be implemented with an extra CF API query or with the proposed DELETE /v3/service_instances/:guid?recursive=true query parameter.