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

Support app delete #294

Conversation

mindprogenitor
Copy link
Contributor

At this moment, deleting an app bound to the autosleep service will fail. It always requires the app to be unbound from the service first. This is because of it trying to delete bindings for routes (which are not even supported, it seems).

This change only tries to delete route bindings if any is found. And then the delete can go on properly and the database is properly updated.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 95.138% when pulling 9249cd2 on mindprogenitor:support-app-delete into 1dff43d on cloudfoundry-community:develop.

@gberche-orange
Copy link
Contributor

thanks @mindprogenitor for your contribution !

In your case, was the delete failing only for apps with no mapped routes ? Are you observing the same symptoms as #293 ?

This PR seems to skip route service listing/deleting when no route is mapped to the application.

Indeed the route binding assigned by autosleep were meant to support autowakeup proxy, but due to current CF limitation, it was half disabled, see #203 for more details.

@metskem
Copy link

metskem commented Jun 22, 2018

#293 is indeed the symptom we are trying to solve (mindprogenitor, lodener and me, we are in the same team over here :-) )

@mindprogenitor
Copy link
Contributor Author

Like metskem said, it is indeed the same issue. I forgot that issue had been opened (I forgot an issue had been opened).

This pr only skips the route deletion (as it can't find any route associated to it, it can't delete route bindings... however, since route binding is not in place, it should not create any clutter anyway. If/when route binding comes in place I suggest that the we use the app id to refer to all route bindings associated to it, by linking app info to the route bindings, or some other way to achieve the same (I must confess I haven't thoroughly checked the domain model to have a more concrete suggestion, but at this moment I would add the app id to the binding table).

So, this pr basically makes sure that a piece of code that currently is actually not used (since the route binding is not active) doesn't break the other normal functionality.

@gberche-orange gberche-orange merged commit 59c8768 into cloudfoundry-community:develop Jun 27, 2018
@gberche-orange
Copy link
Contributor

thanks @mindprogenitor, @metskem !

@mindprogenitor
Copy link
Contributor Author

Thanks ;)

@mindprogenitor mindprogenitor deleted the support-app-delete branch June 27, 2018 08:08
This pull request was closed.
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

Successfully merging this pull request may close these issues.

4 participants