Add healthcheck on root url

[ryantxu]

I am trying to run thumbor on google kubernetes (GKE). The healthcheck infrastructure in gke always needs a 200 response on /

Yes it is annoying!

This easy change makes it easy to run on GKE without extra hoops (proxy)

[ryantxu]

I can work around this when building the Docker image and calling:

RUN sed -i "s#(r'\\/healthcheck', HealthcheckHandler)#(r'\\/', HealthcheckHandler),(r'\\/healthcheck', HealthcheckHandler)#g" /usr/local/lib/python2.7/site-packages/thumbor/app.py

but seems nicer to just have it off-the-shelf :)

[coveralls]

Coverage remained the same at ?% when pulling fa2bb04 on ryantxu:ok-on-root into 34f7c23 on thumbor:master.

[coveralls]

Coverage remained the same at ?% when pulling 16f3463 on ryantxu:ok-on-root into 34f7c23 on thumbor:master.

[heynemann]

I was trying to understand this issue and found this

Have you tried it? If we are to support the healthcheck route being customizable, i'd rather have a configuration like:

Config.define('HEALTHCHECK_ROUTE', r'/healthcheck', 'Healthcheck route.', 'Healthcheck')

Then at app.py:

handlers = [
    (self.context.config.HEALTHCHECK_ROUTE, HealthcheckHandler)
]

If we can avoid another config, I'd rather do it, though.

[ryantxu]

@heynemann Have you tried it? oh yes, and lots of variations!

The readiness/healthcheck probes seem to work for some of their services, but not others. The only solution I have found that reliably works is to have a 200 response off / ☹️ In particular, I am trying to use their load balancer for HTTPS and scaling. When you configure the readiness probes, you can see the checks come in that hit both the configured endpoint and / -- the pods get marked as unhealthy and the service never comes online. To be clear the readinessProbe do get used, but it seems some services also assume a 200 response on /

I am totally agnostic on the approach and if the healthcheck needs configuration. I personally don't see any downside to adding an additional response on /, but I'm sure there are implications I am not considering.

Looks like the tests are failing, so this would take additional work if you agree on an approach

[heynemann]

Can you change to the approach I outlined? Then add a test that verifies that the healthcheck works when it's route is /?

[kkopachev]

@heynemann maybe let it work off of / and that's it?

[heynemann]

Hey,

@kkopachev I didn't understand if you meant changing /healthcheck to / or keeping both. Either way, I worry that some other provider will require /status or any other route for checking.

As far as having both, I think it's a smell having two different routes that do the same thing. And since it's such a small change to have a configuration to do it, and since it's backwards compatible, I figured it to be the best approach.

Does that make sense?

@ryantxu can you please fix the failing test and provide some additional tests for the healthcheck route? Also, the configuration page in the docs should be updated. The file to update is configuration.rst.

[verglor]

Hi @ryantxu, is there an opened issue in gke regarding this problem ? I believe this should be fixed there...

[ryantxu]

thanks @heynemann -- had to use travis as my dev environment :)

@verglor -- yes there are lots of issues! they all seem to get closed without resolution. See kubernetes/ingress-gce#39, kubernetes/ingress-gce#317 -- the real one has been around since 2017 kubernetes/ingress-gce#42

[heynemann]

Released as 6.7.0 on pypi as well.