summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnton Lofgren <alofgren@op5.com>2013-10-18 09:42:46 (GMT)
committerHolger Weiss <holger@zedat.fu-berlin.de>2013-11-19 22:57:27 (GMT)
commit77fc3548ae1be360584082d771fa01696d4f4479 (patch)
tree3410cca89724e1fa1fa64eb94b83422f5d5774ae
parentdc3fc90e2a799e9f835a2d10d824bfcb20128d3d (diff)
downloadmonitoring-plugins-77fc3548ae1be360584082d771fa01696d4f4479.tar.gz
check_procs: ignore plugin parent process
This fixes an issue that appears when running check_procs over NRPE, where the default shell is configured to (for example) dash, as is the case on Debian. dash (and tcsh, and mksh, and probably others), when invoked with -c forks an additional process to execute the argument string. Contrast this with bash, which does not do this, provided that the argument string simply can be exec()'d as-is. To demonstrate: $ bash -c pstree init─┬ .. ... ├─sshd─-─sshd───pstree versus $ dash -c pstree init─┬ .. ... ├─sshd─-─sshd───dash───pstree The consequence of this fork is that the following invocation: /opt/plugins/check_procs -a init will result in this output: PROCS OK: 2 processes with args 'init' | processes=2;;;0; because the check_procs, in addition to finding the actual init process, finds its parent shell as well. This example is a bit contrived, but I think it illustrates the point. This wouldn't really be a problem, and normally isn't, if it weren't for the fact that NRPE uses a call to popen() which does exactly the above (executes '/bin/sh -c ...'), causing inconsistent behaviour between distributions and much confusion for end users. The argument may be made that the dash process spawned by NRPE is just a process like any other, and should therefore be included in the process count just like any other. However, this is not very intuitive, because of the previously mentioned inconsistencies. The argument might also well be made that we're _never_ interested in the immediate ancestor of the plugin, and while it is unknown how many installations have already made the necessary modifications to their setups to make up for the fact that the plugin behaves the way it does, it is not deemed worthwhile to entertain such workarounds. Thus, this patch ignores the parent process. See also these bug reports: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=626913 http://sourceforge.net/p/nagiosplug/bugs/512/ https://github.com/nagios-plugins/nagios-plugins/issues/999 https://bugs.op5.com/view.php?id=4398
-rw-r--r--plugins/check_procs.c8
-rw-r--r--plugins/t/check_procs.t9
2 files changed, 16 insertions, 1 deletions
diff --git a/plugins/check_procs.c b/plugins/check_procs.c
index d20b027..c2239db 100644
--- a/plugins/check_procs.c
+++ b/plugins/check_procs.c
@@ -123,6 +123,7 @@ main (int argc, char **argv)
123 char *procprog; 123 char *procprog;
124 124
125 pid_t mypid = 0; 125 pid_t mypid = 0;
126 pid_t myppid = 0;
126 struct stat statbuf; 127 struct stat statbuf;
127 dev_t mydev = 0; 128 dev_t mydev = 0;
128 ino_t myino = 0; 129 ino_t myino = 0;
@@ -172,6 +173,7 @@ main (int argc, char **argv)
172 173
173 /* find ourself */ 174 /* find ourself */
174 mypid = getpid(); 175 mypid = getpid();
176 myppid = getppid();
175 if (usepid || stat_exe(mypid, &statbuf) == -1) { 177 if (usepid || stat_exe(mypid, &statbuf) == -1) {
176 /* usepid might have been set by -T */ 178 /* usepid might have been set by -T */
177 usepid = 1; 179 usepid = 1;
@@ -241,6 +243,12 @@ main (int argc, char **argv)
241 printf("not considering - is myself or gone\n"); 243 printf("not considering - is myself or gone\n");
242 continue; 244 continue;
243 } 245 }
246 /* Ignore parent*/
247 else if (myppid == procpid) {
248 if (verbose >= 3)
249 printf("not considering - is parent\n");
250 continue;
251 }
244 252
245 /* filter kernel threads (childs of KTHREAD_PARENT)*/ 253 /* filter kernel threads (childs of KTHREAD_PARENT)*/
246 /* TODO adapt for other OSes than GNU/Linux 254 /* TODO adapt for other OSes than GNU/Linux
diff --git a/plugins/t/check_procs.t b/plugins/t/check_procs.t
index 1dea564..e0479ea 100644
--- a/plugins/t/check_procs.t
+++ b/plugins/t/check_procs.t
@@ -13,7 +13,7 @@ my $t;
13if (`uname -s` eq "SunOS\n" && ! -x "/usr/local/nagios/libexec/pst3") { 13if (`uname -s` eq "SunOS\n" && ! -x "/usr/local/nagios/libexec/pst3") {
14 plan skip_all => "Ignoring tests on solaris because of pst3"; 14 plan skip_all => "Ignoring tests on solaris because of pst3";
15} else { 15} else {
16 plan tests => 12; 16 plan tests => 14;
17} 17}
18 18
19my $result; 19my $result;
@@ -26,6 +26,13 @@ $result = NPTest->testCmd( "./check_procs -w 100000 -c 100000 -s Z" );
26is( $result->return_code, 0, "Checking less than 100000 zombie processes" ); 26is( $result->return_code, 0, "Checking less than 100000 zombie processes" );
27like( $result->output, '/^PROCS OK: [0-9]+ process(es)? with /', "Output correct" ); 27like( $result->output, '/^PROCS OK: [0-9]+ process(es)? with /', "Output correct" );
28 28
29SKIP: {
30 skip "No bash available", 2 unless(system("which bash > /dev/null") == 0);
31 $result = NPTest->testCmd( "bash -c './check_procs -a '/sbin/init'; true'" );
32 is( $result->return_code, 0, "Parent process is ignored" );
33 like( $result->output, '/^PROCS OK: 1 process?/', "Output correct" );
34
35}
29$result = NPTest->testCmd( "./check_procs -w 0 -c 100000" ); 36$result = NPTest->testCmd( "./check_procs -w 0 -c 100000" );
30is( $result->return_code, 1, "Checking warning if processes > 0" ); 37is( $result->return_code, 1, "Checking warning if processes > 0" );
31like( $result->output, '/^PROCS WARNING: [0-9]+ process(es)? | procs=[0-9]+;0;100000;0;$/', "Output correct" ); 38like( $result->output, '/^PROCS WARNING: [0-9]+ process(es)? | procs=[0-9]+;0;100000;0;$/', "Output correct" );