ansible / proposals

Repository for sharing and tracking progress on enhancement proposals for Ansible.
Creative Commons Zero v1.0 Universal
93 stars 19 forks source link

Exposing parent role name/path when including roles #146

Closed Xaroth closed 4 years ago

Xaroth commented 5 years ago

Proposal: Exposing parent role name/path when including roles

Author: Steven Noorbergen <@xaroth>

Date: 2018-10-09

Motivation

This proposal is to add two new magic variables to improve the usage of re-include-able sub-tasks of roles.

Problems

We have a large amount of roles, most of which are specific to one target application (for example Nginx, or nftables), where all the main logic is done in tasks/main.yml (and possible others, which are included from main.yml) to get the application running. However, in some cases other applications will want to hook into these roles, as to provide their own application-specific configuration (such as a nginx sites config, or application-specific nftables rules).

To avoid repetition, and to improve the maintainability of our roles tree, we keep specific actions related to an application, inside that application's role, as a separate task file. Other applications can then call that task file via include_role.

For example:

nginx/tasks/enable-site.yml

- name: ensure ownership of config files
  file:
    ...
  register: nginx_ownership_changed

- name: enable nginx config files
  file:
    src: "{{ nginx_sites_available }}/{{ source }}.conf"
    path: "{{ nginx_sites_enabled }}/{{ source }}.conf"
    state: link
  register: nginx_config_changed

- name: register our managed file
  set_fact:
    nginx_enabled_sites: "{{ nginx_enabled_files|d({})|combine({source: True})

- name: restart nginx if needed
  service:
    name: nginx
    state: restarted
  when:
    - "restart_early|d(False)|bool"
    - "nginx_config_changed is changed or nginx_ownership_changed is changed"

icinga/tasks/main.yml

...
- name: generate nginx config
  template:
    src: "nginx/sites-available/template.conf.j2"
    dest: "{{ nginx_sites_available }}/{{ role_name }}.conf"
    ...

- name: enable or disable our nginx config
  include_role:
    name: nginx
    tasks_from: "{{ icinga_nginx_enabled|d(True)|bool|ternary('enable-site.yml', 'disable-site.yml') }}"
  vars:
    source: "{{ role_name }}"
    restart_early: True

Mind, this was the simplest use case I could find, most of the ones we have are more complex, but would be needlessly long for a proposal.

In general, the config files we adhere to contain the role name from which they originate, or we provide extra mechanics to ensure that the system logs where the config files originate from, so that we can diagnose issues more quickly. This ensures that all logic pertaining nginx remains inside the nginx role as much as possible, while other roles only have to worry about their specifics. It also means that should the logic of enabling/disabling a nginx site be changed somehow, we'd only have to worry about one active scenario being used in terms of migration and code-change.

Solution proposal

With the proposed changes as made in https://github.com/ansible/ansible/pull/46687 , the following becomes possible:

nginx/tasks/enable-site.yml

...
- name: enable nginx config files
  file:
    src: "{{ nginx_sites_available }}/{{ ansible_parent_role_name }}.conf"
    path: "{{ nginx_sites_enabled }}/{{ ansible_parent_role_name }}.conf"
    state: link
  register: nginx_config_changed

- name: register our managed file
  set_fact:
    nginx_enabled_sites: "{{ nginx_enabled_files|d({})|combine({ansible_parent_role_name : True})
...

icinga/tasks/main.yml

...
- name: generate nginx config
  template:
    src: "nginx/sites-available/template.conf.j2"
    dest: "{{ nginx_sites_available }}/{{ role_name }}.conf"
    ...

- name: enable or disable our nginx config
  include_role:
    name: nginx
    tasks_from: "{{ icinga_nginx_enabled|d(True)|bool|ternary('enable-site.yml', 'disable-site.yml') }}"
  vars:
    restart_early: True

With this change we no longer have to worry about having to pass our own name to sub-tasks, preventing configuration issues.

Dependencies (optional)

I don't see any dependency requirements

Testing (optional)

It could possibly use testing, however it does not introduce new logic, so existing tests should already cover the code branches affected (assuming they exist).

Documentation (optional)

Documentation is already provided in the referenced PR

Anything else?

These variables cover an extra use-case where roles can now detect whether or not they are being included by another role. We currently do not have this use case active, but it was suggested as a potential feature for our roles.

Xaroth commented 4 years ago

Closed since https://github.com/ansible/ansible/pull/46687 is merged