Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add none recursive service instance deletion parameter #3532

Open
beyhan opened this issue Dec 1, 2023 · 5 comments
Open

Add none recursive service instance deletion parameter #3532

beyhan opened this issue Dec 1, 2023 · 5 comments

Comments

@beyhan
Copy link
Member

beyhan commented Dec 1, 2023

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
Copy link
Member

Gerg commented Dec 1, 2023

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.

@beyhan
Copy link
Member Author

beyhan commented Dec 4, 2023

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
Copy link
Member

Gerg commented Dec 4, 2023

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.

@Gerg
Copy link
Member

Gerg commented Dec 4, 2023

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.

@stephanme
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants