Merge pull request #831 from stevvooe/add-driver-health-check

Provide simple storage driver health check

cmd/registry-api-descriptor-template/main.go

@@ -50,7 +50,10 @@ func main() {
 		ErrorDescriptors []errcode.ErrorDescriptor
 	}{
 		RouteDescriptors: v2.APIDescriptor.RouteDescriptors,
-		ErrorDescriptors: errcode.GetErrorCodeGroup("registry.api.v2"),
+		ErrorDescriptors: append(errcode.GetErrorCodeGroup("registry.api.v2"),
+			// The following are part of the specification but provided by errcode default.
+			errcode.ErrorCodeUnauthorized.Descriptor(),
+			errcode.ErrorCodeUnsupported.Descriptor()),
 	}
 
 	if err := tmpl.Execute(os.Stdout, data); err != nil {

cmd/registry/main.go

@@ -17,7 +17,7 @@ import (
 	"github.com/bugsnag/bugsnag-go"
 	"github.com/docker/distribution/configuration"
 	"github.com/docker/distribution/context"
-	_ "github.com/docker/distribution/health"
+	"github.com/docker/distribution/health"
 	_ "github.com/docker/distribution/registry/auth/htpasswd"
 	_ "github.com/docker/distribution/registry/auth/silly"
 	_ "github.com/docker/distribution/registry/auth/token"
@@ -70,8 +70,10 @@ func main() {
 	uuid.Loggerf = context.GetLogger(ctx).Warnf
 
 	app := handlers.NewApp(ctx, *config)
+	app.RegisterHealthChecks()
 	handler := configureReporting(app)
 	handler = panicHandler(handler)
+	handler = health.Handler(handler)
 	handler = gorhandlers.CombinedLoggingHandler(os.Stdout, handler)
 
 	if config.HTTP.Debug.Addr != "" {

docs/spec/api.md

@@ -2249,7 +2249,7 @@ The following headers will be returned with the response:
 
 |Name|Description|
 |----|-----------|
-|`Content-Length`|Zero|
+|`Content-Length`|0|
 |`Docker-Content-Digest`|Digest of the targeted content for the request.|
 
 

health/health.go

@@ -2,9 +2,13 @@ package health
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"sync"
 	"time"
+
+	"github.com/docker/distribution/context"
+	"github.com/docker/distribution/registry/api/errcode"
 )
 
 var (
@@ -140,7 +144,7 @@ func PeriodicThresholdChecker(check Checker, period time.Duration, threshold int
 }
 
 // CheckStatus returns a map with all the current health check errors
-func CheckStatus() map[string]string {
+func CheckStatus() map[string]string { // TODO(stevvooe) this needs a proper type
 	mutex.RLock()
 	defer mutex.RUnlock()
 	statusKeys := make(map[string]string)
@@ -174,13 +178,13 @@ func RegisterFunc(name string, check func() error) {
 
 // RegisterPeriodicFunc allows the convenience of registering a PeriodicChecker
 // from an arbitrary func() error
-func RegisterPeriodicFunc(name string, check func() error, period time.Duration) {
+func RegisterPeriodicFunc(name string, period time.Duration, check CheckFunc) {
 	Register(name, PeriodicChecker(CheckFunc(check), period))
 }
 
 // RegisterPeriodicThresholdFunc allows the convenience of registering a
 // PeriodicChecker from an arbitrary func() error
-func RegisterPeriodicThresholdFunc(name string, check func() error, period time.Duration, threshold int) {
+func RegisterPeriodicThresholdFunc(name string, period time.Duration, threshold int, check CheckFunc) {
 	Register(name, PeriodicThresholdChecker(CheckFunc(check), period, threshold))
 }
 
@@ -189,25 +193,61 @@ func RegisterPeriodicThresholdFunc(name string, check func() error, period time.
 // Returns 503 if any Error status exists, 200 otherwise
 func StatusHandler(w http.ResponseWriter, r *http.Request) {
 	if r.Method == "GET" {
-		w.Header().Set("Content-Type", "application/json; charset=utf-8")
-		checksStatus := CheckStatus()
-		// If there is an error, return 503
-		if len(checksStatus) != 0 {
-			w.WriteHeader(http.StatusServiceUnavailable)
-		}
-		encoder := json.NewEncoder(w)
-		err := encoder.Encode(checksStatus)
+		checks := CheckStatus()
+		status := http.StatusOK
 
-		// Parsing of the JSON failed. Returning generic error message
-		if err != nil {
-			encoder.Encode(struct {
-				ServerError string `json:"server_error"`
-			}{
-				ServerError: "Could not parse error message",
-			})
+		// If there is an error, return 503
+		if len(checks) != 0 {
+			status = http.StatusServiceUnavailable
 		}
+
+		statusResponse(w, r, status, checks)
 	} else {
-		w.WriteHeader(http.StatusNotFound)
+		http.NotFound(w, r)
+	}
+}
+
+// Handler returns a handler that will return 503 response code if the health
+// checks have failed. If everything is okay with the health checks, the
+// handler will pass through to the provided handler. Use this handler to
+// disable a web application when the health checks fail.
+func Handler(handler http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		checks := CheckStatus()
+		if len(checks) != 0 {
+			errcode.ServeJSON(w, errcode.ErrorCodeUnavailable.
+				WithDetail("health check failed: please see /debug/health"))
+			return
+		}
+
+		handler.ServeHTTP(w, r) // pass through
+	})
+}
+
+// statusResponse completes the request with a response describing the health
+// of the service.
+func statusResponse(w http.ResponseWriter, r *http.Request, status int, checks map[string]string) {
+	p, err := json.Marshal(checks)
+	if err != nil {
+		context.GetLogger(context.Background()).Errorf("error serializing health status: %v", err)
+		p, err = json.Marshal(struct {
+			ServerError string `json:"server_error"`
+		}{
+			ServerError: "Could not parse error message",
+		})
+		status = http.StatusInternalServerError
+
+		if err != nil {
+			context.GetLogger(context.Background()).Errorf("error serializing health status failure message: %v", err)
+			return
+		}
+	}
+
+	w.Header().Set("Content-Type", "application/json; charset=utf-8")
+	w.Header().Set("Content-Length", fmt.Sprint(len(p)))
+	w.WriteHeader(status)
+	if _, err := w.Write(p); err != nil {
+		context.GetLogger(context.Background()).Errorf("error writing health status response body: %v", err)
 	}
 }
 

health/health_test.go

@@ -2,6 +2,7 @@ package health
 
 import (
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -45,3 +46,62 @@ func TestReturns503IfThereAreErrorChecks(t *testing.T) {
 		t.Errorf("Did not get a 503.")
 	}
 }
+
+// TestHealthHandler ensures that our handler implementation correct protects
+// the web application when things aren't so healthy.
+func TestHealthHandler(t *testing.T) {
+	// clear out existing checks.
+	registeredChecks = make(map[string]Checker)
+
+	// protect an http server
+	handler := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNoContent)
+	}))
+
+	// wrap it in our health handler
+	handler = Handler(handler)
+
+	// use this swap check status
+	updater := NewStatusUpdater()
+	Register("test_check", updater)
+
+	// now, create a test server
+	server := httptest.NewServer(handler)
+
+	checkUp := func(t *testing.T, message string) {
+		resp, err := http.Get(server.URL)
+		if err != nil {
+			t.Fatalf("error getting success status: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusNoContent {
+			t.Fatalf("unexpected response code from server when %s: %d != %d", message, resp.StatusCode, http.StatusNoContent)
+		}
+		// NOTE(stevvooe): we really don't care about the body -- the format is
+		// not standardized or supported, yet.
+	}
+
+	checkDown := func(t *testing.T, message string) {
+		resp, err := http.Get(server.URL)
+		if err != nil {
+			t.Fatalf("error getting down status: %v", err)
+		}
+		defer resp.Body.Close()
+
+		if resp.StatusCode != http.StatusServiceUnavailable {
+			t.Fatalf("unexpected response code from server when %s: %d != %d", message, resp.StatusCode, http.StatusServiceUnavailable)
+		}
+	}
+
+	// server should be up
+	checkUp(t, "initial health check")
+
+	// now, we fail the health check
+	updater.Update(fmt.Errorf("the server is now out of commission"))
+	checkDown(t, "server should be down") // should be down
+
+	// bring server back up
+	updater.Update(nil)
+	checkUp(t, "when server is back up") // now we should be back up.
+}

registry/api/errcode/register.go

@@ -13,15 +13,45 @@ var (
 	groupToDescriptors     = map[string][]ErrorDescriptor{}
 )
 
-// ErrorCodeUnknown is a generic error that can be used as a last
-// resort if there is no situation-specific error message that can be used
-var ErrorCodeUnknown = Register("errcode", ErrorDescriptor{
-	Value:   "UNKNOWN",
-	Message: "unknown error",
-	Description: `Generic error returned when the error does not have an
+var (
+	// ErrorCodeUnknown is a generic error that can be used as a last
+	// resort if there is no situation-specific error message that can be used
+	ErrorCodeUnknown = Register("errcode", ErrorDescriptor{
+		Value:   "UNKNOWN",
+		Message: "unknown error",
+		Description: `Generic error returned when the error does not have an
 			                                            API classification.`,
-	HTTPStatusCode: http.StatusInternalServerError,
-})
+		HTTPStatusCode: http.StatusInternalServerError,
+	})
+
+	// ErrorCodeUnsupported is returned when an operation is not supported.
+	ErrorCodeUnsupported = Register("errcode", ErrorDescriptor{
+		Value:   "UNSUPPORTED",
+		Message: "The operation is unsupported.",
+		Description: `The operation was unsupported due to a missing
+		implementation or invalid set of parameters.`,
+		HTTPStatusCode: http.StatusBadRequest,
+	})
+
+	// ErrorCodeUnauthorized is returned if a request is not authorized.
+	ErrorCodeUnauthorized = Register("errcode", ErrorDescriptor{
+		Value:   "UNAUTHORIZED",
+		Message: "access to the requested resource is not authorized",
+		Description: `The access controller denied access for the operation on
+		a resource. Often this will be accompanied by a 401 Unauthorized
+		response status.`,
+		HTTPStatusCode: http.StatusUnauthorized,
+	})
+
+	// ErrorCodeUnavailable provides a common error to report unavialability
+	// of a service or endpoint.
+	ErrorCodeUnavailable = Register("errcode", ErrorDescriptor{
+		Value:          "UNAVAILABLE",
+		Message:        "service unavailable",
+		Description:    "Returned when a service is not available",
+		HTTPStatusCode: http.StatusServiceUnavailable,
+	})
+)
 
 var nextCode = 1000
 var registerLock sync.Mutex

registry/api/v2/descriptors.go

@@ -124,7 +124,7 @@ var (
 			},
 		},
 		ErrorCodes: []errcode.ErrorCode{
-			ErrorCodeUnauthorized,
+			errcode.ErrorCodeUnauthorized,
 		},
 		Body: BodyDescriptor{
 			ContentType: "application/json; charset=utf-8",
@@ -145,7 +145,7 @@ var (
 			},
 		},
 		ErrorCodes: []errcode.ErrorCode{
-			ErrorCodeUnauthorized,
+			errcode.ErrorCodeUnauthorized,
 		},
 		Body: BodyDescriptor{
 			ContentType: "application/json; charset=utf-8",
@@ -374,7 +374,7 @@ var routeDescriptors = []RouteDescriptor{
 									Format:      errorsBody,
 								},
 								ErrorCodes: []errcode.ErrorCode{
-									ErrorCodeUnauthorized,
+									errcode.ErrorCodeUnauthorized,
 								},
 							},
 							{
@@ -451,7 +451,7 @@ var routeDescriptors = []RouteDescriptor{
 									Format:      errorsBody,
 								},
 								ErrorCodes: []errcode.ErrorCode{
-									ErrorCodeUnauthorized,
+									errcode.ErrorCodeUnauthorized,
 								},
 							},
 						},
@@ -506,7 +506,7 @@ var routeDescriptors = []RouteDescriptor{
 									Format:      errorsBody,
 								},
 								ErrorCodes: []errcode.ErrorCode{
-									ErrorCodeUnauthorized,
+									errcode.ErrorCodeUnauthorized,
 								},
 							},
 						},
@@ -568,7 +568,7 @@ var routeDescriptors = []RouteDescriptor{
 									Format:      errorsBody,
 								},
 								ErrorCodes: []errcode.ErrorCode{
-									ErrorCodeUnauthorized,
+									errcode.ErrorCodeUnauthorized,
 								},
 							},
 							{
@@ -645,7 +645,7 @@ var routeDescriptors = []RouteDescriptor{
 									Format:      errorsBody,
 								},
 								ErrorCodes: []errcode.ErrorCode{
-									ErrorCodeUnauthorized,
+									errcode.ErrorCodeUnauthorized,
 								},
 							},
 							{
@@ -682,7 +682,7 @@ var routeDescriptors = []RouteDescriptor{
 									},
 								},
 								ErrorCodes: []errcode.ErrorCode{
-									ErrorCodeUnauthorized,
+									errcode.ErrorCodeUnauthorized,
 								},
 								Body: BodyDescriptor{
 									ContentType: "application/json; charset=utf-8",
@@ -737,7 +737,7 @@ var routeDescriptors = []RouteDescriptor{
 									},
 								},
 								ErrorCodes: []errcode.ErrorCode{
-									ErrorCodeUnauthorized,
+									errcode.ErrorCodeUnauthorized,
 								},
 								Body: BodyDescriptor{
 									ContentType: "application/json; charset=utf-8",
@@ -974,7 +974,7 @@ var routeDescriptors = []RouteDescriptor{
 									Format:      errorsBody,
 								},
 								ErrorCodes: []errcode.ErrorCode{
-									ErrorCodeUnsupported,
+									errcode.ErrorCodeUnsupported,
 								},
 							},
 						},

registry/api/v2/errors.go

@@ -9,24 +9,6 @@ import (
 const errGroup = "registry.api.v2"
 
 var (
-	// ErrorCodeUnsupported is returned when an operation is not supported.
-	ErrorCodeUnsupported = errcode.Register(errGroup, errcode.ErrorDescriptor{
-		Value:   "UNSUPPORTED",
-		Message: "The operation is unsupported.",
-		Description: `The operation was unsupported due to a missing
-		implementation or invalid set of parameters.`,
-	})
-
-	// ErrorCodeUnauthorized is returned if a request is not authorized.
-	ErrorCodeUnauthorized = errcode.Register(errGroup, errcode.ErrorDescriptor{
-		Value:   "UNAUTHORIZED",
-		Message: "access to the requested resource is not authorized",
-		Description: `The access controller denied access for the operation on
-		a resource. Often this will be accompanied by a 401 Unauthorized
-		response status.`,
-		HTTPStatusCode: http.StatusUnauthorized,
-	})
-
 	// ErrorCodeDigestInvalid is returned when uploading a blob if the
 	// provided digest does not match the blob contents.
 	ErrorCodeDigestInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{

registry/client/errors.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 
 	"github.com/docker/distribution/registry/api/errcode"
-	"github.com/docker/distribution/registry/api/v2"
 )
 
 // UnexpectedHTTPStatusError is returned when an unexpected HTTP status is
@@ -52,7 +51,7 @@ func handleErrorResponse(resp *http.Response) error {
 	if resp.StatusCode == 401 {
 		err := parseHTTPErrorResponse(resp.Body)
 		if uErr, ok := err.(*UnexpectedHTTPResponseError); ok {
-			return v2.ErrorCodeUnauthorized.WithDetail(uErr.Response)
+			return errcode.ErrorCodeUnauthorized.WithDetail(uErr.Response)
 		}
 		return err
 	}

registry/client/repository_test.go

@@ -21,7 +21,6 @@ import (
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/distribution/registry/api/errcode"
-	"github.com/docker/distribution/registry/api/v2"
 	"github.com/docker/distribution/testutil"
 )
 
@@ -782,10 +781,10 @@ func TestManifestUnauthorized(t *testing.T) {
 	if !ok {
 		t.Fatalf("Unexpected error type: %#v", err)
 	}
-	if v2Err.Code != v2.ErrorCodeUnauthorized {
+	if v2Err.Code != errcode.ErrorCodeUnauthorized {
 		t.Fatalf("Unexpected error code: %s", v2Err.Code.String())
 	}
-	if expected := v2.ErrorCodeUnauthorized.Message(); v2Err.Message != expected {
+	if expected := errcode.ErrorCodeUnauthorized.Message(); v2Err.Message != expected {
 		t.Fatalf("Unexpected message value: %q, expected %q", v2Err.Message, expected)
 	}
 }

registry/handlers/app.go

@@ -14,6 +14,7 @@ import (
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/configuration"
 	ctxu "github.com/docker/distribution/context"
+	"github.com/docker/distribution/health"
 	"github.com/docker/distribution/notifications"
 	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/distribution/registry/api/v2"
@@ -203,6 +204,20 @@ func NewApp(ctx context.Context, configuration configuration.Configuration) *App
 	return app
 }
 
+// RegisterHealthChecks is an awful hack to defer health check registration
+// control to callers. This should only ever be called once per registry
+// process, typically in a main function. The correct way would be register
+// health checks outside of app, since multiple apps may exist in the same
+// 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() {
+	health.RegisterPeriodicThresholdFunc("storagedriver_"+app.Config.Storage.Type(), 10*time.Second, 3, func() error {
+		_, err := app.driver.List(app, "/") // "/" should always exist
+		return err                          // any error will be treated as failure
+	})
+}
+
 // register a handler with the application, by route name. The handler will be
 // passed through the application filters and context will be constructed at
 // request time.
@@ -560,7 +575,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
 			// base route is accessed. This section prevents us from making
 			// that mistake elsewhere in the code, allowing any operation to
 			// proceed.
-			if err := errcode.ServeJSON(w, v2.ErrorCodeUnauthorized); err != nil {
+			if err := errcode.ServeJSON(w, errcode.ErrorCodeUnauthorized); err != nil {
 				ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors)
 			}
 			return fmt.Errorf("forbidden: no repository name")
@@ -575,7 +590,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
 			// Add the appropriate WWW-Auth header
 			err.SetHeaders(w)
 
-			if err := errcode.ServeJSON(w, v2.ErrorCodeUnauthorized.WithDetail(accessRecords)); err != nil {
+			if err := errcode.ServeJSON(w, errcode.ErrorCodeUnauthorized.WithDetail(accessRecords)); err != nil {
 				ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors)
 			}
 		default:

registry/handlers/app_test.go

@@ -205,8 +205,8 @@ func TestNewApp(t *testing.T) {
 	if !ok {
 		t.Fatalf("not an ErrorCoder: %#v", errs[0])
 	}
-	if err2.ErrorCode() != v2.ErrorCodeUnauthorized {
-		t.Fatalf("unexpected error code: %v != %v", err2.ErrorCode(), v2.ErrorCodeUnauthorized)
+	if err2.ErrorCode() != errcode.ErrorCodeUnauthorized {
+		t.Fatalf("unexpected error code: %v != %v", err2.ErrorCode(), errcode.ErrorCodeUnauthorized)
 	}
 }
 

registry/handlers/blob.go

@@ -81,7 +81,7 @@ func (bh *blobHandler) DeleteBlob(w http.ResponseWriter, r *http.Request) {
 			bh.Errors = append(bh.Errors, v2.ErrorCodeBlobUnknown)
 		case distribution.ErrUnsupported:
 			w.WriteHeader(http.StatusMethodNotAllowed)
-			bh.Errors = append(bh.Errors, v2.ErrorCodeUnsupported)
+			bh.Errors = append(bh.Errors, errcode.ErrorCodeUnsupported)
 		default:
 			bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown)
 		}

registry/handlers/images.go

@@ -213,7 +213,7 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h
 			w.WriteHeader(http.StatusNotFound)
 			return
 		case distribution.ErrUnsupported:
-			imh.Errors = append(imh.Errors, v2.ErrorCodeUnsupported)
+			imh.Errors = append(imh.Errors, errcode.ErrorCodeUnsupported)
 			w.WriteHeader(http.StatusMethodNotAllowed)
 		default:
 			imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown)

registry/proxy/proxymanifeststore.go

@@ -7,7 +7,7 @@ import (
 	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
-	"github.com/docker/distribution/registry/api/v2"
+	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/distribution/registry/client"
 	"github.com/docker/distribution/registry/proxy/scheduler"
 )
@@ -147,9 +147,9 @@ func manifestDigest(sm *manifest.SignedManifest) (digest.Digest, error) {
 }
 
 func (pms proxyManifestStore) Put(manifest *manifest.SignedManifest) error {
-	return v2.ErrorCodeUnsupported
+	return errcode.ErrorCodeUnsupported
 }
 
 func (pms proxyManifestStore) Delete(dgst digest.Digest) error {
-	return v2.ErrorCodeUnsupported
+	return errcode.ErrorCodeUnsupported
 }