Proposal for fixing playbooks with dynamic include problems

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!

Hi

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

René

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).

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`.

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

> 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?

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).

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?

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.

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 ?

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. :slight_smile:

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).

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. :slight_smile:

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...

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).

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.

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

They're definitely not going to go away :slight_smile:

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).

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

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

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.

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.

@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.

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.