Expose a Registry type in health package, so unit tests can stay isolated from each other

Update docs.

Change health_test.go tests to create their own registries and register
the checks there. The tests now call CheckStatus directly instead of
polling the HTTP handler, which returns results from the default
registry.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

docs/handlers/app.go

@@ -234,7 +234,15 @@ func NewApp(ctx context.Context, configuration configuration.Configuration) *App
 // process. Because the configuration and app are tightly coupled,
 // implementing this properly will require a refactor. This method may panic
 // if called twice in the same process.
-func (app *App) RegisterHealthChecks() {
+func (app *App) RegisterHealthChecks(healthRegistries ...*health.Registry) {
+	if len(healthRegistries) > 1 {
+		panic("RegisterHealthChecks called with more than one registry")
+	}
+	healthRegistry := health.DefaultRegistry
+	if len(healthRegistries) == 1 {
+		healthRegistry = healthRegistries[0]
+	}
+
 	if app.Config.Health.StorageDriver.Enabled {
 		interval := app.Config.Health.StorageDriver.Interval
 		if interval == 0 {
@@ -247,9 +255,9 @@ func (app *App) RegisterHealthChecks() {
 		}
 
 		if app.Config.Health.StorageDriver.Threshold != 0 {
-			health.RegisterPeriodicThresholdFunc("storagedriver_"+app.Config.Storage.Type(), interval, app.Config.Health.StorageDriver.Threshold, storageDriverCheck)
+			healthRegistry.RegisterPeriodicThresholdFunc("storagedriver_"+app.Config.Storage.Type(), interval, app.Config.Health.StorageDriver.Threshold, storageDriverCheck)
 		} else {
-			health.RegisterPeriodicFunc("storagedriver_"+app.Config.Storage.Type(), interval, storageDriverCheck)
+			healthRegistry.RegisterPeriodicFunc("storagedriver_"+app.Config.Storage.Type(), interval, storageDriverCheck)
 		}
 	}
 
@@ -260,10 +268,10 @@ func (app *App) RegisterHealthChecks() {
 		}
 		if fileChecker.Threshold != 0 {
 			ctxu.GetLogger(app).Infof("configuring file health check path=%s, interval=%d, threshold=%d", fileChecker.File, interval/time.Second, fileChecker.Threshold)
-			health.Register(fileChecker.File, health.PeriodicThresholdChecker(checks.FileChecker(fileChecker.File), interval, fileChecker.Threshold))
+			healthRegistry.Register(fileChecker.File, health.PeriodicThresholdChecker(checks.FileChecker(fileChecker.File), interval, fileChecker.Threshold))
 		} else {
 			ctxu.GetLogger(app).Infof("configuring file health check path=%s, interval=%d", fileChecker.File, interval/time.Second)
-			health.Register(fileChecker.File, health.PeriodicChecker(checks.FileChecker(fileChecker.File), interval))
+			healthRegistry.Register(fileChecker.File, health.PeriodicChecker(checks.FileChecker(fileChecker.File), interval))
 		}
 	}
 
@@ -274,10 +282,10 @@ func (app *App) RegisterHealthChecks() {
 		}
 		if httpChecker.Threshold != 0 {
 			ctxu.GetLogger(app).Infof("configuring HTTP health check uri=%s, interval=%d, threshold=%d", httpChecker.URI, interval/time.Second, httpChecker.Threshold)
-			health.Register(httpChecker.URI, health.PeriodicThresholdChecker(checks.HTTPChecker(httpChecker.URI), interval, httpChecker.Threshold))
+			healthRegistry.Register(httpChecker.URI, health.PeriodicThresholdChecker(checks.HTTPChecker(httpChecker.URI), interval, httpChecker.Threshold))
 		} else {
 			ctxu.GetLogger(app).Infof("configuring HTTP health check uri=%s, interval=%d", httpChecker.URI, interval/time.Second)
-			health.Register(httpChecker.URI, health.PeriodicChecker(checks.HTTPChecker(httpChecker.URI), interval))
+			healthRegistry.Register(httpChecker.URI, health.PeriodicChecker(checks.HTTPChecker(httpChecker.URI), interval))
 		}
 	}
 }

docs/handlers/health_test.go

@@ -1,7 +1,6 @@
 package handlers
 
 import (
-	"encoding/json"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -15,9 +14,6 @@ import (
 )
 
 func TestFileHealthCheck(t *testing.T) {
-	// In case other tests registered checks before this one
-	health.UnregisterAll()
-
 	interval := time.Second
 
 	tmpfile, err := ioutil.TempFile(os.TempDir(), "healthcheck")
@@ -43,60 +39,29 @@ func TestFileHealthCheck(t *testing.T) {
 	ctx := context.Background()
 
 	app := NewApp(ctx, config)
-	app.RegisterHealthChecks()
+	healthRegistry := health.NewRegistry()
-
+	app.RegisterHealthChecks(healthRegistry)
-	debugServer := httptest.NewServer(nil)
 
 	// Wait for health check to happen
 	<-time.After(2 * interval)
 
-	resp, err := http.Get(debugServer.URL + "/debug/health")
+	status := healthRegistry.CheckStatus()
-	if err != nil {
+	if len(status) != 1 {
-		t.Fatalf("error performing HTTP GET: %v", err)
+		t.Fatal("expected 1 item in health check results")
 	}
-	body, err := ioutil.ReadAll(resp.Body)
+	if status[tmpfile.Name()] != "file exists" {
-	if err != nil {
-		t.Fatalf("error reading HTTP body: %v", err)
-	}
-	resp.Body.Close()
-	var decoded map[string]string
-	err = json.Unmarshal(body, &decoded)
-	if err != nil {
-		t.Fatalf("error unmarshaling json: %v", err)
-	}
-	if len(decoded) != 1 {
-		t.Fatal("expected 1 item in returned json")
-	}
-	if decoded[tmpfile.Name()] != "file exists" {
 		t.Fatal(`did not get "file exists" result for health check`)
 	}
 
 	os.Remove(tmpfile.Name())
 
 	<-time.After(2 * interval)
-	resp, err = http.Get(debugServer.URL + "/debug/health")
+	if len(healthRegistry.CheckStatus()) != 0 {
-	if err != nil {
+		t.Fatal("expected 0 items in health check results")
-		t.Fatalf("error performing HTTP GET: %v", err)
-	}
-	body, err = ioutil.ReadAll(resp.Body)
-	if err != nil {
-		t.Fatalf("error reading HTTP body: %v", err)
-	}
-	resp.Body.Close()
-	var decoded2 map[string]string
-	err = json.Unmarshal(body, &decoded2)
-	if err != nil {
-		t.Fatalf("error unmarshaling json: %v", err)
-	}
-	if len(decoded2) != 0 {
-		t.Fatal("expected 0 items in returned json")
 	}
 }
 
 func TestHTTPHealthCheck(t *testing.T) {
-	// In case other tests registered checks before this one
-	health.UnregisterAll()
-
 	interval := time.Second
 	threshold := 3
 
@@ -132,32 +97,18 @@ func TestHTTPHealthCheck(t *testing.T) {
 	ctx := context.Background()
 
 	app := NewApp(ctx, config)
-	app.RegisterHealthChecks()
+	healthRegistry := health.NewRegistry()
-
+	app.RegisterHealthChecks(healthRegistry)
-	debugServer := httptest.NewServer(nil)
 
 	for i := 0; ; i++ {
 		<-time.After(interval)
 
-		resp, err := http.Get(debugServer.URL + "/debug/health")
+		status := healthRegistry.CheckStatus()
-		if err != nil {
-			t.Fatalf("error performing HTTP GET: %v", err)
-		}
-		body, err := ioutil.ReadAll(resp.Body)
-		if err != nil {
-			t.Fatalf("error reading HTTP body: %v", err)
-		}
-		resp.Body.Close()
-		var decoded map[string]string
-		err = json.Unmarshal(body, &decoded)
-		if err != nil {
-			t.Fatalf("error unmarshaling json: %v", err)
-		}
 
 		if i < threshold-1 {
 			// definitely shouldn't have hit the threshold yet
-			if len(decoded) != 0 {
+			if len(status) != 0 {
-				t.Fatal("expected 1 items in returned json")
+				t.Fatal("expected 1 item in health check results")
 			}
 			continue
 		}
@@ -166,10 +117,10 @@ func TestHTTPHealthCheck(t *testing.T) {
 			continue
 		}
 
-		if len(decoded) != 1 {
+		if len(status) != 1 {
-			t.Fatal("expected 1 item in returned json")
+			t.Fatal("expected 1 item in health check results")
 		}
-		if decoded[checkedServer.URL] != "downstream service returned unexpected status: 500" {
+		if status[checkedServer.URL] != "downstream service returned unexpected status: 500" {
 			t.Fatal("did not get expected result for health check")
 		}
 
@@ -180,21 +131,8 @@ func TestHTTPHealthCheck(t *testing.T) {
 	close(stopFailing)
 
 	<-time.After(2 * interval)
-	resp, err := http.Get(debugServer.URL + "/debug/health")
+
-	if err != nil {
+	if len(healthRegistry.CheckStatus()) != 0 {
-		t.Fatalf("error performing HTTP GET: %v", err)
+		t.Fatal("expected 0 items in health check results")
-	}
-	body, err := ioutil.ReadAll(resp.Body)
-	if err != nil {
-		t.Fatalf("error reading HTTP body: %v", err)
-	}
-	resp.Body.Close()
-	var decoded map[string]string
-	err = json.Unmarshal(body, &decoded)
-	if err != nil {
-		t.Fatalf("error unmarshaling json: %v", err)
-	}
-	if len(decoded) != 0 {
-		t.Fatal("expected 0 items in returned json")
 	}
 }