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

[Question] - why is the "mariadb_data_dir" defined in roles/vars and not in roles/default ? #42

Open
tarek1375 opened this issue Mar 25, 2024 · 7 comments

Comments

@tarek1375
Copy link

Hi,
It's not really an issue but i was wondering why you defined "mariadb_data_dir" and config in the vars file of the role, is there any specifc reason ?
because with our team, we wanted to define another dir for data, but we would use group_vars or host_vars to override those variable, and in some contexts it's not really the ansible way to do it.

@fauust
Copy link
Owner

fauust commented Mar 25, 2024

Everything that may be OS specific is defined in vars, that's the reason. You can override any vars in group_vars or host_vars. That's perfectly fine and is the ansible way to do it.

@fauust fauust closed this as completed Mar 25, 2024
@BenGig
Copy link

BenGig commented Jun 4, 2024

Role vars have a high priority in the precedence list mentioned above. I did a few tests with commandline params and files in group_/host_vars, only commandline params works; and elevating the priority with ansible.builtin.set_fact. That's what I use now as a workaround.

Tests:

- hosts: mariadb-eduit-dev.ethz.ch
  roles:
    - mariadb

and role *mariadb" (outcome is the same as with calling fauust.mariadb directly in the playbook above):

- name: Dump mariadb location variable
  ansible.builtin.debug:
    var: mariadb_data_dir
- name: Apply MariaDB role
  ansible.builtin.include_role:
    name: fauust.mariadb

Command line variables overwrite as expected:

ansible-playbook -e "mariadb_data_dir=/data1/mariadb_data_cmdline" -u root -i inventory play.yml
...
TASK [mariadb : Dump mariadb location variable] **************************************************************************************************************************
ok: [mariadb-eduit-dev.ethz.ch] => {
    "mariadb_data_dir": "/data1/mariadb_data_cmdline"
}
...
TASK [fauust.mariadb : Make sure datadir and log dir exist] **************************************************************************************************************
changed: [mariadb-eduit-dev.ethz.ch] => (item=/data1/mariadb_data_cmdline)

Not from group_vars:

TASK [mariadb : Dump mariadb location variable] **************************************************************************************************************************
ok: [mariadb-eduit-dev.ethz.ch] => {
    "mariadb_data_dir": "/data1/mariadb_in_group"
}
...
TASK [fauust.mariadb : Make sure datadir and log dir exist] **************************************************************************************************************
ok: [mariadb-eduit-dev.ethz.ch] => (item=/var/lib/mysql)

Neither from host_vars:

ok: [mariadb-eduit-dev.ethz.ch] => {
    "mariadb_data_dir": "/data1/mariadb_in_host"
}
...
ok: [mariadb-eduit-dev.ethz.ch] => (item=/var/lib/mysql)

This addition makes it work for me:

- name: Set fact for mariadb_data_dir
  ansible.builtin.set_fact:
    mariadb_data_dir: "{{ mariadb_data_dir }}"

@fauust
Copy link
Owner

fauust commented Jun 4, 2024

@BenGig
Copy link

BenGig commented Jun 4, 2024

The precedence mentioned there is:

\7. playbook group_vars/*
...
\10. playbook host_vars/*
...
\15. role vars
...
\19. set_fact

Playbook vars are defined as having lower precedence than role vars.

But I don't see an intuitive way how you could design it differently. Setting the default storage path based on the OS is important for a proper default behavior, and should suit 90% of the use cases. For the rest, set_fact does the work.

@fauust
Copy link
Owner

fauust commented Jun 4, 2024

Oh, yes, my fault, this is the roles/defaults that has lower precedence. Yep, not sure how to handle this better, I'll take a moment to see if there is no better design. Thanks for the workaround, very much appreciated!

@fauust fauust reopened this Jun 4, 2024
@BenGig
Copy link

BenGig commented Jun 5, 2024

Sadly there is no such thing like include_defaults, which would solve the problem of presetting OS dependent values in roles, and still having them overwritten easily.

Honestly, I would leave it as it is. Most of the settings are rarely changed, and for the few to be changed, a note about set_fact in the docs should be sufficient. And then advertise it as a safety measure against accidential changes of low level parameters ;-)

@atefhaloui
Copy link

atefhaloui commented Jun 9, 2024

Excellent role but we have no other solution that using facts :(

    - name: Update variables and Install packages
      block:
        # Workaround: https://github.com/fauust/ansible-role-mariadb/issues/42
        - name: Update variables
          ansible.builtin.set_fact:
            mariadb_data_dir: "{{ data_mount_path }}"
            mariadb_config_file: "/etc/mysql/my.cnf"
            mariadb_bind_address: "0.0.0.0"
            mariadb_cron_package_name: systemd-cron

        - name: Install MariaDB
          ansible.builtin.include_role:
            name: fauust.mariadb

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

No branches or pull requests

4 participants