Proposal for fixing playbooks with dynamic include problems

[James Cammarata]

Hi all, we've been working on a solution over the last few weeks to address the problems introduced by making includes dynamic in 2.0, and would like some community feedback on my ideas.

As you're all probably well aware by now, we moved task-level includes to be completely dynamic in 2.0 to make doing things like loops easier. In 1.9.x and before, includes functioned like pre-proccessor statements - the files were parsed, turned into a list of tasks, and inserted into the main list of tasks at the time the main playbook or role was parsed.

The downside to this move was to make it quite difficult to handle certain situations, as we now know nothing about those files (and specifically what tasks are in those files) until we encounter them in the regular execution of the playbook. As such, we don' t know what tags are there, and when notifying handlers the task names aren't known so the notifications fail.

To address this shortcoming, our idea was simple: allow includes to be marked as static, and as such they again function as pre-processor statements. Here is the feature branch where we implement this change:

https://github.com/ansible/ansible/compare/static_includes

In a nutshell, there are now two ways to make includes behave as they did in 1.9.x:

1. Use the `static: true` option on the include.

2. Set options in your ansible.cfg. There is one option each for regular tasks, and one for handlers, as some may wish to make all handler includes static without impacting those in regular task sections.

Example:

- hosts: all

  gather_facts: yes

  tasks:

  - include: foo.yml

    static: yes

Or, if you're migrating from 1.9.x (or earlier) to 2.0 and want all of your includes to work as they did before, add the following two options to your ansible.cfg in the [defaults] section:

task_includes_static = yes

handler_includes_static = yes

The caveat to all of this is of course that using loops on a static include will no longer work. Also, any playbook marked as static using variables must have those variables available at compile time (no inventory sources are available at this point).

We're looking at merging this in for the 2.1 release (targeting an April release), so any comments/ideas for making it work better are appreciated. If you've avoided the 2.0 release because of the impact of dynamic includes, we'd really love to hear how your playbooks work (or don't) when using this feature branch with dynamic includes disabled completely.

Thanks!

James Cammarata

Ansible Lead/Sr. Principal Software Engineer

Ansible by Red Hat

twitter: @thejimic, github: jimi-c

[Rene Moser]

Hi

I make it short, the solutions makes sense I am +1 for your solution.

René

> --
> You received this message because you are subscribed to the Google
> Groups "Ansible Development" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to ansible-deve...@googlegroups.com
> <mailto:ansible-deve...@googlegroups.com>.
> For more options, visit https://groups.google.com/d/optout.

[Strahinja Kustudić]

