From 7d32129efb0cad14710e35a9e3f2251a2957fbb2 Mon Sep 17 00:00:00 2001
From: Rick Elrod <rick@elrod.me>
Date: Tue, 28 Jul 2020 10:23:55 -0500
Subject: [PATCH] [dnf] show installations/removals in check_mode (#70892)

Change:
- Previously, we only showed that something would have changed, not what
  would have changed. This allows us to show what will chang as well.

Test Plan:
- Local RHEL8 VM
- New integration tests

Tickets:
- Fixes #66132

Signed-off-by: Rick Elrod <rick@elrod.me>
---
 .../66132_dnf_show_pkgs_in_check_mode.yml     |  2 +
 lib/ansible/modules/dnf.py                    | 20 +++++--
 test/integration/targets/dnf/tasks/dnf.yml    | 60 +++++++++++++++++++
 3 files changed, 76 insertions(+), 6 deletions(-)
 create mode 100644 changelogs/fragments/66132_dnf_show_pkgs_in_check_mode.yml

diff --git a/changelogs/fragments/66132_dnf_show_pkgs_in_check_mode.yml b/changelogs/fragments/66132_dnf_show_pkgs_in_check_mode.yml
new file mode 100644
index 00000000000..7ec57b78bf1
--- /dev/null
+++ b/changelogs/fragments/66132_dnf_show_pkgs_in_check_mode.yml
@@ -0,0 +1,2 @@
+minor_changes:
+  - dnf - now shows specific package changes (installations/removals) under ``results`` in check_mode. (https://github.com/ansible/ansible/issues/66132)
diff --git a/lib/ansible/modules/dnf.py b/lib/ansible/modules/dnf.py
index 67f3f28e548..ab32efbcd11 100644
--- a/lib/ansible/modules/dnf.py
+++ b/lib/ansible/modules/dnf.py
@@ -1158,6 +1158,18 @@ class DnfModule(YumDnf):
                 self.module.exit_json(**response)
             else:
                 response['changed'] = True
+
+                # If packages got installed/removed, add them to the results.
+                # We do this early so we can use it for both check_mode and not.
+                if self.download_only:
+                    install_action = 'Downloaded'
+                else:
+                    install_action = 'Installed'
+                for package in self.base.transaction.install_set:
+                    response['results'].append("{0}: {1}".format(install_action, package))
+                for package in self.base.transaction.remove_set:
+                    response['results'].append("Removed: {0}".format(package))
+
                 if failure_response['failures']:
                     failure_response['msg'] = 'Failed to install some of the specified packages'
                     self.module.fail_json(**failure_response)
@@ -1178,15 +1190,11 @@ class DnfModule(YumDnf):
                     )
 
                 if self.download_only:
-                    for package in self.base.transaction.install_set:
-                        response['results'].append("Downloaded: {0}".format(package))
+                    # No further work left to do, and the results were already updated above.
+                    # Just return them.
                     self.module.exit_json(**response)
                 else:
                     self.base.do_transaction()
-                    for package in self.base.transaction.install_set:
-                        response['results'].append("Installed: {0}".format(package))
-                    for package in self.base.transaction.remove_set:
-                        response['results'].append("Removed: {0}".format(package))
 
                 if failure_response['failures']:
                     failure_response['msg'] = 'Failed to install some of the specified packages'
diff --git a/test/integration/targets/dnf/tasks/dnf.yml b/test/integration/targets/dnf/tasks/dnf.yml
index 3688e7d085a..1d0e871e8cf 100644
--- a/test/integration/targets/dnf/tasks/dnf.yml
+++ b/test/integration/targets/dnf/tasks/dnf.yml
@@ -48,6 +48,20 @@
         - "not dnf_result.changed"
 
 # INSTALL
+- name: install sos (check_mode)
+  dnf:
+    name: sos
+    state: present
+    update_cache: True
+  check_mode: True
+  register: dnf_result
+
+- assert:
+    that:
+      - dnf_result is success
+      - dnf_result.results|length > 0
+      - "dnf_result.results[0].startswith('Installed: ')"
+
 - name: install sos
   dnf:
     name: sos
@@ -74,6 +88,18 @@
         - "'results' in dnf_result"
 
 # INSTALL AGAIN
+- name: install sos again (check_mode)
+  dnf:
+    name: sos
+    state: present
+  check_mode: True
+  register: dnf_result
+
+- assert:
+    that:
+      - dnf_result is not changed
+      - dnf_result.results|length == 0
+
 - name: install sos again
   dnf:
     name: sos
@@ -188,12 +214,33 @@
         - "rpm_sos_result.rc == 0"
         - "rpm_pciutils_result.rc == 0"
 
+- name: uninstall sos and pciutils (check_mode)
+  dnf:
+    name:
+      - sos
+      - pciutils
+    state: removed
+  check_mode: True
+  register: dnf_result
+
+- assert:
+    that:
+      - dnf_result is success
+      - dnf_result.results|length == 2
+      - "dnf_result.results[0].startswith('Removed: ')"
+      - "dnf_result.results[1].startswith('Removed: ')"
+
 - name: uninstall sos and pciutils
   dnf:
     name:
       - sos
       - pciutils
     state: removed
+  register: dnf_result
+
+- assert:
+    that:
+      - dnf_result is changed
 
 - name: install non-existent rpm
   dnf:
@@ -238,6 +285,19 @@
     name: sos
     state: absent
 
+- name: Test download_only (check_mode)
+  dnf:
+    name: sos
+    state: latest
+    download_only: true
+  check_mode: true
+  register: dnf_result
+
+- assert:
+    that:
+      - dnf_result is success
+      - "dnf_result.results[0].startswith('Downloaded: ')"
+
 - name: Test download_only
   dnf:
     name: sos