enable module defaults for implicit gather_facts (#76053)

* deprecated default fact path setting
* subset and timeout on same boat
* avoid pushing module args when none

Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com>

changelogs/fragments/deprecate_default_fact_path_setting.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - setting DEFAULT_FACT_PATH is being deprecated in favor of the generic module_defaults keyword

lib/ansible/config/base.yml

@@ -609,16 +609,23 @@ DEFAULT_EXECUTABLE:
   - {key: executable, section: defaults}
 DEFAULT_FACT_PATH:
   name: local fact path
-  default: ~
   description:
     - "This option allows you to globally configure a custom path for 'local_facts' for the implied M(ansible.builtin.setup) task when using fact gathering."
     - "If not set, it will fallback to the default from the M(ansible.builtin.setup) module: ``/etc/ansible/facts.d``."
     - "This does **not** affect  user defined tasks that use the M(ansible.builtin.setup) module."
+    - The real action being created by the implicit task is currently M(ansible.legacy.gather_facts), which then calls the configured fact modules,
+      by default this will be M(ansible.builtin.setup) for POSIX systems but other platforms might have different defaults.
   env: [{name: ANSIBLE_FACT_PATH}]
   ini:
   - {key: fact_path, section: defaults}
   type: string
-  yaml: {key: facts.gathering.fact_path}
+  # TODO: deprecate in 2.14
+  #deprecated:
+  #    # TODO: when removing set playbook/play.py to default=None
+  #    why: the module_defaults keyword is a more generic version and can apply to all calls to the
+  #         M(ansible.builtin.gather_facts) or M(ansible.builtin.setup) actions
+  #    version: "2.18"
+  #    alternatives: module_defaults
 DEFAULT_FILTER_PLUGIN_PATH:
   name: Jinja2 Filter Plugins Path
   default: ~/.ansible/plugins/filter:/usr/share/ansible/plugins/filter
@@ -665,7 +672,6 @@ DEFAULT_GATHERING:
   choices: ['smart', 'explicit', 'implicit']
 DEFAULT_GATHER_SUBSET:
   name: Gather facts subset
-  default: ['all']
   description:
       - Set the `gather_subset` option for the M(ansible.builtin.setup) task in the implicit fact gathering.
         See the module documentation for specifics.
@@ -676,17 +682,29 @@ DEFAULT_GATHER_SUBSET:
       section: defaults
   version_added: "2.1"
   type: list
+  # TODO: deprecate in 2.14
+  #deprecated:
+  #    # TODO: when removing set playbook/play.py to default=None
+  #    why: the module_defaults keyword is a more generic version and can apply to all calls to the
+  #         M(ansible.builtin.gather_facts) or M(ansible.builtin.setup) actions
+  #    version: "2.18"
+  #    alternatives: module_defaults
 DEFAULT_GATHER_TIMEOUT:
   name: Gather facts timeout
-  default: 10
   description:
-    - Set the timeout in seconds for the implicit fact gathering.
+    - Set the timeout in seconds for the implicit fact gathering, see the module documentation for specifics.
     - "It does **not** apply to user defined M(ansible.builtin.setup) tasks."
   env: [{name: ANSIBLE_GATHER_TIMEOUT}]
   ini:
-  - {key: gather_timeout, section: defaults}
+  - {key: gather_tim8out, section: defaults}
   type: integer
-  yaml: {key: defaults.gather_timeout}
+  # TODO: deprecate in 2.14
+  #deprecated:
+  #    # TODO: when removing set playbook/play.py to default=None
+  #    why: the module_defaults keyword is a more generic version and can apply to all calls to the
+  #         M(ansible.builtin.gather_facts) or M(ansible.builtin.setup) actions
+  #    version: "2.18"
+  #    alternatives: module_defaults
 DEFAULT_HASH_BEHAVIOUR:
   name: Hash merge behaviour
   default: replace

lib/ansible/executor/play_iterator.py

@@ -177,30 +177,28 @@ class PlayIterator(metaclass=MetaPlayIterator):
         self._blocks = []
         self._variable_manager = variable_manager
 
-        # Default options to gather
-        gather_subset = self._play.gather_subset
-        gather_timeout = self._play.gather_timeout
-        fact_path = self._play.fact_path
-
         setup_block = Block(play=self._play)
         # Gathering facts with run_once would copy the facts from one host to
         # the others.
         setup_block.run_once = False
         setup_task = Task(block=setup_block)
         setup_task.action = 'gather_facts'
+        # TODO: hardcoded resolution here, but should use actual resolution code in the end,
+        #       in case of 'legacy' mismatch
+        setup_task.resolved_action = 'ansible.builtin.gather_facts'
         setup_task.name = 'Gathering Facts'
-        setup_task.args = {
+        setup_task.args = {}
-            'gather_subset': gather_subset,
-        }
 
         # Unless play is specifically tagged, gathering should 'always' run
         if not self._play.tags:
             setup_task.tags = ['always']
 
-        if gather_timeout:
+        # Default options to gather
-            setup_task.args['gather_timeout'] = gather_timeout
+        for option in ('gather_subset', 'gather_timeout', 'fact_path'):
-        if fact_path:
+            value = getattr(self._play, option, None)
-            setup_task.args['fact_path'] = fact_path
+            if value is not None:
+                setup_task.args[option] = value
+
         setup_task.set_loader(self._play._loader)
         # short circuit fact gathering if the entire playbook is conditional
         if self._play._included_conditional is not None:

lib/ansible/playbook/play.py

@@ -58,6 +58,8 @@ class Play(Base, Taggable, CollectionSearch):
 
     # Facts
     _gather_facts = FieldAttribute(isa='bool', default=None, always_post_validate=True)
+
+    # defaults to be deprecated, should be 'None' in future
     _gather_subset = FieldAttribute(isa='list', default=(lambda: C.DEFAULT_GATHER_SUBSET), listof=string_types, always_post_validate=True)
     _gather_timeout = FieldAttribute(isa='int', default=C.DEFAULT_GATHER_TIMEOUT, always_post_validate=True)
     _fact_path = FieldAttribute(isa='string', default=C.DEFAULT_FACT_PATH)