Refactor container cleanup

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

@@ -1,7 +1,8 @@
 # Start a container instance (%i) and redirect logs to /log/container
-# Give podman enough time to properly shut down the container, kill:30
-# The pre:script, which is responsibe for fetching a remote image, must
-# not have a timeout.  The cleanup should take no longer than a minute.
+# Give podman enough time to properly shut down the container, kill:30,
+# which is also matched in the container script (podman default is 10!)
+# The pre:script, responsibe for fetching a remote image and calling on
+# 'podman load', must not have a timeout.
 sysv    log:prio:local1,tag:%i kill:30 pid:!/run/container:%i.pid \
-	pre:0,/usr/sbin/container cleanup:60,/usr/sbin/container   \
-	[2345] <!> :%i container -n %i -- container %i
+	[2345] <!> :%i pre:0,/usr/sbin/container                  \
+	container -n %i -- container %i

board/common/rootfs/usr/sbin/container

@@ -12,7 +12,7 @@ BASEDIR=/var/tmp
 container=$0
 checksum=""
 extracted=
-timeout=30
+timeout=30		    # NOTE: matched with [email protected]
 dir=""
 all=""
 env=""
@@ -331,11 +331,56 @@ delete()
 	sleep 1
     done
 
+    # NOTE: -v does not remove *named volumes*
     podman rm -vif "$name" >/dev/null 2>&1
     [ -n "$quiet" ] || log "Container $name has been removed."
+}
+
+# Delete and clean up after containers removed from the running-configuration
+# May take container names as arguments, as '-n NAME', or a batch list from
+# confd in /run/containers/cleanup, one container name per line.
+cleanup()
+{
+    if [ -n "$name" ]; then
+	names="$name"
+    elif [ $# -gt 0 ]; then
+	names="$*"
+    elif [ -f "/run/containers/cleanup" ]; then
+	names=$(cat /run/containers/cleanup)
+	is_cleanup=true
+    else
+	log "cleanup: no containers specified and no cleanup file found"
+	return 0
+    fi
+
+    for name in $names; do
+	image_name=$(podman inspect "$name" 2>/dev/null | jq -r '.[].ImageName' 2>/dev/null || echo "")
+	log "Cleaning up container: $name, potentially also image: $image_name ..."
+
+	# Remove the container instance
+	delete "$name"
+
+	# Clean up associated named volumes (name-*)
+	for vol in $(podman volume ls --format "{{.Name}}" | grep "^${name}-"); do
+	    log "Removing volume: $vol"
+	    podman volume rm -f --time "$timeout" "$vol" 2>/dev/null || true
+	done
+
+	# Clean up the image if it exists and is not used by other containers
+	if [ -n "$image_name" ] && [ "$image_name" != "null" ]; then
+	    if ! podman ps -a --format "{{.Image}}" | grep -q "^${image_name}$"; then
+		log "Removing unused image: $image_name"
+		podman rmi "$image_name" 2>/dev/null || true
+	    else
+		log "Image $image_name still in use by other containers, not removing"
+	    fi
+	fi
+    done
 
-    cnt=$(podman image prune -af | wc -l)
-    log "Pruned $cnt image(s)"
+    if [ -n "$is_cleanup" ]; then
+	rm -f "/run/containers/cleanup"
+	log "Removed cleanup batch file"
+    fi
 }
 
 waitfor()
@@ -375,6 +420,9 @@ stop()
     # Real work is done by wrap() courtesy of finit sysv emulation
 }
 
+# When called by Finit: `initctl stop foo`, the container script
+# is called as `container -n $foo stop`.  For the stop command
+# we want to bypass the default 10 second timeout.
 wrap()
 {
     name=$1
@@ -385,21 +433,28 @@ wrap()
 
     # The setup phase may run forever in the background trying to fetch
     # the image.  It saves its PID in /run/containers/${name}.pid
-    if [ "$cmd" = "stop" ] && [ -f "$pidfile" ]; then
-        pid=$(cat "$pidfile")
-
-	# Check if setup is still running ...
-	if kill -0 "$pid" 2>/dev/null; then
-            kill "$pid"
-            wait "$pid" 2>/dev/null
-        fi
+    if [ "$cmd" = "stop" ]; then
+	if [ -f "$pidfile" ]; then
+            pid=$(cat "$pidfile")
+
+	    # Check if setup is still running ...
+	    if kill -0 "$pid" 2>/dev/null; then
+		kill "$pid"
+		wait "$pid" 2>/dev/null
+            fi
+
+            rm -f "$pidfile"
+            return 0
+	fi
 
-        rm -f "$pidfile"
-        return 0
+	args="-i --timeout $timeout"
+    else
+	args=
     fi
 
     # Skip "echo $name" from podman start in log
-    podman "$cmd" "$name" >/dev/null
+    # shellcheck disable=SC2086
+    podman "$cmd" $args "$name" >/dev/null
 }
 
 # Removes network $1 from all containers
@@ -430,7 +485,7 @@ netrestart()
     done
 }
 
-cleanup()
+atexit()
 {
     pidfile=$(pidfn "$name")
 
@@ -480,6 +535,7 @@ options:
   -v, --volume NAME:PATH   Create named volume mounted inside container on PATH
 
 commands:
+  cleanup [NAME [...]]     Clean up after a deleted container(s): images, 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
@@ -638,14 +694,17 @@ if [ -n "$cmd" ]; then
     shift
 fi
 
-trap cleanup INT HUP TERM
+trap atexit INT HUP TERM
 
 case $cmd in
     # Does not work atm., cannot attach to TTY because
     # we monitor 'podman start -ai foo' with Finit.
     # attach)
     # 	podman attach "$1"
     # 	;;
+    cleanup)
+	cleanup "$@"
+	;;
     create)
 	[ -n "$quiet" ] || log "Got create args: $*"
 	create "$@"
@@ -780,23 +839,37 @@ case $cmd in
 	pidfile=$(pidfn "${name}")
 	echo $$ > "$pidfile"
 
-	while ! "$script"; do
-            log "${name}: setup failed, waiting for network changes ..."
-
-	    # Timeout and retry after 60 seconds, on SIGTERM, or when
-	    # any network event is caught.
-	    timeout -s TERM -k 1 60 sh -c \
-		    'ip monitor address route 2>/dev/null | head -n1 >/dev/null' || true
-
-	    # 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.
-	    log "${name}: retrying ..."
-	    sleep 2
-	done
+	# Try setup once first
+	if ! "$script"; then
+	    # Get image transport to decide if retries make sense
+	    image=$(awk '/^# meta-image:/ {print $3}' "$script" 2>/dev/null || echo "")
+	    case "$image" in
+		oci:* | oci-archive:* | docker-archive:* | docker-daemon:* | dir:* | containers-storage:* | ostree:* | sif:* | /*)
+		    # Local transport - exit immediately on failure
+		    log "${name}: setup failed for local image $image"
+		    rm -f "$pidfile"
+		    exit 1
+		    ;;
+		*)
+		    # Remote transport (docker://, ftp://, http://, https://, etc.) - retry on network changes
+		    while ! "$script"; do
+			log "${name}: setup failed, waiting for network changes ..."
+
+			# Timeout and retry after 60 seconds, on SIGTERM, or when
+			# any network event is caught.
+			timeout -s TERM -k 1 60 sh -c \
+				'ip monitor address route 2>/dev/null | head -n1 >/dev/null' || true
+
+			# 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.
+			log "${name}: retrying ..."
+			sleep 2
+		    done
+		    ;;
+	    esac
+	fi
 
 	rm -f "$pidfile"
-	cnt=$(podman image prune -f | wc -l)
-	log "setup: pruned $cnt image(s)"
 	;;
     shell)
 	if [ -z "$name" ]; then

src/confd/src/core.c

@@ -82,8 +82,19 @@ int core_post_hook(sr_session_ctx_t *session, uint32_t sub_id, const char *modul
 		return SR_ERR_OK;
 	}
 
-	if (systemf("initctl -b reload"))
+	if (systemf("initctl -b reload")) {
+		EMERG("initctl reload: failed applying new configuration!");
 		return SR_ERR_SYS;
+	}
+
+	AUDIT("The new configuration has been applied.");
+
+	/* XXX: This should be weaved into Finit when it has absorbed the golden dagger */
+	if (fexist("/run/containers/cleanup")) {
+		char *const cmd[] = { "container", "cleanup", NULL };
+
+		runbg(cmd, 10 * 1000000); /* microseconds */
+	}
 
 	return SR_ERR_OK;
 }

