Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert reporting of multiple pull errors #19424

Merged
merged 1 commit into from
Jan 19, 2016
Merged

Revert reporting of multiple pull errors #19424

merged 1 commit into from
Jan 19, 2016

Conversation

aaronlehmann
Copy link
Contributor

Revert the portions of #17617 that report all errors when a pull
falls back, and go back to just reporting the last error. This was nice
to have, but causes some UX issues because nonexistent images show
additional "unauthorized" errors.

Keep the part of the PR that handled ENOSPC, as this appears to work
even without tracking multiple errors.

Fixes #19419

@tiborvass tiborvass added this to the 1.10.0 milestone Jan 18, 2016
@tiborvass
Copy link
Contributor

@aaronlehmann thanks, would you mind updating TestPullNonExistingImage to check for unauthorized ?

@runcom
Copy link
Member

runcom commented Jan 18, 2016

This was why we have that weird newline concatenated error in #19159 also I believe

@aaronlehmann aaronlehmann force-pushed the revert-multiple-pull-errors branch from dee5c75 to bdea5fb Compare January 18, 2016 22:41
@aaronlehmann
Copy link
Contributor Author

@tiborvass Added

@tiborvass
Copy link
Contributor

LGTM

1 similar comment
@runcom
Copy link
Member

runcom commented Jan 18, 2016

LGTM

@aaronlehmann
Revert reporting of multiple pull errors
87338bf 
Revert the portions of moby#17617 that report all errors when a pull
falls back, and go back to just reporting the last error. This was nice
to have, but causes some UX issues because nonexistent images show
additional "unauthorized" errors.

Keep the part of the PR that handled ENOSPC, as this appears to work
even without tracking multiple errors.

Fixes moby#19419

Signed-off-by: Aaron Lehmann <[email protected]>
@aaronlehmann aaronlehmann force-pushed the revert-multiple-pull-errors branch from bdea5fb to 87338bf Compare January 18, 2016 23:50
@aaronlehmann
Copy link
Contributor Author

Removed TestPullFallbackOn404, because it doesn't work with only the last error being displayed. It is effectively covered by TestPullNonExistingImage, which makes sure the v1 fallback happens.

@thaJeztah
Copy link
Member

ARM failed;

01:31:31  ---> Running in 5e005a2e3858
01:38:13 error: /v2/armhf/buildpack-deps/manifests/sha256:ca6cce8e5bf5c952129889b5cc15cd6aa8d995d77e55e3749bbaadae50e476cb returned something unexpected:
01:38:13   <html><body><h1>408 Request Time-out</h1>
01:38:13 Your browser didn't send a complete request in time.
01:38:13 </body></html>
01:38:14 Removing intermediate container 5e005a2e3858
01:38:14 The command '/bin/sh -c ./contrib/download-frozen-image-v2.sh /docker-frozen-images

tiborvass added a commit that referenced this pull request Jan 19, 2016
@tiborvass
Merge pull request #19424 from aaronlehmann/revert-multiple-pull-errors
e8ce350 
Revert reporting of multiple pull errors
@tiborvass tiborvass merged commit e8ce350 into moby:master Jan 19, 2016
@runcom runcom mentioned this pull request Jan 19, 2016
Closed
@runcom
Copy link
Member

runcom commented Jan 20, 2016

@aaronlehmann @tiborvass btw I'm now getting:

docker pull myregistrydomain.com:5000/busybox

Using default tag: latest
Error response from daemon: Get http://myregistrydomain.com:5000/v2/: malformed HTTP response "\x15\x03\x01\x00\x02\x02"

when I try to pull from a private registry w/o auth
before:

docker pull myregistrydomain.com:5000/busybox

Using default tag: latest
Error response from daemon: Get https://myregistrydomain.com:5000/v2/runcom/busybox/manifests/latest: no basic auth credentials
Get http://myregistrydomain.com:5000/v2/: malformed HTTP response "\x15\x03\x01\x00\x02\x02"

at least I was getting the correct error: no basic auth credentials
Do you know what's happening? because I think I should get unauthorized and not malformed http response right?

@thaJeztah
Copy link
Member

ping @aaronlehmann @tiborvass ^^

@aaronlehmann aaronlehmann deleted the revert-multiple-pull-errors branch January 21, 2016 00:18
@aaronlehmann
Copy link
Contributor Author

I looked into this today. You are probably using --insecure-registry. When the first attempt to pull from the registry (over HTTPS) fails, it tries using plain HTTP. Then the error you see is just the one from the plain HTTP attempt, because of this PR.

It would be a bit tricky to fix this. I think it may be better to discourage the use of --insecure-registry, which is a security risk anyway.

@aaronlehmann aaronlehmann mentioned this pull request Feb 12, 2016
Merged
@aaronlehmann aaronlehmann mentioned this pull request Mar 28, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.10.0
Development

Successfully merging this pull request may close these issues.
5 participants
@aaronlehmann @tiborvass @runcom @thaJeztah @GordonTheTurtle