fix(swarm): do not call a stack converged while it is still updating

The gate read UpdateStatus but latched only on the states an update cannot
leave by itself, so an in-flight "updating" passed. It then accepted the stack
replica column, whose current count is satisfied by old-spec tasks that are
still running. Between the two, a rolling update reported ready before it had
begun.

Moodle is where that surfaced. A rotation between rounds changes the database
password, the role alters the MariaDB user and renders the new one into the
service environment, and the post-deploy tasks then run inside a container
from the previous spec:

    Access denied for user 'moodle'@'192.168.201.4'

with MariaDB healthy throughout and that address being the manager node's
overlay load-balancer sandbox.

Name "updating" and block on it, and collect any state the case does not know
into its own bucket that reports itself before being treated as in flight, so
a state Docker adds later is diagnosable rather than a silent repository-wide
freeze. Count replicas from the tasks whose desired state is running, which a
rolling update sets to shutdown on the outgoing spec.

The cheap replica comparison stays the fast path and the per-service query
runs only when it passes, which is the only case the old count got wrong. This
gate runs on every swarm deploy, and a converging stack must not pay a
subprocess per service per poll. The service inspect gets one retry before it
fails the gate, so a timeout does not end a deploy on its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

roles/sys-svc-compose/files/shell/swarm/stack_ready.sh

@@ -78,6 +78,20 @@ has_fatal_task() {
 	' <<<"$ps"
 }
 
+inspect_update_states() {
+	timeout 15 docker service inspect \
+		--format '{{.Spec.Name}} {{if .UpdateStatus}}{{.UpdateStatus.State}}{{else}}none{{end}}' \
+		"$@" 2>/dev/null
+}
+
+running_current_spec() {
+	local states
+	states=$(timeout 15 docker service ps "$1" \
+		--filter desired-state=running \
+		--format '{{.CurrentState}}' 2>/dev/null) || return 1
+	printf '%s\n' "$states" | awk '/^Running/ { n++ } END { print n + 0 }'
+}
+
 if ! services=$(timeout 15 docker stack services --format '{{.Name}} {{.Replicas}}' "$STACK"); then
 	echo "not converged: docker stack services failed or timed out for ${STACK}" >&2
 	exit 1
@@ -85,16 +99,29 @@ fi
 
 mapfile -t service_names < <(printf '%s\n' "$services" | awk 'NF {print $1}')
 latched=""
+updating=""
+unknown=""
 if [ ${#service_names[@]} -gt 0 ]; then
-	update_states=$(timeout 15 docker service inspect \
-		--format '{{.Spec.Name}} {{if .UpdateStatus}}{{.UpdateStatus.State}}{{else}}none{{end}}' \
-		"${service_names[@]}" 2>/dev/null || true)
+	if ! update_states=$(inspect_update_states "${service_names[@]}"); then
+		sleep 2
+		if ! update_states=$(inspect_update_states "${service_names[@]}"); then
+			echo "not converged: docker service inspect failed or timed out twice for ${STACK}" >&2
+			exit 1
+		fi
+	fi
 	while read -r name state; do
 		[ -n "$name" ] || continue
 		case "$state" in
+		none | completed) ;;
 		paused | rollback_paused | rollback_started | rollback_completed)
 			latched="${latched} ${name}(${state})"
 			;;
+		updating)
+			updating="${updating} ${name}(${state})"
+			;;
+		*)
+			unknown="${unknown} ${name}(${state})"
+			;;
 		esac
 	done <<<"$update_states"
 fi
@@ -108,11 +135,26 @@ if [ -n "$latched" ]; then
 	exit 2
 fi
 
+if [ -n "$unknown" ]; then
+	echo "not converged: unrecognised update state, treated as in flight:${unknown}" >&2
+	updating="${updating}${unknown}"
+fi
+
+if [ -n "$updating" ]; then
+	echo "not converged: rolling update still in flight:${updating}" >&2
+	printf '%s\n' "$updating" | tr ' ' '\n' | sed 's/(.*//' | while read -r svc; do
+		[ -n "$svc" ] || continue
+		report_tasks "$svc"
+	done
+	exit 1
+fi
+
 not_running=""
 while read -r name reps; do
 	[ -n "$name" ] || continue
 	if awk -v r="$reps" 'BEGIN { split(r, a, "/"); exit (a[1] == a[2]) ? 0 : 1 }'; then
-		continue
+		desired=$(awk -v r="$reps" 'BEGIN { split(r, a, "/"); print a[2] }')
+		[ "$(running_current_spec "$name")" = "$desired" ] && continue
 	fi
 	is_completed_oneshot "$name" && continue
 	not_running="$not_running $name"

tests/unit/python/roles/sys-svc-compose/files/test_stack_ready.py

@@ -19,7 +19,11 @@ waiting rather than arm it.
 
 A service whose ``UpdateStatus`` latched or rolled back is terminal on its own,
 without any task row: swarm will not leave that state until the next stack
-deploy, and a revert leaves the replica count full for the previous spec.
+deploy, and a revert leaves the replica count full for the previous spec. A
+service still ``updating`` keeps the caller waiting instead, and so does a
+service whose desired replicas are only met by old-spec tasks that have not been
+replaced yet - docker's own replica column counts those as running, so the gate
+counts the tasks carrying desired-state=running rather than reading that column.
 """
 
 from __future__ import annotations
@@ -52,7 +56,10 @@ case "$1 $2" in
 	esac
 	;;
 "service ps")
-	case "$5" in
+	case "$*" in
+	*'desired-state=running'*)
+		printf '%s' "${SERVICE_RUNNING}" | awk -F'|' -v s="$3" '$1 == s { print $2 }'
+		;;
 	*'{{.Name}}|{{.CurrentState}}|{{.Error}}'*) printf '%s' "${SERVICE_ERRORS}" ;;
 	*) printf '%s' "${SERVICE_PS}" ;;
 	esac
@@ -60,6 +67,11 @@ case "$1 $2" in
 esac
 exit 0
 """
+WEB_THREE_RUNNING = (
+    "demo_web|Running 2 minutes ago\n"
+    "demo_web|Running 2 minutes ago\n"
+    "demo_web|Running 2 minutes ago\n"
+)
 
 
 class TestStackReady(unittest.TestCase):
@@ -69,6 +81,7 @@ class TestStackReady(unittest.TestCase):
         service_ps: str = "",
         service_errors: str = "",
         service_inspect: str = "",
+        service_running: str = "",
         churning: int = 0,
         updated_at: str | None = None,
     ) -> subprocess.CompletedProcess:
@@ -79,6 +92,8 @@ class TestStackReady(unittest.TestCase):
             service_ps: verbose ``docker service ps`` table for the report.
             service_errors: ``name|current state|error`` rows, newest first.
             service_inspect: ``name update-state`` rows for the latched check.
+            service_running: ``service|current state`` rows carrying
+                desired-state=running, one per task of the current spec.
             churning: seconds since the deploy bumped the service ``UpdatedAt``.
             updated_at: raw ``UpdatedAt`` epoch, overriding ``churning``.
         """
@@ -102,6 +117,7 @@ class TestStackReady(unittest.TestCase):
                 SERVICE_PS=service_ps,
                 SERVICE_ERRORS=service_errors,
                 SERVICE_INSPECT=service_inspect,
+                SERVICE_RUNNING=service_running,
                 SERVICE_UPDATED_AT=updated_at,
                 PATH=f"{stub_bin}:{env['PATH']}",
                 BASH_ENV="",
@@ -115,9 +131,39 @@ class TestStackReady(unittest.TestCase):
             )
 
     def test_a_service_with_no_update_in_flight_converges(self) -> None:
-        proc = self._run("demo_web 3/3\n", service_inspect="demo_web none\n")
+        proc = self._run(
+            "demo_web 3/3\n",
+            service_inspect="demo_web none\n",
+            service_running=WEB_THREE_RUNNING,
+        )
         self.assertEqual(proc.returncode, 0)
 
+    def test_an_update_in_flight_keeps_the_caller_waiting(self) -> None:
+        proc = self._run(
+            "demo_web 3/3\n",
+            service_ps="demo_web.1 desired=Running current=Starting error=\n",
+            service_inspect="demo_web updating\n",
+            service_running=WEB_THREE_RUNNING,
+        )
+        self.assertEqual(proc.returncode, 1)
+        self.assertIn("demo_web(updating)", proc.stderr)
+        self.assertIn("rolling update still in flight", proc.stderr)
+
+    def test_old_spec_tasks_left_running_do_not_fill_the_replica_count(self) -> None:
+        """Docker's replica column counts every Running task, draining old-spec
+        ones included, so a stack mid-rollout reads as full."""
+        proc = self._run(
+            "demo_web 3/3\n",
+            service_ps="demo_web.1 desired=Running current=Preparing error=\n",
+            service_inspect="demo_web completed\n",
+            service_running=(
+                "demo_web|Running 2 minutes ago\ndemo_web|Running 2 minutes ago\n"
+            ),
+        )
+        self.assertEqual(proc.returncode, 1)
+        self.assertIn("not converged", proc.stderr)
+        self.assertIn("demo_web", proc.stderr)
+
     def test_a_latched_update_gives_up_instead_of_polling(self) -> None:
         for latched in ("paused", "rollback_paused"):
             with self.subTest(state=latched):
@@ -142,7 +188,10 @@ class TestStackReady(unittest.TestCase):
                 self.assertIn(f"demo_web({reverted})", proc.stderr)
 
     def test_a_fully_replicated_stack_converges(self) -> None:
-        proc = self._run("demo_web 3/3\ndemo_db 1/1\n")
+        proc = self._run(
+            "demo_web 3/3\ndemo_db 1/1\n",
+            service_running=WEB_THREE_RUNNING + "demo_db|Running 4 minutes ago\n",
+        )
         self.assertEqual(proc.returncode, 0)
 
     def test_a_short_service_does_not_converge(self) -> None: