container: refactor cleanup stage to use Finit cleanup:script

Containers in Infix are read-only, any persistent data is either stored
in volumes or content mounts.  To prevent lingering containers, as have
been seen on some test systems, we move cleanup stage from confd to use
the Finit cleanup:script mechanism.

The resulting code is simpler, shorter, and easier to debug and use.

As an added benefit, confd no longer prunes "unused" volumes by itself
on (any!) reconfiguration event, which with the new container life cycle
in Infix was quite fragile.  Instead the CLI command 'container prune'
is extended with 'podman volume prune', which coincidentally the help
text already alluded to.

This patch requires Finit v4.10-rc1, or later.

Fixes #902

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

board/common/rootfs/etc/finit.d/available/container@.conf

@@ -3,5 +3,5 @@
 # time we start a container we run the setup stage, disable the Finit
 # timeout to allow the setup stage to run to completion.
 sysv    log:prio:local1,tag:%i kill:10 pid:!/run/container:%i.pid \
-	[2345] <!> :%i pre:0,/usr/sbin/container container -n %i  \
-	-- container %i
+	pre:0,/usr/sbin/container cleanup:0,/usr/sbin/container   \
+	[2345] <!> :%i container -n %i -- container %i

board/common/rootfs/usr/sbin/container

@@ -274,7 +274,6 @@ create()
 delete()
 {
     name=$1
-    image=$2
 
     if [ -z "$name" ]; then
 	echo "Usage:"
@@ -283,7 +282,7 @@ delete()
     fi
 
     # Should already be stopped, but if not ...
-    container stop "$name"
+    container stop "$name" >/dev/null
 
     while running "$name"; do
 	_=$((timeout -= 1))
@@ -585,11 +584,12 @@ case $cmd in
 	;;
     delete)
 	cmd=$1
-	name=$2
+	[ -n "$name" ] || name=$2
 	if [ "$cmd" = "network" ] && [ -n "$name" ]; then
 	    netwrm "$name"
 	else
-	    delete "$@"
+	    [ -n "$name" ] || name=$1
+	    delete "$name"
 	fi
 	;;
     exec)
@@ -835,8 +835,19 @@ case $cmd in
 	;;
     *)
 	if [ -n "$SERVICE_SCRIPT_TYPE" ] && [ -n "$SERVICE_ID" ]; then
-	    # Called as pre-script from Finit service
-	    exec $container -n "$SERVICE_ID" setup
+	    case "$SERVICE_SCRIPT_TYPE" in
+		pre)
+		    # Called as pre-script from Finit service
+		    exec $container -q -n "$SERVICE_ID" setup
+		    ;;
+		cleanup)
+		    # Called as cleanup-script from Finit service
+		    exec $container -q -n "$SERVICE_ID" delete
+		    ;;
+		*)
+		    false
+		    ;;
+	    esac
 	fi
 	usage
 	exit 1

src/confd/src/core.c

@@ -75,9 +75,6 @@ int core_post_hook(sr_session_ctx_t *session, uint32_t sub_id, const char *modul
 		return SR_ERR_SYS;
 	}
 
-	/* Everything done, including interfaces, launch all container scripts */
-	infix_containers_post_hook(session, priv);
-
 	/* skip reload in bootstrap, implicit reload in runlevel change */
 	if (systemf("runlevel >/dev/null 2>&1"))
 		return SR_ERR_OK;

src/confd/src/core.h

@@ -213,13 +213,9 @@ int hostnamefmt      (struct confd *confd, const char *fmt, char *hostnm, size_t
 
 /* infix-containers.c */
 #ifdef CONTAINERS
-int  infix_containers_init(struct confd *confd);
-void infix_containers_pre_hook(sr_session_ctx_t *session, struct confd *confd);
-void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd);
+int infix_containers_init(struct confd *confd);
 #else
-static inline int  infix_containers_init(struct confd *confd) { return 0; }
-static inline void infix_containers_pre_hook(sr_session_ctx_t *session, struct confd *confd) {}
-static inline void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd) {}
+static inline int infix_containers_init(struct confd *confd) { return 0; }
 #endif
 
 /* infix-dhcp-common.c */

src/confd/src/infix-containers.c

@@ -21,9 +21,16 @@
 #define  CFG_XPATH    "/infix-containers:containers"
 
 #define  _PATH_CONT   "/run/containers"
-#define  _PATH_INBOX  _PATH_CONT "/INBOX"
-
 
+/*
+ * Create a setup/create/upgrade script and instantiate a new instance
+ * that Finit will start when all networking and other dependencies are
+ * out of the way.  Finit calls the `/usr/sbin/container` wrapper script
+ * in the pre: hook to fetch and create the container instance.
+ *
+ * The script we create here, on every boot, contains all information
+ * needed to recreate and upgrade the user's container at runtime.
+ */
 static int add(const char *name, struct lyd_node *cif)
 {
 	const char *restart_policy, *string;
@@ -206,24 +213,22 @@ static int add(const char *name, struct lyd_node *cif)
 	fchmod(fileno(fp), 0700);
 	fclose(fp);
 
-	/* Enable, or update, container -- both trigger setup script. */
+	/* Enable, or update, container -- both trigger container setup. */
 	systemf("initctl -bnq enable container@%s.conf", name);
 	systemf("initctl -bnq touch container@%s.conf", name);
 
 	return 0;
 }
 
+/*
+ * Remove setup/create/upgrade script and disable the currently running
+ * instance.  The `/usr/sbin/container` wrapper script is called when
+ * Finit removes the instance, and it does not need the file we erase.
+ */
 static int del(const char *name)
 {
-	char fn[strlen(_PATH_CONT) + strlen(name) + 10];
-
-	/* Remove container setup script */
-	snprintf(fn, sizeof(fn), "%s/%s.sh", _PATH_CONT, name);
-	erase(fn);
-
-	/* Stop and schedule for deletion */
-	systemf("initctl -bnq stop container:%s", name);
-	writesf(name, "a", "%s", _PATH_INBOX);
+	erasef("%s/%s.sh", _PATH_CONT, name);
+	systemf("initctl -bnq disable container@%s.conf", name);
 
 	return SR_ERR_OK;
 }
@@ -312,7 +317,7 @@ static int action(sr_session_ctx_t *session, uint32_t sub_id, const char *xpath,
 		return SR_ERR_INTERNAL;
 	cmd += 2;
 
-	DEBUG("CALLING 'container %s %s' (xpath %s)", cmd, name, xpath);
+	DEBUG("RPC xpath %s, calling 'container %s %s'", xpath, cmd, name);
 	if (systemf("container %s %s", cmd, name))
 		return SR_ERR_INTERNAL;
 
@@ -335,34 +340,6 @@ static int oci_load(sr_session_ctx_t *session, uint32_t sub_id, const char *xpat
 	return SR_ERR_OK;
 }
 
-/*
- * 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.  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)
-{
-	char name[256];
-	FILE *fp;
-
-	fp = fopen(_PATH_INBOX, "r");
-	if (!fp)
-		return;		/* nothing to delete */
-
-	while (fgets(name, sizeof(name), fp)) {
-		chomp(name);
-		systemf("initctl -bnq disable container@%s.conf", name);
-		systemf("container delete %s", name);
-		systemf("initctl -bnq cond clr container:%s", name);
-	}
-
-	fclose(fp);
-	erase(_PATH_INBOX);
-
-	systemf("podman volume prune -f");
-}
-
 int infix_containers_init(struct confd *confd)
 {
 	int rc;

src/klish-plugin-infix/xml/containers.xml

@@ -38,6 +38,7 @@
   <COMMAND name="prune" help="Clean up all unused containers, images and volume data">
     <ACTION sym="script" in="tty" out="tty" interrupt="true">
       doas podman system prune
+      doas podman volume prune
     </ACTION>
   </COMMAND>