Bug 197827 - net-mgmt/xymon-client rc script does not handle "faststart" [patch]
Summary: net-mgmt/xymon-client rc script does not handle "faststart" [patch]
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Mark Felder
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-19 17:50 UTC by John Nielsen
Modified: 2015-02-24 13:54 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (feld)


Attachments
xymon client rc script rewrite (1.94 KB, patch)
2015-02-20 22:01 UTC, Mark Felder
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Nielsen 2015-02-19 17:50:47 UTC
I'm not sure if it was an update to the port or -STABLE, but xymon-client no longer starts at boot on my system:

% uname -a
FreeBSD stealth.jnielsen.net 10.1-STABLE FreeBSD 10.1-STABLE #1 r278970M: Wed Feb 18 15:33:54 MST 2015     john@stealth.jnielsen.net:/ssdtmp/obj/usr/src/sys/STEALTH  amd64

Feb 19 10:14:44 stealth kernel: Starting xymon_client.
Feb 19 10:14:45 stealth kernel: Usage: /usr/local/www/xymon/client/runclient.sh start|stop|restart|status

It also does not handle other prefixed commands (fast, force, one, quiet), since it includes "${1}" unsanitized in ${command_args}. I propose the following, to allow run_rc_command to do all of its magic and still pass the correct argument to runclient.sh. The patch works for me, but I am not the most experienced rc programmer.

# diff -u xymon-client.orig xymon-client
--- xymon-client.orig	2015-02-12 13:31:00.784342859 -0700
+++ xymon-client	2015-02-19 10:46:12.134221439 -0700
@@ -16,10 +16,10 @@
 : ${xymon_client_enable:=NO}
 : ${xymon_client_user:=xymon}
 
-command=/usr/local/www/xymon/client/runclient.sh
-command_args="${xymon_client_flags} ${1}"
 procname=/usr/local/www/xymon/client/bin/xymonlaunch
 pidfile="/usr/local/www/xymon/client/logs/clientlaunch.`hostname`.pid"
 start_precmd="chown -R ${xymon_client_user} /usr/local/www/xymon/client/logs"
+start_cmd="/usr/local/www/xymon/client/runclient.sh ${xymon_client_flags} start"
+stop_cmd="/usr/local/www/xymon/client/runclient.sh ${xymon_client_flags} stop"
 
 run_rc_command "$1"
Comment 1 Bugzilla Automation freebsd_committer freebsd_triage 2015-02-19 17:50:47 UTC
Auto-assigned to maintainer feld@FreeBSD.org
Comment 2 John Nielsen 2015-02-19 17:51:44 UTC
Forgot to mention that the issue arises because rc uses "faststart" instead of just "start" at system boot.
Comment 3 John Nielsen 2015-02-19 17:59:00 UTC
Patch against the port file instead of the installed script:

# diff -u xymon-client.in.orig xymon-client.in
--- xymon-client.in.orig	2015-02-19 10:56:54.699249492 -0700
+++ xymon-client.in	2015-02-19 10:57:56.999155736 -0700
@@ -16,10 +16,10 @@
 : ${xymon_client_enable:=NO}
 : ${xymon_client_user:=%%XYMONUSER%%}
 
-command=%%WWWDIR%%/client/runclient.sh
-command_args="${xymon_client_flags} ${1}"
 procname=%%WWWDIR%%/client/bin/xymonlaunch
 pidfile="%%WWWDIR%%/client/logs/clientlaunch.`hostname`.pid"
 start_precmd="chown -R ${xymon_client_user} %%WWWDIR%%/client/logs"
+start_cmd="%%WWWDIR%%/client/runclient.sh ${xymon_client_flags} start"
+stop_cmd="%%WWWDIR%%/client/runclient.sh ${xymon_client_flags} stop"
 
 run_rc_command "$1"
Comment 4 Mark Felder freebsd_committer freebsd_triage 2015-02-20 13:05:37 UTC
The problem is certainly caused by the port being updated -- the old rc script worked, but was very poorly designed and caused some processes to run as root instead of the xymon user.

