confd: use podman stop/start to prevent container corruption

Sending SIGTERM to conmon is not a safe shutdown of a podman container.
To handle gracefully handle shutdown, restarting and provide an orderly
start of dependencies, we use the Finit sysv trick via container script
wrapper to call 'podman stop foo'.

However, since podman does not support syslog as output for containers
we employ an old FIFO trick with another program, k8s-logger, to allow
logs to reach syslog.  Please note that k8s-logger must have properly
started before we call `podman start` -- this makes us fully dependent
on the 'container' wrapper script.  Hence the documentation update.

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

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

@@ -0,0 +1,4 @@
+service :%i pid:!/run/k8s-logger-%i.pid <usr/container:%i> \
+	[2345] k8s-logger -cni %i -f local1 /run/containers/%i.fifo -- Logger for container %i
+sysv    :%i pid:!/run/container:%i.pid <pid/k8s-logger:%i> kill:10 \
+	[2345] container -n %i -- container %i

board/common/rootfs/usr/sbin/container

@@ -99,9 +99,13 @@ create()
     # name, or return docker:// URL for download.
     image=$(unpack_archive "$image")
 
+    if [ -z "$logging" ]; then
+	logging="--log-driver k8s-file --log-opt path=/run/containers/$name.fifo"
+    fi
+
     args="$args --cgroup-parent=containers"
-    args="$args --restart=no --systemd=false --tz=local $privileged --replace --quiet"
-#    args="$args --log-opt tag=$name"
+    args="$args --restart=$restart --systemd=false --tz=local $privileged --replace --quiet"
+    args="$args $ro $vol $mount $hostname $entrypoint $env $port $logging"
     pidfn=/run/container:${name}.pid
 
     [ -n "$quiet" ] || log "---------------------------------------"
@@ -124,13 +128,11 @@ create()
 	network="--net none"
     fi
 
-    #  --syslog --log-level info
     # shellcheck disable=SC2048
-    if podman create --name "$name" --conmon-pidfile="$pidfn" $ro $vol $mount $hostname $entrypoint $env $network $port $args "$image" $*; then
+    if podman create --name "$name" --conmon-pidfile="$pidfn" $args "$image" $*; then
 	[ -n "$quiet" ] || log "Successfully created container $name from $image"
 	rm -f "/run/containers/env/${name}.env"
-	initctl show "container:$name" |grep -q manual:yes || initctl -bq start "container:$name"
-	initctl -nbq cond set container:$name
+	[ -n "$manual" ] || start "$name"
 	exit 0
     fi
 
@@ -149,14 +151,57 @@ delete()
 	exit 1
     fi
 
-    if running "$name"; then
-	initctl -bq stop "container:$name" || podman kill -s KILL "$name" >/dev/null 2>&1
-    fi
-
     podman rm -vif "$name" >/dev/null 2>&1
     [ -n "$quiet" ] || log "Container $name has been removed."
 }
 
