Default ansible_managed to a static string #17947

Closed
abadger opened this Issue Oct 8, 2016 · 11 comments

Projects

None yet

5 participants

@abadger
Member
abadger commented Oct 8, 2016
ISSUE TYPE
  • Feature Idea
COMPONENT NAME

Various templates

ANSIBLE VERSION
devel, 2.3, 2.2
SUMMARY

Using the default values of "{{ ansible_managed }}" in a template file leads to the file being changed. This is due to the variables used in the default definition of ansible_managed:

  • The date replacement leads to it being changed whenever the date of the template's source is different.
    • This can happen when playbooks are stored in git as git does not store file timestamps. Each checkout creates files from that date. In a multi-admin setup, it is common for the playbooks to be shared with other admins via git.
  • The uid replacement leads to it being changed anytime the playbook is run from a different account.
    • Running ansible does not require a single admin account because of its integration with sudo and su. But this portion of the string hampers that because template will report changed each time a different account is used.
  • The host replacement leads it to being changed whenever the playbook is run from a different controller.
    • Ansible is useful to run playbooks from each admin's laptop or workstation rather than having a single central master. This setting makes it so templates cannot be used idempotently in that manner.
  • The file replacement leads to the file being changed whenever the file is substituted from a different path.
    • When ansible gets its playbooks from a git checkout in each admin's home directory the paths to the template file will be different. Thus each admin will get a change in the file.
STEPS TO REPRODUCE

Run the following playbook twice to show the first problem with date. Note that in the real-world, the change in timestamp would come from using a fresh git checkout or a user on another machine, not from state='touch' in the playbook:


---
- hosts: localhost
  gather_facts: False
  tasks:
  - file:
      path: "/var/tmp/not-idempotent.j2"
      state: "touch"
  - copy:
      content: "{{'{{ ansible_managed }}\nSome content\n'}}"
      dest: "/var/tmp/not-idempotent.j2"
  - template:
      src: "/var/tmp/not-idempotent.j2"
      dest: "/var/tmp/not-idempotent"

You can take out the file: task and run the playbook as another user or on a different control host to see the problems with uid and host respectively.

EXPECTED RESULTS

The second run of the playbook should show no changes for the template task.

ACTUAL RESULTS

The template task shows changes every single run:

TASK [template] ****************************************************************
changed: [localhost]
@abadger
Member
abadger commented Oct 8, 2016

This is a summary of #5317 which has been closed for a long time.

@abadger
Member
abadger commented Oct 8, 2016 edited

Updated: two alternate solutions. Some of the variable expansions could be bugs

Solution One:

  • Change the default value of ansible_managed in examples/ansible.cfg and lib/ansible/constants.py to "Ansible Managed: {file} on {host}".
  • Change file to refer to the destination file and host to refer to the inventory_hostname
  • In ansible.cfg, leave the long form of the value as an additional comment and a warning that the various variables can lead to non-idempotent template behaviour.
  • In docsite/rst/intro_configuration.rst change the default to "Ansible Managed: {file} on {host}" and list the uid and date expansions along with the scenarios where they will lead to non-idempotent behaviour as listed above.

Solution Two:

  • Deprecate and eventually remove ansible_managed.
  • Document how users can implement equivalent functionality by adding an ansible_managed group var.
@jsumners
Contributor
jsumners commented Oct 8, 2016

Chiming in to say that it is very annoying to have been following an issue (#5317) for months only to have it closed and locked without a resolution while being told to follow a new issue.

@abadger
Member
abadger commented Oct 8, 2016 edited

@jsumners -- The issue you refer to was closed here: #5317 (comment) (December of 2013). No one is reading closed issues and they don't come up in any of our reports, etc. So I locked it with a message to look here rather than leaving commenting open and people posting new information with the false expectation that it will be seen by people who can take action on it. Sorry for the confusion.

@abadger
Member
abadger commented Oct 8, 2016

Talked about this with @bcoca who suggested that since none of the variables are able to work idempotently we should deprecate and remove this feature and add a recipe to the documentation telling users how to implement ansible_managed via inventory variables:

[all:vars]
ansible_managed="This file is managed by Ansible"

bcoca also said that some of the variables seem to point to things that he thought they weren't supposed to:

  • He thinks host should expand to the inventory_hostname rather than the controller hostname
  • He thinks that file should expand to the destination file rather than the template file.
@jsumners
Contributor
jsumners commented Oct 8, 2016

I'd much rather keep the feature as it is than to lose it altogether.

@abadger
Member
abadger commented Oct 8, 2016

@jsumners how do you use it and what does it add that adding ansible_managed as an inventory var would not?

@sumpfralle

@abadger: I think, the source of confusion regarding #5317 was the combination of a closed ticket, that is not locked but with no way of re-opening it. I am not sure, if this is github's default setup, but it seems to me that closed issues that cannot be re-opened (by non-members) should be locked by default.
Anyway: thank you for taking the time to summarize the discussion of #5317 in detail.

@jsumners
Contributor
jsumners commented Oct 8, 2016

@abadger my default ansible_managed is set to Ansible managed, do not edit directly: last update by {uid} on %Y-%m-%d. As far as I know, those format tokens will not be replaced in a regular inventory variable.

Plus, that would mean I, and probably a whole lot of other people, would have to go back and add the new inventory variable to all of our inventories to fix behavior that has been there for a long time.

@robinro
Contributor
robinro commented Oct 10, 2016

I would prefer solution 1 of #17947 (comment).
I agree idempotency should be the default, so the default value should be constant, but removing the variable would unnecessarily break quite a few existing playbooks, see e.g. 25k results at https://github.com/search?q=ansible_managed&type=Code

@abadger
Member
abadger commented Oct 19, 2016

Okay, at least for 2.2 we went with the original version of this:

Change ansible-managed to a static string. In this case, "Ansible managed".

This lets people continue to set it to something else that isn't idempotent in their config files. I'll update the documentation to give information on this at some point soon.

@abadger abadger added a commit to abadger/ansible that referenced this issue Oct 19, 2016
@abadger abadger Update the ansible_managed documentation.
* New default (a static string)
* Explanation of all the fields and how they impact idempotence

Fixes #17947
cfca71e
@abadger abadger self-assigned this Oct 19, 2016
@abadger abadger added the in progress label Oct 19, 2016
@jimi-c jimi-c removed the in progress label Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment