Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add variable evaluation notes to docs #39302

Closed
wants to merge 3 commits into from

Conversation

maxamillion
Copy link
Contributor

Signed-off-by: Adam Miller admiller@redhat.com

SUMMARY

Docs update to explain variable evaluation

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

docs

ANSIBLE VERSION
ansible 2.6.0 (docs 9a20967402) last updated 2018/04/25 10:31:49 (GMT -500)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/admiller/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/admiller/src/dev/ansible/lib/ansible
  executable location = /home/admiller/src/dev/ansible/bin/ansible
  python version = 2.7.14 (default, Mar 14 2018, 13:36:31) [GCC 7.3.1 20180303 (Red Hat 7.3.1-5)]

Signed-off-by: Adam Miller <admiller@redhat.com>
@ansibot
Copy link
Contributor

ansibot commented Apr 25, 2018

@ansibot ansibot added docs This issue/PR relates to or includes documentation. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Apr 25, 2018
@@ -0,0 +1,30 @@
.. _variable_evaluation:

Variable Evaluation

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My impression is that this topic is more about the lazy evaluation of templates rather than variables

In general, Ansible evaluates any variables in playbook content at the last
possible second, which means that if you define a data structure that data
structure itself can define variable values within it, and everything “just
works” as you would expect. This also means variable strings can include other

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with the phrase "just works" as you would expect: it is subjective and does not illuminate the actual behaviour. In my case, the fact that lazy evaluation is used came as a complete surprise.

b: 3


When using an Ansible :ref:`Lookup Plugin`, do note that the value will be

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph seems to conflate at least two important points which may merit separate paragraphs:

  1. lazy template evaluation has the knock-on effect that lookup plugins may be executed more than once, take care if these are not idempotent or if they have side effects
  2. set_fact is a mechanism by which you can "fix" template evaluation at a point in time

var with the result of the Lookup Plugin be stored in the variable, effectively
resulting in a single evaluation of the variable.

::

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be useful to define more precisely the point(s) at which template evaluation is carried out, perhaps e.g. "templates passed in as arguments to module invocations are fully evaluated at the point of invocation" (which is why set_fact "fixes" template values)

@ansibot
Copy link
Contributor

ansibot commented Apr 25, 2018

The test ansible-test sanity --test docs-build [explain] failed with 1 error:

docs/docsite/rst/shared_snippets/variable_evaluation.rst:4:0: duplicate-label: duplicate label variable_evaluation, other instance in /root/ansible/docs/docsite/rst/user_guide/playbooks_lookups.rst

The test ansible-test sanity --test no-smart-quotes [explain] failed with 2 errors:

docs/docsite/rst/shared_snippets/variable_evaluation.rst:8:71: use ASCII quotes `'` and `"` instead of Unicode quotes
docs/docsite/rst/shared_snippets/variable_evaluation.rst:9:6: use ASCII quotes `'` and `"` instead of Unicode quotes

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Apr 25, 2018
@jborean93 jborean93 removed the needs_triage Needs a first human triage before being processed. label Apr 26, 2018
Signed-off-by: Adam Miller <admiller@redhat.com>
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Apr 26, 2018
@ansibot
Copy link
Contributor

ansibot commented Apr 26, 2018

The test ansible-test sanity --test no-smart-quotes [explain] failed with 2 errors:

docs/docsite/rst/reference_appendices/glossary.rst:481:79: use ASCII quotes `'` and `"` instead of Unicode quotes
docs/docsite/rst/reference_appendices/glossary.rst:482:14: use ASCII quotes `'` and `"` instead of Unicode quotes

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Apr 26, 2018
Signed-off-by: Adam Miller <admiller@redhat.com>
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Apr 26, 2018
@maxamillion
Copy link
Contributor Author

ping @bcoca - would you mind responding to the comments from @jim-minter, I don't think I know the vars code well enough to properly answer a couple of the outlying questions.

@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels May 4, 2018

When using an Ansible :ref:`Lookup Plugin`, do note that the value will be
evaluated every time the variable is expanded in a template or in a playbook
unless it is explicitly used with ``set_fact`` which will evaluate and define
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont put set_fact here, explain below that how it works as it is a task option

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but I don't follow ... what do you mean "how it works as it is a task option"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

friendly nag: ping @bcoca

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that 'set_fact' is not magic onto itself, the reason it works is because module options are evaluated

@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 22, 2018
@ansibot
Copy link
Contributor

ansibot commented Aug 3, 2018

@maxamillion maxamillion deleted the docs branch August 14, 2018 20:54
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 docs This issue/PR relates to or includes documentation. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants