From 98b19f0623b45954515749e84d47316b5fedf69b Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 22 Aug 2017 17:34:59 -0400 Subject: [PATCH] yum: fix changed in group remove --- lib/ansible/modules/packaging/os/yum.py | 64 ++++---- test/integration/targets/yum/tasks/main.yml | 19 ++- .../targets/yum/tasks/yum_group_remove.yml | 142 ++++++++++++++++++ 3 files changed, 190 insertions(+), 35 deletions(-) create mode 100644 test/integration/targets/yum/tasks/yum_group_remove.yml diff --git a/lib/ansible/modules/packaging/os/yum.py b/lib/ansible/modules/packaging/os/yum.py index 39337e8443..4b5a2a1775 100644 --- a/lib/ansible/modules/packaging/os/yum.py +++ b/lib/ansible/modules/packaging/os/yum.py @@ -342,6 +342,18 @@ def po_to_nevra(po): else: return '%s-%s-%s.%s' % (po.name, po.version, po.release, po.arch) + +def is_group_installed(name): + my = yum_base() + groups_list = my.doGroupLists() + for group in groups_list[0]: # list of the installed groups on the first index + name_lower = name.lower() + if name_lower.endswith(group.name.lower()) or name_lower.endswith(group.groupid.lower()): + return True + + return False + + def is_installed(module, repoq, pkgspec, conf_file, qf=def_qf, en_repos=None, dis_repos=None, is_pkg=False, installroot='/'): if en_repos is None: en_repos = [] @@ -746,16 +758,7 @@ def install(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos, i # groups elif spec.startswith('@'): - found = False - my = yum_base() - groups_list = my.doGroupLists() - for group in groups_list[0]: # list of the installed groups on the first index - spec_lower = spec.lower() - if spec_lower.endswith(group.name.lower()) or spec_lower.endswith(group.groupid.lower()): - found = True - break - - if found: + if is_group_installed(spec): continue pkg = spec @@ -860,47 +863,48 @@ def remove(module, items, repoq, yum_basecmd, conf_file, en_repos, dis_repos, in res['rc'] = 0 for pkg in items: - is_group = False - # group remove - this is doom on a stick if pkg.startswith('@'): - is_group = True # nopep8 this will be fixed in next MR this module needs major rewrite anyway. + installed = is_group_installed(pkg) else: - if not is_installed(module, repoq, pkg, conf_file, en_repos=en_repos, dis_repos=dis_repos, installroot=installroot): - res['results'].append('%s is not installed' % pkg) - continue + installed = is_installed(module, repoq, pkg, conf_file, en_repos=en_repos, dis_repos=dis_repos, installroot=installroot) - pkgs.append(pkg) + if installed: + pkgs.append(pkg) + else: + res['results'].append('%s is not installed' % pkg) if pkgs: - # run an actual yum transaction - cmd = yum_basecmd + ["remove"] + pkgs - if module.check_mode: module.exit_json(changed=True, results=res['results'], changes=dict(removed=pkgs)) + # run an actual yum transaction + cmd = yum_basecmd + ["remove"] + pkgs + rc, out, err = module.run_command(cmd) res['rc'] = rc res['results'].append(out) res['msg'] = err + if rc != 0: + module.fail_json(**res) + # compile the results into one batch. If anything is changed # then mark changed # at the end - if we've end up failed then fail out of the rest # of the process - # at this point we should check to see if the pkg is no longer present - + # at this point we check to see if the pkg is no longer present for pkg in pkgs: - if not pkg.startswith('@'): # we can't sensibly check for a group being uninstalled reliably - # look to see if the pkg shows up from is_installed. If it doesn't - if not is_installed(module, repoq, pkg, conf_file, en_repos=en_repos, dis_repos=dis_repos, installroot=installroot): - res['changed'] = True - else: - module.fail_json(**res) + if pkg.startswith('@'): + installed = is_group_installed(pkg) + else: + installed = is_installed(module, repoq, pkg, conf_file, en_repos=en_repos, dis_repos=dis_repos, installroot=installroot) - if rc != 0: - module.fail_json(**res) + if installed: + module.fail_json(**res) + + res['changed'] = True return res diff --git a/test/integration/targets/yum/tasks/main.yml b/test/integration/targets/yum/tasks/main.yml index 78077796e8..cd153f5f9a 100644 --- a/test/integration/targets/yum/tasks/main.yml +++ b/test/integration/targets/yum/tasks/main.yml @@ -19,14 +19,23 @@ # Note: We install the yum package onto Fedora so that this will work on dnf systems # We want to test that for people who don't want to upgrade their systems. - include: 'yum.yml' - when: ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux', 'Fedora'] and - ansible_python.version.major == 2 + when: + - ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux', 'Fedora'] + - ansible_python.version.major == 2 # We can't run yum --installroot tests on dnf systems. Dnf systems revert to # yum-deprecated, and yum-deprecated refuses to run if yum.conf exists # so we cannot configure yum-deprecated correctly in an empty /tmp/fake.root/ # It will always run with $releasever unset - include: 'yuminstallroot.yml' - when: (ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux'] or - (ansible_distribution in ['Fedora'] and ansible_distribution_major_version|int < 23)) and - ansible_python.version.major == 2 + when: + - (ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux'] or (ansible_distribution in ['Fedora'] and ansible_distribution_major_version|int < 23)) + - ansible_python.version.major == 2 + +# el6 has a broken yum group implementation, when you try to remove a group it goes through +# deps and ends up with trying to remove yum itself and the whole process fails +# so don't run the yum group remove tests there +- include: 'yum_group_remove.yml' + when: + - (ansible_distribution in ['RedHat', 'CentOS', 'ScientificLinux'] and ansible_distribution_major_version|int > 6) or ansible_distribution in ['Fedora'] + - ansible_python.version.major == 2 diff --git a/test/integration/targets/yum/tasks/yum_group_remove.yml b/test/integration/targets/yum/tasks/yum_group_remove.yml new file mode 100644 index 0000000000..23b851cde2 --- /dev/null +++ b/test/integration/targets/yum/tasks/yum_group_remove.yml @@ -0,0 +1,142 @@ +- name: install a group to test and yum-utils + yum: + name: "{{ item }}" + state: present + with_items: + - "@Development Tools" + - yum-utils + +- name: check mode remove the group + yum: + name: "@Development Tools" + state: absent + check_mode: yes + register: yum_result + +- name: verify changed + assert: + that: + - "yum_result.changed" + +- name: verify yum module outputs + assert: + that: + - "'changed' in yum_result" + - "'results' in yum_result" + +- name: remove the group + yum: + name: "@Development Tools" + state: absent + register: yum_result + +- name: verify changed + assert: + that: + - "yum_result.rc == 0" + - "yum_result.changed" + +- name: verify yum module outputs + assert: + that: + - "'changed' in yum_result" + - "'msg' in yum_result" + - "'results' in yum_result" + +- name: remove the group again + yum: + name: "@Development Tools" + state: absent + register: yum_result + +- name: verify changed + assert: + that: + - "not yum_result.changed" + +- name: verify yum module outputs + assert: + that: + - "'changed' in yum_result" + - "'msg' in yum_result" + - "'results' in yum_result" + +- name: check mode remove the group again + yum: + name: "@Development Tools" + state: absent + check_mode: yes + register: yum_result + +- name: verify changed + assert: + that: + - "not yum_result.changed" + +- name: verify yum module outputs + assert: + that: + - "'changed' in yum_result" + - "'results' in yum_result" + +- name: install a group and a package to test + yum: + name: "@Development Tools,sos" + state: present + register: yum_output + +- debug: var=yum_output + +- name: check mode remove the group along with the package + yum: + name: "@Development Tools,sos" + state: absent + register: yum_result + check_mode: yes + +- name: verify changed + assert: + that: + - "yum_result.changed" + +- name: verify yum module outputs + assert: + that: + - "'changed' in yum_result" + - "'results' in yum_result" + +- name: remove the group along with the package + yum: + name: "@Development Tools,sos" + state: absent + register: yum_result + +- name: verify changed + assert: + that: + - "yum_result.changed" + +- name: verify yum module outputs + assert: + that: + - "'changed' in yum_result" + - "'msg' in yum_result" + - "'results' in yum_result" + +- name: check mode remove the group along with the package + yum: + name: "@Development Tools,sos" + state: absent + register: yum_result + check_mode: yes + +- name: verify not changed + assert: + that: + - "not yum_result.changed" + +- name: verify yum module outputs + assert: + that: + - "'changed' in yum_result" + - "'results' in yum_result"