From 41685fb5165f1c201133eec2a6cfa6c35fcbbe9f Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 1 Nov 2017 11:42:17 -0400 Subject: [PATCH] fix precedence issue with facts and vars (#32302) avoid making gathered facts high precedence, only set_fact is supposed to be. vars set via set_fact with cacheable are higher precedence than plain facts. Previously (after 6fbd0a8bb5d20c6695aa385fcd064bfe23751a1b) regular facts would end up with a higher precedence than host or play vars, but they should not be. Facts were getting added to 'non_persistent_facts' (equivalent to 'register' vars) which is higher precedence than facts should be. added 'cacheable set_facts' to precedence docs 'ansible_facts_cacheable' -> '_ansible_facts_cacheable' (made 'private') --- docs/docsite/rst/playbooks_variables.rst | 4 +++- lib/ansible/plugins/action/set_fact.py | 2 +- lib/ansible/plugins/strategy/__init__.py | 9 ++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/docsite/rst/playbooks_variables.rst b/docs/docsite/rst/playbooks_variables.rst index 9090ab2002..6d5f915986 100644 --- a/docs/docsite/rst/playbooks_variables.rst +++ b/docs/docsite/rst/playbooks_variables.rst @@ -865,7 +865,7 @@ In 2.x, we have made the order of precedence more specific (with the last listed * inventory file or script host vars [2]_ * inventory host_vars/* * playbook host_vars/* - * host facts + * host facts / cached set_facts [3]_ * play vars * play vars_prompt * play vars_files @@ -884,6 +884,8 @@ Basically, anything that goes into "role defaults" (the defaults folder inside t .. [1] Tasks in each role will see their own role's defaults. Tasks defined outside of a role will see the last role's defaults. .. [2] Variables defined in inventory file or provided by dynamic inventory. +.. [3] When created with set_facts's cacheable option, variables will have the high precedence in the play, + but will be the same as a host facts precedence when they come from the cache. .. note:: Within any section, redefining a var will overwrite the previous instance. If multiple groups have the same variable, the last one loaded wins. diff --git a/lib/ansible/plugins/action/set_fact.py b/lib/ansible/plugins/action/set_fact.py index 65f3a8f322..536bb69c6d 100644 --- a/lib/ansible/plugins/action/set_fact.py +++ b/lib/ansible/plugins/action/set_fact.py @@ -54,5 +54,5 @@ class ActionModule(ActionBase): result['changed'] = False result['ansible_facts'] = facts - result['ansible_facts_cacheable'] = cacheable + result['_ansible_facts_cacheable'] = cacheable return result diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 6937e6f723..ae7e1e8d28 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -511,13 +511,12 @@ class StrategyBase: for target_host in host_list: self._variable_manager.set_host_variable(target_host, var_name, var_value) else: - cacheable = result_item.pop('ansible_facts_cacheable', True) + cacheable = result_item.pop('_ansible_facts_cacheable', False) for target_host in host_list: - if cacheable: + if not original_task.action == 'set_fact' or cacheable: self._variable_manager.set_host_facts(target_host, result_item['ansible_facts'].copy()) - - # If we are setting a fact, it should populate non_persistent_facts as well - self._variable_manager.set_nonpersistent_facts(target_host, result_item['ansible_facts'].copy()) + if original_task.action == 'set_fact': + self._variable_manager.set_nonpersistent_facts(target_host, result_item['ansible_facts'].copy()) if 'ansible_stats' in result_item and 'data' in result_item['ansible_stats'] and result_item['ansible_stats']['data']: