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(mysqld_exporter): Change condition for mysqld_exporter_host check #270

Merged

Conversation

molpako
Copy link
Contributor

@molpako molpako commented Jan 3, 2024

Modify the condition for mysqld_exporter_host in the mysqld_exporter my_cnf.j2 template to address an issue where mysqld_exporter_host: null in roles/defaults/main.yml was not being handled correctly. The updated condition ensures the host setting logic operates properly even when mysqld_exporter_host is set to null.

I also think a useful approach would be to simply remove mysqld_exporter_host[port] from defaults/main.yml and not change the template condition. I would appreciate your comments on this.

Modify the condition for `mysqld_exporter_host` in the mysqld_exporter my_cnf.j2 template to address an issue where `mysqld_exporter_host: null` in roles/defaults/main.yml was not being handled correctly. The updated condition ensures the host setting logic operates properly even when `mysqld_exporter_host` is set to null.

Signed-off-by: molpako <system.222.down@gmail.com>
@molpako molpako force-pushed the fix/mysqld_exporter_use_socket branch from 0f0707b to a38e7ee Compare January 3, 2024 13:57
@github-actions github-actions bot added bugfix and removed bugfix labels Jan 3, 2024
@gardar
Copy link
Member

gardar commented Jan 3, 2024

Good catch, thanks!

I also think a useful approach would be to simply remove mysqld_exporter_host[port] from defaults/main.yml and not change the template condition. I would appreciate your comments on this.

That's up for debate and it's actually a part of a larger discussion.

It's been standard practice in ansible roles that the defaults have served the purpose of giving a overview the variables that users can override. But as we recently introduced the argument spec validation of the roles which serves the same purpose (and is actually used to generate the docs) then perhaps it's ok to remove this variable and other null value ones from the defaults.

@gardar gardar merged commit 906a1d3 into prometheus-community:main Jan 3, 2024
43 checks passed
@molpako
Copy link
Contributor Author

molpako commented Jan 3, 2024

Thank you for merging and explaining the background.

@molpako molpako deleted the fix/mysqld_exporter_use_socket branch January 3, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants