[v2,1/1] package/collectd: add init script for SysV

Signed-off-by: Asaf Kahlon <asafka7@gmail.com>
---
v1->v2: use start-stop-daemon
---
 package/collectd/S90collectd | 36 ++++++++++++++++++++++++++++++++++++
 package/collectd/collectd.mk |  5 +++++
 2 files changed, 41 insertions(+)
 create mode 100644 package/collectd/S90collectd

Hello Asaf,

On Sat, 26 Oct 2019 10:19:04 +0300
Asaf Kahlon <asafka7@gmail.com> wrote:

> Signed-off-by: Asaf Kahlon <asafka7@gmail.com>
> ---
> v1->v2: use start-stop-daemon
> ---
>  package/collectd/S90collectd | 36 ++++++++++++++++++++++++++++++++++++
>  package/collectd/collectd.mk |  5 +++++
>  2 files changed, 41 insertions(+)
>  create mode 100644 package/collectd/S90collectd
> 
> diff --git a/package/collectd/S90collectd b/package/collectd/S90collectd
> new file mode 100644
> index 0000000000..3c5af17674
> --- /dev/null
> +++ b/package/collectd/S90collectd
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +DAEMON="collectd"
> +PIDFILE="/var/run/$DAEMON.pid"
> +
> +start() {
> +	printf "Starting collectd... "
> +	start-stop-daemon -b -m -S -q -p $PIDFILE -x "/usr/sbin/$DAEMON"

Does this actually works? In your v1, you were just starting the
collectd daemon, without anything to put it in the background, so it
suppose it would automatically fork itself and go in the background.
This would not play very well with start-stop-daemon, so I was assuming
you would need to pass some collectd option to keep it in the
foreground, and let start-stop-daemon do the daemonization.

Did you test your v2 on the target ? Does the pidfile really contains
the PID of the daemon that is running ? Does the stop action actually
works ?

Thanks!

Thomas

Hello,

On Sat, Oct 26, 2019 at 10:23 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Asaf,
>
> On Sat, 26 Oct 2019 10:19:04 +0300
> Asaf Kahlon <asafka7@gmail.com> wrote:
>
> > Signed-off-by: Asaf Kahlon <asafka7@gmail.com>
> > ---
> > v1->v2: use start-stop-daemon
> > ---
> >  package/collectd/S90collectd | 36 ++++++++++++++++++++++++++++++++++++
> >  package/collectd/collectd.mk |  5 +++++
> >  2 files changed, 41 insertions(+)
> >  create mode 100644 package/collectd/S90collectd
> >
> > diff --git a/package/collectd/S90collectd b/package/collectd/S90collectd
> > new file mode 100644
> > index 0000000000..3c5af17674
> > --- /dev/null
> > +++ b/package/collectd/S90collectd
> > @@ -0,0 +1,36 @@
> > +#!/bin/sh
> > +
> > +DAEMON="collectd"
> > +PIDFILE="/var/run/$DAEMON.pid"
> > +
> > +start() {
> > +     printf "Starting collectd... "
> > +     start-stop-daemon -b -m -S -q -p $PIDFILE -x "/usr/sbin/$DAEMON"
>
> Does this actually works? In your v1, you were just starting the
> collectd daemon, without anything to put it in the background, so it
> suppose it would automatically fork itself and go in the background.
> This would not play very well with start-stop-daemon, so I was assuming
> you would need to pass some collectd option to keep it in the
> foreground, and let start-stop-daemon do the daemonization.
>
> Did you test your v2 on the target ? Does the pidfile really contains
> the PID of the daemon that is running ? Does the stop action actually
> works ?

Yes, I loaded the image with qemu and the daemon was started.
The pid on /var/run/collectd.pid was the pid of collectd. After that, I manually
run "/etc/init.d/S90collectd stop", made sure the process died and the
/var/run/collectd.pid file was removed. The logs also looked fine and
stated normal
start/stop of the daemon.

>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Asaf.

On Sat, Oct 26, 2019 at 4:19 AM Asaf Kahlon <asafka7@gmail.com> wrote:
>
> Signed-off-by: Asaf Kahlon <asafka7@gmail.com>
> ---
> v1->v2: use start-stop-daemon
> ---
>  package/collectd/S90collectd | 36 ++++++++++++++++++++++++++++++++++++
>  package/collectd/collectd.mk |  5 +++++
>  2 files changed, 41 insertions(+)
>  create mode 100644 package/collectd/S90collectd
>
> diff --git a/package/collectd/S90collectd b/package/collectd/S90collectd
> new file mode 100644
> index 0000000000..3c5af17674
> --- /dev/null
> +++ b/package/collectd/S90collectd
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +DAEMON="collectd"
EXEC="/usr/sbin/$DAEMON"
> +PIDFILE="/var/run/$DAEMON.pid"
> +

Leave room to user customizations

COLLECTD_ARGS=""

# shellcheck source=/dev/null
[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"

> +start() {
> +       printf "Starting collectd... "
> +       start-stop-daemon -b -m -S -q -p $PIDFILE -x "/usr/sbin/$DAEMON"

The -b and -m are not necessary here, since collectd daemonizes by
default. The only reason why it "works" is because collectd takes a
few milliseconds to daemonize and override the /var/run/collectd.pid
that was written by start-stop-daemon. Replacing $PIDFILE by
$PIDFILE.x in the line above will show the truth:

# /etc/init.d/S90collectd stop
Stopping collectd... OK
# /etc/init.d/S90collectd start
Starting collectd... OK
# pidof collectd
161
# ls -ltr /var/run/collectd.pid*
-rw-r--r--    1 root     root             4 Oct 28 18:15
/var/run/collectd.pid.x <<-- written by start-stop-daemon
-rw-r--r--    1 root     root             4 Oct 28 18:15
/var/run/collectd.pid <<-- written by collectd
==> /var/run/collectd.pid <==
161 <<-- daemonized correctd

==> /var/run/collectd.pid.x <==
160 <<-- initial collectd process


> +       [ $? = 0 ] && echo "OK" || echo "FAIL"]

This leads the function to result the result of the "echo" command on
failure, which will be zero. Use this construction, instead

start() {
    printf 'Starting %s: ' "$DAEMON"
    # shellcheck disable=SC2086 # we need the word splitting
    start-stop-daemon -S -q -p "$PIDFILE" -x "$EXEC" \
        -- $COLLECTD_ARGS
    status=$?
    if [ "$status" -eq 0 ]; then
        echo "OK"
    else
        echo "FAIL"
    fi
    return "$status"
}

> +}
> +stop() {
> +       printf "Stopping collectd... "
> +       start-stop-daemon -K -q -p $PIDFILE
> +       [ $? = 0 ] && echo "OK" || echo "FAIL"
> +}

Leave a blank line between each function to improve readability

> +restart() {
> +       stop

Put a one-second pause here, giving time for the daemon to stop.

> +       start
> +}
> +
> +case "$1" in
> +  start)
> +       start
> +       ;;
> +  stop)
> +       stop
> +       ;;
> +  restart|reload)
> +       restart
> +       ;;
> +  *)
> +       echo "Usage: $0 {start|stop|restart}"
> +       exit 1
> +esac

case "$1" in
    start|stop|restart)
        "$1";;
    reload)
        restart;;
    *)
        echo "Usage: $0 {start|stop|restart|reload}"
        exit 1
esac

> +exit $?

This is not required. A Shell script exit status is by default the the
same one of the last command.

> diff --git a/package/collectd/collectd.mk b/package/collectd/collectd.mk
> index 5d94dec46d..1bce196e7e 100644
> --- a/package/collectd/collectd.mk
> +++ b/package/collectd/collectd.mk
> @@ -218,4 +218,9 @@ define COLLECTD_INSTALL_INIT_SYSTEMD
>                 $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/collectd.service
>  endef
>
> +define COLLECTD_INSTALL_INIT_SYSV
> +       $(INSTALL) -D -m 0755 package/collectd/S90collectd \
> +               $(TARGET_DIR)/etc/init.d/S90collectd
> +endef
> +
>  $(eval $(autotools-package))
> --
> 2.20.1
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Refer to package/sysklogd/S01syslogd  and package/acpid/S02acpid for examples.