Add a monitoring uri to the stats port.

[ramr]

@pweil- PTAL Thx

This allows us to not affect hosted backends but rather use the listener on the stats port to
service health check requests. Of course the side effect here is that if you turn off stats, then the monitoring uri will not be available.

I debated putting this into the frontends 80 and 443 but felt it would be too onerous to introduce a config option or even allow a custom uri to be specified and there's of course the issue of clashing within the URI space of the backends (/health won't be available to the backend) But figured its easier to do it on a haproxy specific port we listen on for the web stats service,

Usage examples:

$ echo $(curl -w "%{http_code}" -s -o /dev/null  http://127.0.0.1:1936/healthz)      
200  
$  # old stats uri works as normal - auth required. 
$ echo $(curl -w "%{http_code}" -s -o /dev/null  http://127.0.0.1:1936/)  
401  
$ echo $(curl -w "%{http_code}" --user admin:FNdzbCXEwh  -s -o /dev/null  http://127.0.0.1:1936/)
200  

The return code of 200 satisfies providers like GCE that don't just check connectivity but also the response code.

Update: Usage examples.

[pweil-]

@ramr awesome, putting it within the stats listener is a good idea. LGTM

@smarterclayton @twiest

[pweil-]

My only other comment is maybe we want this always exposed. Ie, if you don't "turn on" stats we still add this listener and just don't include the stats section. If you choose a different stats port the health endpoint moves with it but it is always there. Thoughts?

[liggitt]

healthz?

[ramr]

Yeah, I guess we could do that (did start of doing that after the initial commit but then backed away). Though it does mean that the port would have to be exposed out always and on the flip side of the argument, there's no way to turn off the monitoring uri. We can always address it later if someone wants more granular control on it.

[ramr]

@liggitt next you will be asking for a SOAP envelope around the content!! ;^)

[pweil-]

@liggitt next you will be asking for a SOAP envelope around the content!! ;^)

give em a z and they take the whole alphabet

[liggitt]

Is everything on that port open or protected by the admin password?

[ramr]

@liggitt there's 2 paths exposed on the stats port - the /health[z] one which is open - that one's actually matched before any backend processing is done and everything else is / (stats basically) and that's admin password protected.

[ramr]

The last Zed you will ever wear is in, Agent L!!

[smarterclayton]

LGTM good call

[pweil-]

We can always address it later if someone wants more granular control on it.

I'm cool with a follow up issue. I would suggest in the issue that we should always expose this, change "stats port" to "admin port", and key exposing stats on something other than port > 0 (--enable-stats=false, so we can continue defaulting pw/username). If you REALLY REALLY don't want a health check uri then customize the template.

[liggitt]

just be backwards compatible with any changes to the oadm router invocation

[twiest]

@ramr @pweil- I would prefer an always on health check (even if we don't configure stats port).

Not a huge issue for me, though. We'll make it work either way.

[ramr]

@pweil- / @liggitt created a new issue for follow up - #5166 - a merge tag on this one would be greatly appreciated. thx

[smarterclayton]

Yes every component we have should have an always on, publicly accessible
health check endpoint called /healthz

On Oct 16, 2015, at 9:54 AM, Thomas Wiest notifications@github.com wrote:

@ramr https://github.com/ramr @pweil- https://github.com/pweil- I would
prefer an always on health check (even if we don't configure stats port).

Not a huge issue for me, though. We'll make it work either way.

—
Reply to this email directly or view it on GitHub
#5151 (comment).

[ramr]

/bump @smarterclayton anything else required here?

[smarterclayton]

I think we're saying, if stats port is not specified we should still expose the health check only on the stats port.

[ramr]

@smarterclayton so if stats port is not specified, then the default is to expose stats on port 1936 and the health check would still be enabled. However if I am reading your comment right and stats port is 0 which is what causes the health check to be disabled - that creates other problems (we can't listen/bind on that port as its 0). Now we could use the default port 1936 in that case - but it does sounds a bit hacky to do that here.
Which is why I was suggesting doing it in a follow on PR and creating a specific issue for that.

[smarterclayton]

It wouldn't be the stats port at that point, it'd be the health check
port. I'm fine with it being a follow up.

On Tue, Oct 20, 2015 at 8:24 PM, Ram Ranganathan notifications@github.com
wrote:

@smarterclayton https://github.com/smarterclayton so if stats port is
not specified, then the default is to expose stats on port 1936 and the
health check would still be enabled. However if I am reading your comment
right and stats port is 0 which is what causes the health check to be
disabled - that creates other problems (we can't listen/bind on that port
as its 0). Now we could use the default port 1936 in that case - but it
does sounds a bit hacky to do that here.
Which is why I was suggesting doing it in a follow on PR and creating a
specific issue for that.

—
Reply to this email directly or view it on GitHub
#5151 (comment).

[smarterclayton]

[test]

[openshift-bot]

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/5997/)

[openshift-bot]

Evaluated for origin test up to e6ac51a

[smarterclayton]

LGTM [merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/3758/) (Image: devenv-rhel7_2547)

[openshift-bot]

Evaluated for origin merge up to e6ac51a