Use lists for hooks

[strarsis]

This PR adds functionality that allows using lists for hooks, including the built-in Trellis hooks.

[swalkinshaw]

We don't need all these defaults set right? They already have | default([])

[strarsis]

Is it useful to leave them commented out with comments from the documentation about when each hook is called?

[fullyint]

Could you add back the ending newline for these various files?

[strarsis]

@swalkinshaw: The yet empty hooks are now shown as commented out examples.
Also a commented out example for using site-specific hooks is kept because it can be very useful for site-specific deploys (like rsyncing the build artifacts, I use it for every site myself).

@fullyint: Learned something :)

[fullyint]

@strarsis Your adjustments to these vars in deploy.yml were helpful in initiating internal discussion and the decision to move the deploy role's vars into roles/deploy/defaults/main.yml. This will help deploy.yml be more concise. It will be more consistent with how other roles have their vars in their respective defaults/main.yml files.

Could you help us take the first step toward this goal in this PR by moving just the deploy hook vars out of deploy.yml to a new section at the end of roles/deploy/defaults/main.yml? I think you could start the section with a link to the deploy hooks docs, where all hooks are listed in sequence. Then I think it would be preferable to omit the unused/commented hook variables here in roles/deploy/defaults/main.yml, unless there's a strong argument for duplication.

Example

# roles/deploy/defaults/main.yml
...

# Deploy hooks
# For list of hooks and explanation, see https://roots.io/trellis/docs/deploys/#hooks
deploy_build_before:
  - "{{ playbook_dir }}/deploy-hooks/build-before.yml"

deploy_build_after:
  - "{{ playbook_dir }}/roles/deploy/hooks/build-after.yml"
  # - "{{ playbook_dir }}/deploy-hooks/sites/{{ site }}-build-after.yml"

deploy_finalize_before:
  - "{{ playbook_dir }}/roles/deploy/hooks/finalize-before.yml"

deploy_finalize_after:
  - "{{ playbook_dir }}/roles/deploy/hooks/finalize-after.yml"

[strarsis]

@fullyint: Done

[swalkinshaw]

Looks great 🚀

[fullyint]

Great work @strarsis. Thank you!

My prior test was with the deploy_build_after hook only.
Now that I test ALL hooks via this PR, I get a warning on one task:

TASK [deploy : Update WP theme paths] ******************************************
 [WARNING]: The loop variable 'item' is already in use.
You should set the `loop_var` value in the `loop_control` option for the task
to something else to avoid variable collisions and unexpected behavior.

As the warning suggests, it goes away after using a custom loop_var:

# roles/deploy/tasks/finalize.yml
- - include: "{{ item }}"
+ - include: "{{ include_path }}"
    with_items: "{{ deploy_finalize_after | default([]) }}"
+   loop_control:
+     loop_var: include_path
    tags: deploy-finalize-after

Although there may be some setting to disable the warning, it looks like we should add the custom loop_var to every deploy hook, enabling users to employ with_items in their custom includes without potential variable collisions and unexpected behavior mentioned in the warning.

---

I created #817 to fix the new warning caused by using loop_var on include tasks:

TASK [deploy : include] ********************************************************
 [WARNING]: Failure using method (v2_runner_item_on_ok) in callback plugin
(<ansible.plugins.callback.output.CallbackModule object at 0x104893f90>): 'item'

[fullyint]

@strarsis Thank you for all your work and patience making this great improvement to Trellis!

[pySilver]

per-site hooks aren't working as expected. in order to make it work you need to define hook for each and every website, otherwise it fails

[fullyint]

@pySilver Given that there are no per-site hooks by default, could you help me try to replicate the issue by posting the steps you've taken in setting up your per-site hook? Could you post your definition of the hook (example) and mention in which file you defined it? Also, post the filepath to the file you want included. Do the included tasks never run or do they show errors? If errors, could you post your tasks for review?

[strarsis]

