From bca1818b1e6496d6570ef599983ad605de3d2913 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Fri, 22 Sep 2017 14:24:57 -0400 Subject: [PATCH] Fix pkg_mgr fact on OpenBSD (#30725) * Fix pkg_mgr fact on OpenBSD Add a OpenBSDPkgMgrFactCollector that hardcodes pkg_mgr to 'openbsd_pkg'. The ansible collector will choose the OpenBSD collector if the system is OpenBSD and the 'Generic' one otherwise. This removes PkgMgrFactCollectors depenency on the 'system' fact being in collected_facts, which also avoids ordering issues (if the pkg mgr fact is collected before the system fact...) Fixes #30623 (cherry picked from commit 12404f470abf80d7204d64e71d218c71c5a5a7ec) --- CHANGELOG.md | 1 + .../module_utils/facts/default_collectors.py | 2 + .../module_utils/facts/system/pkg_mgr.py | 18 ++++++--- .../facts/test_ansible_collector.py | 37 +++++++++++++++++-- .../module_utils/facts/test_collectors.py | 25 ++++++++++++- 5 files changed, 74 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 817f082f0ea..4fe97c56e73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Ansible Changes By Release * Fix for win_domain_membership to throw more helpful error messages and check/fix when calling WMI function after changing workgroup * Fix for win_power_plan to compare the OS version's correctly and work on Windows 10/Server 2016 * Fix module doc for typo in telnet command option +* Fix OpenBSD pkg_mgr fact (https://github.com/ansible/ansible/issues/30623) diff --git a/lib/ansible/module_utils/facts/default_collectors.py b/lib/ansible/module_utils/facts/default_collectors.py index ffd14a9e9af..b5969f14bb8 100644 --- a/lib/ansible/module_utils/facts/default_collectors.py +++ b/lib/ansible/module_utils/facts/default_collectors.py @@ -32,6 +32,7 @@ from ansible.module_utils.facts.system.fips import FipsFactCollector from ansible.module_utils.facts.system.local import LocalFactCollector from ansible.module_utils.facts.system.lsb import LSBFactCollector from ansible.module_utils.facts.system.pkg_mgr import PkgMgrFactCollector +from ansible.module_utils.facts.system.pkg_mgr import OpenBSDPkgMgrFactCollector from ansible.module_utils.facts.system.platform import PlatformFactCollector from ansible.module_utils.facts.system.python import PythonFactCollector from ansible.module_utils.facts.system.selinux import SelinuxFactCollector @@ -108,6 +109,7 @@ collectors = [ApparmorFactCollector, SunOSNetworkCollector, PkgMgrFactCollector, + OpenBSDPkgMgrFactCollector, PlatformFactCollector, PythonFactCollector, SelinuxFactCollector, diff --git a/lib/ansible/module_utils/facts/system/pkg_mgr.py b/lib/ansible/module_utils/facts/system/pkg_mgr.py index 578a78deccf..502dfe8570a 100644 --- a/lib/ansible/module_utils/facts/system/pkg_mgr.py +++ b/lib/ansible/module_utils/facts/system/pkg_mgr.py @@ -50,20 +50,28 @@ PKG_MGRS = [{'path': '/usr/bin/yum', 'name': 'yum'}, ] +class OpenBSDPkgMgrFactCollector(BaseFactCollector): + name = 'pkg_mgr' + _fact_ids = set() + _platform = 'OpenBSD' + + def collect(self, module=None, collected_facts=None): + facts_dict = {} + + facts_dict['pkg_mgr'] = 'openbsd_pkg' + return facts_dict + + # the fact ends up being 'pkg_mgr' so stick with that naming/spelling class PkgMgrFactCollector(BaseFactCollector): name = 'pkg_mgr' _fact_ids = set() + _platform = 'Generic' def collect(self, module=None, collected_facts=None): facts_dict = {} collected_facts = collected_facts or {} - pkg_mgr_name = None - if collected_facts.get('system') == 'OpenBSD': - facts_dict['pkg_mgr'] = 'openbsd_pkg' - return facts_dict - pkg_mgr_name = 'unknown' for pkg in PKG_MGRS: if os.path.exists(pkg['path']): diff --git a/test/units/module_utils/facts/test_ansible_collector.py b/test/units/module_utils/facts/test_ansible_collector.py index 52290a8835f..27bb6da9fc7 100644 --- a/test/units/module_utils/facts/test_ansible_collector.py +++ b/test/units/module_utils/facts/test_ansible_collector.py @@ -1,4 +1,3 @@ -# unit tests for ansible/module_utils/facts/__init__.py # -*- coding: utf-8 -*- # # @@ -22,7 +21,7 @@ __metaclass__ = type # for testing from ansible.compat.tests import unittest -from ansible.compat.tests.mock import Mock +from ansible.compat.tests.mock import Mock, patch from ansible.module_utils.facts import collector from ansible.module_utils.facts import ansible_collector @@ -40,7 +39,7 @@ from ansible.module_utils.facts.system.dns import DnsFactCollector from ansible.module_utils.facts.system.fips import FipsFactCollector from ansible.module_utils.facts.system.local import LocalFactCollector from ansible.module_utils.facts.system.lsb import LSBFactCollector -from ansible.module_utils.facts.system.pkg_mgr import PkgMgrFactCollector +from ansible.module_utils.facts.system.pkg_mgr import PkgMgrFactCollector, OpenBSDPkgMgrFactCollector from ansible.module_utils.facts.system.platform import PlatformFactCollector from ansible.module_utils.facts.system.python import PythonFactCollector from ansible.module_utils.facts.system.selinux import SelinuxFactCollector @@ -59,6 +58,7 @@ ALL_COLLECTOR_CLASSES = \ SystemCapabilitiesFactCollector, FipsFactCollector, PkgMgrFactCollector, + OpenBSDPkgMgrFactCollector, ServiceMgrFactCollector, LSBFactCollector, DateTimeFactCollector, @@ -311,3 +311,34 @@ class TestOhaiCollectedFacts(TestCollectedFacts): expected_facts = ['gather_subset', 'module_setup'] not_expected_facts = ['lsb'] + + +class TestPkgMgrFacts(TestCollectedFacts): + gather_subset = ['pkg_mgr'] + min_fact_count = 1 + max_fact_count = 10 + expected_facts = ['gather_subset', + 'module_setup', + 'pkg_mgr'] + + +class TestOpenBSDPkgMgrFacts(TestPkgMgrFacts): + def test_is_openbsd_pkg(self): + self.assertIn('pkg_mgr', self.facts) + self.assertEqual(self.facts['pkg_mgr'], 'openbsd_pkg') + + def setUp(self): + self.patcher = patch('platform.system') + mock_platform = self.patcher.start() + mock_platform.return_value = 'OpenBSD' + + mock_module = self._mock_module() + collectors = self._collectors(mock_module) + + fact_collector = \ + ansible_collector.AnsibleFactCollector(collectors=collectors, + namespace=ns) + self.facts = fact_collector.collect(module=mock_module) + + def tearDown(self): + self.patcher.stop() diff --git a/test/units/module_utils/facts/test_collectors.py b/test/units/module_utils/facts/test_collectors.py index a356422ea98..4e00b40c030 100644 --- a/test/units/module_utils/facts/test_collectors.py +++ b/test/units/module_utils/facts/test_collectors.py @@ -32,7 +32,7 @@ from ansible.module_utils.facts.system.distribution import DistributionFactColle from ansible.module_utils.facts.system.dns import DnsFactCollector from ansible.module_utils.facts.system.env import EnvFactCollector from ansible.module_utils.facts.system.fips import FipsFactCollector -from ansible.module_utils.facts.system.pkg_mgr import PkgMgrFactCollector +from ansible.module_utils.facts.system.pkg_mgr import PkgMgrFactCollector, OpenBSDPkgMgrFactCollector from ansible.module_utils.facts.system.platform import PlatformFactCollector from ansible.module_utils.facts.system.python import PythonFactCollector from ansible.module_utils.facts.system.selinux import SelinuxFactCollector @@ -225,6 +225,29 @@ class TestPkgMgrFacts(BaseFactsTest): fact_namespace = 'ansible_pkgmgr' collector_class = PkgMgrFactCollector + def test_collect(self): + 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'] + valid_subsets = ['pkg_mgr'] + fact_namespace = 'ansible_pkgmgr' + collector_class = OpenBSDPkgMgrFactCollector + + def test_collect(self): + 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) + self.assertEqual(facts_dict['pkg_mgr'], 'openbsd_pkg') + class TestPlatformFactCollector(BaseFactsTest): __test__ = True