Fix fact failures cause by ordering of collectors (#30777)

* Fix fact failures cause by ordering of collectors

Some fact collectors need info collected by other facts.
(for ex, service_mgr needs to know 'ansible_system').
This info is passed to the Collector.collect method via
the 'collected_facts' info.

But, the order the fact collectors were running in is
not a set order, so collectors like service_mgr could
run before the PlatformFactCollect ('ansible_system', etc),
so the 'ansible_system' fact would not exist yet.

Depending on the collector and the deps, this can result
in incorrect behavior and wrong or missing facts.

To make the ordering of the collectors more consistent
and predictable, the code that builds that list is now
driven by the order of collectors in default_collectors.py,
and the rest of the code tries to preserve it.

* Flip the loops when building collector names

iterate over the ordered default_collectors list
selecting them for the final list in order instead
of driving it from the unordered collector_names set.

This lets the list returned by select_collector_classes
to stay in the same order as default_collectors.collectors

For collectors that have implicit deps on other fact collectors,
the default collectors can be ordered to include those early.

* default_collectors.py now uses a handful of sub lists of
collectors that can be ordered in default_collectors.collectors.

fixes #30753
fixes #30623

(cherry picked from commit 95abc1d82e4c40832c802253107ad3f9aeebc68d)

2 files changed, 98 insertions, 65 deletions

diff --git a/lib/ansible/module_utils/facts/collector.py b/lib/ansible/module_utils/facts/collector.py
index 4baf5bd338..232c506803 100644
--- a/lib/ansible/module_utils/facts/collector.py
+++ b/lib/ansible/module_utils/facts/collector.py
@@ -203,6 +203,28 @@ def build_fact_id_to_collector_map(collectors_for_platform):
     return fact_id_to_collector_map, aliases_map
 
 
+def select_collector_classes(collector_names, all_fact_subsets, all_collector_classes):
+    # TODO: can be a set()
+    seen_collector_classes = []
+
+    selected_collector_classes = []
+
+    for candidate_collector_class in all_collector_classes:
+        candidate_collector_name = candidate_collector_class.name
+
+        if candidate_collector_name not in collector_names:
+            continue
+
+        collector_classes = all_fact_subsets.get(candidate_collector_name, [])
+
+        for collector_class in collector_classes:
+            if collector_class not in seen_collector_classes:
+                selected_collector_classes.append(collector_class)
+                seen_collector_classes.append(collector_class)
+
+    return selected_collector_classes
+
+
 def collector_classes_from_gather_subset(all_collector_classes=None,
                                          valid_subsets=None,
                                          minimal_gather_subset=None,
@@ -248,19 +270,8 @@ def collector_classes_from_gather_subset(all_collector_classes=None,
                                           aliases_map=aliases_map,
                                           platform_info=platform_info)
 
-    # TODO: can be a set()
-    seen_collector_classes = []
-
-    selected_collector_classes = []
-
-    for collector_name in collector_names:
-        collector_classes = all_fact_subsets.get(collector_name, [])
-
-        # TODO? log/warn if we dont find an implementation of a fact_id?
-
-        for collector_class in collector_classes:
-            if collector_class not in seen_collector_classes:
-                selected_collector_classes.append(collector_class)
-                seen_collector_classes.append(collector_class)
+    selected_collector_classes = select_collector_classes(collector_names,
+                                                          all_fact_subsets,
+                                                          all_collector_classes)
 
     return selected_collector_classes
diff --git a/lib/ansible/module_utils/facts/default_collectors.py b/lib/ansible/module_utils/facts/default_collectors.py
index b5969f14bb..a302393c94 100644
--- a/lib/ansible/module_utils/facts/default_collectors.py
+++ b/lib/ansible/module_utils/facts/default_collectors.py
@@ -73,59 +73,81 @@ from ansible.module_utils.facts.virtual.netbsd import NetBSDVirtualCollector
 from ansible.module_utils.facts.virtual.openbsd import OpenBSDVirtualCollector
 from ansible.module_utils.facts.virtual.sunos import SunOSVirtualCollector
 
-# TODO: make config driven
-collectors = [ApparmorFactCollector,
-              CmdLineFactCollector,
-              DateTimeFactCollector,
-              DistributionFactCollector,
-              DnsFactCollector,
-              EnvFactCollector,
-              FipsFactCollector,
+# these should always be first due to most other facts depending on them
+_base = [
+    PlatformFactCollector,
+    DistributionFactCollector,
+    LSBFactCollector
+]
+
+# These restrict what is possible in others
+_restrictive = [
+    SelinuxFactCollector,
+    ApparmorFactCollector,
+    FipsFactCollector
+]
 
-              HardwareCollector,
-              AIXHardwareCollector,
-              DarwinHardwareCollector,
-              DragonFlyHardwareCollector,
-              FreeBSDHardwareCollector,
-              HPUXHardwareCollector,
-              HurdHardwareCollector,
-              LinuxHardwareCollector,
-              NetBSDHardwareCollector,
-              OpenBSDHardwareCollector,
-              SunOSHardwareCollector,
-              LocalFactCollector,
-              LSBFactCollector,
+# general info, not required but probably useful for other facts
+_general = [
+    PythonFactCollector,
+    SystemCapabilitiesFactCollector,
+    PkgMgrFactCollector,
+    OpenBSDPkgMgrFactCollector,
+    ServiceMgrFactCollector,
+    CmdLineFactCollector,
+    DateTimeFactCollector,
+    EnvFactCollector,
+    SshPubKeyFactCollector,
+    UserFactCollector
+]
 
-              NetworkCollector,
-              AIXNetworkCollector,
-              DarwinNetworkCollector,
-              DragonFlyNetworkCollector,
-              FreeBSDNetworkCollector,
-              HPUXNetworkCollector,
-              HurdNetworkCollector,
-              LinuxNetworkCollector,
-              NetBSDNetworkCollector,
-              OpenBSDNetworkCollector,
-              SunOSNetworkCollector,
+# virtual, this might also limit hardware/networking
+_virtual = [
+    VirtualCollector,
+    DragonFlyVirtualCollector,
+    FreeBSDVirtualCollector,
+    LinuxVirtualCollector,
+    OpenBSDVirtualCollector,
+    NetBSDVirtualCollector,
+    SunOSVirtualCollector,
+    HPUXVirtualCollector
+]
 
-              PkgMgrFactCollector,
-              OpenBSDPkgMgrFactCollector,
-              PlatformFactCollector,
-              PythonFactCollector,
-              SelinuxFactCollector,
-              ServiceMgrFactCollector,
-              SshPubKeyFactCollector,
-              SystemCapabilitiesFactCollector,
-              UserFactCollector,
+_hardware = [
+    HardwareCollector,
+    AIXHardwareCollector,
+    DarwinHardwareCollector,
+    DragonFlyHardwareCollector,
+    FreeBSDHardwareCollector,
+    HPUXHardwareCollector,
+    HurdHardwareCollector,
+    LinuxHardwareCollector,
+    NetBSDHardwareCollector,
+    OpenBSDHardwareCollector,
+    SunOSHardwareCollector
+]
 
-              VirtualCollector,
-              DragonFlyVirtualCollector,
-              FreeBSDVirtualCollector,
-              LinuxVirtualCollector,
-              OpenBSDVirtualCollector,
-              NetBSDVirtualCollector,
-              SunOSVirtualCollector,
-              HPUXVirtualCollector,
+_network = [
+    DnsFactCollector,
+    NetworkCollector,
+    AIXNetworkCollector,
+    DarwinNetworkCollector,
+    DragonFlyNetworkCollector,
+    FreeBSDNetworkCollector,
+    HPUXNetworkCollector,
+    HurdNetworkCollector,
+    LinuxNetworkCollector,
+    NetBSDNetworkCollector,
+    OpenBSDNetworkCollector,
+    SunOSNetworkCollector
+]
 
-              FacterFactCollector,
-              OhaiFactCollector]
+# other fact sources
+_extra_facts = [
+    LocalFactCollector,
+    FacterFactCollector,
+    OhaiFactCollector
+]
+
+# TODO: make config driven
+collectors = _base + _restrictive + _general + _virtual + _hardware + _network + _extra_facts