evolix/evocheck
evolix/evocheck
21
2
Fork 2
Code Issues 48 Pull requests 5 Releases 37 Wiki Activity

fix check_mysqlnrpe #96

Merged
benpro merged 2 commits from fix-mysqlnrpe into master 2019-06-21 14:00:41 +02:00
Conversation 0 Commits 2 Files changed 1 +11 -6
jlecour commented 2019-06-18 13:53:06 +02:00
Owner
Copy link

the shell doesn't expand "~nagios", which makes the first test fail.
the grep wasn't looking for the nrpe config file :/

the shell doesn't expand "~nagios", which makes the first test fail. the grep wasn't looking for the nrpe config file :/
benpro reviewed 2019-06-18 14:13:13 +02:00
evocheck.sh
@ -891,12 +891,19 @@ check_mysqlmunin() {
}
check_mysqlnrpe() {
if is_debian_stretch && is_installed mariadb-server; then
nagios_file="~nagios/.my.cnf"
benpro commented 2019-06-18 14:13:03 +02:00
Contributor
Copy link

It doesn't expands because you have put double quotes.

nagios_file=~nagios/.my.cnf

should works.

14:03:05 [bserie@huit:/tmp] % bash -c 'echo ~bserie'
/home/bserie
14:12:54 [bserie@huit:/tmp] % bash -c 'echo "~bserie"'
~bserie
14:12:59 [bserie@huit:/tmp] %
It doesn't expands because you have put double quotes. ~~~ nagios_file=~nagios/.my.cnf ~~~ should works. ~~~ 14:03:05 [bserie@huit:/tmp] % bash -c 'echo ~bserie' /home/bserie 14:12:54 [bserie@huit:/tmp] % bash -c 'echo "~bserie"' ~bserie 14:12:59 [bserie@huit:/tmp] % ~~~
benpro commented 2019-06-18 14:15:52 +02:00
Contributor
Copy link

I'm not fond of checking nagios_file_abs and nagios_file_sym. We should keep tilde expansion. (With no quotes of course).

I'm not fond of checking `nagios_file_abs` and `nagios_file_sym`. We should keep tilde expansion. (With no quotes of course).
jlecour commented 2019-06-18 16:33:12 +02:00
Author
Owner
Copy link

I understand, but without the quotes the variable holds the expanded value.
The nrpe check currently contains the non-expanded version so it's not easy to check without rewritting all nrpe configurations.

I understand, but without the quotes the variable holds the expanded value. The nrpe check currently contains the non-expanded version so it's not easy to check without rewritting all nrpe configurations.
jlecour commented 2019-06-18 16:34:26 +02:00
Author
Owner
Copy link

We can loosen the check a bit and only verify that a "check_mysql" is present and not the entire line.

We can loosen the check a bit and only verify that a "check_mysql" is present and not the entire line.
benpro added the
bug
 label 2019-06-18 14:17:18 +02:00
benpro reviewed 2019-06-18 16:56:17 +02:00
evocheck.sh
@ -892,4 +892,2 @@
check_mysqlnrpe() {
if is_debian_stretch && is_installed mariadb-server; then
nagios_file="~nagios/.my.cnf"
{ test -f $nagios_file \
benpro commented 2019-06-18 16:56:13 +02:00
Contributor
Copy link

Or you could add eval to evaluate the variable even if quoted.

16:53:11 [bserie@mikubook:~] 130 % nagios_file="~bserie"
16:53:22 [bserie@mikubook:~] % echo $nagios_file
~bserie
16:53:28 [bserie@mikubook:~] % eval echo $nagios_file
/home/bserie

This should works:

eval test -f $nagios_file
Or you could add eval to evaluate the variable even if quoted. ``` 16:53:11 [bserie@mikubook:~] 130 % nagios_file="~bserie" 16:53:22 [bserie@mikubook:~] % echo $nagios_file ~bserie 16:53:28 [bserie@mikubook:~] % eval echo $nagios_file /home/bserie ``` This should works: ~~~ eval test -f $nagios_file ~~~
jlecour commented 2019-06-18 17:36:43 +02:00
Author
Owner
Copy link

I think it's overcomplicated :/

I think it's overcomplicated :/
benpro commented 2019-06-18 17:38:47 +02:00
Contributor
Copy link

Huh? You take the actual check. You add eval and its done.

Huh? You take the actual check. You add `eval` and its done.
benpro commented 2019-06-21 11:23:15 +02:00
Contributor
Copy link

Any news? You don't agree?

@vlaborie what's your point of view?

Any news? You don't agree? @vlaborie what's your point of view?
jlecour commented 2019-06-21 11:30:37 +02:00
Author
Owner
Copy link

It's not that I disagree. I think adding an eval is technically correct but needlessly complicated.

I offered another solution : simplifying the NRPE check by not checking the exact command parameters (which we don't care about).
It verifies that there is an NRPE check for MySQL.

It's not that I disagree. I think adding an `eval` is technically correct but needlessly complicated. I offered another solution : simplifying the NRPE check by not checking the exact command parameters (which we don't care about). It verifies that there is an NRPE check for MySQL.
benpro commented 2019-06-21 11:34:12 +02:00
Contributor
Copy link

It verifies that there is an NRPE check for MySQL.

Ok go for it.

> It verifies that there is an NRPE check for MySQL. Ok go for it.
benpro approved these changes 2019-06-21 14:00:15 +02:00
benpro closed this pull request 2019-06-21 14:00:41 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something is not working

  
bullseye

   
discussion

This need to be discussed

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
suggestion

Suggest to add/change something

  
wontfix

This won't be fixed

No labels
bug
bullseye
discussion
duplicate
enhancement
help wanted
invalid
question
suggestion
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: evolix/evocheck#96
No description provided.
Delete branch "fix-mysqlnrpe"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?