confd: drop fragile asynchronous container create/delete using execd

This is a complete redesign of how the system creates/deletes containers
from the running-config.  Containers are now removed synchronously from
confd before any interfaces they may be using are removed, and created
in parallel, using a Finit task, well after confd has finished setting
up interfaces.

The logic previously provided by execd to retry container create on any
route/address changes, or periodically every 60 seconds, is now handled
by a new 'setup' command in the container wrapper script.

Additionally, container create is now split in wget/curl/podman pull of
the image and 'podman create'.  This to both consolidate image fetching
and improve user feedback since most of the retry logic (above) revolves
around the image download.

Fixes #835

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

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

@@ -1,4 +1,6 @@
-service :%i pid:!/run/k8s-logger-%i.pid <usr/container:%i> \
+task    name:container-%i :setup \
+	[2345] container -n %i setup -- Setup container %i
+service <usr/container:%i> :%i pid:!/run/k8s-logger-%i.pid \
 	[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> log kill:10 \
+sysv    <!pid/k8s-logger:%i> :%i pid:!/run/container:%i.pid kill:10 \
 	[2345] container -n %i -- container %i

board/common/rootfs/usr/lib/tmpfiles.d/containers.conf

@@ -0,0 +1,6 @@
+d  /run/containers/args		0700 - -
+d  /run/containers/files	0700 - -
+d  /var/lib/containers		0700 - -
+d  /var/lib/containers/oci	0700 - -
+d  /run/cni			0755 - -
+L+ /var/lib/cni                 -    - - - /run/cni

board/common/rootfs/usr/sbin/container

@@ -1,17 +1,17 @@
 #!/bin/sh
 # This script can be used to start, stop, create, and delete containers.
-# It is primarily used by confd to create jobs for execd to run from its
-# /run/containers/queue, but it can also be used manually.
+# It is what confd use, with the Finit container@.conf template, to set
+# up, run, and delete containers.
 #
 # NOTE: when creating/deleting containers, remember 'initctl reload' to
-#       activate the changes!  When called by confd, via execd, this is
-#       already handled.
+#       activate the changes!  In confd this is already handled.
 #
 DOWNLOADS=/var/lib/containers/oci
 BUILTIN=/lib/oci
 TMPDIR=/var/tmp
 checksum=""
 extracted=
+timeout=30
 dir=""
 all=""
 env=""
@@ -126,7 +126,17 @@ unpack_archive()
 	    fi
             ;;
 	*)			# docker://*, docker-archive:*, or URL
-	    echo "$image"
+	    if podman image exists "$image"; then
+		echo "$image"
+		return 0
+	    fi
+	    # XXX: use --retry=0 with Podman 5.0 or later.
+	    if ! id=$(podman pull --quiet "$image"); then
+		log "Failed pulling $image"
+		return 1
+	    fi
+	    # Echo image name to caller
+	    podman images --filter id="$id" --format "{{.Repository}}:{{.Tag}}"
 	    return 0
 	    ;;
     esac
@@ -219,10 +229,7 @@ create()
 	logging="--log-driver k8s-file --log-opt path=/run/containers/$name.fifo"
     fi
 
-    # Pull quietly and don't retry on failure, we use execd for this,
-    # or user retry manually when run interactively, we may have other
-    # containers waiting to start that have an image locally already.
-    # Use --retry=0 with Podman 5.0 or later.
+    # When we get here we've already fetched, or pulled, the image
     args="$args --read-only --replace --quiet --cgroup-parent=containers $caps"
     args="$args --restart=$restart --systemd=false --tz=local $privileged"
     args="$args $vol $mount $hostname $entrypoint $env $port $logging"
@@ -253,6 +260,7 @@ create()
     if podman create --name "$name" --conmon-pidfile="$pidfn" $args "$image" $*; then
 	[ -n "$quiet"  ] || log "Successfully created container $name from $image"
 	[ -n "$manual" ] || start "$name"
