Update pull error handling

Translate pull errors to provide a more consistent and user friendly
error message.

Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)

distribution/errors.go

@@ -5,11 +5,14 @@ import (
 	"strings"
 	"syscall"
 
+	"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/auth"
 	"github.com/docker/docker/distribution/xfer"
+	"github.com/docker/docker/reference"
+	"github.com/pkg/errors"
 )
 
 // ErrNoSupport is an error type used for errors indicating that an operation
@@ -56,6 +59,37 @@ func shouldV2Fallback(err errcode.Error) bool {
 	return false
 }
 
+func translatePullError(err error, ref reference.Named) error {
+	switch v := err.(type) {
+	case errcode.Errors:
+		if len(v) != 0 {
+			for _, extra := range v[1:] {
+				logrus.Infof("Ignoring extra error returned from registry: %v", extra)
+			}
+			return translatePullError(v[0], ref)
+		}
+	case errcode.Error:
+		var newErr error
+		switch v.Code {
+		case errcode.ErrorCodeDenied:
+			// ErrorCodeDenied is used when access to the repository was denied
+			newErr = errors.Errorf("repository %s not found: does not exist or no read access", ref.Name())
+		case v2.ErrorCodeManifestUnknown:
+			newErr = errors.Errorf("manifest for %s not found", ref.String())
+		case v2.ErrorCodeNameUnknown:
+			newErr = errors.Errorf("repository %s not found", ref.Name())
+		}
+		if newErr != nil {
+			logrus.Infof("Translating %q to %q", err, newErr)
+			return newErr
+		}
+	case xfer.DoNotRetry:
+		return translatePullError(v.Err, ref)
+	}
+
+	return err
+}
+
 // continueOnError returns true if we should fallback to the next endpoint
 // as a result of this error.
 func continueOnError(err error) bool {

distribution/pull.go

@@ -168,7 +168,7 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
 				continue
 			}
 			logrus.Errorf("Not continuing with pull after error: %v", err)
-			return err
+			return translatePullError(err, ref)
 		}
 
 		imagePullConfig.ImageEventLogger(ref.String(), repoInfo.Name(), "pull")
@@ -179,7 +179,7 @@ func Pull(ctx context.Context, ref reference.Named, imagePullConfig *ImagePullCo
 		lastErr = fmt.Errorf("no endpoints found for %s", ref.String())
 	}
 
-	return lastErr
+	return translatePullError(lastErr, ref)
 }
 
 // writeStatus writes a status message to out. If layersDownloaded is true, the

integration-cli/docker_cli_by_digest_test.go

@@ -114,7 +114,7 @@ func testPullByDigestNoFallback(c *check.C) {
 	imageReference := fmt.Sprintf("%s@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", repoName)
 	out, _, err := dockerCmdWithError("pull", imageReference)
 	c.Assert(err, checker.NotNil, check.Commentf("expected non-zero exit status and correct error message when pulling non-existing image"))
-	c.Assert(out, checker.Contains, "manifest unknown", check.Commentf("expected non-zero exit status and correct error message when pulling non-existing image"))
+	c.Assert(out, checker.Contains, fmt.Sprintf("manifest for %s not found", imageReference), check.Commentf("expected non-zero exit status and correct error message when pulling non-existing image"))
 }
 
 func (s *DockerRegistrySuite) TestPullByDigestNoFallback(c *check.C) {

integration-cli/docker_cli_pull_test.go

@@ -48,12 +48,12 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
 	}
 
 	entries := []entry{
-		{"library/asdfasdf", "asdfasdf", "foobar"},
-		{"library/asdfasdf", "library/asdfasdf", "foobar"},
-		{"library/asdfasdf", "asdfasdf", ""},
-		{"library/asdfasdf", "asdfasdf", "latest"},
-		{"library/asdfasdf", "library/asdfasdf", ""},
-		{"library/asdfasdf", "library/asdfasdf", "latest"},
+		{"asdfasdf", "asdfasdf", "foobar"},
+		{"asdfasdf", "library/asdfasdf", "foobar"},
+		{"asdfasdf", "asdfasdf", ""},
+		{"asdfasdf", "asdfasdf", "latest"},
+		{"asdfasdf", "library/asdfasdf", ""},
+		{"asdfasdf", "library/asdfasdf", "latest"},
 	}
 
 	// The option field indicates "-a" or not.
@@ -98,18 +98,11 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
 	for record := range recordChan {
 		if len(record.option) == 0 {
 			c.Assert(record.err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", record.out))
-			// Hub returns 401 rather than 404 for nonexistent repos over
-			// the v2 protocol - but we should end up falling back to v1,
-			// which does return a 404.
-			tag := record.e.tag
-			if tag == "" {
-				tag = "latest"
-			}
-			c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s:%s not found", record.e.repo, tag), check.Commentf("expected image not found error messages"))
+			c.Assert(record.out, checker.Contains, fmt.Sprintf("repository %s not found: does not exist or no read access", record.e.repo), check.Commentf("expected image not found error messages"))
 		} else {
 			// pull -a on a nonexistent registry should fall back as well
 			c.Assert(record.err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", record.out))
-			c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s not found", record.e.repo), check.Commentf("expected image not found error messages"))
+			c.Assert(record.out, checker.Contains, fmt.Sprintf("repository %s not found", record.e.repo), check.Commentf("expected image not found error messages"))
 			c.Assert(record.out, checker.Not(checker.Contains), "unauthorized", check.Commentf(`message should not contain "unauthorized"`))
 		}
 	}