[Buildroot] [PATCH 1/2] package/chrony: rewrite start script

[Buildroot] [PATCH 1/2] package/chrony: rewrite start script

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

This patch brings package/chrony/S49chronyd in line with the start
script standards. Note that users will need to update the location of
any script config from /etc/default/chrony to /etc/default/chronyd.

One deviation from the standard remains (and thus the check-package
override): The PID file is placed in /var/run/chrony/$DAEMON.pid. This
is necessary because chronyd drops privileges, and would not be able
to delete the PID file from root-owned /var/run on shutdown.

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
I'm happy to drop the config rename if it's considered too disruptive,
but it's a simple rename, so I hope it's fine.

 .checkpackageignore       |  2 +-
 package/chrony/S49chrony  | 26 ----------------
 package/chrony/S49chronyd | 65 +++++++++++++++++++++++++++++++++++++++
 package/chrony/chrony.mk  |  2 +-
 4 files changed, 67 insertions(+), 28 deletions(-)
 delete mode 100644 package/chrony/S49chrony
 create mode 100644 package/chrony/S49chronyd

diff --git a/.checkpackageignore b/.checkpackageignore
index e9fc94b863..bf70e2c22c 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -360,7 +360,7 @@ package/cfm/S65cfm lib_sysv.Indent lib_sysv.Variables
 package/cgroupfs-mount/S30cgroupfs Shellcheck lib_sysv.Indent lib_sysv.Variables
 package/chipmunk/0001-Fix-build-failure-on-musl.patch lib_patch.Upstream
 package/chocolate-doom/0001-Remove-redundant-demoextend-definition.patch lib_patch.Upstream
-package/chrony/S49chrony Shellcheck lib_sysv.Indent lib_sysv.Variables
+package/chrony/S49chronyd lib_sysv.Variables
 package/cmake/0001-rename-cmake-rootfile.patch lib_patch.Upstream
 package/cmocka/0001-Don-t-redefine-uintptr_t.patch lib_patch.Upstream
 package/collectd/0001-src-netlink.c-remove-REG_NOERROR.patch lib_patch.Upstream
diff --git a/package/chrony/S49chrony b/package/chrony/S49chrony
deleted file mode 100644
index f75233fe5e..0000000000
--- a/package/chrony/S49chrony
+++ /dev/null
@@ -1,26 +0,0 @@
-#!/bin/sh
-#
-# Start chrony
-
-[ -r /etc/default/chrony ] && . /etc/default/chrony
-
-case "$1" in
-  start)
-	printf "Starting chrony: "
-	chronyd $CHRONY_ARGS && echo "OK" || echo "FAIL"
-	;;
-  stop)
-	printf "Stopping chrony: "
-	killall chronyd && echo "OK" || echo "FAIL"
-	;;
-  restart|reload)
-	"$0" stop
-	sleep 1
-	"$0" start
-	;;
-  *)
-	echo "Usage: $0 {start|stop|restart}"
-	exit 1
-esac
-
-exit $?
diff --git a/package/chrony/S49chronyd b/package/chrony/S49chronyd
new file mode 100644
index 0000000000..6c1525dd76
--- /dev/null
+++ b/package/chrony/S49chronyd
@@ -0,0 +1,65 @@
+#!/bin/sh
+#
+# Start chrony
+
+DAEMON="chronyd"
+# /var/run/chrony is owned by the chrony user, this allows chrony to
+# delete the PID file during shutdown, long after dropping privileges.
+PIDFILE="/var/run/chrony/$DAEMON.pid"
+CHRONY_ARGS=""
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+
+start() {
+	printf "Starting %s: " "$DAEMON"
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon --start --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON" \
+		-- $CHRONY_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+stop() {
+	printf "Stopping %s: " "$DAEMON"
+	start-stop-daemon --stop --pidfile "$PIDFILE" \
+		--exec "/usr/sbin/$DAEMON"
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	while [ -f "$PIDFILE" ]; do
+		sleep 0.1
+	done
+	return "$status"
+}
+
+restart() {
+	stop
+	start
+}
+
+case "$1" in
+	start)
+		start
+		;;
+	stop)
+		stop
+		;;
+	restart)
+		restart
+		;;
+	*)
+		echo "Usage: $0 {start|stop|restart}"
+		exit 1
+esac
+
+exit $?
diff --git a/package/chrony/chrony.mk b/package/chrony/chrony.mk
index 11270206bd..bcf69a59db 100644
--- a/package/chrony/chrony.mk
+++ b/package/chrony/chrony.mk
@@ -79,7 +79,7 @@ define CHRONY_INSTALL_TARGET_CMDS
 endef

 define CHRONY_INSTALL_INIT_SYSV
-	$(INSTALL) -D -m 755 package/chrony/S49chrony $(TARGET_DIR)/etc/init.d/S49chrony
+	$(INSTALL) -D -m 755 package/chrony/S49chronyd $(TARGET_DIR)/etc/init.d/S49chronyd
 endef

 define CHRONY_INSTALL_INIT_SYSTEMD
--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/2] package/openssh: consistently use $DAEMON in start script output

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

This was already done in start(), but not in stop() and reload().

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
 package/openssh/S50sshd | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/package/openssh/S50sshd b/package/openssh/S50sshd
index 7e5ccf079d..a5ff385acc 100644
--- a/package/openssh/S50sshd
+++ b/package/openssh/S50sshd
@@ -28,7 +28,7 @@ start() {
 }

 stop() {
-	printf "Stopping sshd: "
+	printf "Stopping %s: " "$DAEMON"
 	start-stop-daemon --stop --pidfile "$PIDFILE" \
 		--exec "/usr/sbin/$DAEMON"
 	status=$?
@@ -50,7 +50,7 @@ restart() {
 }

 reload() {
-	printf "Reloading sshd config: "
+	printf "Reloading %s config: " "$DAEMON"
 	start-stop-daemon --stop --signal HUP -q --pidfile "$PIDFILE" \
 		--exec "/usr/sbin/$DAEMON"
 	status=$?
--
2.45.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/2] package/chrony: rewrite start script

[Thomas Petazzoni via buildroot]

On Tue, 29 Oct 2024 20:04:19 +0100
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> This patch brings package/chrony/S49chronyd in line with the start
> script standards. Note that users will need to update the location of
> any script config from /etc/default/chrony to /etc/default/chronyd.
> 
> One deviation from the standard remains (and thus the check-package
> override): The PID file is placed in /var/run/chrony/$DAEMON.pid. This
> is necessary because chronyd drops privileges, and would not be able
> to delete the PID file from root-owned /var/run on shutdown.
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
> ---
> I'm happy to drop the config rename if it's considered too disruptive,
> but it's a simple rename, so I hope it's fine.

Both patches, applied, thanks! I also wondered a bit about the rename,
and thought it was OK. We'll see if users complain.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot