confd: refactor and refine container creation

- Reduce the amount of queues: 3 -> 1
 - Simplify post hook
 - Refine execd

The resulting simplification of infix_containers_post_hook(), and
touching execd, also ensure container environment variable changes
are propagated.

Fixes #822

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

board/common/rootfs/usr/sbin/container

@@ -627,7 +627,7 @@ case $cmd in
 	;;
     upgrade)
 	# Start script used to initially create container
-	script=/run/containers/active/S01-${1}.sh
+	script=/run/containers/${1}.sh
 
 	# Find container image
 	img=$(podman inspect "$1" | jq -r .[].ImageName)

package/execd/execd.conf

@@ -1,4 +1,4 @@
 task log:prio:local1.notice,tag:container-flush <pid/syslogd> \
 	[S] container flush -- Cleaning up lingering containers
 service log:prio:local1.err,tag:container \
-	[2345] execd /run/containers/queue /run/containers/active -- Container job runner
+	[2345] execd /run/containers/queue -- Container job runner

package/execd/tmpfiles.conf

@@ -3,6 +3,5 @@ d  /run/containers/files	0700 - -
 d  /var/lib/containers/oci	0755 - -
 d  /run/containers/inbox	0700 - -
 d  /run/containers/queue	0700 - -
-d  /run/containers/active	0700 - -
 d  /run/cni			0755 - -
 L+ /var/lib/cni                 -    - - - /run/cni

src/confd/src/infix-containers.c

@@ -13,13 +13,16 @@
 #include <srx/srx_val.h>
 
 #include "core.h"
+
 #define  ARPING_MSEC  1000
+#define  LOGGER       "logger -t container -p local1.notice"
+
 #define  MODULE       "infix-containers"
 #define  CFG_XPATH    "/infix-containers:containers"
-#define  INBOX_QUEUE  "/run/containers/inbox"
-#define  JOB_QUEUE    "/run/containers/queue"
-#define  ACTIVE_QUEUE "/run/containers/active"
-#define  LOGGER       "logger -t container -p local1.notice"
+
+#define  _PATH_CONT   "/run/containers"
+#define  _PATH_INBOX  _PATH_CONT "/INBOX"
+#define  _PATH_QUEUE  _PATH_CONT "/queue"
 
 
 static int add(const char *name, struct lyd_node *cif)
@@ -27,11 +30,13 @@ static int add(const char *name, struct lyd_node *cif)
 	const char *image = lydx_get_cattr(cif, "image");
 	const char *restart_policy, *string;
 	struct lyd_node *node, *nets, *caps;
+	char script[strlen(name) + 5];
 	FILE *fp, *ap;
 
-	fp = fopenf("w", "%s/S01-%s.sh", INBOX_QUEUE, name);
+	snprintf(script, sizeof(script), "%s.sh", name);
+	fp = fopenf("w", "%s/%s", _PATH_CONT, script);
 	if (!fp) {
-		ERRNO("Failed adding job S01-%s.sh to job queue" INBOX_QUEUE, name);
+		ERRNO("Failed creating container script %s/%s", _PATH_CONT, script);
 		return SR_ERR_SYS;
 	}
 
@@ -176,37 +181,45 @@ static int add(const char *name, struct lyd_node *cif)
 		fprintf(fp, " %s", string);
 
 	fprintf(fp, "\n");
+
+	if (lydx_is_enabled(cif, "manual"))
+		fprintf(fp, "initctl -bnq cond set container:%s\n", name);
+
 	fchmod(fileno(fp), 0700);
 	fclose(fp);
 
 	systemf("initctl -bnq enable container@%s.conf", name);
 