+waitfor()
+{
+    timeout=$2
+    while [ ! -f "$1" ]; do
+	_=$((timeout -= 1))
+	if [ $timeout -le 0 ]; then
+	    log "Timeout waiting for $1, aborting!"
+	    exit 1
+	fi
+	sleep 1;
+    done
+}
+
+start()
+{
+    name=$1
+
+    if running "$name"; then
+	[ -n "$quiet" ] || echo "$name: already running."
+	return
+    fi
+
+    initctl -bq cond set "container:$name"
+    # Real work is done by wrap() courtesy of finit sysv emulation
+}
+
+stop()
+{
+    name=$1
+
+    if ! running "$name"; then
+	[ -n "$quiet" ] || echo "$name: not running."
+	return
+    fi
+
+    initctl -bq cond clr "container:$name"
+    # Real work is done by wrap() courtesy of finit sysv emulation
+}
+
+wrap()
+{
+    name=$1
+    cmd=$2
+
+    podman "$cmd" "$name"
+}
+
 # Removes network $1 from all containers
 netwrm()
 {
@@ -178,43 +223,51 @@ usage:
   container [opt] cmd [arg]
 
 options:
-  -a, --all               Show all, of something
-      --dns NAMESERVER    Set nameserver(s) when creating a container
-      --dns-search LIST   Set host lookup search list when creating container
-  -c, --creds USR[:PWD]   Credentials to pass to curl -u for remote ops
-  -d, --detach            Detach a container started with 'run IMG [CMD]'
-  -e, --env FILE          Environment variables when creating container
-      --entrypoint        Disable container image's ENTRYPOINT, run cmd + arg
-  -f, --force             Force operation, e.g. remove
-  -h, --help              Show this help text
-      --hostname NAME     Set hostname when creating container
-      --net NETWORK       Set network(s) when creating container
-  -m, --mount HOST:DEST   Bind mount a read-only file inside a container
-      --privileged        Give container extended privileges
-  -p, --publish PORT      Publish ports when creating container
-                          Syntax: [[ip:][hostPort]:]containerPort[/protocol]
-  -q, --quiet             Quiet operation, called from confd
-      --read-only         Do not create a writable layer
-  -s, --simple            Show output in simplified format
-  -v, --volume NAME:PATH  Create named volume mounted inside container on PATH
+  -a, --all                Show all, of something
+      --dns NAMESERVER     Set nameserver(s) when creating a container
+      --dns-search LIST    Set host lookup search list when creating container
+  -c, --creds USR[:PWD]    Credentials to pass to curl -u for remote ops
+  -d, --detach             Detach a container started with 'run IMG [CMD]'
+  -e, --env FILE           Environment variables when creating container
+      --entrypoint         Disable container image's ENTRYPOINT, run cmd + arg
+  -f, --force              Force operation, e.g. remove
+  -h, --help               Show this help text
+      --hostname NAME      Set hostname when creating container
+      --net NETWORK        Network interface(s) when creating or finding container
+  -l, --log-driver DRV     Log driver to use
+      --log-opt OPT        Logging options to log driver
+      --log-path PATH      Path for k8s-file log pipe
+  -m, --mount HOST:DEST    Bind mount a read-only file inside a container
+      --manual             Do not start container automatically after creation
+  -n, --name NAME          Alternative way of supplying name to start/stop/restart
+      --privileged         Give container extended privileges
+  -p, --publish PORT       Publish ports when creating container
+                           Syntax: [[ip:][hostPort]:]containerPort[/protocol]
+  -q, --quiet              Quiet operation, called from confd
+  -r, --restart POLICY     One of "no", "always", or "on-failure:NUM"
+      --read-only          Do not create a writable layer
+  -s, --simple             Show output in simplified format
+  -v, --volume NAME:PATH   Create named volume mounted inside container on PATH
 
 commands:
-  create  NAME IMAGE NET  Create container NAME using IMAGE with networks NET
-  delete [network] NAME   Remove container NAME or network NAME from all containers
-  exec    NAME CMD        Run a command inside a container
-  help                    Show this help text
-  list   [image | oci]    List names (only) of containers, images, or OCI archives
-  load   [NAME | URL] NM  Load OCI tarball fileNAME or URL to image NM
-  remove  IMAGE           Remove an (unused) container image
-  restart NAME            Restart a crashed container
-  run     NAME [CMD]      Run a container interactively, with an optional command
-  save    IMAGE FILE      Save a container image to an OCI tarball FILE[.tar.gz]
-  shell                   Start a shell inside a container
-  show   [image | volume] Show containers, images, or volumes
-  stat                    Show continuous stats about containers (Ctrl-C aborts)
-  start   NAME            Start a container
-  stop    NAME            Stop a container
-  volume [prune]          Prune unused volumes
+  create   NAME IMAGE NET  Create container NAME using IMAGE with networks NET
+  delete  [network] NAME   Remove container NAME or network NAME from all containers
+  exec     NAME CMD        Run a command inside a container
+  find    [ifname PID]     Find PID of container where '--net IFNAME' currently lives
+                           or, find the name of our IFNAME inside the container @PID
+  help                     Show this help text
+  list    [image | oci]    List names (only) of containers, images, or OCI archives
+  load    [NAME | URL] NM  Load OCI tarball fileNAME or URL to image NM
+  remove   IMAGE           Remove an (unused) container image
+  restart [NAME]           Restart a crashed container
+  run      NAME [CMD]      Run a container interactively, with an optional command
+  save     IMAGE FILE      Save a container image to an OCI tarball FILE[.tar.gz]
+  shell                    Start a shell inside a container
+  show    [image | volume] Show containers, images, or volumes
+  stat                     Show continuous stats about containers (Ctrl-C aborts)
+  start   [NAME]           Start a container, see -n
+  stop    [NAME]           Stop a container, see -n
+  volume  [prune]          Prune unused volumes
 EOF
 }
 
@@ -256,10 +309,30 @@ while [ "$1" != "" ]; do
 	    shift
 	    hostname="--hostname $1"
 	    ;;
+	-l | --log-driver)
+	    shift
+	    logging=" --log-driver=$1"
+	    ;;
+	--log-opt)
+	    shift
+	    logging="$logging --log-opt $1"
+	    ;;
+	--log-path)
+	    shift
+	    logging="$logging --log-opt path=$1"
+	    log_path="$1"
+	    ;;
 	-m | --mount)
 	    shift
 	    mount="--mount=$1"
 	    ;;
+	--manual)
+	    manual=true
+	    ;;
+	-n | --name)
+	    shift
+	    name="$1"
+	    ;;
 	--net)
 	    shift
 	    if [ -n "$network" ]; then
@@ -278,6 +351,10 @@ while [ "$1" != "" ]; do
 	-q | --quiet)
 	    quiet="-q"
 	    ;;
+	-r | --restart)
+	    shift
+	    restart=$1
+	    ;;
 	--read-only)
 	    ro="--read-only=true"
 	    ;;
@@ -451,21 +528,35 @@ case $cmd in
 	esac
 	;;
     start)
-	if running "$1"; then
-	    [ -n "$quiet" ] || echo "$1: already running."
+	if [ -n "$name" ]; then
+	    wrap "$name" start
+	elif [ -n "$1" ]; then
+	    start "$1"
+	else
+	    usage
 	    exit 1
 	fi
-	initctl -bq start "container:$1"
 	;;
     restart)
-	initctl -bq restart "container:$1"
+	if [ -n "$name" ]; then
+	    wrap "$name" restart
+	elif [ -n "$1" ]; then
+	    stop  "$1"
+	    start "$1"
+	else
+	    usage
+	    exit 1
+	fi
 	;;
     stop)
-	if ! running "$1"; then
-	    [ -n "$quiet" ] || echo "$1: not running."
+	if [ -n "$name" ]; then
+	    wrap "$name" stop
+	elif [ -n "$1" ]; then
+	    stop "$1"
+	else
+	    usage
 	    exit 1
 	fi
-	initctl -bq stop "container:$1" || podman kill "$1" >/dev/null 2>&1
 	;;
     stat*)
 	podman stats -i 2

configs/aarch64_defconfig

@@ -131,6 +131,7 @@ BR2_PACKAGE_FINIT_PLUGIN_RTC=y
 BR2_PACKAGE_FINIT_PLUGIN_TTY=y
 BR2_PACKAGE_FINIT_PLUGIN_URANDOM=y
 BR2_PACKAGE_IITO=y
+BR2_PACKAGE_K8S_LOGGER=y
 BR2_PACKAGE_KEYACK=y
 BR2_PACKAGE_KLISH_PLUGIN_INFIX=y
 BR2_PACKAGE_LOWDOWN=y

configs/r2s_defconfig

@@ -161,6 +161,7 @@ BR2_PACKAGE_FINIT_PLUGIN_RTC=y
 BR2_PACKAGE_FINIT_PLUGIN_TTY=y
 BR2_PACKAGE_FINIT_PLUGIN_URANDOM=y
 BR2_PACKAGE_IITO=y
+BR2_PACKAGE_K8S_LOGGER=y
 BR2_PACKAGE_KEYACK=y
 BR2_PACKAGE_KLISH_PLUGIN_INFIX=y
 BR2_PACKAGE_LOWDOWN=y

configs/x86_64_defconfig

