plugin/proxy: Don't enable HTTP healthchecking if not configured (#1441)

HTTP healthchecking will be implicitely activated for proxy upstream
hosts, even if not configured. The README states that not using the
health_check directive will disable HTTP healthchecks though.

It seems to me that the availability of the HealthCheck.Path attribute
is used as indicator whether HTTP healthchecks should be used or not.
The normalizeCheckURL() function didn't check that attribute though,
always returning a CheckURL. This would increase the healthcheck failure
on every third failure in plugin/proxy, without any possibility for the
upstream host to be marked as healthy again. This would eventually
remove all upstream hosts from the serving pool.

2 files changed, 26 insertions, 2 deletions

diff --git a/plugin/pkg/healthcheck/healthcheck.go b/plugin/pkg/healthcheck/healthcheck.go
index 29a904c34..2a1a89478 100644
--- a/plugin/pkg/healthcheck/healthcheck.go
+++ b/plugin/pkg/healthcheck/healthcheck.go
@@ -212,6 +212,10 @@ func (u *HealthCheck) Select() *UpstreamHost {
 
 // normalizeCheckURL creates a proper URL for the health check.
 func (u *HealthCheck) normalizeCheckURL(name string) string {
+	if u.Path == "" {
+		return ""
+	}
+
 	// The DNS server might be an HTTP server.  If so, extract its name.
 	hostName := name
 	ret, err := url.Parse(name)
diff --git a/plugin/pkg/healthcheck/policy_test.go b/plugin/pkg/healthcheck/policy_test.go
index 60ed46b23..d3f03b7e3 100644
--- a/plugin/pkg/healthcheck/policy_test.go
+++ b/plugin/pkg/healthcheck/policy_test.go
@@ -52,17 +52,16 @@ func TestRegisterPolicy(t *testing.T) {
 func TestHealthCheck(t *testing.T) {
 	u := &HealthCheck{
 		Hosts:       testPool(),
+		Path:        "/",
 		FailTimeout: 10 * time.Second,
 		MaxFails:    1,
 	}
 
 	for i, h := range u.Hosts {
-		u.Hosts[i].Fails = 1
 		u.Hosts[i].CheckURL = u.normalizeCheckURL(h.Name)
 	}
 
 	u.healthCheck()
-
 	time.Sleep(time.Duration(1 * time.Second)) // sleep a bit, it's async now
 
 	if u.Hosts[0].Down() {
@@ -73,6 +72,27 @@ func TestHealthCheck(t *testing.T) {
 	}
 }
 
+func TestHealthCheckDisabled(t *testing.T) {
+	u := &HealthCheck{
+		Hosts:       testPool(),
+		FailTimeout: 10 * time.Second,
+		MaxFails:    1,
+	}
+
+	for i, h := range u.Hosts {
+		u.Hosts[i].CheckURL = u.normalizeCheckURL(h.Name)
+	}
+
+	u.healthCheck()
+	time.Sleep(time.Duration(1 * time.Second)) // sleep a bit, it's async now
+
+	for i, h := range u.Hosts {
+		if h.Down() {
+			t.Errorf("Expected host %d in testpool to not be down with healthchecks disabled.", i+1)
+		}
+	}
+}
+
 func TestRoundRobinPolicy(t *testing.T) {
 	pool := testPool()
 	rrPolicy := &RoundRobin{}