I did not catch the faststart situation. It's sort of annoying that xymon comes with its own elaborate "rc script" under the guise of "runclient.sh". I will test your proposed solution immediately.


Thanks for the report!
Comment 5 Mark Felder freebsd_committer freebsd_triage 2015-02-20 22:01:47 UTC
Created attachment 153250 [details]
xymon client rc script rewrite

Ok, I'm going to have you try this which seems to work well for me. Basically we are going to bypass the runclient.sh script that comes with Xymon because it duplicates too much of the functionality that already exists with the FreeBSD rc framework. It's good if you have a custom install of Xymon, but unnecessary for our needs.

Please try the below patch. It seems to be working well for me, including using xymon_client_flags in rc.conf and passing --hostname=foo.com, etc.


Thanks!
Comment 6 Mark Felder freebsd_committer freebsd_triage 2015-02-21 16:01:47 UTC
I forgot to mention -- the reason why your approach doesn't work is that when you specify start_cmd the xymon_client_user is ignored and the xymon client doesn't start as a non-root user which is what I was trying to solve. Many of the child processes do run as non-root because Xymon knows they should, but the xymonlaunch and vmstat run as root.

This could be avoided by making start_cmd also include "su -m $xymon_client_user -c ..." but for a long time I've wanted to bypass the duplicated functionality provided by the runclient.sh script
Comment 7 commit-hook freebsd_committer freebsd_triage 2015-02-21 16:50:45 UTC
A commit references this bug:

Author: feld
Date: Sat Feb 21 16:50:31 UTC 2015
New revision: 379535
URL: https://svnweb.freebsd.org/changeset/ports/379535

Log:
  Execute the xymonlaunch process directly. The supplied runclient.sh
  script duplicates much of the rc script and adds unnecessary complexity.

  While here ensure that leftover processes are cleaned up.

  This also fixes a recent regression which prevented xymon-client from
  reliably starting on boot.

  PR:		197827

Changes:
  head/net-mgmt/xymon-client/Makefile
  head/net-mgmt/xymon-client/files/xymon-client.in
Comment 8 John Nielsen 2015-02-23 17:40:26 UTC
Sorry for the delay. I'm testing the updated version from ports now.

Found a small error:
# service xymon-client stop
/usr/local/etc/rc.d/xymon-client: 1: Syntax error: Unterminated quoted string
/usr/local/etc/rc.d/xymon-client: 38: Syntax error: Error in command substitution

It is due to a transposed back-tick and double-quote at the end of line 38 of the installed script.

Also the new script was unable to stop my previously-running client since it was running as root. This is probably just due to the problem with my patch that you've already discussed and won't affect others.

Otherwise looks good. "faststart" and friends work as expected. Thanks!
Comment 9 John Nielsen 2015-02-23 17:49:32 UTC
And a pedantic nit--you can shorten your tr strings by using the :upper: and :lower: classes a la:

tr '[:upper:]/' '[:lower:]_'
Comment 10 Mark Felder freebsd_committer freebsd_triage 2015-02-23 21:48:42 UTC
(In reply to John Nielsen from comment #9)

Yeah I thought so too, but learned that's a dangerous unless you know exactly where you're using it. The effects of [:upper:] [:lower:] can differ by LOCALE -- some characters can be missed entirely. Same goes for [A-Z] [a-z]. I was going to submit a patch to Xymon to fix that in their runclient.sh script but decided not to after researching the possible side effects.
Comment 11 commit-hook freebsd_committer freebsd_triage 2015-02-24 13:53:45 UTC
A commit references this bug:

Author: feld
Date: Tue Feb 24 13:53:42 UTC 2015
New revision: 379791
URL: https://svnweb.freebsd.org/changeset/ports/379791

Log:
  Fix typo that slipped in previous rc script patch

  Pointyhat -> me

  PR:		197827

Changes:
  head/net-mgmt/xymon-client/Makefile
  head/net-mgmt/xymon-client/files/xymon-client.in
Comment 12 Mark Felder freebsd_committer freebsd_triage 2015-02-24 13:54:39 UTC
I don't know how that typo managed to get into my patch... I must have mangled it when reviewing it.

Thanks for the heads up...