Fix container upgrade regression

In 0edc2d5 and afbe5ca, just prior to v24.06.0-rc1, support for deleting
containers in the background was added.  However, this also broke support
for upgrading containers.

In afbe5ca the start script for containers was renamed from NAME.sh to
S01-NAME.sh, but the container wrapper script's upgrade command was not
updated.  Neither was the cleanup and post-hook callbacks in confd!  So
when a container had been added to the system, the cleanup callback just
simply deleted the script, preventing it from being recreated at ugprade

This patch fixes the container identification code and also refactors
the execd code to ensure that kill scripts (for deleting in background)
and start scripts are run in the proper order *and* ensuring that execd
also does not accidentally remove the container start script.

Some cosmetic changes to the output at upgrade have also been added.

Fixes #623

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

board/common/rootfs/usr/sbin/container

@@ -626,20 +626,32 @@ case $cmd in
 	podman stats -i 2
 	;;
     upgrade)
+	# Start script used to initially create container
+	script=/var/lib/containers/active/S01-${1}.sh
+
+	# Find container image
 	img=$(podman inspect "$1" | jq -r .[].ImageName)
 	if [ -z "$img" ]; then
 	    echo "No such container ($1), or invalid ImageName.  Cannot upgrade."
 	    exit 1;
 	fi
+
+	# Likely an OCI archive, or local directory, assume user has updated image.
 	if echo "$img" | grep -Eq '^localhost/'; then
-	    # Likely an OCI archive, or local directory, assume user has updated image.
-	    file=$(awk '{s=$NF} END{print s}' "/var/lib/containers/active/${1}.sh")
+	    file=$(awk '{s=$NF} END{print s}' "$script")
 	    echo "Upgrading container ${1} with local archive: $file ..."
 	else
+	    printf ">> Stopping ... "
 	    podman stop "$1"
+	    printf ">> "
 	    podman pull "$img" || (echo "Failed fetching $img, check your network (settings)."; exit 1)
 	fi
-	"/var/lib/containers/active/${1}.sh" || (echo "Failed recreating container $1"; exit 1)
+	echo ">> Starting $1 ..."
+	if ! "$script"; then
+	    echo ">> Failed recreating container $1"
+	    exit 1
+	fi
+	echo ">> Done."
 	;;
     volume)
 	cmd=$1

src/confd/src/infix-containers.c

@@ -36,8 +36,8 @@ static int add(const char *name, struct lyd_node *cif)
 
 	/* Stop any running container gracefully so it releases its IP addresses. */
 	fprintf(fp, "#!/bin/sh\n"
-		"container stop %s\n"
-		"container delete %s\n"
+		"container stop %s   >/dev/null\n" /* Silence "not running" on upgrade */
+		"container delete %s >/dev/null\n" /* Silence any hashes when deleting */
 		"container", name, name);
 
 	LYX_LIST_FOR_EACH(lyd_child(cif), node, "dns")
@@ -367,7 +367,10 @@ static void cleanup(sr_session_ctx_t *session, struct confd *confd)
 			continue; /* odd, non-script file? */
 		*ptr = 0;
 
-		if (is_active(session, name))
+		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. */
@@ -421,11 +424,15 @@ void infix_containers_post_hook(sr_session_ctx_t *session, struct confd *confd)
 			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];
 
 				/* New job is already active, no changes, skipping ... */
-				if (!is_manual(session, name))
-					systemf("initctl -bnq cond set container:%s", name);
+				if (nm && !is_manual(session, nm))
+					systemf("initctl -bnq cond set container:%s", nm);
 			}
 			remove(next);
 			continue;

src/execd/execd.c

@@ -57,7 +57,7 @@ static void run_job(const char *path, char *file, int archive)
 		return;
 	}
 
-	dbg("job %s in %s done", file, path);
+	dbg("job %s in %s done %p, archive: %d", file, path, done, archive);
 	if (done && archive)
 		movefile(cmd, done);
 	else
@@ -69,28 +69,43 @@ 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 check(const char *name, int type, int *archive)
+static int should_run(const char *name, int type, int *archive)
 {
 	if (!name || strlen(name) < 3)
-		return -1;
+		return 0;
 
 	if (isdigit(name[1]) && isdigit(name[2])) {
-		if (type == '*')
-			type = name[0];
+		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;
+			}
+		}
 
 		switch (type) {
 		case 'K':
 			*archive = 0;
-			return 0;
+			break;
 		case 'S':
 			*archive = 1;
-			return 0;
-		default:
 			break;
+		default:
+			return 0;
 		}
+
+		dbg("name:%s type:'%c' archive:%d => run:%d", name, type, *archive, type == name[0]);
+		return type == name[0];
 	}
 
-	return 1;
+	errx("unsupported script %s, must follow pattern SNN/KNN", name);
+	return 0;
 }
 
 static void run_dir(const char *path, int type)
@@ -106,15 +121,24 @@ static void run_dir(const char *path, int type)
 	}
 
 	for (i = 0; i < n; i++) {
-		if (!check(namelist[i]->d_name, type, &archive))
-			run_job(path, namelist[i]->d_name, archive);
+		struct dirent *d = namelist[i];
 
-		free(namelist[i]);
+		if (d->d_type == DT_DIR)
+			continue;
+
+		if (should_run(d->d_name, type, &archive))
+			run_job(path, d->d_name, archive);
+
+		free(d);
 	}
 
 	free(namelist);
 }
 
+/*
+ * Call stop/cleanup jobs first, may use same container name or
+ * resources as replacement container start scripts use.
+ */
 static void run_queue(char *path)
 {
 	run_dir(path, 'K');
@@ -154,7 +178,7 @@ static void inotify_cb(uev_t *w, void *arg, int _)
 
 		if (event->mask & (IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVED_TO)) {
 			dbg("Got inotify event %s 0x%04x", name, event->mask);
-			if (check(name, '*', &archive))
+			if (!should_run(name, '*', &archive))
 				continue;
 
 			run_job(arg, name, archive);