Fix app deletion in case of asynchronous service bindings #547
Conversation
| // error handling | ||
| if err != nil { | ||
| // https://github.com/cloudfoundry/cloud_controller_ng/issues/3589 -> Delete app when bound to service fails with async service brokers -> Retry that | ||
| specialError, _ := regexp.MatchString("An operation for the service binding between app .* and service instance .* is in progress.", err.Error()) |
There was a problem hiding this comment.
Out of curiosity, will the API return an error matching this regardless of the service broker implementation?
There was a problem hiding this comment.
They even have a test for this at this point: https://github.com/cloudfoundry/cloud_controller_ng/blob/ee25971edd577137ecc1e7104ad343e55043de9f/spec/unit/actions/app_delete_spec.rb#L236-L248 -> whenever possible, they go async, and then they expect this message
The app delete action uses the bindings deletion mixin: https://github.com/cloudfoundry/cloud_controller_ng/blob/main/app/actions/mixins/bindings_delete.rb
The actual delete action responds here: https://github.com/cloudfoundry/cloud_controller_ng/blob/ee25971edd577137ecc1e7104ad343e55043de9f/app/actions/v3/service_binding_delete.rb#L78
If the action is not complete yet, this here is triggered: https://github.com/cloudfoundry/cloud_controller_ng/blob/ee25971edd577137ecc1e7104ad343e55043de9f/app/actions/mixins/bindings_delete.rb#L23
Which resolves to https://github.com/cloudfoundry/cloud_controller_ng/blob/main/app/actions/app_delete.rb#L106, where the message is hardcoded
| Session: session, | ||
| JobURL: jobURL, | ||
| }) | ||
| err = v3appdeployers.SafeAppDeletion(*session.ClientV3, appGUID, 5) |
There was a problem hiding this comment.
Small note: this would mean that we can potentially be waiting up to 5 minutes for each venerable app deletion. We'd probably want to discuss with @loafoe on whether this is too much time or not.
There was a problem hiding this comment.
Hm, you are right. I'm not sure what actually will take up time in the process. The assumption is that if deletion of services fails (due to asynchronicity), it does fail fast.
See the delete action at cloud controller:
- first, they delete the nontransactional resources (the binding): https://github.com/cloudfoundry/cloud_controller_ng/blob/ee25971edd577137ecc1e7104ad343e55043de9f/app/actions/app_delete.rb#L41
- only afterwards, they delete packages, tasks, droplets, revisions, ..... https://github.com/cloudfoundry/cloud_controller_ng/blob/ee25971edd577137ecc1e7104ad343e55043de9f/app/actions/app_delete.rb#L46
As such, when the retry happens, it should happen fast and not after lots of iterations in the polling loop.
Fixes #546