@@ -134,6 +134,7 @@ BR2_PACKAGE_FINIT_PLUGIN_RTC=y
 BR2_PACKAGE_FINIT_PLUGIN_TTY=y
 BR2_PACKAGE_FINIT_PLUGIN_URANDOM=y
 BR2_PACKAGE_IITO=y
+BR2_PACKAGE_K8S_LOGGER=y
 BR2_PACKAGE_KEYACK=y
 BR2_PACKAGE_KLISH_PLUGIN_INFIX=y
 BR2_PACKAGE_LOWDOWN=y

doc/container.md

@@ -33,6 +33,11 @@ All network specific settings are done using the IETF interfaces YANG
 model, with augments for containers to ensure smooth integration with
 container networking in podman.
 
+> **Note:** even though the `podman` command can be used directly from a
+> shell prompt, we strongly recommend using the CLI commands instead.
+> They employ the services of a wrapper `container` script which handles
+> the integration of containers in the system.
+
 
 Caution
 -------

src/confd/src/infix-containers.c

@@ -33,7 +33,6 @@ static int add(const char *name, struct lyd_node *cif)
 	const char *restart_policy, *string;
 	struct lyd_node *node, *network;
 	FILE *fp, *ap;
-	char *restart = "";	/* Default restart:10 */
 
 	fp = fopenf("w", "%s/%s.sh", INBOX_QUEUE, name);
 	if (!fp) {
@@ -152,6 +151,19 @@ static int add(const char *name, struct lyd_node *cif)
 		}
 	}
 
+	restart_policy = lydx_get_cattr(cif, "restart-policy");
+	if (restart_policy) {
+		if (!strcmp(restart_policy, "never"))
+			fprintf(fp, " -r no"); /* for docker compat */
+		else if (!strcmp(restart_policy, "always"))
+			fprintf(fp, " -r always");
+		else
+			fprintf(fp, " -r on-failure:10");
+	}
+
+	if (lydx_is_enabled(cif, "manual"))
+		fprintf(fp, " --manual");
+
 	if ((string = lydx_get_cattr(cif, "command")))
 		fprintf(fp, " --entrypoint");
 
@@ -164,31 +176,7 @@ static int add(const char *name, struct lyd_node *cif)
 	fchmod(fileno(fp), 0700);
 	fclose(fp);
 
-	fp = fopenf("w", "/etc/finit.d/available/container:%s.conf", name);
-	if (!fp) {
-		ERROR("Failed creating container %s monitor", name);
-		return SR_ERR_SYS;
-	}
-
-	restart_policy = lydx_get_cattr(cif, "restart-policy");
-	if (restart_policy) {
-		if (!strcmp(restart_policy, "no"))
-			restart = "norestart";
-		else if (!strcmp(restart_policy, "always"))
-			restart = "restart:always";
-		/* default is to restart up to 10 times */
-	}
-
-	fprintf(fp, "service name:container :%s log:prio:local1.err,tag:%s pid:!/run/container:%s.pid \\\n"
-		"	[2345] %s %s <usr/container:%s> podman start -a %s -- Container %s\n",
-		name, name, name, lydx_is_enabled(cif, "manual") ? "manual:yes" : "", restart,
-		name, name, name);
-	fclose(fp);
-
-	if (systemf("initctl -nbq enable container:%s", name)) {
-		ERROR("Failed enabling container %s monitor", name);
-		return SR_ERR_SYS;
-	}
+	systemf("initctl -bnq enable container@%s.conf", name);
 
 	return 0;
 }
@@ -200,7 +188,6 @@ static int del(const char *name)
 		INBOX_QUEUE,
 		ACTIVE_QUEUE,
 	};
-	int rc;
 
 	/* Remove any pending download/create job first */
 	for (size_t i = 0; i < NELEMS(queue); i++) {
@@ -210,11 +197,8 @@ static int del(const char *name)
 		erase(fn);
 	}
 
