From 499ddeadd546bfb4c044cbc505c51c7b457cb410 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Tue, 4 Sep 2018 17:46:28 -0500 Subject: [PATCH] Backport package manager detection fixes to stable 2.6 (#44415) * Fix pkg_mgr_name fact finding for Fedora (#40922) * Properly handle default package manager vs apt For distros where apt might be installed but is not the default package manager for the distro, properly identify the default distro package manager during fact finding and re-use fact finding from DistributionFactCollector and instead of reimplementing small portions of it in PkgMgrFactCollector Add unit test to always check the apt + Fedora combination to test the new code. Fixes #34014 Signed-off-by: Adam Miller * remove q debugging output I accidentally left behind Signed-off-by: Adam Miller * add os_family to the conditional so we're only hitting that code path when needed Signed-off-by: Adam Miller * setup for a _check* pattern for general os_family group pkg_mgr checking Signed-off-by: Adam Miller * use Mock.patch decorator for os.path.exists in TestPkgMgrFactsAptFedora Signed-off-by: Adam Miller * fix fedora version dnf fact, default pkg_mgr detection per distro family (#43261) * fix fedora version dnf fact, default pkg_mgr detection per distro family * loop over possible dnf/yum paths in case there are multiple canonical sources later in life Signed-off-by: Adam Miller * pkg_mgr: fixed apt_rpm detection (#43769) Instead of checking the distribution name (which apparently is tricky to find out) check if /usr/bin/apt-get is managed by RPM. Fixes #43539 * Ensure that apt is always chosen on debian/ubuntu One can install alternate packages managers on debuntu machines. However, doing so doesn't mean you want to suddenly start using them. Add in a check similar to the fedora yum/dnf check that sets apt as the pkg_mgr if the ansible_os_family is Debian. --- changelogs/fragments/zypper-on-ubuntu.yaml | 4 ++ .../module_utils/facts/system/pkg_mgr.py | 65 +++++++++++++++++-- .../targets/package/tasks/main.yml | 48 +++++++++++++- .../facts/test_ansible_collector.py | 12 +++- .../module_utils/facts/test_collectors.py | 34 ++++++++++ 5 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/zypper-on-ubuntu.yaml diff --git a/changelogs/fragments/zypper-on-ubuntu.yaml b/changelogs/fragments/zypper-on-ubuntu.yaml new file mode 100644 index 00000000000..6afe40fc3b1 --- /dev/null +++ b/changelogs/fragments/zypper-on-ubuntu.yaml @@ -0,0 +1,4 @@ +bugfixes: + - Fixed an issue where ``ansible_facts.pkg_mgr`` would incorrectly set + to ``zypper`` on Debian/Ubuntu systems that happened to have the + command installed. diff --git a/lib/ansible/module_utils/facts/system/pkg_mgr.py b/lib/ansible/module_utils/facts/system/pkg_mgr.py index a8fcf0929dc..3bb18343c8f 100644 --- a/lib/ansible/module_utils/facts/system/pkg_mgr.py +++ b/lib/ansible/module_utils/facts/system/pkg_mgr.py @@ -19,6 +19,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import os +import subprocess from ansible.module_utils.facts.collector import BaseFactCollector @@ -68,6 +69,43 @@ class PkgMgrFactCollector(BaseFactCollector): name = 'pkg_mgr' _fact_ids = set() _platform = 'Generic' + required_facts = set(['distribution']) + + def _check_rh_versions(self, pkg_mgr_name, collected_facts): + if collected_facts['ansible_distribution'] == 'Fedora': + try: + if int(collected_facts['ansible_distribution_major_version']) < 23: + for yum in [pkg_mgr for pkg_mgr in PKG_MGRS if pkg_mgr['name'] == 'yum']: + if os.path.exists(yum['path']): + pkg_mgr_name = 'yum' + break + else: + for dnf in [pkg_mgr for pkg_mgr in PKG_MGRS if pkg_mgr['name'] == 'dnf']: + if os.path.exists(dnf['path']): + pkg_mgr_name = 'dnf' + break + except ValueError: + # If there's some new magical Fedora version in the future, + # just default to dnf + pkg_mgr_name = 'dnf' + return pkg_mgr_name + + def _check_apt_flavor(self, pkg_mgr_name): + # Check if '/usr/bin/apt' is APT-RPM or an ordinary (dpkg-based) APT. + # There's rpm package on Debian, so checking if /usr/bin/rpm exists + # is not enough. Instead ask RPM if /usr/bin/apt-get belongs to some + # RPM package. + rpm_query = '/usr/bin/rpm -q --whatprovides /usr/bin/apt-get'.split() + if os.path.exists('/usr/bin/rpm'): + with open(os.devnull, 'w') as null: + try: + subprocess.check_call(rpm_query, stdout=null, stderr=null) + pkg_mgr_name = 'apt_rpm' + except subprocess.CalledProcessError: + # No apt-get in RPM database. Looks like Debian/Ubuntu + # with rpm package installed + pkg_mgr_name = 'apt' + return pkg_mgr_name def collect(self, module=None, collected_facts=None): facts_dict = {} @@ -78,10 +116,29 @@ class PkgMgrFactCollector(BaseFactCollector): if os.path.exists(pkg['path']): pkg_mgr_name = pkg['name'] - if pkg_mgr_name == 'apt' and \ - os.path.exists('/usr/bin/rpm') and \ - not os.path.exists('/usr/bin/dpkg'): - pkg_mgr_name = 'apt_rpm' + # Handle distro family defaults when more than one package manager is + # installed, the ansible_fact entry should be the default package + # manager provided by the distro. + if collected_facts['ansible_os_family'] == "RedHat": + if pkg_mgr_name not in ('yum', 'dnf'): + pkg_mgr_name = self._check_rh_versions(pkg_mgr_name, collected_facts) + elif collected_facts['ansible_os_family'] == 'Debian' and pkg_mgr_name != 'apt': + # It's possible to install yum, dnf, zypper, rpm, etc inside of + # Debian. Doing so does not mean the system wants to use them. + pkg_mgr_name = 'apt' + elif collected_facts['ansible_os_family'] == 'Altlinux': + if pkg_mgr_name == 'apt': + pkg_mgr_name = 'apt_rpm' + + # Check if /usr/bin/apt-get is ordinary (dpkg-based) APT or APT-RPM + if pkg_mgr_name == 'apt': + pkg_mgr_name = self._check_apt_flavor(pkg_mgr_name) + + # pacman has become available by distros other than those that are Arch + # based by virtue of a dependency to the systemd mkosi project, this + # handles some of those scenarios as they are reported/requested + if pkg_mgr_name == 'pacman' and collected_facts['ansible_os_family'] in ["RedHat"]: + pkg_mgr_name = self._check_rh_versions(collected_facts) facts_dict['pkg_mgr'] = pkg_mgr_name return facts_dict diff --git a/test/integration/targets/package/tasks/main.yml b/test/integration/targets/package/tasks/main.yml index 5fc7a5a01d4..4fc3a8a6796 100644 --- a/test/integration/targets/package/tasks/main.yml +++ b/test/integration/targets/package/tasks/main.yml @@ -24,11 +24,57 @@ - name: create our testing sub-directory file: path="{{ output_dir_test }}" state=directory +# Verify correct default package manager for Fedora +# Validates: https://github.com/ansible/ansible/issues/34014 +- block: + - name: install apt + dnf: + name: apt + state: present + - name: gather facts again + setup: + - name: validate output + assert: + that: + - 'ansible_pkg_mgr == "dnf"' + always: + - name: remove apt + dnf: + name: apt + state: absent + - name: gather facts again + setup: + when: ansible_distribution == "Fedora" + +# Verify correct default package manager for Debian/Ubuntu when Zypper installed +- block: + # Just make an executable file called "zypper" - installing zypper itself + # consistently is hard - and we're not going to use it + - name: install fake zypper + file: + state: touch + mode: 0755 + path: /usr/bin/zypper + - name: gather facts again + setup: + - name: validate output + assert: + that: + - 'ansible_pkg_mgr == "apt"' + always: + - name: remove fake zypper + file: + path: /usr/bin/zypper + state: absent + - name: gather facts again + setup: + when: ansible_os_family == "Debian" + ## ## package ## -- name: define distros to attempt installing at on +- name: define distros to attempt installing at on set_fact: package_distros: - RedHat diff --git a/test/units/module_utils/facts/test_ansible_collector.py b/test/units/module_utils/facts/test_ansible_collector.py index 3d61dce4bcb..51172090956 100644 --- a/test/units/module_utils/facts/test_ansible_collector.py +++ b/test/units/module_utils/facts/test_ansible_collector.py @@ -202,6 +202,8 @@ class TestCollectedFacts(unittest.TestCase): 'env'] not_expected_facts = ['facter', 'ohai'] + collected_facts = {} + def _mock_module(self, gather_subset=None): return mock_module(gather_subset=self.gather_subset) @@ -212,7 +214,8 @@ class TestCollectedFacts(unittest.TestCase): fact_collector = \ ansible_collector.AnsibleFactCollector(collectors=collectors, namespace=ns) - self.facts = fact_collector.collect(module=mock_module) + self.facts = fact_collector.collect(module=mock_module, + collected_facts=self.collected_facts) def _collectors(self, module, all_collector_classes=None, @@ -466,10 +469,15 @@ class TestOhaiCollectedFacts(TestCollectedFacts): class TestPkgMgrFacts(TestCollectedFacts): gather_subset = ['pkg_mgr'] min_fact_count = 1 - max_fact_count = 10 + max_fact_count = 20 expected_facts = ['gather_subset', 'module_setup', 'pkg_mgr'] + collected_facts = { + "ansible_distribution": "Fedora", + "ansible_distribution_major_version": "28", + "ansible_os_family": "RedHat" + } class TestOpenBSDPkgMgrFacts(TestPkgMgrFacts): diff --git a/test/units/module_utils/facts/test_collectors.py b/test/units/module_utils/facts/test_collectors.py index 6311934c3b3..668344d3684 100644 --- a/test/units/module_utils/facts/test_collectors.py +++ b/test/units/module_utils/facts/test_collectors.py @@ -224,6 +224,11 @@ class TestPkgMgrFacts(BaseFactsTest): valid_subsets = ['pkg_mgr'] fact_namespace = 'ansible_pkgmgr' collector_class = PkgMgrFactCollector + collected_facts = { + "ansible_distribution": "Fedora", + "ansible_distribution_major_version": "28", + "ansible_os_family": "RedHat" + } def test_collect(self): module = self._mock_module() @@ -233,6 +238,35 @@ class TestPkgMgrFacts(BaseFactsTest): self.assertIn('pkg_mgr', facts_dict) +def _sanitize_os_path_apt_get(path): + if path == '/usr/bin/apt-get': + return True + else: + return False + + +class TestPkgMgrFactsAptFedora(BaseFactsTest): + __test__ = True + gather_subset = ['!all', 'pkg_mgr'] + valid_subsets = ['pkg_mgr'] + fact_namespace = 'ansible_pkgmgr' + collector_class = PkgMgrFactCollector + collected_facts = { + "ansible_distribution": "Fedora", + "ansible_distribution_major_version": "28", + "ansible_os_family": "RedHat", + "ansible_pkg_mgr": "apt" + } + + @patch('ansible.module_utils.facts.system.pkg_mgr.os.path.exists', side_effect=_sanitize_os_path_apt_get) + def test_collect(self, mock_os_path_exists): + module = self._mock_module() + fact_collector = self.collector_class() + facts_dict = fact_collector.collect(module=module, collected_facts=self.collected_facts) + self.assertIsInstance(facts_dict, dict) + self.assertIn('pkg_mgr', facts_dict) + + class TestOpenBSDPkgMgrFacts(BaseFactsTest): __test__ = True gather_subset = ['!all', 'pkg_mgr']