+	/*
+	 * All start scripts must wait for the rest of confd to complete
+	 * before being enqueued to execd, so we postpone it using this
+	 * "inbox" to the post hook.
+	 */
+	writesf(script, "a", "%s", _PATH_INBOX);
+
 	return 0;
 }
 
 static int del(const char *name)
 {
-	const char *queue[] = {
-		JOB_QUEUE,
-		INBOX_QUEUE,
-		ACTIVE_QUEUE,
-	};
+	char fn[strlen(_PATH_QUEUE) + strlen(name) + 10];
 	FILE *fp;
 
 	/* Remove any pending download/create job first */
-	for (size_t i = 0; i < NELEMS(queue); i++) {
-		char fn[strlen(queue[i]) + strlen(name) + 5];
+	snprintf(fn, sizeof(fn), "%s/S01-%s.sh", _PATH_QUEUE, name);
+	erase(fn);
 
-		snprintf(fn, sizeof(fn), "%s/%s.sh", queue[i], name);
-		erase(fn);
-	}
+	/* Remove container script itself */
+	snprintf(fn, sizeof(fn), "%s/%s.sh", _PATH_CONT, name);
+	erase(fn);
 
 	/* Disable service and schedule for deletion. */
 	systemf("initctl -bnq disable container@%s.conf", name);
 
-	fp = fopenf("w", "%s/K01-%s.sh", INBOX_QUEUE, name);
+	snprintf(fn, sizeof(fn), "%s/K01-%s.sh", _PATH_CONT, name);
+	fp = fopen(fn, "w");
 	if (!fp) {
-		ERRNO("Failed adding job 00-delete-%s.sh to job queue" INBOX_QUEUE, name);
+		ERRNO("Failed creating container stop script %s", fn);
 		return SR_ERR_SYS;
 	}
 
@@ -215,6 +228,9 @@ static int del(const char *name)
 	fchmod(fileno(fp), 0700);
 	fclose(fp);
 
+	/* Enqueue kill job immediately on execd */
+	movefile(fn, _PATH_QUEUE);
+
 	return SR_ERR_OK;
 }
 
@@ -330,109 +346,42 @@ static int oci_load(sr_session_ctx_t *session, uint32_t sub_id, const char *xpat
 	return SR_ERR_OK;
 }
 
-static int is_active(sr_session_ctx_t *session, const char *name)
-{
-	return srx_enabled(session, CFG_XPATH "/container[name='%s']/enabled", name);
-}
-
-static int is_manual(sr_session_ctx_t *session, const char *name)
-{
-	return srx_enabled(session, CFG_XPATH "/container[name='%s']/manual", name);
-}
-
-/*
- * When container configurations are not saved to startup-config and the
- * user reboot the system (or lose power) we will have lingering active
- * containers cached on persistent storage.
- *
- * This function runs every time a configuration is applied to clean up
- * any lingering active jobs to prevent false matches in the cmp magic
- * in the below post-hook.
- */
-static void cleanup(sr_session_ctx_t *session, struct confd *confd)
-{
-	struct dirent *d;
-	DIR *dir;
-
-	dir = opendir(ACTIVE_QUEUE);
-	if (!dir)
-		return;
-
-	while ((d = readdir(dir))) {
-		char name[strlen(ACTIVE_QUEUE) + strlen(d->d_name) + 2];
-		char *ptr;
-
-		if (d->d_name[0] == '.')
-			continue;
-
-		strlcpy(name, d->d_name, sizeof(name));
-		ptr = strstr(name, ".sh");
-		if (!ptr)
-			continue; /* odd, non-script file? */
-		*ptr = 0;
-
-		if (strncmp(name, "S01-", 4))
-			continue; /* odd, not start script? */
-
-		if (is_active(session, &name[4]))
-			continue;
-
-		/* Not found in running-config, remove stale cache. */
-		snprintf(name, sizeof(name), "%s/%s", ACTIVE_QUEUE, d->d_name);
-		if (erase(name))
-			ERRNO("Failed removing stale container job %s", name);
-	}
-
-	closedir(dir);
-}
-
 /*
  * Containers depend on a lot of other system resources being properly
  * set up, e.g., networking, which is run by dagger.  So we need to wait
- * for all that before we can launch new, or modified, containers.
+ * for all that before we can launch new, or modified, containers.  This
+ * post hook runs as (one of) the last actions on a config change/boot.
  */
 void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd)
 {
-	struct dirent *d;
-	DIR *dir;
+	char script[256];
+	FILE *fp;
 
-	cleanup(session, confd);
+	fp = fopen(_PATH_INBOX, "r");
+	if (!fp)
+		return;		/* nothing to do today */
 
-	dir = opendir(INBOX_QUEUE);
-	if (!dir) {
-		ERROR("Cannot open %s to launch scripts.", INBOX_QUEUE);
-		return;
+	while (fgets(script, sizeof(script), fp)) {
+		char link[strlen(_PATH_QUEUE) + strlen(script) + 10];
+		char path[strlen(script) + 10];
+
+		chomp(script);
+
+		/*
+		 * Enqueue start job on execd, use a symlink since we
+		 * want to be able to reuse the script for manual image
+		 * uprgade (and debugging) purposes.
+		 */
+		snprintf(link, sizeof(link), "%s/S01-%s", _PATH_QUEUE, script);
+		snprintf(path, sizeof(path), "../%s", script);
+		if (symlink(path, link) && errno != EEXIST)
+			ERRNO("Creating symlink %s -> %s", link, path);
 	}
 
-	while ((d = readdir(dir))) {
-		char next[strlen(INBOX_QUEUE) + strlen(d->d_name) + 2];
-		char name[strlen(d->d_name) + 1];
-		char *ptr;
+	fclose(fp);
+	erase(_PATH_INBOX);
 
-		if (d->d_name[0] == '.')
-			continue;
-
-		snprintf(next, sizeof(next), "%s/%s", INBOX_QUEUE, d->d_name);
-
-		strlcpy(name, d->d_name, sizeof(name));
-		ptr = strstr(name, ".sh");
-		if (ptr) {
-			char *nm = NULL;
-
-			*ptr = 0;
-			if (!strncmp(name, "S01-", 4))
-				nm = &name[4];
-
-			if (nm && !is_manual(session, nm))
-				systemf("initctl -bnq cond set container:%s", nm);
-		}
-
-		if (movefile(next, JOB_QUEUE))
-			ERRNO("Failed moving %s to job queue %s", next, JOB_QUEUE);
-	}
-
-	closedir(dir);
-	systemf("container volume prune -f >/dev/null 2>&1");
+	systemf("initctl -bnq touch execd");
 }
 
 int infix_containers_init(struct confd *confd)

src/execd/execd.c

@@ -30,9 +30,8 @@
 
 static int   logmask = LOG_UPTO(LOG_NOTICE);
 static char  buffer[BUFSIZ];
-static char *done;
 
-static void run_job(const char *path, char *file, int archive)
+static void run_job(const char *path, char *file)
 {
 	char cmd[strlen(path) + strlen(file) + 2];
 	int rc;
@@ -57,11 +56,7 @@ static void run_job(const char *path, char *file, int archive)
 		return;
 	}
 
-	dbg("job %s in %s done %p, archive: %d", file, path, done, archive);
-	if (done && archive)
-		movefile(cmd, done);
-	else
-		erase(cmd);
+	erase(cmd);
 }
 
 /*
@@ -69,7 +64,7 @@ static void run_job(const char *path, char *file, int archive)
  * a type '*' just to figure out if a job should be archived in
  * the done directory.
  */
-static int should_run(const char *name, int type, int *archive)
+static int should_run(const char *name, int type)
 {
 	if (!name || strlen(name) < 3)
 		return 0;
@@ -78,32 +73,25 @@ static int should_run(const char *name, int type, int *archive)
 		if (type == '*') {
 			switch (name[0]) {
 			case 'K':
-				*archive = 0;
-				return 1;
 			case 'S':
-				*archive = 1;
 				return 1;
 			default:
-				errx("unsupported '%s', scripts must start with S or K", name);
-				return 0;
+				goto done;
 			}
 		}
 
 		switch (type) {
 		case 'K':
-			*archive = 0;
-			break;
 		case 'S':
-			*archive = 1;
 			break;
 		default:
 			return 0;
 		}
 
-		dbg("name:%s type:'%c' archive:%d => run:%d", name, type, *archive, type == name[0]);
+		dbg("name:%s type:'%c' => run:%d", name, type, type == name[0]);
 		return type == name[0];
 	}
-
+done:
 	errx("unsupported script %s, must follow pattern SNN/KNN", name);
 	return 0;
 }
@@ -111,7 +99,6 @@ static int should_run(const char *name, int type, int *archive)
 static void run_dir(const char *path, int type)
 {
 	struct dirent **namelist;
-	int archive = 0;
 	int n, i;
 
 	n = scandir(path, &namelist, NULL, alphasort);
@@ -126,8 +113,8 @@ static void run_dir(const char *path, int type)
 		if (d->d_type == DT_DIR)
 			continue;
 
-		if (should_run(d->d_name, type, &archive))
-			run_job(path, d->d_name, archive);
+		if (should_run(d->d_name, type))
+			run_job(path, d->d_name);
 
 		free(d);
 	}
@@ -139,7 +126,7 @@ static void run_dir(const char *path, int type)
  * Call stop/cleanup jobs first, may use same container name or
  * resources as replacement container start scripts use.
  */
-static void run_queue(char *path)
+static void run_queue(const char *path)
 {
 	run_dir(path, 'K');
 	run_dir(path, 'S');
@@ -174,14 +161,13 @@ static void inotify_cb(uev_t *w, void *arg, int _)
 	for (char *p = buffer; p < buffer + bytes;) {
 		struct inotify_event *event = (struct inotify_event *)p;
 		char *name = event->name;
-		int archive;
 
 		if (event->mask & (IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVED_TO)) {
 			dbg("Got inotify event %s 0x%04x", name, event->mask);
-			if (!should_run(name, '*', &archive))
+			if (!should_run(name, '*'))
 				continue;
 
-			run_job(arg, name, archive);
+			run_job(arg, name);
 		}
 
 		p += sizeof(struct inotify_event) + event->len;
@@ -225,13 +211,13 @@ int logmask_from_str(const char *str)
 static int usage(char *arg0, int rc)
 {
 	printf("Usage:\n"
-	       "  %s [-dh] [-l LVL] JOBDIR\n"
+	       "  %s [-dh] [-l LVL] QUEUE\n"
 	       "Options:\n"
 	       "  -d        Log to stderr as well\n"
 	       "  -h        This help text\n"
 	       "  -l LVL  Set log level: none, err, warn, notice*, info, debug\n"
 	       "\n"
-	       "Runs jobs from JOBDIR, re-runs failing jobs on route changes or SIGHUP.\n"
+	       "Runs jobs from QUEUE, re-runs failing jobs on route changes or SIGHUP.\n"
 	       "Use SIGUSR1 to toggle debug messages at runtime.\n", arg0);
 
 	return rc;
@@ -246,7 +232,7 @@ int main(int argc, char *argv[])
 	uev_t sighup_watcher;
 	int logopt = LOG_PID;
 	int wd, sd, fd, c;
-	char *jobdir;
+	char *queue;
 	uev_ctx_t ctx;
 	int rc = 0;
 
@@ -272,13 +258,11 @@ int main(int argc, char *argv[])
 	if (optind >= argc)
 		return usage(argv[0], 1);
 
-	jobdir = argv[optind++];
-	if (optind < argc)
-		done = argv[optind];
+	queue = argv[optind];
 
-	if (access(jobdir, X_OK)) {
+	if (access(queue, X_OK)) {
 		fprintf(stderr, "Cannot find job directory %s, errno %d: %s\n",
-			jobdir, errno, strerror(errno));
+			queue, errno, strerror(errno));
 		return 1;
 	}
 
@@ -299,7 +283,7 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
-	wd = inotify_add_watch(fd, jobdir, IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVED_TO);
+	wd = inotify_add_watch(fd, queue, IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVED_TO);
 	if (wd == -1) {
 		err("inotify_add_watch");
 		close(fd);
@@ -323,7 +307,7 @@ int main(int argc, char *argv[])
 	}
 
 	uev_init(&ctx);
-	if (uev_signal_init(&ctx, &sighup_watcher, signal_cb, jobdir, SIGHUP) == -1) {
+	if (uev_signal_init(&ctx, &sighup_watcher, signal_cb, queue, SIGHUP) == -1) {
 		err("uev_signal_init (sighup)");
 		rc = 1;
 		goto done;
@@ -334,19 +318,19 @@ int main(int argc, char *argv[])
 		goto done;
 	}
 
-	if (uev_io_init(&ctx, &inotify_watcher, inotify_cb, jobdir, fd, UEV_READ) == -1) {
+	if (uev_io_init(&ctx, &inotify_watcher, inotify_cb, queue, fd, UEV_READ) == -1) {
 		err("uev_io_init (inotify)");
 		rc = 1;
 		goto done;
 	}
 
-	if (uev_io_init(&ctx, &netlink_watcher, netlink_cb, jobdir, sd, UEV_READ) == -1) {
+	if (uev_io_init(&ctx, &netlink_watcher, netlink_cb, queue, sd, UEV_READ) == -1) {
 		err("uev_io_init (netlink)");
 		rc = 1;
 		goto done;
 	}
 
-	run_queue(jobdir);
+	run_queue(queue);
 	if (uev_run(&ctx, 0) == -1) {
 		err("uev_run");
 		rc = 1;