Browse code
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 <aaron.lehmann@docker.com>
Aaron Lehmann authored on 2016/03/29 09:16:56Showing 3 changed files
- distribution/pull_v1.go
- integration-cli/docker_cli_logout_test.go
- integration-cli/docker_cli_pull_test.go
| ... | ... |
@@ -75,9 +75,14 @@ func (p *v1Puller) Pull(ctx context.Context, ref reference.Named) error {
|
| 75 | 75 |
func (p *v1Puller) pullRepository(ctx context.Context, ref reference.Named) error {
|
| 76 | 76 |
progress.Message(p.config.ProgressOutput, "", "Pulling repository "+p.repoInfo.FullName()) |
| 77 | 77 |
|
| 78 |
+ tagged, isTagged := ref.(reference.NamedTagged) |
|
| 79 |
+ |
|
| 78 | 80 |
repoData, err := p.session.GetRepositoryData(p.repoInfo) |
| 79 | 81 |
if err != nil {
|
| 80 | 82 |
if strings.Contains(err.Error(), "HTTP code: 404") {
|
| 83 |
+ if isTagged {
|
|
| 84 |
+ return fmt.Errorf("Error: image %s:%s not found", p.repoInfo.RemoteName(), tagged.Tag())
|
|
| 85 |
+ } |
|
| 81 | 86 |
return fmt.Errorf("Error: image %s not found", p.repoInfo.RemoteName())
|
| 82 | 87 |
} |
| 83 | 88 |
// Unexpected HTTP error |
| ... | ... |
@@ -86,7 +91,6 @@ func (p *v1Puller) pullRepository(ctx context.Context, ref reference.Named) erro |
| 86 | 86 |
|
| 87 | 87 |
logrus.Debugf("Retrieving the tag list")
|
| 88 | 88 |
var tagsList map[string]string |
| 89 |
- tagged, isTagged := ref.(reference.NamedTagged) |
|
| 90 | 89 |
if !isTagged {
|
| 91 | 90 |
tagsList, err = p.session.GetRemoteTags(repoData.Endpoints, p.repoInfo) |
| 92 | 91 |
} else {
|
| ... | ... |
@@ -52,5 +52,5 @@ func (s *DockerRegistryAuthHtpasswdSuite) TestLogoutWithExternalAuth(c *check.C) |
| 52 | 52 |
// check I cannot pull anymore |
| 53 | 53 |
out, _, err := dockerCmdWithError("--config", tmp, "pull", repoName)
|
| 54 | 54 |
c.Assert(err, check.NotNil, check.Commentf(out)) |
| 55 |
- c.Assert(out, checker.Contains, fmt.Sprintf("Error: image dockercli/busybox not found"))
|
|
| 55 |
+ c.Assert(out, checker.Contains, "Error: image dockercli/busybox:authtest not found") |
|
| 56 | 56 |
} |
| ... | ... |
@@ -42,17 +42,18 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
|
| 42 | 42 |
testRequires(c, DaemonIsLinux) |
| 43 | 43 |
|
| 44 | 44 |
type entry struct {
|
| 45 |
- Repo string |
|
| 46 |
- Alias string |
|
| 45 |
+ repo string |
|
| 46 |
+ alias string |
|
| 47 |
+ tag string |
|
| 47 | 48 |
} |
| 48 | 49 |
|
| 49 | 50 |
entries := []entry{
|
| 50 |
- {"library/asdfasdf", "asdfasdf:foobar"},
|
|
| 51 |
- {"library/asdfasdf", "library/asdfasdf:foobar"},
|
|
| 52 |
- {"library/asdfasdf", "asdfasdf"},
|
|
| 53 |
- {"library/asdfasdf", "asdfasdf:latest"},
|
|
| 54 |
- {"library/asdfasdf", "library/asdfasdf"},
|
|
| 55 |
- {"library/asdfasdf", "library/asdfasdf:latest"},
|
|
| 51 |
+ {"library/asdfasdf", "asdfasdf", "foobar"},
|
|
| 52 |
+ {"library/asdfasdf", "library/asdfasdf", "foobar"},
|
|
| 53 |
+ {"library/asdfasdf", "asdfasdf", ""},
|
|
| 54 |
+ {"library/asdfasdf", "asdfasdf", "latest"},
|
|
| 55 |
+ {"library/asdfasdf", "library/asdfasdf", ""},
|
|
| 56 |
+ {"library/asdfasdf", "library/asdfasdf", "latest"},
|
|
| 56 | 57 |
} |
| 57 | 58 |
|
| 58 | 59 |
// The option field indicates "-a" or not. |
| ... | ... |
@@ -71,15 +72,19 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
|
| 71 | 71 |
group.Add(1) |
| 72 | 72 |
go func(e entry) {
|
| 73 | 73 |
defer group.Done() |
| 74 |
- out, err := s.CmdWithError("pull", e.Alias)
|
|
| 74 |
+ repoName := e.alias |
|
| 75 |
+ if e.tag != "" {
|
|
| 76 |
+ repoName += ":" + e.tag |
|
| 77 |
+ } |
|
| 78 |
+ out, err := s.CmdWithError("pull", repoName)
|
|
| 75 | 79 |
recordChan <- record{e, "", out, err}
|
| 76 | 80 |
}(e) |
| 77 |
- if !strings.ContainsRune(e.Alias, ':') {
|
|
| 81 |
+ if e.tag == "" {
|
|
| 78 | 82 |
// pull -a on a nonexistent registry should fall back as well |
| 79 | 83 |
group.Add(1) |
| 80 | 84 |
go func(e entry) {
|
| 81 | 85 |
defer group.Done() |
| 82 |
- out, err := s.CmdWithError("pull", "-a", e.Alias)
|
|
| 86 |
+ out, err := s.CmdWithError("pull", "-a", e.alias)
|
|
| 83 | 87 |
recordChan <- record{e, "-a", out, err}
|
| 84 | 88 |
}(e) |
| 85 | 89 |
} |
| ... | ... |
@@ -96,11 +101,15 @@ func (s *DockerHubPullSuite) TestPullNonExistingImage(c *check.C) {
|
| 96 | 96 |
// Hub returns 401 rather than 404 for nonexistent repos over |
| 97 | 97 |
// the v2 protocol - but we should end up falling back to v1, |
| 98 | 98 |
// which does return a 404. |
| 99 |
- c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s not found", record.e.Repo), check.Commentf("expected image not found error messages"))
|
|
| 99 |
+ tag := record.e.tag |
|
| 100 |
+ if tag == "" {
|
|
| 101 |
+ tag = "latest" |
|
| 102 |
+ } |
|
| 103 |
+ 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"))
|
|
| 100 | 104 |
} else {
|
| 101 | 105 |
// pull -a on a nonexistent registry should fall back as well |
| 102 | 106 |
c.Assert(record.err, checker.NotNil, check.Commentf("expected non-zero exit status when pulling non-existing image: %s", record.out))
|
| 103 |
- c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s not found", record.e.Repo), check.Commentf("expected image not found error messages"))
|
|
| 107 |
+ c.Assert(record.out, checker.Contains, fmt.Sprintf("Error: image %s not found", record.e.repo), check.Commentf("expected image not found error messages"))
|
|
| 104 | 108 |
c.Assert(record.out, checker.Not(checker.Contains), "unauthorized", check.Commentf(`message should not contain "unauthorized"`)) |
| 105 | 109 |
} |
| 106 | 110 |
} |
| ... | ... |
@@ -261,5 +270,5 @@ func (s *DockerRegistryAuthHtpasswdSuite) TestPullNoCredentialsNotFound(c *check |
| 261 | 261 |
// gives a 404 (in this case the test registry doesn't handle v1 at all) |
| 262 | 262 |
out, _, err := dockerCmdWithError("pull", privateRegistryURL+"/busybox")
|
| 263 | 263 |
c.Assert(err, check.NotNil, check.Commentf(out)) |
| 264 |
- c.Assert(out, checker.Contains, "Error: image busybox not found") |
|
| 264 |
+ c.Assert(out, checker.Contains, "Error: image busybox:latest not found") |
|
| 265 | 265 |
} |