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

distribution: registry: do not access the errors slice if it's empty #21030

Merged
merged 1 commit into from
Mar 8, 2016
Merged

distribution: registry: do not access the errors slice if it's empty #21030

merged 1 commit into from
Mar 8, 2016

Conversation

runcom
Copy link
Member

@runcom runcom commented Mar 8, 2016

Fix #21027
Regression from 1.9 which just prompt for user/pass on 401
See the discussion in the issue for more information

Signed-off-by: Antonio Murdaca runcom@redhat.com

@runcom
Copy link
Member Author

runcom commented Mar 8, 2016

ping @tiborvass @thaJeztah @Soulou

@thaJeztah thaJeztah added this to the 1.10.3 milestone Mar 8, 2016
@runcom
distribution: registry: do not access the errors slice if it's empty
0186f4d 
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@tiborvass
Copy link
Contributor

LGTM

1 similar comment
@thaJeztah
Copy link
Member

LGTM

@thaJeztah
Copy link
Member

Thanks so much for the quick turnaround @runcom

@runcom
Copy link
Member Author

runcom commented Mar 8, 2016

tests pass but jobs always end with something like this:

15:41:39 Failed to remove container (docker-pr-userns7100): Error response from daemon: No such container: docker-pr-userns7100

@tiborvass
Copy link
Contributor

@runcom yes its a CI issue, i'm investigating.

@tiborvass
Copy link
Contributor

We don't care about experimental since this is a release.

@tiborvass tiborvass added the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 8, 2016
@tiborvass
Copy link
Contributor

Janky also passed, except for that stupid tar --include bug.
Gccgo is the only one that's actually failing...

@runcom
Copy link
Member Author

runcom commented Mar 8, 2016

gccgo tests don't fail: 16:25:58 OK: 1123 passed, 87 skipped

@tiborvass
Copy link
Contributor

@runcom you're right I can't read. ok then we're good.

@tiborvass tiborvass removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 8, 2016
tiborvass added a commit that referenced this pull request Mar 8, 2016
@tiborvass
Merge pull request #21030 from runcom/fix-daemon-panic
0fc4a0d 
distribution: registry: do not access the errors slice if it's empty
@tiborvass tiborvass merged commit 0fc4a0d into moby:bump_v1.10.3 Mar 8, 2016
@runcom runcom deleted the fix-daemon-panic branch March 8, 2016 16:45
@Soulou
Copy link
Contributor

Soulou commented Mar 8, 2016

Great, thanks @runcom for acting quickly!

@aaronlehmann
Copy link
Contributor

Kind of surprised this doesn't also affect earlier versions: https://github.com/docker/docker/blob/v1.9.1/registry/registry.go#L215

Anyway, nice work with the quick fix.

@runcom
Copy link
Member Author

runcom commented Mar 8, 2016

@aaronlehmann acutally in 1.9, if the registry reply with 401 it will prompt for user/pass (which is not happening anymore in 1.10.x see #19159)

@thaJeztah thaJeztah mentioned this pull request Mar 8, 2016
Closed
This was referenced Mar 10, 2016
Closed
Closed
@ekristen
Copy link
Contributor

Much appreciated! Thanks for the quick fix!

@tiborvass tiborvass mentioned this pull request Mar 15, 2016
Merged
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
status/2-code-review
Projects
None yet
Milestone
1.10.3
Development

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