+
 	# Should already be enabled by confd (this is for manual use)
 	initctl -bnq enable "container@${name}.conf"
 	exit 0
@@ -272,8 +280,16 @@ delete()
 	exit 1
     fi
 
-    # Should already be disabled (and stopped) by confd (this is for manual use)
-    initctl -bnq disable "container@${name}.conf"
+    # Should already be stopped, but if not ...
+    container stop "$name"
+
+    while running "$name"; do
+	_=$((timeout -= 1))
+	if [ $timeout -le 0 ]; then
+	    err 1 "timed out waiting for container $1 to stop before deleting it."
+	fi
+	sleep 1
+    done
 
     podman rm -vif "$name" >/dev/null 2>&1
     [ -n "$quiet" ] || log "Container $name has been removed."
@@ -281,7 +297,6 @@ delete()
 
 waitfor()
 {
-    timeout=$2
     while [ ! -f "$1" ]; do
 	_=$((timeout -= 1))
 	if [ $timeout -le 0 ]; then
@@ -353,6 +368,12 @@ netrestart()
     done
 }
 
+cleanup()
+{
+    log "Received signal, exiting."
+    exit 1
+}
+
 usage()
 {
 	cat <<EOF
@@ -386,6 +407,7 @@ options:
   -q, --quiet              Quiet operation, called from confd
   -r, --restart POLICY     One of "no", "always", or "on-failure:NUM"
   -s, --simple             Show output in simplified format
+  -t, --timeout SEC        Set timeout for delete/restart commands, default: 20
   -v, --volume NAME:PATH   Create named volume mounted inside container on PATH
 
 commands:
@@ -403,6 +425,7 @@ commands:
   restart [network] NAME   Restart a (crashed) container or container(s) using network
   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]
+  setup    NAME            Create and set up container as a Finit task
   shell                    Start a shell inside a container
   show    [image | volume] Show containers, images, or volumes
   stat                     Show continuous stats about containers (Ctrl-C aborts)
@@ -525,6 +548,10 @@ while [ "$1" != "" ]; do
 	-s | --simple)
 	    simple=true
 	    ;;
+	-t | --timeout)
+	    shift
+	    timeout=$1
+	    ;;
 	-v | --volume)
 	    shift
 	    vol="$vol -v $1"
@@ -541,6 +568,8 @@ if [ -n "$cmd" ]; then
     shift
 fi
 
+trap cleanup INT TERM
+
 case $cmd in
     # Does not work atm., cannot attach to TTY because
     # we monitor 'podman start -ai foo' with Finit.
@@ -666,6 +695,20 @@ case $cmd in
 	    gzip "$file"
 	fi
 	;;
+    setup)
+	[ -n "$name" ] || err 1 "setup: missing container name."
+	script=/run/containers/${name}.sh
+	[ -x "$script" ] || err 1 "setup: $script does not exist or is not executable."
+	while ! "$script"; do
+	    # Wait for address/route changes, or retry every 60 secods
+	    # shellcheck disable=2162,3045
+	    ip monitor address route | while read -t 60 _; do break; done
+
+	    # On IP address/route changes, wait a few seconds more to ensure
+	    # the system has ample time to react and set things up for us.
+	    sleep 2
+	done
+	;;
     shell)
 	podman exec -it "$1" sh -l
 	;;
@@ -720,7 +763,6 @@ case $cmd in
 	    else
 		name=$1
 		stop "$name"
-		timeout=20
 		while running "$name"; do
 		    _=$((timeout -= 1))
 		    if [ $timeout -le 0 ]; then
@@ -781,7 +823,7 @@ case $cmd in
 	[ -n "$cmd" ] && shift
 	case $cmd in
 	    prune)
-		podman volume $force prune
+		podman volume prune $force
 		;;
 	    *)
 		false

patches/podman/4.5.0/0001-disable-pull-retry.patch

