search

cloudfoundry-community / autosleep

Auto sleep service for CloudFoundry
Apache License 2.0
39 stars 21 forks source link

Support app delete #294

Closed mindprogenitor closed 6 years ago

mindprogenitor commented 6 years ago

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 commented 6 years ago

Coverage Status

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

gberche-orange commented 6 years ago

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 commented 6 years ago

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

mindprogenitor commented 6 years ago

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 commented 6 years ago

thanks @mindprogenitor, @metskem !

mindprogenitor commented 6 years ago

Thanks ;)