From 1c4e491eac3228bad51d3710bbed4d1ebdc15764 Mon Sep 17 00:00:00 2001
From: Martin Krizek <martin.krizek@gmail.com>
Date: Thu, 10 Aug 2017 19:57:08 +0200
Subject: [PATCH] yum: do not hide yum's errors (#27696)

---
 lib/ansible/modules/packaging/os/yum.py    |  23 ++--
 test/integration/targets/yum/tasks/yum.yml | 117 ++++++++++++++++++---
 2 files changed, 110 insertions(+), 30 deletions(-)

diff --git a/lib/ansible/modules/packaging/os/yum.py b/lib/ansible/modules/packaging/os/yum.py
index 8c641df60fc..002c1d14455 100644
--- a/lib/ansible/modules/packaging/os/yum.py
+++ b/lib/ansible/modules/packaging/os/yum.py
@@ -755,7 +755,6 @@ def install(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos, i
         if module.check_mode:
             module.exit_json(changed=True, results=res['results'], changes=dict(installed=pkgs))
 
-        changed = True
 
         lang_env = dict(LANG='C', LC_ALL='C', LC_MESSAGES='C')
         rc, out, err = module.run_command(cmd, environ_update=lang_env)
@@ -765,27 +764,25 @@ def install(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos, i
                 # Fail on invalid urls:
                 if '://' in spec and ('No package %s available.' % spec in out or 'Cannot open: %s. Skipping.' % spec in err):
                     module.fail_json(msg='Package at %s could not be installed' % spec, rc=1, changed=False)
-        if (rc != 0 and 'Nothing to do' in err) or 'Nothing to do' in out:
-            # avoid failing in the 'Nothing To Do' case
-            # this may happen with an URL spec.
-            # for an already installed group,
-            # we get rc = 0 and 'Nothing to do' in out, not in err.
-            rc = 0
-            err = ''
-            out = '%s: Nothing to do' % spec
-            changed = False
 
         res['rc'] = rc
         res['results'].append(out)
         res['msg'] += err
+        res['changed'] = True
+
+        # special case for groups
+        if spec.startswith('@'):
+            if ('Nothing to do' in out and rc == 0) or ('does not have any packages to install' in err):
+                res['changed'] = False
+
+        if rc != 0:
+            res['changed'] = False
+            module.fail_json(**res)
 
         # FIXME - if we did an install - go and check the rpmdb to see if it actually installed
         # look for each pkg in rpmdb
         # look for each pkg via obsoletes
 
-        # Record change
-        res['changed'] = changed
-
     return res
 
 
diff --git a/test/integration/targets/yum/tasks/yum.yml b/test/integration/targets/yum/tasks/yum.yml
index 07e1590d5e6..f212b2661b3 100644
--- a/test/integration/targets/yum/tasks/yum.yml
+++ b/test/integration/targets/yum/tasks/yum.yml
@@ -1,21 +1,4 @@
-# UNINSTALL 'yum-utils'
-#  The `yum` module has the smarts to auto-install `yum-utils`.  To test, we
-#  will first uninstall `yum-utils`.
-- name: check yum-utils with rpm
-  shell: rpm -q yum-utils
-  register: rpm_result
-  ignore_errors: true
-
-# Don't uninstall yum-utils with the `yum` module, it would be bad.  The `yum`
-# module does some `repoquery` magic after removing a package.  It fails when you
-# remove `yum-utils.
-- name: uninstall yum-utils with shell
-  shell: yum -y remove yum-utils
-  when: rpm_result|success
-
 # UNINSTALL
-#   With 'yum-utils' uninstalled, the first call to 'yum' should install
-#   yum-utils.
 - name: uninstall sos
   yum: name=sos state=removed
   register: yum_result
@@ -232,3 +215,103 @@
     installroot: '/'
     state: removed
   register: yum_result
+
+- name: install group
+  yum:
+    name: "@Development Tools"
+    state: present
+  register: yum_result
+
+- name: verify installation of the group
+  assert:
+    that:
+        - "yum_result.rc == 0"
+        - "yum_result.changed"
+
+- name: verify yum module outputs
+  assert:
+    that:
+        - "'changed' in yum_result"
+        - "'msg' in yum_result"
+        - "'rc' in yum_result"
+        - "'results' in yum_result"
+
+- name: install the group again
+  yum:
+    name: "@Development Tools"
+    state: present
+  register: yum_result
+
+- name: verify nothing changed
+  assert:
+    that:
+        - "yum_result.rc == 0"
+        - "not yum_result.changed"
+
+- name: verify yum module outputs
+  assert:
+    that:
+        - "'changed' in yum_result"
+        - "'msg' in yum_result"
+        - "'rc' in yum_result"
+        - "'results' in yum_result"
+
+- name: try to install non existing group
+  yum:
+    name: "@non-existing-group"
+    state: present
+  register: yum_result
+  ignore_errors: True
+
+- name: verify installation of the non existing group failed
+  assert:
+    that:
+        - "yum_result.rc == 1"
+        - "not yum_result.changed"
+        - "yum_result|failed"
+
+- name: verify yum module outputs
+  assert:
+    that:
+        - "'changed' in yum_result"
+        - "'msg' in yum_result"
+        - "'rc' in yum_result"
+        - "'results' in yum_result"
+
+- name: try to install non existing file
+  yum:
+    name: /tmp/non-existing-1.0.0.fc26.noarch.rpm
+    state: present
+  register: yum_result
+  ignore_errors: yes
+
+- name: verify installation failed
+  assert:
+    that:
+        - "yum_result|failed"
+        - "not yum_result.changed"
+
+- name: verify yum module outputs
+  assert:
+    that:
+        - "'changed' in yum_result"
+        - "'msg' in yum_result"
+
+- name: try to install from non existing url
+  yum:
+    name: http://non-existing.com/non-existing-1.0.0.fc26.noarch.rpm
+    state: present
+  register: yum_result
+  ignore_errors: yes
+
+- name: verify installation failed
+  assert:
+    that:
+        - "yum_result|failed"
+        - "not yum_result.changed"
+
+- name: verify yum module outputs
+  assert:
+    that:
+        - "'changed' in yum_result"
+        - "'msg' in yum_result"