src/confd/src/infix-containers.c

@@ -38,16 +38,6 @@ static int add(const char *name, struct lyd_node *cif)
 	char script[strlen(name) + 5];
 	FILE *fp, *ap;
 
-	/*
-	 * If running already, disable the service, keeping the created
-	 * container and any volumes for later if the user re-enables
-	 * it again.
-	 */
-	if (!lydx_is_enabled(cif, "enabled")) {
-		systemf("initctl -bnq disable container@%s.conf", name);
-		return 0;
-	}
-
 	snprintf(script, sizeof(script), "%s.sh", name);
 	fp = fopenf("w", "%s/%s", _PATH_CONT, script);
 	if (!fp) {
@@ -256,9 +246,9 @@ static int add(const char *name, struct lyd_node *cif)
 	fchmod(fileno(fp), 0700);
 	fclose(fp);
 
-	/* Enable, or update, container -- both trigger container setup. */
-	systemf("initctl -bnq enable container@%s.conf", name);
 	systemf("initctl -bnq touch container@%s.conf", name);
+	systemf("initctl -bnq %s container@%s.conf", lydx_is_enabled(cif, "enabled")
+		? "enable" : "disable", name);
 
 	return 0;
 }
@@ -273,6 +263,9 @@ static int del(const char *name)
 	erasef("%s/%s.sh", _PATH_CONT, name);
 	systemf("initctl -bnq disable container@%s.conf", name);
 
+	/* Schedule a cleanup job for this container as soon as it has stopped */
+	writesf(name, "a", "/run/containers/cleanup");
+
 	return SR_ERR_OK;
 }
 

src/confd/yang/confd/infix-containers.yang

@@ -22,6 +22,11 @@ module infix-containers {
     prefix infix-sys;
   }
 
+  revision 2025-09-09 {
+    description "Add dedicated 'ident' type for container and volume names.";
+    reference "internal";
+  }
+
   revision 2025-06-25 {
     description "Add file mode option to content mounts, allows creating scripts.";
     reference "internal";
@@ -62,6 +67,14 @@ module infix-containers {
    * Typedefs
    */
 
+  typedef ident {
+    description "Container name or volume identifiers may not contain spaces.";
+    type string {
+      length "2..64";
+      pattern '[a-zA-Z0-9][a-zA-Z0-9_.-]+';
+    }
+  }
+
   typedef mount-type {
     type enumeration {
       enum bind {
@@ -128,7 +141,7 @@ module infix-containers {
 
       leaf name {
 	description "Name of the container";
-	type string;
+	type ident;
       }
 
       leaf id {
@@ -344,7 +357,7 @@ module infix-containers {
 
                        For persistent writable directories, *volumes* may be a
                        better fit for your container and easier to set up.";
-          type string;
+          type ident;
         }
 
         leaf type {
@@ -436,10 +449,7 @@ module infix-containers {
                        with the contents of the container's file system on first
                        use.  Hence, upgrading the container image will not update
                        the volume if the image has new/removed files at 'path'.";
-	  type string {
-            pattern '[a-zA-Z_][a-zA-Z0-9_]*';
-            length "1..64";
-          }
+	  type ident;
 	}
 
 	leaf target {

src/confd/yang/containers.inc

@@ -1,5 +1,5 @@
 # -*- sh -*-
 MODULES=(
     "infix-interfaces -e containers"
-    "infix-containers@2025-06-25.yang"
+    "infix-containers@2025-09-09.yang"
 )