Refactor ready:script to support new notify framework

Follow-up to abcb3ce, calling the service ready:script when readiness
has been signaled to or detected by Finit.

Issue #300

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

plugins/pidfile.c

@@ -63,7 +63,6 @@ static int pidfile_add_path(struct iwatch *iw, char *path)
 
 static void pidfile_update_conds(char *dir, char *name, uint32_t mask)
 {
-	char ready[MAX_COND_LEN];
 	char cond[MAX_COND_LEN];
 	char fn[PATH_MAX];
 	svc_t *svc;
@@ -81,7 +80,6 @@ static void pidfile_update_conds(char *dir, char *name, uint32_t mask)
 	}
 
 	dbg("Found svc %s for %s with pid %d", svc->name, fn, svc->pid);
-	snprintf(ready, sizeof(ready), "service/%s/ready", svc_ident(svc, NULL, 0));
 	mkcond(svc, cond, sizeof(cond));
 
 	if (mask & (IN_CLOSE_WRITE | IN_ATTRIB | IN_MODIFY | IN_MOVED_TO)) {
@@ -117,11 +115,11 @@ static void pidfile_update_conds(char *dir, char *name, uint32_t mask)
 
 		cond_set(cond);
 		if (!svc->notify)
-			cond_set(ready);
+			service_ready(svc);
 	} else if (mask & IN_DELETE) {
 		cond_clear(cond);
 		if (!svc->notify)
-			cond_set(ready);
+			service_ready(svc);
 	}
 }
 

src/cond-w.c

@@ -297,17 +297,10 @@ int cond_set_noupdate(const char *name)
 
 void cond_set(const char *name)
 {
-	svc_t *svc;
-
 	dbg("%s", name);
 	if (cond_set_noupdate(name))
 		return;
 
-	/* XXX: refactor when new notify framework is in place */
-	svc = svc_find_by_cond(name);
-	if (svc && svc_has_ready(svc))
-		service_ready_script(svc);
-
 	cond_update(name);
 }
 

src/service.c

@@ -1797,7 +1797,7 @@ static void service_pre_script(svc_t *svc)
 {
 	svc->pid = service_fork(svc);
 	if (svc->pid < 0) {
-		err(1, "Failed forking off %s pre-script %s", svc_ident(svc, NULL, 0), svc->pre_script);
+		err(1, "Failed forking off %s pre:script %s", svc_ident(svc, NULL, 0), svc->pre_script);
 		return;
 	}
 
@@ -1822,6 +1822,8 @@ static void service_pre_script(svc_t *svc)
 		_exit(EX_OSERR);
 	}
 
+	dbg("%s: pre:script %s started as PID %d", svc_ident(svc, NULL, 0), svc->pre_script, svc->pid);
+
 	/* Short hard-coded timeout to prevent locking up Finit */
 	service_timeout_after(svc, svc->killdelay, service_kill_script);
 }
@@ -1830,7 +1832,7 @@ static void service_post_script(svc_t *svc)
 {
 	svc->pid = service_fork(svc);
 	if (svc->pid < 0) {
-		err(1, "Failed forking off %s post-script %s", svc_ident(svc, NULL, 0), svc->post_script);
+		err(1, "Failed forking off %s post:script %s", svc_ident(svc, NULL, 0), svc->post_script);
 		return;
 	}
 
@@ -1870,6 +1872,8 @@ static void service_post_script(svc_t *svc)
 		_exit(EX_OSERR);
 	}
 
+	dbg("%s: post:script %s started as PID %d", svc_ident(svc, NULL, 0), svc->post_script, svc->pid);
+
 	/* Short hard-coded timeout to prevent locking up Finit */
 	service_timeout_after(svc, svc->killdelay, service_kill_script);
 }
@@ -1908,6 +1912,9 @@ void service_ready_script(svc_t *svc)
 		_exit(EX_OSERR);
 	}
 
+	dbg("%s: ready:script %s started as PID %d", svc_ident(svc, NULL, 0), svc->ready_script, pid);
+
+	/* Short hard-coded timeout to prevent locking up Finit */
 	service_script_add(svc, pid);
 }
 
@@ -2031,6 +2038,18 @@ void service_forked(svc_t *svc)
 	svc_set_state(svc, SVC_RUNNING_STATE);
 }
 
