diff mbox

[PATCHv2] inadyn: fix init script and default config file

Message ID 1445090722-4297-1-git-send-email-thomas.petazzoni@free-electrons.com
State Accepted
Headers show

Commit Message

Thomas Petazzoni Oct. 17, 2015, 2:05 p.m. UTC
This commit does a number of fixes to the inadyn package to make it
work properly "out of the box":

 * inadyn is installed in /usr/sbin, not /usr/bin, so we fix the path
   in the init script

 * Use "printf" for the Starting and Stopping messages, so that the OK
   / FAIL stay on the same line.

 * Pass the -q option to the start sequence, since it's passed in the
   stop sequence.

 * Fix the configuration file to use an existing dyndns_system and
   avoid a failure at startup.

 * Use a variable called ENABLED in /etc/default/inadyn to decide
   whether to start the service or not. By default, it is not started,
   as suggested by Gustavo, and an explicit ENABLED="yes" is needed.

 * Store the PID file in /var/run/inadyn.pid, like we do for all other
   daemons.

Cc: Alex Suykov <alex.suykov@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---

Changes since v1:

 * Store PID file in /var/run/inadyn.pid instead of
   /var/run/inadyn/inadyn.pid.

 * Use the background functionality of start-stop-daemon instead of
   the one of inadyn itself (suggested by Gustavo)

 * Specify the path to the inadyn executable even when stopping, so
   that start-stop-daemon can check we're not killing the wrong
   program (suggested by Arnout).

 * Do not start inadyn by default, and use an ENABLED variable in
   /etc/default/inadyn to decide if it should be started (suggested by
   Gustavo).
---
 package/inadyn/S70inadyn   | 24 ++++++++++++++++--------
 package/inadyn/inadyn.conf |  4 ++--
 2 files changed, 18 insertions(+), 10 deletions(-)

Comments

Yann E. MORIN Oct. 17, 2015, 8 p.m. UTC | #1
Thomas, All,

On 2015-10-17 16:05 +0200, Thomas Petazzoni spake thusly:
> This commit does a number of fixes to the inadyn package to make it
> work properly "out of the box":
> 
>  * inadyn is installed in /usr/sbin, not /usr/bin, so we fix the path
>    in the init script
> 
>  * Use "printf" for the Starting and Stopping messages, so that the OK
>    / FAIL stay on the same line.
> 
>  * Pass the -q option to the start sequence, since it's passed in the
>    stop sequence.
> 
>  * Fix the configuration file to use an existing dyndns_system and
>    avoid a failure at startup.
> 
>  * Use a variable called ENABLED in /etc/default/inadyn to decide
>    whether to start the service or not. By default, it is not started,
>    as suggested by Gustavo, and an explicit ENABLED="yes" is needed.
> 
>  * Store the PID file in /var/run/inadyn.pid, like we do for all other
>    daemons.
> 
> Cc: Alex Suykov <alex.suykov@gmail.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Twice your SoB line.

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Yet, still a minor thingy...

[--SNIP--]
> diff --git a/package/inadyn/inadyn.conf b/package/inadyn/inadyn.conf
> index b5877f7..aef5870 100644
> --- a/package/inadyn/inadyn.conf
> +++ b/package/inadyn/inadyn.conf
> @@ -1,11 +1,11 @@
>  # Basic configuration file for inadyn
>  #
>  # /etc/inadyn.conf
> -background
> +pidfile /var/run/inadyn.pid
>  update_period_sec 600 # Check for a new IP every 600 seconds
>  username test		# replace 'test' with your username
>  password test		# replace 'test' with your password
> -dyndns_system dyndns@dyndns.org   # replace w/ your provider
> +dyndns_system default@dyndns.org   # replace w/ your provider
>  
>  #  uncomment the alias statement below to test it on your system
>  alias test.homeip.net

The alias line is not commented, which makes the comment above a bit
confusing...

Regards,
Yann E. MORIN.
Thomas Petazzoni Oct. 18, 2015, 1:46 p.m. UTC | #2
Dear Yann E. MORIN,

On Sat, 17 Oct 2015 22:00:31 +0200, Yann E. MORIN wrote:

> >  #  uncomment the alias statement below to test it on your system
> >  alias test.homeip.net
> 
> The alias line is not commented, which makes the comment above a bit
> confusing...

Indeed, thanks. I've dropped it when applying.

Thomas
Thomas Petazzoni Oct. 18, 2015, 1:47 p.m. UTC | #3
Hello,

