From 745892a7b271cf0f1770a8ec3698aaf61573e5f2 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Mon, 28 Mar 2016 17:16:56 -0700 Subject: [PATCH] Change v1 pull 404 message to include tag The current error message is "Error: image [name] not found". This makes sense from the perspective of the v1 pull, since we found the repository doesn't exist over the v1 protocol. However, in the vast majority of cases, this error will be produced by fallback situations, where we first try to pull the tag with the v2 protocol, and then fall back the v1 protocol, which probably isn't even supported by the server. Including the tag in the error message makes a lot more sense since the actual repository may exist on v2, but not the tag. Signed-off-by: Aaron Lehmann --- distribution/pull_v1.go | 6 +++- integration-cli/docker_cli_logout_test.go | 2 +- integration-cli/docker_cli_pull_test.go | 37 ++++++++++++++--------- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/distribution/pull_v1.go b/distribution/pull_v1.go index a8ae624915..192e321b7d 100644 --- a/distribution/pull_v1.go +++ b/distribution/pull_v1.go @@ -75,9 +75,14 @@ func (p *v1Puller) Pull(ctx context.Context, ref reference.Named) error { func (p *v1Puller) pullRepository(ctx context.Context, ref reference.Named) error { progress.Message(p.config.ProgressOutput, "", "Pulling repository "+p.repoInfo.FullName()) + tagged, isTagged := ref.(reference.NamedTagged) + repoData, err := p.session.GetRepositoryData(p.repoInfo) if err != nil { if strings.Contains(err.Error(), "HTTP code: 404") { + if isTagged { + return fmt.Errorf("Error: image %s:%s not found", p.repoInfo.RemoteName(), tagged.Tag()) + } return fmt.Errorf("Error: image %s not found", p.repoInfo.RemoteName()) } // Unexpected HTTP error @@ -86,7 +91,6 @@ func (p *v1Puller) pullRepository(ctx context.Context, ref reference.Named) erro logrus.Debugf("Retrieving the tag list") var tagsList map[string]string - tagged, isTagged := ref.(reference.NamedTagged) if !isTagged { tagsList, err = p.session.GetRemoteTags(repoData.Endpoints, p.repoInfo) } else { diff --git a/integration-cli/docker_cli_logout_test.go b/integration-cli/docker_cli_logout_test.go index 6658da54c2..52d4fff303 100644 --- a/integration-cli/docker_cli_logout_test.go +++ b/integration-cli/docker_cli_logout_test.go @@ -52,5 +52,5 @@ func (s *DockerRegistryAuthHtpasswdSuite) TestLogoutWithExternalAuth(c *check.C) // check I cannot pull anymore out, _, err := dockerCmdWithError("--config", tmp, "pull", repoName) c.Assert(err, check.NotNil, check.Commentf(out)) - c.Assert(out, checker.Contains, fmt.Sprintf("Error: image dockercli/busybox not found")) + c.Assert(out, checker.Contains, "Error: image dockercli/busybox:authtest not found") } diff --git a/integration-cli/docker_cli_pull_test.go b/integration-cli/docker_cli_pull_test.go index ac211d2519..c9f4ef195f 100644 --- a/integration-cli/docker_cli_pull_test.go +++ b/integration-cli/docker_cli_pull_test.go @@ -42,17 +42,18 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) { testRequires(c, DaemonIsLinux) type entry struct { - Repo string - Alias string + repo string + alias string + tag string } 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"}, + {"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"}, } // The option field indicates "-a" or not. @@ -71,15 +72,19 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) { group.Add(1) go func(e entry) { defer group.Done() - out, err := s.CmdWithError("pull", e.Alias) + repoName := e.alias + if e.tag != "" { + repoName += ":" + e.tag + } + out, err := s.CmdWithError("pull", repoName) recordChan <- record{e, "", out, err} }(e) - if !strings.ContainsRune(e.Alias, ':') { + if e.tag == "" { // pull -a on a nonexistent registry should fall back as well group.Add(1) go func(e entry) { defer group.Done() - out, err := s.CmdWithError("pull", "-a", e.Alias) + out, err := s.CmdWithError("pull", "-a", e.alias) recordChan <- record{e, "-a", out, err} }(e) } @@ -96,11 +101,15 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) { // 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. - c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s not found", record.e.Repo), check.Commentf("expected image not found error messages")) + 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")) } 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("Error: image %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"`)) } } @@ -261,5 +270,5 @@ func (s *DockerRegistryAuthHtpasswdSuite) TestPullNoCredentialsNotFound(c *check // gives a 404 (in this case the test registry doesn't handle v1 at all) out, _, err := dockerCmdWithError("pull", privateRegistryURL+"/busybox") c.Assert(err, check.NotNil, check.Commentf(out)) - c.Assert(out, checker.Contains, "Error: image busybox not found") + c.Assert(out, checker.Contains, "Error: image busybox:latest not found") }