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

Fix interface lookup #244

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

thesefer
Copy link

@thesefer thesefer commented Feb 6, 2025

Description

In https://github.com/mrlesmithjr/ansible-mariadb-galera-cluster/blob/master/templates/etc/mysql/conf.d/galera.cnf.j2 the subvalue of hostvars[node]['ansible_' ~ galera_cluster_bind_interface]['ipv4']['address'] cannot be found because the galera_cluster_bind_interface contains only the interface for the current host. You have to lookup the variable for each host individually.

EDIT:
To be more precise, in it's old form running the template on host 1, the galera_cluster_bind_interface would be eth0 while on the second host it is eth1. The check running on host1 would check hostvars[host1]['ansible_' ~ eth0]['ipv4']['address'] and gets a result but would not get anything for hostvars[host2]['ansible_' ~ eth0]['ipv4']['address'] because eth0 does not exist there. Same vice versa. The galera_cluster_bind_interface is set fixed for the host it is running on.

Related Issue

#243

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
    Couldn't test properly due to company proxy restrictions.

In https://github.com/mrlesmithjr/ansible-mariadb-galera-cluster/blob/master/templates/etc/mysql/conf.d/galera.cnf.j2 the subvalue of `hostvars[node]['ansible_' ~ galera_cluster_bind_interface]['ipv4']['address']` cannot be found because the `galera_cluster_bind_interface` contains only the interface for the current host. You have to lookup the variable for each host individually.
@eRadical
Copy link
Collaborator

eRadical commented Feb 6, 2025

This means on each node one has to set "galera_cluster_bind_interface".
I find it rather cumbersome...

I'm also not sure if this is backward compatible.

@thesefer
Copy link
Author

thesefer commented Feb 6, 2025

I didn't check or change the following occurrences:

roles/mrlesmithjr.mariadb-galera-cluster/templates/etc/my.cnf.d/server.cnf.j2
51:{%   set _ = _galera_cluster_node_addresses.append( hostvars[node]['galera_cluster_bind_address'] | default(hostvars[node]['ansible_' ~ galera_cluster_bind_interface]['ipv4']['address']) | mandatory ) %}

roles/mrlesmithjr.mariadb-galera-cluster/templates/etc/my.cnf.d/server.cnf.temp.j2
51:{%   set _ = _galera_cluster_node_addresses.append( hostvars[node]['galera_cluster_bind_address'] | default(hostvars[node]['ansible_' ~ galera_cluster_bind_interface]['ipv4']['address']) | mandatory ) %}

roles/mrlesmithjr.mariadb-galera-cluster/templates/etc/mysql/conf.d/galera.cnf.temp.j2
51:{%   set _ = _galera_cluster_node_addresses.append( hostvars[node]['galera_cluster_bind_address'] | default(hostvars[node]['ansible_' ~ galera_cluster_bind_interface]['ipv4']['address']) | mandatory ) %}

@eRadical
The default is galera_cluster_bind_interface: "eth0" which wouldn't match either so one has to set it. I worked around this by setting it to the following: galera_cluster_bind_interface: "{{ ansible_default_ipv4['interface'] }}" in my group vars.

@thesefer
Copy link
Author

thesefer commented Feb 6, 2025

Btw the following default variable isn't working / expanding and needs to be changed according to my tests but is not covered in this PR:

galera_cluster_bind_address: "{{ hostvars[inventory_hostname]['ansible_' + galera_cluster_bind_interface]['ipv4']['address'] }}" to galera_cluster_bind_address: "{{ ansible_default_ipv4['address'] }}"

@thesefer
Copy link
Author

thesefer commented Feb 18, 2025

I implemented the change to all files considered. I added the example for galera_cluster_bind_interface to the defaults file (which will make it completely dynamic because it is using ansible runtime variables and values) and replaced the +galera_cluster_bind_address to do the same.

Please note that the fix from 0d3180f will not work (Please test it by removing the default completely), you will only get The error was: ansible.errors.AnsibleFilterError: Mandatory variable '10.10.10.0' not defined because it is using the value of the variable.

If one didn't do some weird magic this should be 100% backwards compatible (with all fixes applied) because it is using exactly the same values which ansible would gather anyways.

EDIT: It is completely ok to use jinja2 magic in templates and tasks but better not in host variables.

@eRadical
Copy link
Collaborator

Hi @thesefer - please rebase & resolve conflicts so I can give it a run.

@thesefer
Copy link
Author

Hi @eRadical. Conflicts resolved.

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

Successfully merging this pull request may close these issues.

2 participants