Bug 118325
| Summary: | [patch] [request] new periodic script to test statuses of daemons started via rc system | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Matthew Seaman <m.seaman> | ||||
| Component: | bin | Assignee: | freebsd-bugs (Nobody) <bugs> | ||||
| Status: | Open --- | ||||||
| Severity: | Affects Only Me | Keywords: | patch | ||||
| Priority: | Normal | ||||||
| Version: | 7.0-BETA1 | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
|
Description
Matthew Seaman
2007-11-29 06:40:02 UTC
Responsible Changed From-To: freebsd-bugs->freebsd-rc Over to maintainer(s) for discussion. State Changed From-To: open->analyzed I like this idea, and I have some feedback for it. Responsible Changed From-To: freebsd-rc->dougb I'd like to manage this one. I like this idea a lot, I think it would be a good addition to periodic. I do have some feedback for you, which I hope you will take in the constructive manner in which it's offered. First some meta-feedback. I am disturbed to hear that some of our system scripts don't support the 'status' argument. I'm moderately disturbed that some of the ones that don't start services return something that could be interpreted as meaningful for it. Could you work up a list for both categories, and send it to freebsd-rc@freebsd.org? Bonus points if you include patches. :) Now the feedback for your script. A general comment is that a very important rule of shell scripting is that you always want to avoid forking a subshell if you can. Thus I'd like to see your 'echo foo | grep' statements converted to use case. You can take a look at mergemaster if you need examples, I use that trick a lot. In terms of style, while I wouldn't hold it up for this the style we use most often is 'if [ blah ]; then' rather than putting 'then' on its own line. It saves a little space is the primary reason, although in the case of contributing code to an existing project maintaining a consistent style is its own reward. And while I'm picking nits, "result" is usually spelled "rc" (return code). In terms of what the script actually outputs, I'm torn. On the one hand I can't help feeling that it shouldn't output anything if the service is running. On the other I know some people will want that warm fuzzy feeling. So I'd like to propose that you add a _verbose variable to this mix. There are a few examples in /etc/periodic/daily you can take a look at. I also don't think that you should duplicate the name of the service if it's running, I think that's wasted space. What I have in mind is something like this: output=$( foo ) rc=$? case "$rc" in 0) case "$daily_status_rc_verbose" in yes) echo "$output" ;; esac ;; *) echo "$output" ;; esac Now, that's the simple version. :) If you really want to get fancy, what I'd do is to run a counter for problem scripts, and increment it if you find any, and save the outputs of the various status commands. Then at the end you would do something like: case "verbose" in yes) echo "Services running normally:" echo "outputs" ;; esac case "number_of_problem_scripts" in 0) case "verbose" in yes) echo "No problems found!" ;; esac *) echo "Services not running:" echo "bad-outputs" ;; esac There is another meta-message here, which is an output of "rc script status problems" followed by no output would probably be alarming to users, so one way or another you should find a way around that. hope this helps, Doug PS, if you're only submitting one file, it doesn't have to be a shar archive. :) -- This .signature sanitized for your protection This is a multi-part message in MIME format. -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
It seems there is a problem with /etc/rc.d/tmp which does not use the
normal /etc/rc.subr machinery. Effectively all calls to /etc/rc.d/tmp
cause the 'start' action. Most of the time this is harmless unless $tmpmfs
is set to "YES" in /etc/rc.conf -- in that case, the script will create and
mount yet another mfs over /tmp each time it is called.
This just adds 'tmp' to the exceptions list and changes the logic slightly
so that the scripts in the exceptions list are never called. I'll send-pr
about the tmp script separately.
Matthew
% cvs diff -u 490.status-rc
Index: 490.status-rc
===================================================================
RCS file: /home/matthew/cvsroot/scripts/490.status-rc,v
retrieving revision 1.5
diff -u -u -r1.5 490.status-rc
- --- 490.status-rc 9 Dec 2007 11:27:16 -0000 1.5
+++ 490.status-rc 31 Dec 2007 10:37:59 -0000
@@ -8,11 +8,11 @@
# /etc/periodic.conf.local to override the default values.
#
# daily_status_rc_enable="YES" # run tests
- -# daily_status_rc_exceptions="newsyslog bsdstats.sh othermta"
- -# # Startup scripts that don't give meaningful status output
+# daily_status_rc_exceptions="tmp newsyslog bsdstats.sh othermta"
+# # Startup scripts not to call 'status' on
daily_status_rc_enable="YES"
- -daily_status_rc_exceptions="newsyslog bsdstats.sh othermta"
+daily_status_rc_exceptions="tmp newsyslog bsdstats.sh othermta"
# If there is a global system configuration file, suck it in.
#
@@ -43,6 +43,19 @@
# Now loop over all of the rc files and query their status
for _rc_elem in ${files}; do
+
+ # Some rc-scripts do not start long-running processes but
+ # still provide the default 'status' action. Worse:
+ # /etc/rc.d/tmp does the 'start' action even when called as
+ # 'status', which is harmful if $tmpmfs is defined in
+ # /etc/rc.conf. Skip the known problem cases.
+
+ case ${daily_status_rc_exceptions} in
+ *${_rc_elem##*/}*)
+ continue ;;
+ *) ;;
+ esac
+
output=$( run_rc_script $_rc_elem status 2>&1 )
if [ $? -ne 0 ]; then
@@ -57,17 +70,6 @@
* ) ;;
esac
- - # Some rc-scripts do not start long-running processes but
- - # still provide the default 'status' action. This doesn't
- - # make a great deal of sense given the default action is
- - # to test that some process is alive.
- -
- - case ${daily_status_rc_exceptions} in
- - *${_rc_elem##*/}*)
- - continue ;;
- - *) ;;
- - esac
- -
echo "==>> $_rc_elem"
echo $output
rc=1
- --
Dr Matthew J Seaman MA, D.Phil. 7 Priory Courtyard
Flat 3
PGP: http://www.infracaninophile.co.uk/pgpkey Ramsgate
Kent, CT11 9PW
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4 (FreeBSD)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFHeMh78Mjk52CukIwRCKQFAJ4wVbdum79ZDzIGxKewAmTfwUG6EgCfW0Js
faR1N+0Ck1Sdr3E8KtKYrNc=
=iZAD
-----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Matthew Seaman wrote:
Dammit. Scratch the previous patch. /etc/rc.d/var does the same
thing as /etc/rc.d/tmp.
Matthew
% cvs diff -u 490.status-rc
Index: 490.status-rc
===================================================================
RCS file: /home/matthew/cvsroot/scripts/490.status-rc,v
retrieving revision 1.5
diff -u -u -r1.5 490.status-rc
- --- 490.status-rc 9 Dec 2007 11:27:16 -0000 1.5
+++ 490.status-rc 31 Dec 2007 11:02:50 -0000
@@ -8,11 +8,11 @@
# /etc/periodic.conf.local to override the default values.
#
# daily_status_rc_enable="YES" # run tests
- -# daily_status_rc_exceptions="newsyslog bsdstats.sh othermta"
- -# # Startup scripts that don't give meaningful status output
+# daily_status_rc_exceptions="tmp var newsyslog bsdstats.sh othermta"
+# # Startup scripts not to call 'status' on
daily_status_rc_enable="YES"
- -daily_status_rc_exceptions="newsyslog bsdstats.sh othermta"
+daily_status_rc_exceptions="tmp var newsyslog bsdstats.sh othermta"
# If there is a global system configuration file, suck it in.
#
@@ -43,6 +43,19 @@
# Now loop over all of the rc files and query their status
for _rc_elem in ${files}; do
+
+ # Some rc-scripts do not start long-running processes but
+ # still provide the default 'status' action. Worse:
+ # /etc/rc.d/{tmp,var} do the 'start' action even when called
+ # as 'status', which is harmful if $tmpmfs or $varmfs is
+ # defined in /etc/rc.conf. Skip the known problem cases.
+
+ case ${daily_status_rc_exceptions} in
+ *${_rc_elem##*/}*)
+ continue ;;
+ *) ;;
+ esac
+
output=$( run_rc_script $_rc_elem status 2>&1 )
if [ $? -ne 0 ]; then
@@ -57,17 +70,6 @@
* ) ;;
esac
- - # Some rc-scripts do not start long-running processes but
- - # still provide the default 'status' action. This doesn't
- - # make a great deal of sense given the default action is
- - # to test that some process is alive.
- -
- - case ${daily_status_rc_exceptions} in
- - *${_rc_elem##*/}*)
- - continue ;;
- - *) ;;
- - esac
- -
echo "==>> $_rc_elem"
echo $output
rc=1
- --
Dr Matthew J Seaman MA, D.Phil. 7 Priory Courtyard
Flat 3
PGP: http://www.infracaninophile.co.uk/pgpkey Ramsgate
Kent, CT11 9PW
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4 (FreeBSD)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFHeMzk8Mjk52CukIwRCGh/AJ4/QCGHWycLmyWnlm6+GQlntwdgWgCff5tO
Mmy7/HtPqW8J098kbWQHi3o=
=K87v
-----END PGP SIGNATURE-----
State Changed From-To: analyzed->open Cast this one back into the pool, unfortunately I won't have time to give it proper attention any time soon. Responsible Changed From-To: dougb->freebsd-rc Cast this one back into the pool, unfortunately I won't have time to give it proper attention any time soon. For bugs matching the following criteria: Status: In Progress Changed: (is less than) 2014-06-01 Reset to default assignee and clear in-progress tags. Mail being skipped Keyword:
patch
or patch-ready
– in lieu of summary line prefix:
[patch]
* bulk change for the keyword
* summary lines may be edited manually (not in bulk).
Keyword descriptions and search interface:
<https://bugs.freebsd.org/bugzilla/describekeywords.cgi>
|