@pySilver: From the related discussion on
https://discourse.roots.io/t/deploy-build-before-for-multiple-site-boxes/5274/3:

[...]
deploy_build_before:
  - "{{ playbook_dir }}/deploy-hooks/build-before/{{ site }}.yml"
[...]

So in this example, {{ site }} is interpolated and ansible tries to find that hook yml file - for all sites, regardless whether there is a hook file intended for the site or not.

[pySilver]

@fullyint @strarsis ok to simplify things I'll break down the issue to an example.

Typically you might want a per-site hook for some websites. Let's say you have a.com and b.com. The first one needs some specific build tasks to be executed, while latter b.com is fine with general build-before.yml (or even none).

There is no way to handle this easily at the moment.
If you define deploy_build_before this way:

deploy_build_before:
  - "{{ playbook_dir }}/deploy-hooks/build-before.yml"  # works for b.com
  - "{{ playbook_dir }}/deploy-hooks/build-before/{{ site }}.yml" # works for a.com

you need to create both hook files:

deploy-hooks/build-before/a.com
deploy-hooks/build-before/b.com

and one of them would be empty, since it's not required. If you omit any of them – your deployment playbook would fail once it will try to include non-existence yml.

It would be much easier if we first check file presence before inclusion. I did handle it this way:

- name: Validate build before hooks
  local_action: "stat path={{ item }}"
  register: deploy_build_before_hooks
  with_items: "{{ deploy_build_before | default([]) }}"
  tags: deploy-build-before

- include: "{{ item.item }}"
  with_items: "{{ deploy_build_before_hooks.results | default([]) }}"
  when: item.stat.exists
  tags: deploy-build-before

I imagine this could be accomplished to all hooks at once using some custom plugin.. which I had no time to create.

[fullyint]

@pySilver Thanks for the clear example. 👍

This raises the question of whether in this context we should override Ansible's default behavior of failing when a specified include file is missing. I tend to think the default behavior is worth keeping.

I certainly understand the desire to only include files if they exist, particularly in this context. I wish there were such an option built-in as standard in Ansible (not requiring the extra stat task).

Your approach above is very straightforward. Here are a few alternatives that would avoid modifying Trellis core files. These ideas build on the fact that Ansible's with_items does nothing with an item that is an empty list: []

# output is ONLY "one"
- debug:
    msg: "{{ item }}"
  with_items:
    - one
    - []

Specify include in wordpress_sites

Capitalizing on how a site's variables are loaded into a project variable, you could indicate the include file per site in wordpress_sites:

wordpress_sites:
  a.com:
    build_before_include: "{{ playbook_dir }}/deploy-hooks/build-before/a.j2"

deploy_build_before:
  - "{{ playbook_dir }}/deploy-hooks/build-before.yml"
  - "{{ project.build_before_include | default([]) }}"

Check site name

An alternative would be to use a ternary filter to check site name then return the include file or [].

deploy_build_before:
  - "{{ playbook_dir }}/deploy-hooks/build-before.yml"
  - "{{ (site == 'theONE.com') | ternary(playbook_dir + '/deploy-hooks/xyz/' + site + '.j2', []) }}"
# or 
  - "{{ (site in ['theONE.com', 'otherone.com']) | ternary(playbook_dir + '/deploy-hooks/xyz/' + site + '.j2', []) }}"

Check file existence

The above approaches require manual adjustment over time if you add/remove include files. A method to achieve your original intent of only including a file if it exists avoids that extra work, but is a little more complex. Suppose you use a pipe lookup to check the file's existence: if exists return file path, else return []. Based on the following bash command:
[ -f path/to/file ] && echo path/to/file || echo []

deploy_build_before:
  - "{{ playbook_dir }}/deploy-hooks/build-before.yml"
  - "{{ lookup('pipe', '[ -f '+ playbook_dir + '/deploy-hooks/xyz/' + site + '.j2 ] && echo ' + playbook_dir + '/deploy-hooks/xyz/' + site + '.j2 || echo []') }}"