Abstract
All role-specific variables must use the role prefix in Ansible (ex: timemaster_ptp_network_transport for the timemaster role).
We must harmonize how variables shared by multiple roles are handled:
- How they are named
- How they are passed to all the roles that need them
Current status
Shared role variables are named using legacy naming and passed directly to the role without any mention in the role's defaults file, nor any validation in the role.
For example: hostname and cluster_ip_addr are used by the cephadm role, but they are not present in the defaults file, nor do they use the cephadm_ prefix.
Detailed Description
I advise creating one variable per role and passing the global variable to them in the inventory.
For the cephadm role, it would mean:
- Create
cephadm_hostname and cephadm_ip_addr.
- Mention them in the role defaults (make them default to
null as they are required).
- Write
cephadm_hostname: "{{ hostname }}" and cephadm_ip_addr: "{{ cluster_ip_addr }}" in the inventory.
I proposed this approach in #866 for the podman_registry_mirror_url variable.
6d725a9
The main drawback of this method is that it could make the inventory even bigger. However, it would be possible to put these in group_vars files.
Alternatives to be discussed
It would also be possible to get the shared variable in the defaults file of each role, for example:
cephadm_hostname: "{{ hostname | default(null) }}"
But I find it less convenient.
It is also possible to put that in the playbook variables, but:
- It would be impossible for the user to overwrite it, as task vars have precedence over inventory (see Ansible docs).
- It would probably confuse people trying to launch the role themselves, IMO.
- name: Cephadm
hosts:
cluster_machines
become: true
vars:
cephadm_hostname: "{{ hostname }}"
cephadm_ip_addr: "{{ cluster_ip_addr }}"
roles:
- detect_seapath_distro
- cephadm
- deploy_cephfs
I'm open to discussion, mostly on choosing between inventories vs task vars.
Abstract
All role-specific variables must use the role prefix in Ansible (ex:
timemaster_ptp_network_transportfor thetimemasterrole).We must harmonize how variables shared by multiple roles are handled:
Current status
Shared role variables are named using legacy naming and passed directly to the role without any mention in the role's
defaultsfile, nor any validation in the role.For example:
hostnameandcluster_ip_addrare used by thecephadmrole, but they are not present in thedefaultsfile, nor do they use thecephadm_prefix.Detailed Description
I advise creating one variable per role and passing the global variable to them in the inventory.
For the cephadm role, it would mean:
cephadm_hostnameandcephadm_ip_addr.nullas they are required).cephadm_hostname: "{{ hostname }}"andcephadm_ip_addr: "{{ cluster_ip_addr }}"in the inventory.I proposed this approach in #866 for the
podman_registry_mirror_urlvariable.6d725a9
The main drawback of this method is that it could make the inventory even bigger. However, it would be possible to put these in group_vars files.
Alternatives to be discussed
It would also be possible to get the shared variable in the defaults file of each role, for example:
cephadm_hostname: "{{ hostname | default(null) }}"But I find it less convenient.
It is also possible to put that in the playbook variables, but:
I'm open to discussion, mostly on choosing between inventories vs task vars.