Mv AnsibleFactCollector back to module_utils (#26150)

It was in lib/ansible/modules/system/setup.py since it
was the only thing using it, but move it back to module_utils
and add a ansible_collector.get_ansible_collector() to build
a facts collector just like the one used by setup.py

mv test_setup.py -> test_ansible_collector.py
All the code it was testing is now in ansible_collector

rm code to create 'ansible_facts' subkey from namespace

Just leave it up to the caller to do, and just return a
flat dictionary from AnsibleFactCollector.collect()

lib/ansible/module_utils/facts/ansible_collector.py

@@ -0,0 +1,129 @@
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import fnmatch
+import sys
+
+from ansible.module_utils.facts import timeout
+from ansible.module_utils.facts import collector
+
+
+class AnsibleFactCollector(collector.BaseFactCollector):
+    '''A FactCollector that returns results under 'ansible_facts' top level key.
+
+       If a namespace if provided, facts will be collected under that namespace.
+       For ex, a ansible.module_utils.facts.namespace.PrefixFactNamespace(prefix='ansible_')
+
+       Has a 'from_gather_subset() constructor that populates collectors based on a
+       gather_subset specifier.'''
+
+    def __init__(self, collectors=None, namespace=None, filter_spec=None):
+
+        super(AnsibleFactCollector, self).__init__(collectors=collectors,
+                                                   namespace=namespace)
+
+        self.filter_spec = filter_spec
+
+    def _filter(self, facts_dict, filter_spec):
+        # assume a filter_spec='' is equilv to filter_spec='*'
+        if not filter_spec or filter_spec == '*':
+            return facts_dict
+
+        return [(x, y) for x, y in facts_dict.items() if fnmatch.fnmatch(x, filter_spec)]
+
+    def collect(self, module=None, collected_facts=None):
+        collected_facts = collected_facts or {}
+
+        facts_dict = {}
+
+        for collector_obj in self.collectors:
+            info_dict = {}
+
+            # shallow copy of the accumulated collected facts to pass to each collector
+            # for reference.
+            collected_facts.update(facts_dict.copy())
+
+            try:
+
+                # Note: this collects with namespaces, so collected_facts also includes namespaces
+                info_dict = collector_obj.collect_with_namespace(module=module,
+                                                                 collected_facts=collected_facts)
+            except Exception as e:
+                sys.stderr.write(repr(e))
+                sys.stderr.write('\n')
+
+            # NOTE: If we want complicated fact dict merging, this is where it would hook in
+            facts_dict.update(self._filter(info_dict, self.filter_spec))
+
+        return facts_dict
+
+
+class CollectorMetaDataCollector(collector.BaseFactCollector):
+    '''Collector that provides a facts with the gather_subset metadata.'''
+
+    name = 'gather_subset'
+    _fact_ids = set([])
+
+    def __init__(self, collectors=None, namespace=None, gather_subset=None, module_setup=None):
+        super(CollectorMetaDataCollector, self).__init__(collectors, namespace)
+        self.gather_subset = gather_subset
+        self.module_setup = module_setup
+
+    def collect(self, module=None, collected_facts=None):
+        meta_facts = {'gather_subset': self.gather_subset}
+        if self.module_setup:
+            meta_facts['module_setup'] = self.module_setup
+        return meta_facts
+
+
+def get_ansible_collector(all_collector_classes,
+                          namespace=None,
+                          filter_spec=None,
+                          gather_subset=None,
+                          gather_timeout=None,
+                          minimal_gather_subset=None):
+
+    filter_spec = filter_spec or '*'
+    gather_subset = gather_subset or ['all']
+    gather_timeout = gather_timeout or timeout.DEFAULT_GATHER_TIMEOUT
+    minimal_gather_subset = minimal_gather_subset or frozenset()
+
+    collector_classes = \
+        collector.collector_classes_from_gather_subset(
+            all_collector_classes=all_collector_classes,
+            minimal_gather_subset=minimal_gather_subset,
+            gather_subset=gather_subset,
+            gather_timeout=gather_timeout)
+
+    collectors = []
+    for collector_class in collector_classes:
+        collector_obj = collector_class(namespace=namespace)
+        collectors.append(collector_obj)
+
+    # Add a collector that knows what gather_subset we used so it it can provide a fact
+    collector_meta_data_collector = \
+        CollectorMetaDataCollector(gather_subset=gather_subset,
+                                   module_setup=True)
+    collectors.append(collector_meta_data_collector)
+
+    fact_collector = \
+        AnsibleFactCollector(collectors=collectors,
+                             filter_spec=filter_spec,
+                             namespace=namespace)
+
+    return fact_collector

lib/ansible/modules/system/setup.py

@@ -118,93 +118,16 @@ EXAMPLES = """
 # Display facts from Windows hosts with custom facts stored in C(C:\\custom_facts).
 # ansible windows -m setup -a "fact_path='c:\\custom_facts'"
 """
-import fnmatch
-import sys
 
 # import module snippets
 from ansible.module_utils.basic import AnsibleModule
 
-from ansible.module_utils.facts import collector
 from ansible.module_utils.facts.namespace import PrefixFactNamespace
+from ansible.module_utils.facts import ansible_collector
 
 from ansible.module_utils.facts import default_collectors
 
 
-# This is the main entry point for setup.py facts.py.
-# FIXME: This is coupled to AnsibleModule (it assumes module.params has keys 'gather_subset',
-#        'gather_timeout', 'filter' instead of passing those are args or oblique ds
-#        module is passed in and self.module.misc_AnsibleModule_methods
-#        are used, so hard to decouple.
-
-class AnsibleFactCollector(collector.BaseFactCollector):
-    '''A FactCollector that returns results under 'ansible_facts' top level key.
-
-       Has a 'from_gather_subset() constructor that populates collectors based on a
-       gather_subset specifier.'''
-
-    def __init__(self, collectors=None, namespace=None, filter_spec=None):
-
-        super(AnsibleFactCollector, self).__init__(collectors=collectors,
-                                                   namespace=namespace)
-
-        self.filter_spec = filter_spec
-
-    def _filter(self, facts_dict, filter_spec):
-        # assume a filter_spec='' is equilv to filter_spec='*'
-        if not filter_spec or filter_spec == '*':
-            return facts_dict
-
-        return [(x, y) for x, y in facts_dict.items() if fnmatch.fnmatch(x, filter_spec)]
-
-    def collect(self, module=None, collected_facts=None):
-        collected_facts = collected_facts or {}
-
-        facts_dict = {}
-        facts_dict['ansible_facts'] = {}
-
-        for collector_obj in self.collectors:
-            info_dict = {}
-
-            # shallow copy of the accumulated collected facts to pass to each collector
-            # for reference.
-            collected_facts.update(facts_dict['ansible_facts'].copy())
-
-            try:
-
-                # Note: this collects with namespaces, so collected_facts also includes namespaces
-                info_dict = collector_obj.collect_with_namespace(module=module,
-                                                                 collected_facts=collected_facts)
-            except Exception as e:
-                sys.stderr.write(repr(e))
-                sys.stderr.write('\n')
-
-            # filtered_info_dict = self._filter(info_dict, self.filter_spec)
-            # NOTE: If we want complicated fact dict merging, this is where it would hook in
-            facts_dict['ansible_facts'].update(self._filter(info_dict, self.filter_spec))
-
-        # TODO: this may be best place to apply fact 'filters' as well. They
-        #       are currently ignored -akl
-        return facts_dict
-
-
-class CollectorMetaDataCollector(collector.BaseFactCollector):
-    '''Collector that provides a facts with the gather_subset metadata.'''
-
-    name = 'gather_subset'
-    _fact_ids = set([])
-
-    def __init__(self, collectors=None, namespace=None, gather_subset=None, module_setup=None):
-        super(CollectorMetaDataCollector, self).__init__(collectors, namespace)
-        self.gather_subset = gather_subset
-        self.module_setup = module_setup
-
-    def collect(self, module=None, collected_facts=None):
-        meta_facts = {'gather_subset': self.gather_subset}
-        if self.module_setup:
-            meta_facts['module_setup'] = self.module_setup
-        return meta_facts
-
-
 def main():
     module = AnsibleModule(
         argument_spec=dict(
@@ -231,38 +154,21 @@ def main():
 
     all_collector_classes = default_collectors.collectors
 
-    collector_classes = \
+    # rename namespace_name to root_key?
-        collector.collector_classes_from_gather_subset(
-            all_collector_classes=all_collector_classes,
-            minimal_gather_subset=minimal_gather_subset,
-            gather_subset=gather_subset,
-            gather_timeout=gather_timeout)
-
-    # print('collector_classes: %s' % pprint.pformat(collector_classes))
-
     namespace = PrefixFactNamespace(namespace_name='ansible',
                                     prefix='ansible_')
 
-    collectors = []
-    for collector_class in collector_classes:
-        collector_obj = collector_class(namespace=namespace)
-        collectors.append(collector_obj)
-
-    # Add a collector that knows what gather_subset we used so it it can provide a fact
-    collector_meta_data_collector = \
-        CollectorMetaDataCollector(gather_subset=gather_subset,
-                                   module_setup=True)
-    collectors.append(collector_meta_data_collector)
-
-    # print('collectors: %s' % pprint.pformat(collectors))
-
     fact_collector = \
-        AnsibleFactCollector(collectors=collectors,
+        ansible_collector.get_ansible_collector(all_collector_classes=all_collector_classes,
-                             filter_spec=filter_spec)
+                                                namespace=namespace,
+                                                filter_spec=filter_spec,
+                                                gather_subset=gather_subset,
+                                                gather_timeout=gather_timeout,
+                                                minimal_gather_subset=minimal_gather_subset)
 
     facts_dict = fact_collector.collect(module=module)
 
-    module.exit_json(**facts_dict)
+    module.exit_json(ansible_facts=facts_dict)
 
 
 if __name__ == '__main__':

test/units/module_utils/facts/test_ansible_collector.py

@@ -25,6 +25,8 @@ from ansible.compat.tests import unittest
 from ansible.compat.tests.mock import Mock
 
 from ansible.module_utils.facts import collector
+from ansible.module_utils.facts import ansible_collector
+from ansible.module_utils.facts import namespace
 
 from ansible.module_utils.facts.other.facter import FacterFactCollector
 from ansible.module_utils.facts.other.ohai import OhaiFactCollector
@@ -49,10 +51,6 @@ from ansible.module_utils.facts.system.user import UserFactCollector
 from ansible.module_utils.facts.network.base import NetworkCollector
 from ansible.module_utils.facts.virtual.base import VirtualCollector
 
-# module under test
-from ansible.modules.system import setup
-
-
 ALL_COLLECTOR_CLASSES = \
     [PlatformFactCollector,
      DistributionFactCollector,
@@ -109,13 +107,16 @@ def _collectors(module,
 
     # Add a collector that knows what gather_subset we used so it it can provide a fact
     collector_meta_data_collector = \
-        setup.CollectorMetaDataCollector(gather_subset=gather_subset,
+        ansible_collector.CollectorMetaDataCollector(gather_subset=gather_subset,
-                                         module_setup=True)
+                                                     module_setup=True)
     collectors.append(collector_meta_data_collector)
 
     return collectors
 
 
+ns = namespace.PrefixFactNamespace('ansible_facts', 'ansible_')
+
+
 # FIXME: this is brute force, but hopefully enough to get some refactoring to make facts testable
 class TestInPlace(unittest.TestCase):
     def _mock_module(self, gather_subset=None):
@@ -136,15 +137,14 @@ class TestInPlace(unittest.TestCase):
                                       all_collector_classes=all_collector_classes)
 
         fact_collector = \
-            setup.AnsibleFactCollector(collectors=collectors)
+            ansible_collector.AnsibleFactCollector(collectors=collectors,
+                                                   namespace=ns)
 
         res = fact_collector.collect(module=mock_module)
         self.assertIsInstance(res, dict)
-        self.assertIn('ansible_facts', res)
+        self.assertIn('env', res)
-        self.assertIsInstance(res['ansible_facts'], dict)
+        self.assertIn('gather_subset', res)
-        self.assertIn('env', res['ansible_facts'])
+        self.assertEqual(res['gather_subset'], ['all'])
-        self.assertIn('gather_subset', res['ansible_facts'])
-        self.assertEqual(res['ansible_facts']['gather_subset'], ['all'])
 
     def test1(self):
         gather_subset = ['all']
@@ -152,14 +152,14 @@ class TestInPlace(unittest.TestCase):
         collectors = self._collectors(mock_module)
 
         fact_collector = \
-            setup.AnsibleFactCollector(collectors=collectors)
+            ansible_collector.AnsibleFactCollector(collectors=collectors,
+                                                   namespace=ns)
 
         res = fact_collector.collect(module=mock_module)
         self.assertIsInstance(res, dict)
-        self.assertIn('ansible_facts', res)
         # just assert it's not almost empty
         # with run_command and get_file_content mock, many facts are empty, like network
-        self.assertGreater(len(res['ansible_facts']), 20)
+        self.assertGreater(len(res), 20)
 
     def test_empty_all_collector_classes(self):
         mock_module = self._mock_module()
@@ -169,13 +169,13 @@ class TestInPlace(unittest.TestCase):
                                       all_collector_classes=all_collector_classes)
 
         fact_collector = \
-            setup.AnsibleFactCollector(collectors=collectors)
+            ansible_collector.AnsibleFactCollector(collectors=collectors,
+                                                   namespace=ns)
 
         res = fact_collector.collect()
         self.assertIsInstance(res, dict)
-        self.assertIn('ansible_facts', res)
         # just assert it's not almost empty
-        self.assertLess(len(res['ansible_facts']), 3)
+        self.assertLess(len(res), 3)
 
 #    def test_facts_class(self):
 #        mock_module = self._mock_module()
@@ -207,7 +207,8 @@ class TestCollectedFacts(unittest.TestCase):
         collectors = self._collectors(mock_module)
 
         fact_collector = \
-            setup.AnsibleFactCollector(collectors=collectors)
+            ansible_collector.AnsibleFactCollector(collectors=collectors,
+                                                   namespace=ns)
         self.facts = fact_collector.collect(module=mock_module)
 
     def _collectors(self, module,
@@ -228,47 +229,33 @@ class TestCollectedFacts(unittest.TestCase):
 
     def _assert_basics(self, facts):
         self.assertIsInstance(facts, dict)
-        self.assertIn('ansible_facts', facts)
         # just assert it's not almost empty
-        self.assertGreaterEqual(len(facts['ansible_facts']), self.min_fact_count)
+        self.assertGreaterEqual(len(facts), self.min_fact_count)
         # and that is not huge number of keys
-        self.assertLess(len(facts['ansible_facts']), self.max_fact_count)
+        self.assertLess(len(facts), self.max_fact_count)
 
     # everything starts with ansible_ namespace
     def _assert_ansible_namespace(self, facts):
-        subfacts = facts['ansible_facts']
 
         # FIXME: kluge for non-namespace fact
-        subfacts.pop('module_setup', None)
+        facts.pop('module_setup', None)
-        subfacts.pop('gather_subset', None)
+        facts.pop('gather_subset', None)
 
-        for fact_key in subfacts:
+        for fact_key in facts:
             self.assertTrue(fact_key.startswith('ansible_'),
                             'The fact name "%s" does not startwith "ansible_"' % fact_key)
 
     def _assert_expected_facts(self, facts):
-        subfacts = facts['ansible_facts']
 
-        import pprint
+        facts_keys = sorted(facts.keys())
-        pprint.pprint(subfacts)
-        subfacts_keys = sorted(subfacts.keys())
         for expected_fact in self.expected_facts:
-            self.assertIn(expected_fact, subfacts_keys)
+            self.assertIn(expected_fact, facts_keys)
-        #    self.assertIsInstance(subfacts['ansible_env'], dict)
-
-        # self.assertIsInstance(subfacts['ansible_env'], dict)
-
-        # self._assert_ssh_facts(subfacts)
 
     def _assert_not_expected_facts(self, facts):
-        subfacts = facts['ansible_facts']
 
-        subfacts_keys = sorted(subfacts.keys())
+        facts_keys = sorted(facts.keys())
         for not_expected_fact in self.not_expected_facts:
-            self.assertNotIn(not_expected_fact, subfacts_keys)
+            self.assertNotIn(not_expected_fact, facts_keys)
-
-    def _assert_ssh_facts(self, subfacts):
-        self.assertIn('ssh_host_key_rsa_public', subfacts.keys())
 
 
 class ExceptionThrowingCollector(collector.BaseFactCollector):