diff --git a/cmd/registry-api-descriptor-template/main.go b/cmd/registry-api-descriptor-template/main.go index 05a1b487..61eab8ec 100644 --- a/cmd/registry-api-descriptor-template/main.go +++ b/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 { diff --git a/cmd/registry/main.go b/cmd/registry/main.go index 9196d316..2832c0d7 100644 --- a/cmd/registry/main.go +++ b/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 != "" { diff --git a/docs/spec/api.md b/docs/spec/api.md index 9b56b6c5..40e21e41 100644 --- a/docs/spec/api.md +++ b/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.| diff --git a/health/health.go b/health/health.go index 8a4df776..ba954919 100644 --- a/health/health.go +++ b/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) } } diff --git a/health/health_test.go b/health/health_test.go index 7989f0b2..3228cb80 100644 --- a/health/health_test.go +++ b/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. +} diff --git a/registry/api/errcode/register.go b/registry/api/errcode/register.go index 42f911b3..e1c93f38 100644 --- a/registry/api/errcode/register.go +++ b/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 diff --git a/registry/api/v2/descriptors.go b/registry/api/v2/descriptors.go index 0ef64f88..09289b96 100644 --- a/registry/api/v2/descriptors.go +++ b/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, }, }, }, diff --git a/registry/api/v2/errors.go b/registry/api/v2/errors.go index 87e27f2e..ece52a2c 100644 --- a/registry/api/v2/errors.go +++ b/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{ diff --git a/registry/client/errors.go b/registry/client/errors.go index ebd1c36c..7305c021 100644 --- a/registry/client/errors.go +++ b/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 } diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index 26201763..8a7a598e 100644 --- a/registry/client/repository_test.go +++ b/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) } } diff --git a/registry/handlers/app.go b/registry/handlers/app.go index f60290d0..11d91120 100644 --- a/registry/handlers/app.go +++ b/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: diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go index 6f597527..3ef2342c 100644 --- a/registry/handlers/app_test.go +++ b/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) } } diff --git a/registry/handlers/blob.go b/registry/handlers/blob.go index b7c06ea2..fd514ec0 100644 --- a/registry/handlers/blob.go +++ b/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) } diff --git a/registry/handlers/images.go b/registry/handlers/images.go index dbe7b706..f5354399 100644 --- a/registry/handlers/images.go +++ b/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) diff --git a/registry/proxy/proxymanifeststore.go b/registry/proxy/proxymanifeststore.go index 5b79c8ce..8921998a 100644 --- a/registry/proxy/proxymanifeststore.go +++ b/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 }