Summary:	ASTERISK-15322: [patch] ASTARGS in sysconfig not inherited as startup options
Reporter:	Nico Kooijman (syspert)	Labels:
Date Opened:	2009-12-16 10:10:16.000-0600	Date Closed:	2010-01-11 17:01:50.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 20091228__issue16454__3.diff.txt ( 1) parm_ast.patch
Description:	An option specified in /etc/sysconfig/asterisk is not passed on in ASTARGS when starting asterisk. Two minor changes made the process work.
Comments:	By: Nico Kooijman (syspert) 2009-12-16 10:14:47.000-0600 Learnig quickly patch made against trunk. The file /etc/sysconfig/asterisk was expected to be executable, which is questionable for a config file, and when $AST_USER (always) the var ASTARGS was initialized. By: Tilghman Lesher (tilghman) 2009-12-22 21:16:54.000-0600 I disagree with this patch.  If you want to make additional arguments available, the way to do this has always been to include individual options for each, not bundle all extra options into a single variable. ASTARGS is properly an internal variable, not available for modification by an external file. By: dant (dant) 2009-12-23 21:45:32.000-0600 I'd be willing to make a bigger more complex script that uses a variable per argument, but, I don't think it's required... There are plenty of init scripts already in existance that allow OPTS or ARGS to be overridden or extended from /etc/sysconfig/*... I was just looking to pass an additional argument when starting asterisk using the init script to attempt to diagnose another problem when I found it didn't work as expected, the patch I just created to resolve this is exactly the same as the patch attached by syspert, so I won't bother uploading it :)... the -x check on /etc/sysconfig/asterisk is definitely just wrong, it should be -f, I'd guess this was probably done at some point by someone not understanding the difference between '. /etc/sysconfig/asterisk' meaning 'source /etc/sysconfig/asterisk' and './etc/sysconfig/asterisk' meaning execute the file asterisk in the etc/sysconfig subdirectory. I would suggest that the attached patch is both a fix and an enhancement that would be valid even if /etc/sysconfig/asterisk was allowed to contain user friendly yes/no activation of specific options. By: Tilghman Lesher (tilghman) 2009-12-23 23:37:09.000-0600 I'm fine with the -f change, just not a default-everything option patch.  Break it out into individual yes/no options, and I'm better about it. By: Tilghman Lesher (tilghman) 2009-12-27 13:03:42.000-0600 I've uploaded a suggested changeset for the Debian init file, which might be a model for the redhat init file to follow. By: Tilghman Lesher (tilghman) 2009-12-27 13:04:45.000-0600 tzafrir: could you look at this suggested changeset for Debian, and give me some feedback? By: Tzafrir Cohen (tzafrir) 2009-12-27 13:33:43.000-0600 I don't like the following part of the patch: +if ! test -f /etc/default/asterisk ; then + echo >/etc/default/asterisk <<EOF By: Tilghman Lesher (tilghman) 2009-12-28 00:35:59.000-0600 tzafrir: how would you suggest ensuring that the external config file gets created? By: dant (dant) 2009-12-28 01:22:54.000-0600 The file should get created as part of the install... If it doesn't exist for whatever reason it shouldn't be a show stopper... By: Tilghman Lesher (tilghman) 2009-12-28 09:00:14.000-0600 dant:  it's not a showstopper in this case, either.  It simply gets created as a matter of course. By: Nico Kooijman (syspert) 2009-12-28 09:24:04.000-0600 I assumed that the variable ASTARGS was a global one because it was already initialized, I just came across the code and looked at the -x option and thought it was a simple bug. We don't really need this one, it's okay for me to drop this one. By: Tilghman Lesher (tilghman) 2009-12-28 13:04:12.000-0600 tzafrir: patch updated. By: Tilghman Lesher (tilghman) 2009-12-28 13:38:36.000-0600 There!  Config files for all! By: Tzafrir Cohen (tzafrir) 2009-12-28 14:13:42.000-0600 One small nit: Instead of:  if [ -f filename ]; then  . filename; fi use: if [ -r filename ]; then . filename; fi Just in case the file exists but is not readable for whatever reason. Another thing to note: the Debian package (and maybe also others) runs Asterisk as user 'asterisk' by default and forbids you to run Asterisk as root. By: Tilghman Lesher (tilghman) 2009-12-28 14:54:33.000-0600 Packagers are downstream, and they may do a great many things.  However, we're looking at the default behavior here, and I'm satisfied with it as is. -r is a good idea, though. By: Nico Kooijman (syspert) 2009-12-29 03:53:14.000-0600 Just tested it and the startup options are now included! Only get a warning: Starting asterisk: /etc/init.d/asterisk: line 87: [: -gt: unary operator expected /etc/init.d/asterisk: line 90: [: -gt: unary operator expected /etc/init.d/asterisk: line 93: [: -gt: unary operator expected The same goes for $VERBOSITY By: Leif Madsen (lmadsen) 2010-01-04 13:58:52.000-0600 Unassigned myself as the reporter has tested and provided feedback. By: Digium Subversion (svnbot) 2010-01-11 17:01:48.000-0600 Repository: asterisk Revision: 239231 U   trunk/Makefile U   trunk/contrib/init.d/rc.debian.asterisk U   trunk/contrib/init.d/rc.mandriva.asterisk U   trunk/contrib/init.d/rc.redhat.asterisk U   trunk/contrib/init.d/rc.suse.asterisk ------------------------------------------------------------------------ r239231 | tilghman | 2010-01-11 17:00:55 -0600 (Mon, 11 Jan 2010) | 7 lines Permit more options in the Makefile as to startup options (closes issue ASTERISK-15322) Reported by: syspert Patches:       20091228__issue16454__3.diff.txt uploaded by tilghman (license 14) Tested by: syspert ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=239231