I like the idea, but I don't like the proposal. I would do the same thing, but the other way around. With your proposal it would break backward compatibility with 1.9 if you don't update all your includes, or update ansible.ini. Also most of the includes are static anyway (since dynamic ones didn't work pre 2.0), so why not make them all static by default.

My proposal is to make all includes static by default and introduce a new keyword dynamic: yes (or use static: no if you like it more) which you would set on includes where you need them to be dynamic. This would make old 1.9 playbooks backward compatible without any changes anywhere, and if you need a dynamic include in a 2.0 playbook, you will need to change it in just a few places (since there are far less 2.0 playbooks anyway and even lower number of dynamic includes).

[James Cammarata]

On Mon, Mar 14, 2016 at 8:22 PM, Strahinja Kustudić <strah...@nordeus.eu> wrote:

I like the idea, but I don't like the proposal. I would do the same thing, but the other way around. With your proposal it would break backward compatibility with 1.9 if you don't update all your includes, or update ansible.ini. Also most of the includes are static anyway (since dynamic ones didn't work pre 2.0), so why not make them all static by default.

I went this way to avoid introducing problems with those who have already adopted 2.0. We discussed this internally in depth, and flipping the defaults would break everyone who was using 2.0 and upgrading to 2.1. We feel at this point that most people who have not upgraded are avoiding doing so because of backwards incompatibilities with 2.0, so this is mainly targeted at giving them an upgrade path.

 

My proposal is to make all includes static by default and introduce a new keyword dynamic: yes (or use static: no if you like it more) which you would set on includes where you need them to be dynamic. This would make old 1.9 playbooks backward compatible without any changes anywhere, and if you need a dynamic include in a 2.0 playbook, you will need to change it in just a few places (since there are far less 2.0 playbooks anyway and even lower number of dynamic includes).

Again, this is something we discussed internally, the option name is flexible and we can flip it. However for the reasons above we'd default it to `dynamic: yes`.

[Mark Kusch]

What issues are there with dynamic includes except for included files
which do not do anything?

If there was such a huge problem to discuss this, I wouldn't go for a
change in behaviour of include itself but add a new plugin, e.g.
static_include, or include_static.

Inline...

# kraM

On 21:57 Mon 14 Mar, James Cammarata wrote:
> On Mon, Mar 14, 2016 at 8:22 PM, Strahinja Kustudić <strah...@nordeus.eu>
> wrote:
>
> > I like the idea, but I don't like the proposal. I would do the same thing,
> > but the other way around. With your proposal it would break backward
> > compatibility with 1.9 if you don't update all your includes, or update
> > ansible.ini. Also most of the includes are static anyway (since dynamic
> > ones didn't work pre 2.0), so why not make them all static by default.
> >
>
> I went this way to avoid introducing problems with those who have already
> adopted 2.0. We discussed this internally in depth, and flipping the
> defaults would break everyone who was using 2.0 and upgrading to 2.1. We
> feel at this point that most people who have not upgraded are avoiding
> doing so because of backwards incompatibilities with 2.0, so this is mainly
> targeted at giving them an upgrade path.

Upgraded everything to 2.0.0.2, currently upgrading everything to 2.0.1.0
(deprecation warnings not visible due to a bug in 2.0.0.2). If we need to
update everything for 2.1.0, ... Where would that lead to?

>
>
> > My proposal is to make all includes static by default and introduce a new

> > keyword *dynamic: yes* (or use *static: no* if you like it more) which

> > you would set on includes where you need them to be dynamic. This would
> > make old 1.9 playbooks backward compatible without any changes anywhere,
> > and if you need a dynamic include in a 2.0 playbook, you will need to
> > change it in just a few places (since there are far less 2.0 playbooks
> > anyway and even lower number of dynamic includes).
> >
>
> Again, this is something we discussed internally, the option name is
> flexible and we can flip it. However for the reasons above we'd default it
> to `dynamic: yes`.
>

> --
> You received this message because you are subscribed to the Google Groups "Ansible Development" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to ansible-deve...@googlegroups.com.

> For more options, visit https://groups.google.com/d/optout.

--
kraM

Think can be useful if someone is needing some thinking about something.

gpg --recv-keys 0x6C215719

[James Cammarata]

On Tue, Mar 15, 2016 at 1:19 AM, Mark Kusch <mark....@gmail.com> wrote:

What issues are there with dynamic includes except for included files
which do not do anything?

Because we do not load the included file until we reach that point in the execution, we no longer know anything about the tasks contained within that file. This means that we don't know about tags on those tasks, and notifying handlers within those files do not work either (which breaks a common use case for many people).

[Strahinja Kustudić]

James do you have an adoption rate of 2.0 compared to older versions? 2.0 is out for just a few months and older versions which use static includes have been out for years now and people are used to them working that way. But more importantly how many of those who switched to 2.0 are really using dynamic includes, so how many playbooks would really get broken? I guess most of the people using 2.0 had more trouble with dynamic includes than fun.

You should also think of the new users, since 2.0 basically broke the include statement and now in 2.1 you want to leave it broken by default. So future users will now always have to write static: yes for every include (except the dynamic ones, which are far less used) if they wanted their plays to behave as they expect them to work. Wouldn't the documentation be much easier for new users if the first time you mention a dynamic include is when you actual need it and not the first time you show them a simple include. So basically this doc page http://docs.ansible.com/ansible/playbooks_roles.html#task-include-files-and-encouraging-reuse, which is one of the first pages new users read, will have to be changed so that it adds static: yes and you would need to explain the concept of a static/dynamic include to a new user, who just learned that there is an include statement. But if you left them to be static by default, then you would need to change basically one part of the documentation and mention it the first time you need a dynamic one which is here http://docs.ansible.com/ansible/playbooks_loops.html#loops-and-includes, which as we all know is far less used than a static one and by the time the user reaches loops, they are already using Ansible.

Also what exactly is a dynamic include? I thought it is only used when you use include + with_items and when using when + include on playbook, or there are more situations when an include is useful to be dynamic?

I have one more idea/question. Would it be possible for Ansible to know all the situations when it should use a dynamic include, so that it automatically treats it as a dynamic one and threats the rest as static?

[Strahinja Kustudić]

On Tue, Mar 15, 2016 at 7:12 AM, James Cammarata <jcamm...@ansible.com> wrote:

Because we do not load the included file until we reach that point in the execution, we no longer know anything about the tasks contained within that file. This means that we don't know about tags on those tasks, and notifying handlers within those files do not work either (which breaks a common use case for many people).

Your reply basically sums up my answer above. Include was broken with 2.0 for a common use case for many people and now you would like to fix it by making all those people change their playbooks for it to work as they expected it to work in the first place, because of an uncommon use case which didn't even work in older versions.

[haad]

Hi,

Wouldn't be easier to just preproccess those files at beginning of playbook run (so you will evaluate everything to true and load all includes/plays/roles into memory at beginning). So you know what tags/handlers are inside and when that's done you just execute tasks one by one ?

On Tue, Mar 15, 2016 at 7:12 AM, James Cammarata <jcamm...@ansible.com> wrote:

--
You received this message because you are subscribed to the Google Groups "Ansible Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ansible-deve...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Regards.

Adam

[Udondan]

I also agree it would have made more sense if the dynamic include was optional and the default behavior was the version 1 way. But I also see how this is hard to justify now that Ansible 2 works as it does, it's hard to go back. If Ansible follows semver that would actually mean we have Ansible 3 knocking on our door if the behavior is switched back. 

Ideally, in my opinion, the new functionality would have been added with a different name, for example "import" instead of "include". This would have allowed a clean way to migrate without breaking things. It also makes it possible to distinguish between two complete different features by name.

We have adopted Ansible 2 but I would not mind if the behavior changed back to the old static include. In any case, I will adjust our roles. Most of the includes do not need to be dynamic and I would set them to static just to get rid of the hundreds blue include statements during playtime.

Now, I'm wondering what happens if a static include includes a file which has a dynamic include. :)

[James Cammarata]

No, we can't do this because of variables and the possibility that they will do loops (which may also be based on a variable).

Take for exampe:

- include: "{{ansible_distribution|lower}}.yml"

^ This is completely valid in 2.0, and a much nicer alternative to the include/when syntax you used to have to do. However, we don't know which files will be brought in until that's actually run, so we can't pre-process anything (unless we also read in and parse every yaml file in the playbook/roles path, which presents other problems).

James Cammarata

Ansible Lead/Sr. Principal Software Engineer

Ansible by Red Hat

twitter: @thejimic, github: jimi-c

[James Cammarata]

On Tue, Mar 15, 2016 at 8:10 AM, Udondan <deem...@googlemail.com> wrote:

I also agree it would have made more sense if the dynamic include was optional and the default behavior was the version 1 way. But I also see how this is hard to justify now that Ansible 2 works as it does, it's hard to go back. If Ansible follows semver that would actually mean we have Ansible 3 knocking on our door if the behavior is switched back. 

Ideally, in my opinion, the new functionality would have been added with a different name, for example "import" instead of "include". This would have allowed a clean way to migrate without breaking things. It also makes it possible to distinguish between two complete different features by name.

Agreed, hind sight is 20/20. That is also something we considered, however again at this point it would break playbooks as written for 2.0.
 

We have adopted Ansible 2 but I would not mind if the behavior changed back to the old static include. In any case, I will adjust our roles. Most of the includes do not need to be dynamic and I would set them to static just to get rid of the hundreds blue include statements during playtime.

Now, I'm wondering what happens if a static include includes a file which has a dynamic include. :) 

Thanks for this, this is something I need to test with my feature branch. I don't think I recursively parse tasks brought in from static includes...

[James Cammarata]

No, the entire point of the config options is to allow users to switch to 2.0 without having to change their playbooks when moving from 1.x. Include loops were not an uncommon use-case, in the 1.5 years or so since we removed the capability, that was one of the most often requested features, which is why we implemented dynamic includes in 2.0 (to make them actually work like people expected).

[Jan Grant]

I certainly don't want to see dynamic includes go away.

Assuming the preprocessing and running code is neatly split, how about:

1. Preprocess "static" includes - which can be identified by the absence of J2 constructs in them
2. Dynamically preprocess dynamic includes and run them as currently.
3. Add a "dynamic: yes" flag which optionally adds the dynamic behaviour for things that look like static includes.

[James Cammarata]

On Tue, Mar 15, 2016 at 10:12 AM, Jan Grant <ja...@ioctl.org> wrote:

I certainly don't want to see dynamic includes go away.

They're definitely not going to go away :)

 

Assuming the preprocessing and running code is neatly split, how about:

1. Preprocess "static" includes - which can be identified by the absence of J2 constructs in them
2. Dynamically preprocess dynamic includes and run them as currently.
3. Add a "dynamic: yes" flag which optionally adds the dynamic behaviour for things that look like static includes.

#1 is something which can be done pretty easily, if there's no loop and the file name contains no variables. For #3 though, I went the other way because to me the more tricky thing to figure out is when you see something like my earlier example, when you've got `- include: "{{some_var}}.yml"`, and you want to force it to be pre-processed instead of dynamic. This is why I picked the `static: yes` syntax instead of the converse.

I can look at tweaking my feature branch to do the above, though this is why I added config options. Using variables in the include name is valid in 1.x, so to 2.0 those would look like dynamic includes (as they should, because now the variable COULD come from an inventory source as well).

[James Cammarata]

Just pushed an update to my feature branch to implement #1 above:

https://github.com/ansible/ansible/compare/static_includes#diff-69130db4f6e9ef75876cab5ca756d9d6R118

[Strahinja Kustudić]

So basically now simple includes without any loops or variables in file name will be static by default? That is great news, since most of the includes are like that and it makes them at least backward compatible with 1.x.

Could anyone tell me what exactly is the difference between:

- include: "{{ some_var }}.yml"
  static: yes

and

- include: "{{ some_var }}.yml"
  static: no

Does it mean for static: yes that some_var would need to be defined statically as a var, and for static: no some_var could be defined during play execution, e.g. set_fact: some_var=something.

[James Cammarata]

On Tue, Mar 15, 2016 at 11:03 AM, Strahinja Kustudić <strah...@nordeus.eu> wrote:

So basically now simple includes without any loops or variables in file name will be static by default? That is great news, since most of the includes are like that and it makes them at least backward compatible with 1.x.

Could anyone tell me what exactly is the difference between:

- include: "{{ some_var }}.yml"
  static: yes

In this case, the include would be processed at playbook parsing time, meaning there is no inventory. As such, `some_var` would have to be available in playbook vars (vars/vars_files/role vars and defaults), or extra vars. Tasks and all related info would be available to things like --star-at-task, --list-tags, and --list-tasks.

 

and

- include: "{{ some_var }}.yml"
  static: no

Does it mean for static: yes that some_var would need to be defined statically as a var, and for static: no some_var could be defined during play execution, e.g. set_fact: some_var=something.

In this case, the include would not be processed until that task was hit at execution time, meaning `some_var` could come from any and all available variable sources in ansible. Nothing about the tasks in this file would be known at compile time, and as such that information would not be available for the CLI switches mentioned above.

[Timothy Appnel]

@jimi-c: Is it possible to evaluate static includes after the setup module runs to gathering facts? The most common use pattern I see in users plays and roles are dynamic includes that define the filename based on a fact like ansible_os_family. Collecting the fact before evaluating includes would make things even more seamless for users.

Timothy Appnel
Principal Architect
Ansible by Red Hat

+1 718.404.6429 | ansible.com 

GitHub: tima

Twitter: @appnelgroup

[James Cammarata]

In theory, yes we could, however that wouldn't buy us much. The problem is that, if the include uses a variable for the file name or loop, that variable may change of the execution of the playbook. This is particularly common for loops, where some previously registered result is used for the loop variable to an include.

James Cammarata

Ansible Lead/Sr. Principal Software Engineer

Ansible by Red Hat

twitter: @thejimic, github: jimi-c

[Timothy Appnel]

So many things are broken with Ansible's notion of "composability" that it's hard to know where to begin.

https://github.com/ansible/proposals/pull/4