Start health checks early.

[timoreimann]

Do not wait a full tick cycle to execute the first health check.

Additional changes:

• Make request timeout configurable (for testing purposes).
• Support synchronizing on health check goroutine termination through an internal wait group (for testing purposes).
• Stop leaking by closing the HTTP response body.
• Extend health check logging and use WARNING level for (continuously) failing health checks.

[timoreimann]

@dtomcej feel free to continue where you left off. :-)

[emilevauge]

I would prefer to not modify the code for testing purpose only.
It could lead to confusion/issue in the future.
Can with think of another solution for testing?

[timoreimann]

We need some kind of synchronization point to make sure all health check goroutines have completed by the time the test asserts the results. Without modifying the code, the only alternative I see is to wait a constant amount of time and hope that all requests have finished. However, that leads to both longer-than-necessary test runtimes and a potential source of flakiness, both of which I consider worse that the addition of a wait group.

I tried to use the injected loadBalancer to signal when the health checks are complete, but it turned out we can't use that because not all test paths call methods on the loadBalancer (e.g., "healthy server staying healthy"). I'm at a point where I think it might be possible to make it work, but only at the price of a much more complicated test implementation.

Thus, a handful of extra prod code to simplify testing seems justified. I've made sure the wait group is commented and unexported so code outside of the healthcheck package won't be able to access it.

WDYT?

[emilevauge]

Ouch, not sure on this, sorry.
Designing the code to make it easily testable, I'm OK with that. But adding test parts directly into the code is a whole different story. Especially sync stuff ^^
@containous/traefik WDYT ?

[timoreimann]

@emilevauge I think I found a way: Did a bit of refactoring so that I can start the health-checking goroutine concurrently from the test; that way, I can also define the wait group in the test.

The downside of this is that I'm not strictly testing the public API anymore but an unexported function (i.e., we start testing further down the stack). But that's not the worst either.
You never get a lunch for free. :-)

Let me know what you think.

(PS: Rebased too.)

[emilevauge]

Thanks a lot @timoreimann !
I love the way your refactored this.
LGTM :)

[emilevauge]

Could you rebase again ?

[timoreimann]

@emilevauge squashed and rebased.