Add healthz checker

[aledbf]

fixes #27

[coveralls]

Coverage remained the same at 40.231% when pulling 2fb0c3db17625eec3881c400c1e0967b2dc3a0ba on aledbf:health-check into d1fb96a on kubernetes:master.

[bprashanth]

I assume all our yaml exposes a probe through a standard port, and the output of this health method call is automatically exported via http endpoint on that port by the generic_controller?

I also think you need to update the comment in doc.go since we've added a new interface method?

[bprashanth]

are we exposing the output of a Check call on some standard port as a liveness or readiness check?

[aledbf]

Yes, the port if 10254 (can be changed, is behind a flag) and all the examples use this port for liveness and readiness check
https://github.com/kubernetes/ingress/blob/master/core/pkg/ingress/controller/launch.go#L67
https://github.com/kubernetes/ingress/pull/31/files#diff-ce130ef9249cc52ef757bf889e4ba07dR160

[bprashanth]

please clarify what happens if the check is +ve/-ve

[bprashanth]

actually i prefer giving this interface a plain Check method, the name "healthz" implies a http endpoint and the Check method call may do anything else to validate health

[aledbf]

I'm embedding the interface not using healthz as name (not sure what's the issue here)

[bprashanth]

If the only purpose of the healthzChecker is to have a Check method, it should be renamed to healthChecker. If that is not the case (i.e healthzChecker is an interface for especially http health checks), we shouldn't embed it, because adding methods to healthzChecker will directly bubble up here. Reading the interface definition one could get tricked into thinking that all backends must return a http error from the Check() call, when that's not the case. The inclusion of z in the name leads to unnecessary confusion IMO.

[bprashanth]

Oh sorry, I missed that Check() actually takes a url. Does it need to? What if I want to ps aux | grep nginx as a health check in my backend?

[aledbf]

What if I want to ps aux | grep nginx as a health check in my backend?

you can do that inside the Check method of you backend.
Just in case no matter what/how you backend is checking the yaml always remains like this:

        readinessProbe:
          httpGet:
            path: /healthz
            port: 10254
            scheme: HTTP
        livenessProbe:
          httpGet:
            path: /healthz
            port: 10254
            scheme: HTTP

In nginx I know that the URL exposed in the nginx default server http://127.0.0.1:18080/healthz tells me if nginx is running or not

[bprashanth]

Right I'm asking why "Check(_ *http.Request) error" vs "Check() error", since check is not restricted to http anyway.

[aledbf]

because the check is method is called from http (https://github.com/kubernetes/ingress/pull/31/files#diff-ce130ef9249cc52ef757bf889e4ba07dR159)

[aledbf]

I also think you need to update the comment in doc.go since we've added a new interface method?

done

[coveralls]

Coverage increased (+0.005%) to 40.236% when pulling d49579e9b4c6a1c9b88317a9bf12d1f4f7546fac on aledbf:health-check into d1fb96a on kubernetes:master.

[coveralls]

Coverage decreased (-0.08%) to 40.148% when pulling 478d51c on aledbf:health-check into d1fb96a on kubernetes:master.

[bprashanth]

LGTM