[fuel-qa] Incorrect repos list processing which leads to unpredictiable results

[Sergey Kulanov]

We have incorrect logic (in replace_repos [1] module). While working with repos we modify list in `for` loop which is wrong:

        for repo in repos:
            if repo['name'] in ('mos-updates', 'mos-security'):
                repos.remove(repo)

Steps to reproduce:
run any test, smoke for example with:
PATCHING_DISABLE_UPDATES=True
MIRROR_UBUNTU=with_required_repos

not all repos will be removed, please see some of my tests, as you can see mos-security was not removed:

1)
2016-03-25 22:12:04,715 - DEBUG __init__.py:61 -- Done: add_ubuntu_mirrors with result: [{'name': 'ubuntu-0', 'section': 'main universe multiverse', 'uri': 'http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133', 'priority': 1001, 'suite': 'trusty', 'type': 'deb'}, {'name': 'ubuntu-1', 'section': 'main universe multiverse', 'uri': 'http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133', 'priority': 1001, 'suite': 'trusty-updates', 'type': 'deb'}, {'name': 'ubuntu-2', 'section': 'main universe multiverse', 'uri': 'http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133', 'priority': 1001, 'suite': 'trusty-security', 'type': 'deb'}, {'name': 'ubuntu-3', 'section': 'main universe multiverse', 'uri': 'http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133', 'priority': 1001, 'suite': 'trusty-proposed', 'type': 'deb'}]
2)
2016-03-25 22:12:04,715 - INFO replace_repos.py:33 -- repos_attr: 'ubuntu http://archive.ubuntu.com/ubuntu'
2016-03-25 22:12:04,715 - DEBUG replace_repos.py:39 -- Removing mirror: 'ubuntu http://archive.ubuntu.com/ubuntu'
2016-03-25 22:12:04,715 - INFO replace_repos.py:33 -- repos_attr: 'ubuntu-updates http://archive.ubuntu.com/ubuntu'
2016-03-25 22:12:04,715 - DEBUG replace_repos.py:39 -- Removing mirror: 'ubuntu-updates http://archive.ubuntu.com/ubuntu'
2016-03-25 22:12:04,715 - INFO replace_repos.py:33 -- repos_attr: 'ubuntu-security http://archive.ubuntu.com/ubuntu'
2016-03-25 22:12:04,716 - DEBUG replace_repos.py:39 -- Removing mirror: 'ubuntu-security http://archive.ubuntu.com/ubuntu'
2016-03-25 22:12:04,716 - INFO replace_repos.py:33 -- repos_attr: 'mos http://127.0.0.1:8080/ubuntu/x86_64'
2016-03-25 22:12:04,716 - INFO replace_repos.py:33 -- repos_attr: 'mos-updates http://mirror.fuel-infra.org/mos-repos/ubuntu/10.0'
2016-03-25 22:12:04,716 - INFO replace_repos.py:33 -- repos_attr: 'mos-security http://mirror.fuel-infra.org/mos-repos/ubuntu/10.0'
2016-03-25 22:12:04,716 - INFO replace_repos.py:33 -- repos_attr: 'mos-holdback http://mirror.fuel-infra.org/mos-repos/ubuntu/10.0'

3) Replacing:
2016-03-25 22:12:04,716 - INFO replace_repos.py:48 -- Repo: 'ubuntu-0 http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133'
2016-03-25 22:12:04,716 - INFO replace_repos.py:48 -- Repo: 'ubuntu-1 http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133'
2016-03-25 22:12:04,716 - INFO replace_repos.py:48 -- Repo: 'ubuntu-2 http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133'
2016-03-25 22:12:04,716 - INFO replace_repos.py:48 -- Repo: 'ubuntu-3 http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133'
2016-03-25 22:12:04,716 - INFO replace_repos.py:48 -- Repo: 'mos http://127.0.0.1:8080/ubuntu/x86_64'
2016-03-25 22:12:04,716 - INFO replace_repos.py:48 -- Repo: 'mos-updates http://mirror.fuel-infra.org/mos-repos/ubuntu/10.0'
2016-03-25 22:12:04,716 - INFO replace_repos.py:51 -- Removing MOS mirror: 'mos-updates http://mirror.fuel-infra.org/mos-repos/ubuntu/10.0'
2016-03-25 22:12:04,716 - INFO replace_repos.py:48 -- Repo: 'mos-holdback http://mirror.fuel-infra.org/mos-repos/ubuntu/10.0'
2016-03-25 22:12:04,716 - INFO replace_repos.py:51 -- Removing MOS mirror: 'mos-holdback http://mirror.fuel-infra.org/mos-repos/ubuntu/10.0'

4) result:
2016-03-25 22:12:04,717 - DEBUG __init__.py:61 -- Done: replace_ubuntu_repos with result: [{'name': 'ubuntu-0', 'section': 'main universe multiverse', 'uri': 'http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133', 'priority': 1001, 'suite': 'trusty', 'type': 'deb'}, {'name': 'ubuntu-1', 'section': 'main universe multiverse', 'uri': 'http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133', 'priority': 1001, 'suite': 'trusty-updates', 'type': 'deb'}, {'name': 'ubuntu-2', 'section': 'main universe multiverse', 'uri': 'http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133', 'priority': 1001, 'suite': 'trusty-security', 'type': 'deb'}, {'name': 'ubuntu-3', 'section': 'main universe multiverse', 'uri': 'http://mirror.seed-cz1.fuel-infra.org/pkgs/ubuntu-2016-03-25-170133', 'priority': 1001, 'suite': 'trusty-proposed', 'type': 'deb'}, {'priority': 1050, 'name': 'mos', 'suite': 'mos10.0', 'section': 'main restricted', 'type': 'deb', 'uri': 'http://127.0.0.1:8080/ubuntu/x86_64'}, {'priority': 1050, 'name': 'mos-security', 'suite': 'mos10.0-security', 'section': 'main restricted', 'type': 'deb', 'uri': 'http://mirror.fuel-infra.org/mos-repos/ubuntu/10.0'}]

Expected results:
All repos should be removed mos-updates and mos-security

How to fix:
If you need to modify the sequence you are iterating over while inside the loop (for example to duplicate selected items), it is recommended that you first make a copy. Iterating over a sequence does not implicitly make a copy.

Set to High since can lead to unpredictable results in repo-lists

[1]. https://github.com/openstack/fuel-qa/blob/master/fuelweb_test/helpers/replace_repos.py

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/298042

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/298042
Committed: https://git.openstack.org/cgit/openstack/fuel-qa/commit/?id=3990d8a34f91c1afae5d72a5a3c4d487d846220f
Submitter: Jenkins
Branch: master

commit 3990d8a34f91c1afae5d72a5a3c4d487d846220f
Author: Sergey Kulanov <email address hidden>
Date: Sun Mar 27 17:32:07 2016 +0300

    Fix repos list processing flow

      We must not modify lists in `for` loops because one builds a
    completely new list and then gives it the same name the old list
    has [1]

    [1]. https://docs.python.org/2/reference/compound_stmts.html#for

    Change-Id: Id77ed7814701d4cac83c4c9d72c667a839ed05f9
    Closes-bug: #1562530

[Miroslav Anashkin (manashkin)]

This bug is critical for all 8.0 users.
Correct mos-holdback processing is mandatory part for the following released technical bulletin:
https://content.mirantis.com/rs/451-RBY-185/images/Mirantis-Technical-Bulletin-33-systemd.pdf

Please build updated fuel-mirror package for 8.0.

[Miroslav Anashkin (manashkin)]

Sorry, not this bug.