On Sat, 17 Oct 2015 16:05:22 +0200, Thomas Petazzoni wrote:
> This commit does a number of fixes to the inadyn package to make it
> work properly "out of the box":
> 
>  * inadyn is installed in /usr/sbin, not /usr/bin, so we fix the path
>    in the init script
> 
>  * Use "printf" for the Starting and Stopping messages, so that the OK
>    / FAIL stay on the same line.
> 
>  * Pass the -q option to the start sequence, since it's passed in the
>    stop sequence.
> 
>  * Fix the configuration file to use an existing dyndns_system and
>    avoid a failure at startup.
> 
>  * Use a variable called ENABLED in /etc/default/inadyn to decide
>    whether to start the service or not. By default, it is not started,
>    as suggested by Gustavo, and an explicit ENABLED="yes" is needed.
> 
>  * Store the PID file in /var/run/inadyn.pid, like we do for all other
>    daemons.
> 
> Cc: Alex Suykov <alex.suykov@gmail.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

Applied after dropping the silly comment in inadyn.conf, as suggested
by Yann.

Thomas
Peter Korsgaard Oct. 19, 2015, 7:56 a.m. UTC | #4
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

Hi,

 > Changes since v1:

 >  * Store PID file in /var/run/inadyn.pid instead of
 >    /var/run/inadyn/inadyn.pid.

 >  * Use the background functionality of start-stop-daemon instead of
 >    the one of inadyn itself (suggested by Gustavo)

Didn't Gustavoz suggest the opposite? E.G. to use the inadyn option to
force the background mode on, independently of what the config file
states?

I also see that you are not passing the --syslog option to inadyn, so
log messages go to stdout (which start-stop-daemon redirects to
/dev/null) :/

> +++ b/package/inadyn/inadyn.conf
 > @@ -1,11 +1,11 @@
 >  # Basic configuration file for inadyn
 >  #
 >  # /etc/inadyn.conf
 > -background
 > +pidfile /var/run/inadyn.pid

We should instead use the --pidfile inadyn command line option to
force this as we _REQUIRE_ this for start-stop-daemon.
diff mbox

Patch

diff --git a/package/inadyn/S70inadyn b/package/inadyn/S70inadyn
index b20048c..ca7b414 100644
--- a/package/inadyn/S70inadyn
+++ b/package/inadyn/S70inadyn
@@ -4,25 +4,33 @@ 
 #
 
 CONFIG=/etc/inadyn.conf
-VR_INADYN=/var/run/inadyn
 
 # check if CONFIG exists, print message & exit if it doesn't
 [ ! -f $CONFIG ] && ( echo "The config file "$CONFIG" is missing...exiting now." && exit 2 )
 
-# check if VR_INADYN exists, create it if not
-[ ! -d $VR_INADYN ] && mkdir -p $VR_INADYN
+# Allow a few customizations from a config file. Especially inadyn
+# must be explicitly enabled by adding ENABLED="yes" in this file.
+test -r /etc/default/inadyn && . /etc/default/inadyn
 
 case "$1" in
 	start)
-		echo "Starting inadyn: "
-		start-stop-daemon -S -x /usr/bin/inadyn
+		printf "Starting inadyn: "
+		if test "${ENABLED}" != "yes" ; then
+		    echo "SKIPPED"
+		    exit 0
+		fi
+		start-stop-daemon -b -q -S -p /var/run/inadyn.pid -x /usr/sbin/inadyn
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
 		;;
 	stop)
-		echo  "Stopping inadyn: "
-		start-stop-daemon -q -K -x /usr/bin/inadyn
+		printf "Stopping inadyn: "
+		if test "${ENABLED}" != "yes" ; then
+		    echo "SKIPPED"
+		    exit 0
+		fi
+		start-stop-daemon -q -K -p /var/run/inadyn.pid -x /usr/sbin/inadyn
 		[ $? = 0 ] && echo "OK" || echo "FAIL"
-		rm -f /var/run/inadyn/inadyn.pid
+		rm -f /var/run/inadyn.pid
 		;;
 	restart)
 		"$0" stop
diff --git a/package/inadyn/inadyn.conf b/package/inadyn/inadyn.conf
index b5877f7..aef5870 100644
--- a/package/inadyn/inadyn.conf
+++ b/package/inadyn/inadyn.conf
@@ -1,11 +1,11 @@ 
 # Basic configuration file for inadyn
 #
 # /etc/inadyn.conf
-background
+pidfile /var/run/inadyn.pid
 update_period_sec 600 # Check for a new IP every 600 seconds
 username test		# replace 'test' with your username
 password test		# replace 'test' with your password
-dyndns_system dyndns@dyndns.org   # replace w/ your provider
+dyndns_system default@dyndns.org   # replace w/ your provider
 
 #  uncomment the alias statement below to test it on your system
 alias test.homeip.net