Skip to content

Improve the mk_mysql agent plugin script - allow alias per line#585

Closed
flybyray wants to merge 2 commits intoCheckmk:masterfrom
ndgit:FEED-7806
Closed

Improve the mk_mysql agent plugin script - allow alias per line#585
flybyray wants to merge 2 commits intoCheckmk:masterfrom
ndgit:FEED-7806

Conversation

@flybyray
Copy link
Copy Markdown

example cfg section for check_mk with alias= entries. aliases should work too when alias= is not present socket= is only moved to its corresponding alias=

improved the mk_mysql logic
should allow what was previously configured too

Thank you for your interest in contributing to Checkmk!
Consider looking into Readme regarding process details.

General information

Update agent plugin mk_mysql to allow alias= per line instead of one aliases= with each alias as an item of a comma seperated list.
This makes it easier to provision "$MK_CONFDIR"/mysql{.local,}.cfg with ansible module block in file.

Bug reports

no bug report.
the previous solution was not useful for a lot of galera cluster instances.

Proposed changes

The new code should work with old config.
But now also allows to write a check_mk cfg file in the following stile.

[check_mk]
...
# BEGIN ANSIBLE MANAGED BLOCK Alias1
alias=Alias1
socket=/path/to/my-alias1.sock
# END ANSIBLE MANAGED BLOCK Alias1
# BEGIN ANSIBLE MANAGED BLOCK Alias1
alias=Alias2
socket=/path/to/my-alias2.sock
# END ANSIBLE MANAGED BLOCK Alias1

While it may work for you, it is our job to ensure that it works for everybody.
I hope the provided solution should work for everybody using the mk_mysql agent plugin.

  • If it's not obvious from the above: In what way does your patch change the current behavior?
    If there is no alias= entry but if there is the aliases=Alias1,Alias2 the code should still work.

  • Consider writing a unit test that would have failed without your fix.
    There is no fix. The previous behaviour should work too.

  • Is this a new problem? What made you submit this PR (new firmware, new device, changed device behavior)?
    It allows/provides a cleaner structure for cfg files with a lot of mysql instances.
    Actually it fixes even some issues with parsing sockets from running mysqld processes where the --socket=path/to/sock is not at the end.

example cfg section for check_mk with alias= entries.
aliases should work too when alias= is not present
socket= is only moved to its corresponding alias=

improved the mk_mysql logic
should allow what was previously configured too
@TLI29 TLI29 added bug Something isn't working Component: Checks & Agents labels Apr 4, 2023
@scolakovic scolakovic added enhancement New feature or request and removed bug Something isn't working labels May 11, 2023
@scolakovic
Copy link
Copy Markdown
Contributor

scolakovic commented May 11, 2023

Hi flybyray,

thank you for contributing to Checkmk.

It's currently possible to specify a socket, even though the alias is missing.
In the agents/cfg_examples/mysql.cfg file there is a section which explains it:

# There is always one alias per socket which can also be empty
# Example with three sockets, the second one has an empty alias:
# [client]
# user=monitoring
# password="password"
# socket=/var/run/mysqld/mysqld.sock
# socket=/var/run/mysqld/mysqld2.sock
# socket=/var/run/mysqld/mysqld3.sock

# host=127.0.0.1
# !include /etc/check_mk/mysql.local.cfg
# [check_mk]
# aliases=Alias1,,Alias3

Is there another problem your PR is tackling?
If not, I'm inclined to close the PR, because changing the configuration format would mean we also have to change the agent bakery on our side.

@flybyray
Copy link
Copy Markdown
Author

Hi flybyray,

thank you for contributing to Checkmk.

It's currently possible to specify a socket, even though the alias is missing. In the agents/cfg_examples/mysql.cfg file there is a section which explains it:

# There is always one alias per socket which can also be empty
# Example with three sockets, the second one has an empty alias:
# [client]
# user=monitoring
# password="password"
# socket=/var/run/mysqld/mysqld.sock
# socket=/var/run/mysqld/mysqld2.sock
# socket=/var/run/mysqld/mysqld3.sock

# host=127.0.0.1
# !include /etc/check_mk/mysql.local.cfg
# [check_mk]
# aliases=Alias1,,Alias3

Is there another problem your PR is tackling? If not, I'm inclined to close the PR, because changing the configuration format would mean we also have to change the agent bakery on our side.

The PR just would allow both configurations.

The current configuration format is just awefull for reading

  • sockets in lines
  • aliases in columns

When bakery tries to make configuration human unreadable. I will fear the bakery until it dies. ;-)

@flybyray flybyray closed this May 11, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants