diff --git a/changelogs/fragments/fix_set_fact.yml b/changelogs/fragments/fix_set_fact.yml new file mode 100644 index 00000000000..0b739c82096 --- /dev/null +++ b/changelogs/fragments/fix_set_fact.yml @@ -0,0 +1,2 @@ +bugfixes: + - '[set_fact] Corrected and expanded documentation as well as now raise errors that were previously ignored.' diff --git a/lib/ansible/modules/set_fact.py b/lib/ansible/modules/set_fact.py index 0f5300d070b..fe8eadf1a65 100644 --- a/lib/ansible/modules/set_fact.py +++ b/lib/ansible/modules/set_fact.py @@ -11,22 +11,20 @@ __metaclass__ = type DOCUMENTATION = r''' --- module: set_fact -short_description: Set host facts from a task +short_description: Set host variable(s) and fact(s). version_added: "1.2" description: - - This module allows setting new variables. - - Variables are set on a host-by-host basis just like facts discovered by the setup module. - - These variables will be available to subsequent plays during an ansible-playbook run. - - Set C(cacheable) to C(yes) to save variables across executions - using a fact cache. Variables created with set_fact have different precedence depending on whether they are or are not cached. - - Per the standard Ansible variable precedence rules, many other types of variables have a higher priority, so this value may be overridden. - - This module is also supported for Windows targets. + - This action allows setting variables associated to the current host. + - These variables will be available to subsequent plays during an ansible-playbook run via the host they were set on. + - Set C(cacheable) to C(yes) to save variables across executions using a fact cache. + Variables will keep the set_fact precedence for the current run, but will used 'cached fact' precedence for subsequent ones. + - Per the standard Ansible variable precedence rules, other types of variables have a higher priority, so this value may be overridden. options: key_value: description: - - The C(set_fact) module takes key=value pairs as variables to set - in the playbook scope. Or alternatively, accepts complex arguments - using the C(args:) statement. + - "The C(set_fact) module takes ``key=value`` pairs or ``key: value``(YAML notation) as variables to set in the playbook scope. + The 'key' is the resulting variable name and the value is, of course, the value of said variable." + - You can create multiple variables at once, by supplying multiple pairs, but do NOT mix notations. required: true cacheable: description: @@ -41,10 +39,15 @@ options: default: no version_added: "2.4" notes: - - "The C(var=value) notation can only create strings or booleans. - If you want to create lists/arrays or dictionary/hashes use C(var: [val1, val2])." - - Since 'cacheable' is now a module param, 'cacheable' is no longer a valid fact name as of Ansible 2.4. - - This module is also supported for Windows targets. + - Because of the nature of tasks, set_fact will produce 'static' values for a variable. + Unlike normal 'lazy' variables, the value gets evaluated and templated on assignment. + - Some boolean values (yes, no, true, false) will always be converted to boolean type, + unless C(DEFAULT_JINJA2_NATIVE) is enabled. This is done so the C(var=value) booleans, + otherwise it would only be able to create strings, but it also prevents using those values to create YAML strings. + Using the setting will restrict k=v to strings, but will allow you to specify string or boolean in YAML. + - "To create lists/arrays or dictionary/hashes use YAML notation C(var: [val1, val2])." + - Since 'cacheable' is now a module param, 'cacheable' is no longer a valid fact name. + - This action does not use a connection and always executes on the controller. seealso: - module: ansible.builtin.include_vars - ref: ansible_variable_precedence @@ -54,7 +57,7 @@ author: ''' EXAMPLES = r''' -- name: Setting host facts using key=value pairs, note that this always creates strings or booleans +- name: Setting host facts using key=value pairs, this format can only create strings or booleans set_fact: one_fact="something" other_fact="{{ local_var }}" - name: Setting host facts using complex arguments @@ -69,12 +72,18 @@ EXAMPLES = r''' other_fact: "{{ local_var * 2 }}" cacheable: yes -# As of Ansible 1.8, Ansible will convert boolean strings ('true', 'false', 'yes', 'no') -# to proper boolean values when using the key=value syntax, however it is still -# recommended that booleans be set using the complex argument style: -- name: Setting booleans using complex argument style +- name: Creating list and dictionary variables set_fact: - one_fact: yes - other_fact: no + one_dict: + something: here + other: there + one_list: + - a + - b + - c +- name: Creating list and dictionary variables using 'shorthand' YAML + set_fact: + two_dict: {'something': here2, 'other': somewhere} + two_list: [1,2,3] ''' diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index f492c08f05e..9f5103be675 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -89,6 +89,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): * Module parameters. These are stored in self._task.args """ + # does not default to {'changed': False, 'failed': False}, as it breaks async result = {} if tmp is not None: diff --git a/lib/ansible/plugins/action/set_fact.py b/lib/ansible/plugins/action/set_fact.py index d7fe573c1a2..df1c0a43da9 100644 --- a/lib/ansible/plugins/action/set_fact.py +++ b/lib/ansible/plugins/action/set_fact.py @@ -18,6 +18,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +from ansible.errors import AnsibleActionFail from ansible.module_utils.six import iteritems, string_types from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase @@ -37,8 +38,7 @@ class ActionModule(ActionBase): result = super(ActionModule, self).run(tmp, task_vars) del tmp # tmp no longer has any effect - facts = dict() - + facts = {} cacheable = boolean(self._task.args.pop('cacheable', False)) if self._task.args: @@ -46,16 +46,23 @@ class ActionModule(ActionBase): k = self._templar.template(k) if not isidentifier(k): - result['failed'] = True - result['msg'] = ("The variable name '%s' is not valid. Variables must start with a letter or underscore character, and contain only " - "letters, numbers and underscores." % k) - return result + raise AnsibleActionFail("The variable name '%s' is not valid. Variables must start with a letter or underscore character, " + "and contain only letters, numbers and underscores." % k) + # NOTE: this should really use BOOLEANS from convert_bool, but only in the k=v case, + # right now it converts matching explicit YAML strings also when 'jinja2_native' is disabled. if not C.DEFAULT_JINJA2_NATIVE and isinstance(v, string_types) and v.lower() in ('true', 'false', 'yes', 'no'): v = boolean(v, strict=False) facts[k] = v + else: + raise AnsibleActionFail('No key/value pairs provided, at least one is required for this action to succeed') + + if facts: + # just as _facts actions, we don't set changed=true as we are not modifying the actual host + result['ansible_facts'] = facts + result['_ansible_facts_cacheable'] = cacheable + else: + # this should not happen, but JIC we get here + raise AnsibleActionFail('Unable to create any variables with provided arguments') - result['changed'] = False - result['ansible_facts'] = facts - result['_ansible_facts_cacheable'] = cacheable return result