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

Explain use of pre_tasks #207

Closed
gaddman opened this issue Feb 20, 2020 · 8 comments
Closed

Explain use of pre_tasks #207

gaddman opened this issue Feb 20, 2020 · 8 comments

Comments

@gaddman
Copy link

@gaddman gaddman commented Feb 20, 2020

Book version: 1.21
Chapter 4 introduces pre_tasks (page 63) with a brief explanation that they're run before the main set of tasks. It's not clear (at this point in the book anyway) how that's any different to just adding a task at the beginning of the tasks section.
Assuming the reasoning is covered later in the book, it would be useful to reference that section, or provide a simple explanation at this point of the book.

@geerlingguy

This comment has been minimized.

Copy link
Owner

@geerlingguy geerlingguy commented Feb 26, 2020

@gaddman - Good point; if you don't know the context (especially how roles are run prior to tasks) it might seem arbitrary. I've made the line read this way to hopefully clear that up:

Ansible lets you run tasks before or after the main tasks (defined in tasks:) or roles (defined in roles:—we'll get to roles later) using pre_tasks and post_tasks, respectively.

@geerlingguy

This comment has been minimized.

Copy link
Owner

@geerlingguy geerlingguy commented Feb 26, 2020

This will be included in the next batch of book updates.

@ssbarnea

This comment has been minimized.

Copy link

@ssbarnea ssbarnea commented Feb 27, 2020

To be honest I plan to deprecate pre/post tasks in ansible-lint as I see no use for them since include_role was created.

I would be very happy if you would avoid using them in your book. Obsolete syntax.

If you wonder why, think about: task, role, task, role,... not possible with old style.

@geerlingguy

This comment has been minimized.

Copy link
Owner

@geerlingguy geerlingguy commented Feb 27, 2020

@ssbarnea - pre_ and post_ tasks are extremely useful, and I avoid using include_role as much as possible for two reasons:

  1. Most playbooks work with pre_tasks (prep for roles), then roles, then that's it (very little to write, don't have to have a bunch of redundant include_role: lines when you can just list the roles like - role1, - role2).
  2. Almost every automation yaml format I use (like Travis CI, Jenkinsfile, etc.) with 'build stages' uses a similar set of concepts (e.g. pre/post install, pre/post run, pre/post whatever), and even though it's not strictly needed, it is useful IMO to separate out "prep" stuff from the "main" stuff (e.g. prep is updating apt cache, main is installing and configuring stuff, post is cleanup, like delete archives or caches that aren't needed).
@geerlingguy

This comment has been minimized.

Copy link
Owner

@geerlingguy geerlingguy commented Feb 27, 2020

As an illustration (pseudo-playbook):

- hosts: all

  pre_tasks:
    - name: Update apt cache.

  roles:
    - accounts
    - security
    - webserver
    - app

versus:

- hosts: all

  tasks:
    - name: Update apt cache.
    - include_role: accounts
    - include_role: security
    - include_role: webserver 
    - include_role: app

The playbook is shorter, but the layout is all compacted (which I don't particularly like), and now I have an extra 48 characters that make reading the playbook more annoying.

@dglinder

This comment has been minimized.

Copy link
Contributor

@dglinder dglinder commented Feb 27, 2020

I too like the pre_tasks/post_tasks but you can achieve the same "look" using loops:

- hosts: all

  tasks:
    - name: Update apt cache.
    - include_role:
         name: "{{ item }}"
      loop:
        - accounts
        - security
        - webserver
        - app

This also gives back the less-compact view.

You can also use the "name:" field in the include_roles to expand them for readability too:

  tasks:
    - name: Update apt cache.
    - include_role:
        name: accounts
    - include_role:
        name: security
    - include_role:
        name: webserver 
    - include_role:
        name: app

Though I'm secretly hoping that @ssbarnea keeps the pre_/post_tasks

@ssbarnea

This comment has been minimized.

Copy link

@ssbarnea ssbarnea commented Feb 27, 2020

Nobody is removing them, but they do not scale, and for sake of having two ways of writing code, I am inclined to discourage their use. The net is full of users confused about where to stick tasks between roles. Even if I succeed introducing a lint rule for that, it will be optional, so anyone could bypass it.

@geerlingguy

This comment has been minimized.

Copy link
Owner

@geerlingguy geerlingguy commented Feb 27, 2020

@ssbarnea but for a case like mine (I have around 300 different playbooks in different projects that I have set up with ansible-lint), that means any of those playbooks would start failing CI tests and I'd need to add yet another rule exception (once each for all those repos) for something that is not discouraged in Ansible core, at least.

And to @dglinder's point, loops increase complexity for someone new to a system a lot more than a list of roles. My book tries to start out slow/easy/approachable (the roles: keyword is definitely that) and then gets into more advanced use cases as the user grows more comfortable with how things work behind the scenes.

But still, I use roles: a lot because 80% of my playbooks are literally one or two setup tasks (in pre_tasks:, then 5-10 roles, and that's it. Outside of a few very complex use cases (and at last resort, for me), I've never had to use include_role in my playbooks. Instead, I like to make my roles composable and insular, so they can easily be put together and controlled via a few vars.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.