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.

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

Fix search order returned by sysregistriesv2 for v1 config files #520

Merged
merged 4 commits into from Oct 17, 2018

Conversation

Projects
None yet
4 participants
@mtrmac
Contributor

mtrmac commented Oct 13, 2018

While working on containers/buildah#909 I have noticed that that the order of entries returned by FindUnqualifiedSearchRegistries is randomized; this should fix it.

See individual commit messages for details.

Cc @vrothberg

@vrothberg

This comment has been minimized.

Member

vrothberg commented Oct 13, 2018

LGTM, thanks a lot for fixing that!

@runcom

This comment has been minimized.

Member

runcom commented Oct 13, 2018

LGTM

Approved with PullApprove

@runcom

This comment has been minimized.

Member

runcom commented Oct 13, 2018

Travis failure is going to be fixed by #521

@runcom

This comment has been minimized.

Member

runcom commented Oct 13, 2018

please rebase after that's merged and merge this one as well, thanks!

mtrmac added some commits Oct 13, 2018

Test that reading v2 config preserves the registry search order
(AFAICS the code works correctly, but the existing code tests
only presence, not order.)

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Remove an obsolete comment
Commit 313b5ff has removed the scheme
support and changed the test line below the comment not to have a scheme.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Add an assertSearchRegistryURLsEqual helper
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Fix V1 compatibility code to preserve search registry order
... and also to make the rest of the order consistent as well,
to minimize difficult-to-reproduce heisenbugs.

The Golang map iterating code is somewhat random, so the old code
_sometimes_ passes the updaed test; the failures are very
obvious in
> go test -count 1000 ./pkg/sysregistriesv2/
though.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac

This comment has been minimized.

Contributor

mtrmac commented Oct 17, 2018

👍

Approved with PullApprove

@mtrmac mtrmac merged commit bd10b1b into containers:master Oct 17, 2018

2 checks passed

code-review/pullapprove Approved by mtrmac, runcom
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mtrmac mtrmac deleted the mtrmac:search-order branch Oct 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment