Sync from a mirror list feed if provided

[lubosmj]

ref #6225

[pep8speaks]

Hello @lubosmj! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-06-19 22:25:18 UTC

[pulpbot]

Attached issue: https://pulp.plan.io/issues/6225

[goosemania]

@lubosmj , please, re-push (git commit --amend, git push -f ...), so the required check for your PR passes. We moved to travis-ci.com and your PR hasn't been tested there yet. Thanks.

[lubosmj]

I converted this to a draft PR because it is not possible to call docker exec anymore in order to replace the invalid fixtures.

[ipanova]

if i understand correctly how mirrolist works is - is looks for the first available mirror and tries it. If there is some issue with the repodata it just fails. So once the download starts from a url there is not way back.
There are some BZ opened around this behaviour and we agreed that amatej will inform us once they fix this( complicated change)
https://bugzilla.redhat.com/show_bug.cgi?id=1819188
https://bugzilla.redhat.com/show_bug.cgi?id=1819188
I think we should follow same logic as currently mirrorlist has implemented. Thoughts?

[bmbouter]

For the initial feature I think we should do the very simplest thing possible. I believe that would be:

1. Use the repodata from the very first available mirror that will serve it, and try to fetch all content from that mirror exclusively. If anything goes wrong in fetching content fail (just like Pulp normally would in cases where there is no mirrorlist).
2. Do not try to sort mirrors based on how fast or close they are
3. Do not try to fetch content from multiple mirrors

We can improve (2) and (3) later, but having even basic mirrorlist support is still a useful feature for users because we currently do not support it.

[ipanova]

After rpm meeting we decided that for now we would want to follow the current implementation of mirrorlist( aka fail if the the first fetched url fails during download)
In the future we could add this kind of enhancement where other urls would be tried if previous fail, but for that we'd rather implement a mirrorlist downloader to be more flexibile

[ipanova]

@lubosmj i don't think you handled case for treeinfo. It will always be ignored https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/kickstart/treeinfo.py#L21

[ipanova]

i don't think we should check whether the repodata for the url exists, i think we should just parse the file and add the urls to the list of feeds

[ipanova]

@bmbouter i don't have a strong opinion on this approach, thoughts?

[bmbouter]

I was thinking we would do no parsing and try to use the URL blind. If the resultant works it works, if it fails try the next one.

[ipanova]

there have been some discussion around what should be the behaviour and this is what we agreed on: 1) identify whether this is a mirrorlist 2) find url that matches the regexp and check whether it has repomd.xml, once such url has been found return it and use it in the the sync, no need to parse other urls 3) if further issues will appear during the sync with that url just fail the sync

[ipanova]

for mirror in mirror_list_file:
    match = re.match(url_pattern, mirror)
    if match and repodata_exists(remote, match.group(2)):
        mirror_list_feed.append(match.group(2))
        break

[ipanova]

you can also get rid of the list and return the url instead

[daviddavis]

I have a couple PRs that should set the base url in mirror lists:

pulp/pulp-fixtures#164
pulp/plugin_template#233

[daviddavis]

This should enable it: #1750

[ipanova]

Thank you for all your work 🍰