From 8f26fe4f59ce515c68440da1443ace4c96e89d4a Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 11 Feb 2016 14:08:49 -0800 Subject: [PATCH] Push/pull errors improvement and cleanup Several improvements to error handling: - Introduce ImageConfigPullError type, wrapping errors related to downloading the image configuration blob in schema2. This allows for a more descriptive error message to be seen by the end user. - Change some logrus.Debugf calls that display errors to logrus.Errorf. Add log lines in the push/pull fallback cases to make sure the errors leading to the fallback are shown. - Move error-related types and functions which are only used by the distribution package out of the registry package. Signed-off-by: Aaron Lehmann --- distribution/errors.go | 102 +++++++++++++++++++++++++++++++++++++++ distribution/pull.go | 5 +- distribution/pull_v1.go | 2 +- distribution/pull_v2.go | 27 ++++++++--- distribution/push.go | 3 +- distribution/push_v2.go | 2 +- distribution/registry.go | 46 ------------------ registry/registry.go | 49 ------------------- 8 files changed, 128 insertions(+), 108 deletions(-) create mode 100644 distribution/errors.go diff --git a/distribution/errors.go b/distribution/errors.go new file mode 100644 index 0000000000..9f9dcf6978 --- /dev/null +++ b/distribution/errors.go @@ -0,0 +1,102 @@ +package distribution + +import ( + "net/url" + "strings" + "syscall" + + "github.com/docker/distribution/registry/api/errcode" + "github.com/docker/distribution/registry/api/v2" + "github.com/docker/distribution/registry/client" + "github.com/docker/docker/distribution/xfer" +) + +// ErrNoSupport is an error type used for errors indicating that an operation +// is not supported. It encapsulates a more specific error. +type ErrNoSupport struct{ Err error } + +func (e ErrNoSupport) Error() string { + if e.Err == nil { + return "not supported" + } + return e.Err.Error() +} + +// fallbackError wraps an error that can possibly allow fallback to a different +// endpoint. +type fallbackError struct { + // err is the error being wrapped. + err error + // confirmedV2 is set to true if it was confirmed that the registry + // supports the v2 protocol. This is used to limit fallbacks to the v1 + // protocol. + confirmedV2 bool +} + +// Error renders the FallbackError as a string. +func (f fallbackError) Error() string { + return f.err.Error() +} + +// shouldV2Fallback returns true if this error is a reason to fall back to v1. +func shouldV2Fallback(err errcode.Error) bool { + switch err.Code { + case errcode.ErrorCodeUnauthorized, v2.ErrorCodeManifestUnknown, v2.ErrorCodeNameUnknown: + return true + } + return false +} + +// continueOnError returns true if we should fallback to the next endpoint +// as a result of this error. +func continueOnError(err error) bool { + switch v := err.(type) { + case errcode.Errors: + if len(v) == 0 { + return true + } + return continueOnError(v[0]) + case ErrNoSupport: + return continueOnError(v.Err) + case errcode.Error: + return shouldV2Fallback(v) + case *client.UnexpectedHTTPResponseError: + return true + case ImageConfigPullError: + return false + case error: + return !strings.Contains(err.Error(), strings.ToLower(syscall.ENOSPC.Error())) + } + // let's be nice and fallback if the error is a completely + // unexpected one. + // If new errors have to be handled in some way, please + // add them to the switch above. + return true +} + +// retryOnError wraps the error in xfer.DoNotRetry if we should not retry the +// operation after this error. +func retryOnError(err error) error { + switch v := err.(type) { + case errcode.Errors: + return retryOnError(v[0]) + case errcode.Error: + switch v.Code { + case errcode.ErrorCodeUnauthorized, errcode.ErrorCodeUnsupported, errcode.ErrorCodeDenied: + return xfer.DoNotRetry{Err: err} + } + case *url.Error: + return retryOnError(v.Err) + case *client.UnexpectedHTTPResponseError: + return xfer.DoNotRetry{Err: err} + case error: + if strings.Contains(err.Error(), strings.ToLower(syscall.ENOSPC.Error())) { + return xfer.DoNotRetry{Err: err} + } + } + // let's be nice and fallback if the error is a completely + // unexpected one. + // If new errors have to be handled in some way, please + // add them to the switch above. + return err +} diff --git a/distribution/pull.go b/distribution/pull.go index 49c0d3b660..debe378d51 100644 --- a/distribution/pull.go +++ b/distribution/pull.go @@ -136,7 +136,7 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo } } if fallback { - if _, ok := err.(registry.ErrNoSupport); !ok { + if _, ok := err.(ErrNoSupport); !ok { // Because we found an error that's not ErrNoSupport, discard all subsequent ErrNoSupport errors. discardNoSupportErrors = true // append subsequent errors @@ -147,9 +147,10 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo // append subsequent errors lastErr = err } + logrus.Errorf("Attempting next endpoint for pull after error: %v", err) continue } - logrus.Debugf("Not continuing with error: %v", err) + logrus.Errorf("Not continuing with pull after error: %v", err) return err } diff --git a/distribution/pull_v1.go b/distribution/pull_v1.go index a7080df697..3e0cbdb46c 100644 --- a/distribution/pull_v1.go +++ b/distribution/pull_v1.go @@ -38,7 +38,7 @@ type v1Puller struct { func (p *v1Puller) Pull(ctx context.Context, ref reference.Named) error { if _, isCanonical := ref.(reference.Canonical); isCanonical { // Allowing fallback, because HTTPS v1 is before HTTP v2 - return fallbackError{err: registry.ErrNoSupport{Err: errors.New("Cannot pull by digest with v1 registry")}} + return fallbackError{err: ErrNoSupport{Err: errors.New("Cannot pull by digest with v1 registry")}} } tlsConfig, err := p.config.RegistryService.TLSConfig(p.repoInfo.Index.Name) diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index 00cf7a5f41..3d315ca413 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -35,6 +35,17 @@ import ( var errRootFSMismatch = errors.New("layers from manifest don't match image configuration") +// ImageConfigPullError is an error pulling the image config blob +// (only applies to schema2). +type ImageConfigPullError struct { + Err error +} + +// Error returns the error string for ImageConfigPullError. +func (e ImageConfigPullError) Error() string { + return "error pulling image configuration: " + e.Err.Error() +} + type v2Puller struct { V2MetadataService *metadata.V2MetadataService endpoint registry.APIEndpoint @@ -58,8 +69,8 @@ func (p *v2Puller) Pull(ctx context.Context, ref reference.Named) (err error) { if _, ok := err.(fallbackError); ok { return err } - if registry.ContinueOnError(err) { - logrus.Debugf("Error trying v2 registry: %v", err) + if continueOnError(err) { + logrus.Errorf("Error trying v2 registry: %v", err) return fallbackError{err: err, confirmedV2: p.confirmedV2} } } @@ -170,7 +181,7 @@ func (ld *v2LayerDescriptor) Download(ctx context.Context, progressOutput progre layerDownload, err := blobs.Open(ctx, ld.digest) if err != nil { - logrus.Debugf("Error initiating layer download: %v", err) + logrus.Errorf("Error initiating layer download: %v", err) if err == distribution.ErrBlobUnknown { return nil, 0, xfer.DoNotRetry{Err: err} } @@ -280,12 +291,12 @@ func (ld *v2LayerDescriptor) truncateDownloadFile() error { ld.verifier = nil if _, err := ld.tmpFile.Seek(0, os.SEEK_SET); err != nil { - logrus.Debugf("error seeking to beginning of download file: %v", err) + logrus.Errorf("error seeking to beginning of download file: %v", err) return err } if err := ld.tmpFile.Truncate(0); err != nil { - logrus.Debugf("error truncating download file: %v", err) + logrus.Errorf("error truncating download file: %v", err) return err } @@ -484,7 +495,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s go func() { configJSON, err := p.pullSchema2ImageConfig(ctx, target.Digest) if err != nil { - errChan <- err + errChan <- ImageConfigPullError{Err: err} cancel() return } @@ -704,12 +715,12 @@ func allowV1Fallback(err error) error { switch v := err.(type) { case errcode.Errors: if len(v) != 0 { - if v0, ok := v[0].(errcode.Error); ok && registry.ShouldV2Fallback(v0) { + if v0, ok := v[0].(errcode.Error); ok && shouldV2Fallback(v0) { return fallbackError{err: err, confirmedV2: false} } } case errcode.Error: - if registry.ShouldV2Fallback(v) { + if shouldV2Fallback(v) { return fallbackError{err: err, confirmedV2: false} } case *url.Error: diff --git a/distribution/push.go b/distribution/push.go index d0622f82c9..c25f545ce0 100644 --- a/distribution/push.go +++ b/distribution/push.go @@ -144,11 +144,12 @@ func Push(ctx context.Context, ref reference.Named, imagePushConfig *ImagePushCo confirmedV2 = confirmedV2 || fallbackErr.confirmedV2 err = fallbackErr.err lastErr = err + logrus.Errorf("Attempting next endpoint for push after error: %v", err) continue } } - logrus.Debugf("Not continuing with error: %v", err) + logrus.Errorf("Not continuing with push after error: %v", err) return err } diff --git a/distribution/push_v2.go b/distribution/push_v2.go index ac2017a8d0..b4805e7807 100644 --- a/distribution/push_v2.go +++ b/distribution/push_v2.go @@ -68,7 +68,7 @@ func (p *v2Pusher) Push(ctx context.Context) (err error) { } if err = p.pushV2Repository(ctx); err != nil { - if registry.ContinueOnError(err) { + if continueOnError(err) { return fallbackError{err: err, confirmedV2: p.pushState.confirmedV2} } } diff --git a/distribution/registry.go b/distribution/registry.go index 4a8988f13f..3b50bb2751 100644 --- a/distribution/registry.go +++ b/distribution/registry.go @@ -6,38 +6,19 @@ import ( "net/http" "net/url" "strings" - "syscall" "time" "github.com/docker/distribution" distreference "github.com/docker/distribution/reference" - "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/auth" "github.com/docker/distribution/registry/client/transport" - "github.com/docker/docker/distribution/xfer" "github.com/docker/docker/dockerversion" "github.com/docker/docker/registry" "github.com/docker/engine-api/types" "golang.org/x/net/context" ) -// fallbackError wraps an error that can possibly allow fallback to a different -// endpoint. -type fallbackError struct { - // err is the error being wrapped. - err error - // confirmedV2 is set to true if it was confirmed that the registry - // supports the v2 protocol. This is used to limit fallbacks to the v1 - // protocol. - confirmedV2 bool -} - -// Error renders the FallbackError as a string. -func (f fallbackError) Error() string { - return f.err.Error() -} - type dumbCredentialStore struct { auth *types.AuthConfig } @@ -141,30 +122,3 @@ func (th *existingTokenHandler) AuthorizeRequest(req *http.Request, params map[s req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", th.token)) return nil } - -// retryOnError wraps the error in xfer.DoNotRetry if we should not retry the -// operation after this error. -func retryOnError(err error) error { - switch v := err.(type) { - case errcode.Errors: - return retryOnError(v[0]) - case errcode.Error: - switch v.Code { - case errcode.ErrorCodeUnauthorized, errcode.ErrorCodeUnsupported, errcode.ErrorCodeDenied: - return xfer.DoNotRetry{Err: err} - } - case *url.Error: - return retryOnError(v.Err) - case *client.UnexpectedHTTPResponseError: - return xfer.DoNotRetry{Err: err} - case error: - if strings.Contains(err.Error(), strings.ToLower(syscall.ENOSPC.Error())) { - return xfer.DoNotRetry{Err: err} - } - } - // let's be nice and fallback if the error is a completely - // unexpected one. - // If new errors have to be handled in some way, please - // add them to the switch above. - return err -} diff --git a/registry/registry.go b/registry/registry.go index 6214d41af3..9071d9dc14 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -13,13 +13,9 @@ import ( "path/filepath" "runtime" "strings" - "syscall" "time" "github.com/Sirupsen/logrus" - "github.com/docker/distribution/registry/api/errcode" - "github.com/docker/distribution/registry/api/v2" - "github.com/docker/distribution/registry/client" "github.com/docker/distribution/registry/client/transport" "github.com/docker/go-connections/tlsconfig" ) @@ -169,51 +165,6 @@ func addRequiredHeadersToRedirectedRequests(req *http.Request, via []*http.Reque return nil } -// ShouldV2Fallback returns true if this error is a reason to fall back to v1. -func ShouldV2Fallback(err errcode.Error) bool { - switch err.Code { - case errcode.ErrorCodeUnauthorized, v2.ErrorCodeManifestUnknown, v2.ErrorCodeNameUnknown: - return true - } - return false -} - -// ErrNoSupport is an error type used for errors indicating that an operation -// is not supported. It encapsulates a more specific error. -type ErrNoSupport struct{ Err error } - -func (e ErrNoSupport) Error() string { - if e.Err == nil { - return "not supported" - } - return e.Err.Error() -} - -// ContinueOnError returns true if we should fallback to the next endpoint -// as a result of this error. -func ContinueOnError(err error) bool { - switch v := err.(type) { - case errcode.Errors: - if len(v) == 0 { - return true - } - return ContinueOnError(v[0]) - case ErrNoSupport: - return ContinueOnError(v.Err) - case errcode.Error: - return ShouldV2Fallback(v) - case *client.UnexpectedHTTPResponseError: - return true - case error: - return !strings.Contains(err.Error(), strings.ToLower(syscall.ENOSPC.Error())) - } - // let's be nice and fallback if the error is a completely - // unexpected one. - // If new errors have to be handled in some way, please - // add them to the switch above. - return true -} - // NewTransport returns a new HTTP transport. If tlsConfig is nil, it uses the // default TLS configuration. func NewTransport(tlsConfig *tls.Config) *http.Transport {