[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 <rick@elrod.me>

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).

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'],
+                            pkg['version'],
-                            updates[other_pkg]['dist'],
+                            pkg['dist'],
-                            updates[other_pkg]['repo']
+                            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)

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,
+        }

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', '<=')

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

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/