@@ -1,10 +1,11 @@
-This patch disables the default "podman pull" retry value, which otherwise
-blocks execd from trying other jobs in its queue.
+This patch disables the default "podman pull" retry value, which is not
+used by Infix.  Instead, the container wrapper script retries on network
+related changes, or every 60 seconds.
 
-As of podman v5.0.0 a --retry=NUM has been added to the podman create, run,
-and pull commands.  However, CNI is no longer supported, and a lot of other
-breaking changes have been made, e.g., output of podman inspect.  So there's
-a lot of work upgrading.
+As of podman v5.0.0 a '--retry=NUM' has been added to the podman create,
+run, and pull commands.  However, CNI is no longer supported, and a lot
+of other breaking changes have been made, eg., output of podman inspect.
+So there's a lot of work to upgrade.
 
    -- Joachim
 

src/confd/src/infix-containers.c

@@ -22,7 +22,6 @@
 
 #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)
@@ -40,11 +39,15 @@ static int add(const char *name, struct lyd_node *cif)
 		return SR_ERR_SYS;
 	}
 
-	/* Stop any running container gracefully so it releases its IP addresses. */
+	/*
+	 * Create /run/containers/<NAME>.sh it is used both for initial
+	 * setup at creation/boot and for manual upgrade.  The delete
+	 * command ensures any already running container is stopped and
+	 * deleted so that it releases all claimed resources.
+	 */
 	fprintf(fp, "#!/bin/sh\n"
-		"container --quiet stop %s   >/dev/null\n" /* Silence "not running" on upgrade */
-		"container --quiet delete %s >/dev/null\n" /* Silence any hashes when deleting */
-		"container --quiet", name, name);
+		"container --quiet delete %s >/dev/null\n"
+		"container --quiet", name);
 
 	LYX_LIST_FOR_EACH(lyd_child(cif), node, "dns")
 		fprintf(fp, " --dns %s", lyd_get_value(node));
@@ -191,55 +194,27 @@ 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);
 
+	/* Enable, or update, container -- both trigger setup script. */
 	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);
+	systemf("initctl -bnq touch container@%s.conf", name);
 
 	return 0;
 }
 
 static int del(const char *name)
 {
-	char fn[strlen(_PATH_QUEUE) + strlen(name) + 10];
-	FILE *fp;
+	char fn[strlen(_PATH_CONT) + strlen(name) + 10];
 
-	/* Remove any pending download/create job first */
-	snprintf(fn, sizeof(fn), "%s/S01-%s.sh", _PATH_QUEUE, name);
-	erase(fn);
-
-	/* Remove container script itself */
+	/* Remove container setup script */
 	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);
-
-	snprintf(fn, sizeof(fn), "%s/K01-%s.sh", _PATH_CONT, name);
-	fp = fopen(fn, "w");
-	if (!fp) {
-		ERRNO("Failed creating container stop script %s", fn);
-		return SR_ERR_SYS;
-	}
-
-	fprintf(fp, "#!/bin/sh\n"
-		"container delete %s\n", name);
-	fchmod(fileno(fp), 0700);
-	fclose(fp);
-
-	/* Enqueue kill job immediately on execd */
-	movefile(fn, _PATH_QUEUE);
+	/* Stop and schedule for deletion */
+	systemf("initctl -bnq stop container:%s", name);
+	writesf(name, "a", "%s", _PATH_INBOX);
 
 	return SR_ERR_OK;
 }
@@ -364,34 +339,24 @@ static int oci_load(sr_session_ctx_t *session, uint32_t sub_id, const char *xpat
  */
 void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd)
 {
-	char script[256];
+	char name[256];
 	FILE *fp;
 
 	fp = fopen(_PATH_INBOX, "r");
 	if (!fp)
-		return;		/* nothing to do today */
+		return;		/* nothing to delete */
 
-	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 (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("initctl -bnq touch execd");
+	systemf("podman volume prune -f");
 }
 
 int infix_containers_init(struct confd *confd)