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 12404f470a)

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)
 
 <a id="2.4"></a>
 

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,

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']):

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()

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