+/* Set service/foo/ready condition and call optional ready:script */
+void service_ready(svc_t *svc)
+{
+	char buf[MAX_COND_LEN];
+
+	snprintf(buf, sizeof(buf), "service/%s/ready", svc_ident(svc, NULL, 0));
+	cond_set(buf);
+
+	if (svc_has_ready(svc))
+		service_ready_script(svc);
+}
+
 /*
  * Transition task/run/service
  *
@@ -2213,11 +2232,8 @@ restart:
 
 				svc_mark_clean(svc);
 			}
-			if (svc->notify == 2 && svc->notify_watcher.fd == 0) {
+			if (svc->notify == 2 && svc->notify_watcher.fd == 0)
-				char buf[120];
+				service_ready(svc);
-				snprintf(buf, sizeof(buf), "service/%s/ready", svc_ident(svc, NULL, 0));
-				cond_set(buf);
-			}
 			break;
 		}
 		break;
@@ -2355,7 +2371,7 @@ int service_completed(void)
 void service_notify_cb(uev_t *w, void *arg, int events)
 {
 	svc_t *svc = (svc_t *)arg;
-	char buf[120];
+	char buf[32];
 	ssize_t len;
 
 	if (UEV_ERROR == events) {
@@ -2374,8 +2390,7 @@ void service_notify_cb(uev_t *w, void *arg, int events)
 
 	/* systemd and s6, respectively.  The latter then closes the socket */
 	if (!strcmp(buf, "READY=1\n") || !strcmp(buf, "\n")) {
-		snprintf(buf, sizeof(buf), "service/%s/ready", svc_ident(svc, NULL, 0));
+		service_ready(svc);
-		cond_set(buf);
 		/* s6 applications close their socket after notification */
 		if (svc->notify == 2) {
 			uev_io_stop(w);

src/service.h

@@ -41,6 +41,7 @@ int       service_timeout_after  (svc_t *svc, int timeout, void (*cb)(svc_t *svc
 int       service_timeout_cancel (svc_t *svc);
 
 void      service_forked         (svc_t *svc);
+void      service_ready          (svc_t *svc);
 
 int       service_stop           (svc_t *svc);
 int       service_step           (svc_t *svc);

test/Makefile.am

@@ -1,12 +1,12 @@
 SUBDIRS                  = common tenv
 
-EXTRA_DIST		 = skel/bin/busybox-x86_64.sha256 skel/sbin/service.sh skel/bin/slay \
+EXTRA_DIST		 = skel/bin/busybox-x86_64.sha256 skel/sbin/service.sh skel/etc/env  \
-			   skel/etc/env skel/bin/pre.sh skel/bin/post.sh skel/sbin/task.sh   \
+			   skel/bin/pre.sh skel/bin/post.sh skel/bin/ready.sh skel/bin/slay  \
 			   skel/cdrom/.empty skel/dev/shm/.empty skel/dev/pts/.empty	     \
 			   skel/etc/inittab skel/etc/hostname skel/etc/fstab 		     \
 			   skel/etc/init.d/rcS skel/etc/init.d/rcK skel/tmp/.empty	     \
-			   skel/etc/finit.d/.empty skel/etc/init.d/S01-service.sh 	     \
+			   skel/etc/finit.d/.empty skel/sbin/task.sh 	                     \
-			   skel/etc/init.d/S02-serv.sh					     \
+			   skel/etc/init.d/S01-service.sh skel/etc/init.d/S02-serv.sh	     \
 			   skel/proc/.empty skel/root/.empty skel/run/.empty		     \
 			   skel/sbin/chrootsetup.sh skel/srv/.empty skel/sys/.empty	     \
 			   skel/usr/bin/.empty skel/usr/lib/.empty skel/usr/sbin/.empty	     \
@@ -26,6 +26,7 @@ EXTRA_DIST		+= global-envs.sh
 EXTRA_DIST		+= initctl-status-subset.sh
 EXTRA_DIST		+= notify.sh
 EXTRA_DIST		+= pre-post-serv.sh
+EXTRA_DIST		+= ready-serv.sh
 EXTRA_DIST		+= restart-self.sh
 EXTRA_DIST		+= runlevel.sh
 EXTRA_DIST		+= start-stop-service.sh
@@ -56,6 +57,7 @@ TESTS			+= global-envs.sh
 TESTS			+= initctl-status-subset.sh
 TESTS			+= notify.sh
 TESTS			+= pre-post-serv.sh
+TESTS			+= ready-serv.sh
 TESTS			+= restart-self.sh
 TESTS			+= runlevel.sh
 TESTS			+= start-stop-service.sh

test/ready-serv.sh

@@ -0,0 +1,50 @@
+#!/bin/sh
+# Verify ready:script for pid/systemd/s6 style services
+set -eu
+
+TEST_DIR=$(dirname "$0")
+
+test_setup()
+{
+	say "Test start $(date)"
+}
+
+test_teardown()
+{
+	say "Test done $(date)"
+
+	texec rm -f "$FINIT_CONF"
+}
+
+test_one()
+{
+	service=$1
+	file=/tmp/ready
+
+	say "Add service stanza '$service' to $FINIT_CONF ..."
+	texec sh -c "echo '$service' > $FINIT_CONF"
+
+	say 'Reload Finit'
+	texec sh -c "initctl reload"
+
+	retry 'assert_num_children 1 serv'
+	assert_file_contains "$file" "READY"
+	texec sh -c "rm -f $file"
+
+	say 'Stop the service'
+	texec sh -c "initctl stop serv"
+	retry 'assert_num_children 0 serv'
+
+	say "Done, drop service from $FINIT_CONF ..."
+	texec sh -c "rm $FINIT_CONF"
+	texec sh -c "initctl reload"
+}
+
+# shellcheck source=/dev/null
+. "$TEST_DIR/tenv/lib.sh"
+
+#texec sh -c "initctl debug"
+
+test_one "service                ready:/bin/ready.sh serv -np       -- Native style notification"
+test_one "service notify:s6      ready:/bin/ready.sh serv -np -N %n -- s6 style notification"
+test_one "service notify:systemd ready:/bin/ready.sh serv -np       -- systemd style notification"

test/skel/bin/ready.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+echo "READY" > /tmp/ready