-	systemf("initctl -nbq disable container:%s", name);
-	rc = systemf("container delete %s", name);
-	erasef("/etc/finit.d/available/container:%s.conf", name);
-
-	return rc;
+	return  systemf("container delete %s", name) ||
+		systemf("initctl -bnq disable container@%s.conf", name);
 }
 
 static int change(sr_session_ctx_t *session, uint32_t sub_id, const char *module,
@@ -329,9 +313,14 @@ static int oci_load(sr_session_ctx_t *session, uint32_t sub_id, const char *xpat
 	return SR_ERR_OK;
 }
 
-static int is_active(struct confd *confd, const char *name)
+static int is_active(sr_session_ctx_t *session, const char *name)
 {
-	return srx_enabled(confd->session, CFG_XPATH "/container[name='%s']/enabled", 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);
 }
 
 /*
@@ -343,16 +332,14 @@ static int is_active(struct confd *confd, const char *name)
  * any lingering active jobs to prevent false matches in the cmp magic
  * in the below post-hook.
  */
-void infix_containers_pre_hook(sr_session_ctx_t *session, struct confd *confd)
+static void cleanup(sr_session_ctx_t *session, struct confd *confd)
 {
 	struct dirent *d;
 	DIR *dir;
 
 	dir = opendir(ACTIVE_QUEUE);
-	if (!dir) {
-		ERROR("Failed opening %s for cleanup, skipping.", ACTIVE_QUEUE);
+	if (!dir)
 		return;
-	}
 
 	while ((d = readdir(dir))) {
 		char name[strlen(ACTIVE_QUEUE) + strlen(d->d_name) + 2];
@@ -367,7 +354,7 @@ void infix_containers_pre_hook(sr_session_ctx_t *session, struct confd *confd)
 			continue; /* odd, non-script file? */
 		*ptr = 0;
 
-		if (is_active(confd, name))
+		if (is_active(session, name))
 			continue;
 
 		/* Not found in running-config, remove stale cache. */
@@ -390,15 +377,14 @@ void infix_containers_pre_hook(sr_session_ctx_t *session, struct confd *confd)
  * them on a reboot.  Hence the cmp magic below: we check if the command
  * to create a container is the same as what is already activated, if it
  * is already activated we know 'podman create' has done its thing and
- * we can safely reassert the Finit condition and allo core_post_hook()
- * to launch the container with 'initctl reload'.
+ * we can safely start the container.
  */
 void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd)
 {
 	struct dirent *d;
 	DIR *dir;
 
-	(void)confd;		/* unused atm. */
+	cleanup(session, confd);
 
 	dir = opendir(INBOX_QUEUE);
 	if (!dir) {
@@ -416,8 +402,18 @@ void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd)
 		snprintf(curr, sizeof(curr), "%s/%s", ACTIVE_QUEUE, d->d_name);
 		snprintf(next, sizeof(next), "%s/%s", INBOX_QUEUE, d->d_name);
 		if (!systemf("cmp %s %s >/dev/null 2>&1", curr, next)) {
-			/* New job is already active, no changes, skipping ... */
-			systemf("initctl -nbq cond set container:$(basename %s .sh)", d->d_name);
+			char name[strlen(d->d_name) + 1];
+			char *ptr;
+
+			strlcpy(name, d->d_name, sizeof(name));
+			ptr = strstr(name, ".sh");
+			if (ptr) {
+				*ptr = 0;
+
+				/* New job is already active, no changes, skipping ... */
+				if (!is_manual(session, name))
+					systemf("initctl -bnq cond set container:%s", name);
+			}
 			remove(next);
 			continue;
 		}
@@ -427,7 +423,6 @@ void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd)
 	}
 
 	closedir(dir);
-	NOTE("Pruning unused container volumes from system.");
 	systemf("container volume prune -f >/dev/null 2>&1");
 }
 

src/confd/yang/infix-containers@2024-02-01.yang

@@ -46,7 +46,7 @@ module infix-containers {
 
   typedef restart-policy {
     type enumeration {
-      enum no {
+      enum never {
 	description "Do not restart containers that exit/crash.";
 	value 1;
       }
@@ -147,7 +147,7 @@ module infix-containers {
       leaf restart-policy {
 	description "Restart policy to when containers exit/crash.";
 	type restart-policy;
-	default retry;
+	default always;
       }
 
       leaf manual {