From 3504f4c45fc044b3ffd3fc96f02a9f261ec87048 Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Sat, 10 Apr 2021 02:36:20 -0500 Subject: [PATCH] [yum] report upgraded multiarch packages (#73548) Change: - Previously when the same package name was installed twice under different architectures, we only reported it once in changes.updated. - This was the result of using a dict internally and keying on package name alone. - This change still keys on package name but turns the values into lists which can contain multiple packages per name. Test Plan: - Added a lot of tests around multi-arch support - Added some tests around virtual provides as well Tickets: - Fixes #73284 Signed-off-by: Rick Elrod --- changelogs/fragments/73284-yum-multiarch.yml | 2 + lib/ansible/modules/yum.py | 62 ++++++- .../filter_list_of_tuples_by_first_param.py | 25 +++ test/integration/targets/yum/tasks/main.yml | 9 +- .../targets/yum/tasks/multiarch.yml | 154 ++++++++++++++++++ test/integration/targets/yum/vars/main.yml | 1 + 6 files changed, 246 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/73284-yum-multiarch.yml create mode 100644 test/integration/targets/yum/filter_plugins/filter_list_of_tuples_by_first_param.py create mode 100644 test/integration/targets/yum/tasks/multiarch.yml create mode 100644 test/integration/targets/yum/vars/main.yml diff --git a/changelogs/fragments/73284-yum-multiarch.yml b/changelogs/fragments/73284-yum-multiarch.yml new file mode 100644 index 00000000000..bf6e8ef6411 --- /dev/null +++ b/changelogs/fragments/73284-yum-multiarch.yml @@ -0,0 +1,2 @@ +bugfixes: + - yum - When upgrading, every architecture of a package is now included in the module results, instead of just one (https://github.com/ansible/ansible/issues/73284). diff --git a/lib/ansible/modules/yum.py b/lib/ansible/modules/yum.py index d417394a9b0..7c5c2759021 100644 --- a/lib/ansible/modules/yum.py +++ b/lib/ansible/modules/yum.py @@ -1257,12 +1257,20 @@ class YumModule(YumDnf): pkg, version, repo = line[0], line[1], line[2] name, dist = pkg.rsplit('.', 1) - updates.update({name: {'version': version, 'dist': dist, 'repo': repo}}) + + if name not in updates: + updates[name] = [] + + updates[name].append({'version': version, 'dist': dist, 'repo': repo}) if len(line) == 6: obsolete_pkg, obsolete_version, obsolete_repo = line[3], line[4], line[5] obsolete_name, obsolete_dist = obsolete_pkg.rsplit('.', 1) - obsoletes.update({obsolete_name: {'version': obsolete_version, 'dist': obsolete_dist, 'repo': obsolete_repo}}) + + if obsolete_name not in obsoletes: + obsoletes[obsolete_name] = [] + + obsoletes[obsolete_name].append({'version': obsolete_version, 'dist': obsolete_dist, 'repo': obsolete_repo}) return updates, obsoletes @@ -1403,22 +1411,64 @@ class YumModule(YumDnf): to_update = [] for w in will_update: if w.startswith('@'): + # yum groups to_update.append((w, None)) elif w not in updates: + # There are (at least, probably more) 2 ways we can get here: + # + # * A virtual provides (our user specifies "webserver", but + # "httpd" is the key in 'updates'). + # + # * A wildcard. emac* will get us here if there's a package + # called 'emacs' in the pending updates list. 'updates' will + # of course key on 'emacs' in that case. + other_pkg = will_update_from_other_package[w] + + # We are guaranteed that: other_pkg in updates + # ...based on the logic above. But we only want to show one + # update in this case (given the wording of "at least") below. + # As an example, consider a package installed twice: + # foobar.x86_64, foobar.i686 + # We want to avoid having both: + # ('foo*', 'because of (at least) foobar-1.x86_64 from repo') + # ('foo*', 'because of (at least) foobar-1.i686 from repo') + # We just pick the first one. + # + # TODO: This is something that might be nice to change, but it + # would be a module UI change. But without it, we're + # dropping potentially important information about what + # was updated. Instead of (given_spec, random_matching_package) + # it'd be nice if we appended (given_spec, [all_matching_packages]) + # + # ... But then, we also drop information if multiple + # different (distinct) packages match the given spec and + # we should probably fix that too. + pkg = updates[other_pkg][0] to_update.append( ( w, 'because of (at least) %s-%s.%s from %s' % ( other_pkg, - updates[other_pkg]['version'], - updates[other_pkg]['dist'], - updates[other_pkg]['repo'] + pkg['version'], + pkg['dist'], + pkg['repo'] ) ) ) else: - to_update.append((w, '%s.%s from %s' % (updates[w]['version'], updates[w]['dist'], updates[w]['repo']))) + # Otherwise the spec is an exact match + for pkg in updates[w]: + to_update.append( + ( + w, + '%s.%s from %s' % ( + pkg['version'], + pkg['dist'], + pkg['repo'] + ) + ) + ) if self.update_only: res['changes'] = dict(installed=[], updated=to_update) diff --git a/test/integration/targets/yum/filter_plugins/filter_list_of_tuples_by_first_param.py b/test/integration/targets/yum/filter_plugins/filter_list_of_tuples_by_first_param.py new file mode 100644 index 00000000000..27f38ce5ce7 --- /dev/null +++ b/test/integration/targets/yum/filter_plugins/filter_list_of_tuples_by_first_param.py @@ -0,0 +1,25 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible.errors import AnsibleError, AnsibleFilterError + + +def filter_list_of_tuples_by_first_param(lst, search, startswith=False): + out = [] + for element in lst: + if startswith: + if element[0].startswith(search): + out.append(element) + else: + if search in element[0]: + out.append(element) + return out + + +class FilterModule(object): + ''' filter ''' + + def filters(self): + return { + 'filter_list_of_tuples_by_first_param': filter_list_of_tuples_by_first_param, + } diff --git a/test/integration/targets/yum/tasks/main.yml b/test/integration/targets/yum/tasks/main.yml index 3a7f4cf51da..6ea63fa9c6c 100644 --- a/test/integration/targets/yum/tasks/main.yml +++ b/test/integration/targets/yum/tasks/main.yml @@ -10,7 +10,7 @@ name: - sos state: absent - + - import_tasks: yum.yml always: - name: remove installed packages @@ -69,3 +69,10 @@ - import_tasks: lock.yml when: - ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux'] + +- import_tasks: multiarch.yml + when: + - ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux'] + - ansible_architecture == 'x86_64' + # Our output parsing expects us to be on yum, not dnf + - ansible_distribution_major_version is version('7', '<=') diff --git a/test/integration/targets/yum/tasks/multiarch.yml b/test/integration/targets/yum/tasks/multiarch.yml new file mode 100644 index 00000000000..bced634c4fa --- /dev/null +++ b/test/integration/targets/yum/tasks/multiarch.yml @@ -0,0 +1,154 @@ +- block: + - name: Set up test yum repo + yum_repository: + name: multiarch-test-repo + description: ansible-test multiarch test repo + baseurl: "{{ multiarch_repo_baseurl }}" + gpgcheck: no + repo_gpgcheck: no + + - name: Install two out of date packages from the repo + yum: + name: + - multiarch-a-1.0 + - multiarch-b-1.0 + register: outdated + + - name: See what we installed + command: rpm -q multiarch-a multiarch-b + register: rpm_q + + # Here we assume we're running on x86_64 (and limit to this in main.yml) + # (avoid comparing ansible_architecture because we only have test RPMs + # for i686 and x86_64 and ansible_architecture could be other things.) + - name: Assert that we got the right architecture + assert: + that: + - outdated is changed + - outdated.changes.installed | length == 2 + - rpm_q.stdout_lines | length == 2 + - rpm_q.stdout_lines[0].endswith('x86_64') + - rpm_q.stdout_lines[1].endswith('x86_64') + + - name: Install the same versions, but i686 instead + yum: + name: + - multiarch-a-1.0*.i686 + - multiarch-b-1.0*.i686 + register: outdated_i686 + + - name: See what we installed + command: rpm -q multiarch-a multiarch-b + register: rpm_q + + - name: Assert that all four are installed + assert: + that: + - outdated_i686 is changed + - outdated.changes.installed | length == 2 + - rpm_q.stdout_lines | length == 4 + + - name: Update them all to 2.0 + yum: + name: multiarch-* + state: latest + update_only: true + register: yum_latest + + - name: Assert that all were updated and shown in results + assert: + that: + - yum_latest is changed + # This is just testing UI stability. The behavior is arguably not + # correct, because multiple packages are being updated. But the + # "because of (at least)..." wording kinda locks us in to only + # showing one update in this case. :( + - yum_latest.changes.updated | length == 1 + + - name: Downgrade them so we can upgrade them a different way + yum: + name: + - multiarch-a-1.0* + - multiarch-b-1.0* + allow_downgrade: true + register: downgrade + + - name: See what we installed + command: rpm -q multiarch-a multiarch-b --queryformat '%{name}-%{version}.%{arch}\n' + register: rpm_q + + - name: Ensure downgrade worked + assert: + that: + - downgrade is changed + - rpm_q.stdout_lines | sort == ['multiarch-a-1.0.i686', 'multiarch-a-1.0.x86_64', 'multiarch-b-1.0.i686', 'multiarch-b-1.0.x86_64'] + + # This triggers a different branch of logic that the partial wildcard + # above, but we're limited to check_mode here since it's '*'. + - name: Upgrade with full wildcard + yum: + name: '*' + state: latest + update_only: true + update_cache: true + check_mode: true + register: full_wildcard + + # https://github.com/ansible/ansible/issues/73284 + - name: Ensure we report things correctly (both arches) + assert: + that: + - full_wildcard is changed + - full_wildcard.changes.updated | filter_list_of_tuples_by_first_param('multiarch', startswith=True) | length == 4 + + - name: Downgrade them so we can upgrade them a different way + yum: + name: + - multiarch-a-1.0* + - multiarch-b-1.0* + allow_downgrade: true + register: downgrade + + - name: Try to install again via virtual provides, should be unchanged + yum: + name: + - virtual-provides-multiarch-a + - virtual-provides-multiarch-b + state: present + register: install_vp + + - name: Ensure the above did not change + assert: + that: + - install_vp is not changed + + - name: Try to upgrade via virtual provides + yum: + name: + - virtual-provides-multiarch-a + - virtual-provides-multiarch-b + state: latest + update_only: true + register: upgrade_vp + + - name: Ensure we report things correctly (both arches) + assert: + that: + - upgrade_vp is changed + # This is just testing UI stability, like above. + # We'll only have one package in "updated" per spec, even though + # (in this case) two are getting updated per spec. + - upgrade_vp.changes.updated | length == 2 + + always: + - name: Remove test yum repo + yum_repository: + name: multiarch-test-repo + state: absent + + - name: Remove all test packages installed + yum: + name: + - multiarch-* + - virtual-provides-multiarch-* + state: absent diff --git a/test/integration/targets/yum/vars/main.yml b/test/integration/targets/yum/vars/main.yml new file mode 100644 index 00000000000..2be151325c7 --- /dev/null +++ b/test/integration/targets/yum/vars/main.yml @@ -0,0 +1 @@ +multiarch_repo_baseurl: https://ansible-ci-files.s3.amazonaws.com/test/integration/targets/yum/multiarch-test-repo/RPMS/