From db6f37a07018d36c6cd5a14de2b7922c6950af25 Mon Sep 17 00:00:00 2001
From: Evan Jarrett <evan@evanjarrett.com>
Date: Fri, 11 Sep 2026 19:53:06 -0500
Subject: [PATCH] deploy: never install a template config during update, and
 poll every restarted service's health

On 2026-09-09 the labeler's config was replaced with the rendered template,
which has empty identity fields (owner DID, DID, rotation key). The running
process kept its in-memory config, so nothing failed until the next restart
on 2026-09-12, when the labeler crash-looped on "labeler.owner_did is
required" and nobody was told: the deploy tool never probed the labeler at
all. The sync's missing-file branch is the only code that writes a whole
template, so an update now refuses when the file is gone and says to restore
it from the predeploy backup; provision keeps the first-install behaviour.

The health check was a single curl two seconds after restart. The hold takes
longer than that to open its listener, so tonight's deploy printed
HEALTH_FAIL for a hold that answered seconds later. Worse, the verdict was a
substring match on HEALTH_OK, which the scanner's SCANNER_HEALTH_OK line also
satisfies, so a failed hold next to a healthy scanner was reported healthy.
Each restarted service (hold, scanner, appview, labeler) is now polled every
two seconds for up to thirty, and reports on its own whole-label line.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EvFJr4Dwz8p2NDAeXmgmBt
---
 deploy/upcloud/cloudinit.go   | 15 +++++++-
 deploy/upcloud/health.go      | 72 +++++++++++++++++++++++++++++++++++
 deploy/upcloud/health_test.go | 32 ++++++++++++++++
 deploy/upcloud/provision.go   |  8 ++--
 deploy/upcloud/update.go      | 46 ++++++++++------------
 5 files changed, 142 insertions(+), 31 deletions(-)
 create mode 100644 deploy/upcloud/health.go
 create mode 100644 deploy/upcloud/health_test.go

diff --git a/deploy/upcloud/cloudinit.go b/deploy/upcloud/cloudinit.go
index 8cc0953..b0e2a7b 100644
--- a/deploy/upcloud/cloudinit.go
+++ b/deploy/upcloud/cloudinit.go
@@ -417,15 +417,28 @@ func syncServiceUnit(name, ip, serviceName, renderedUnit string) (bool, error) {
 
 // syncConfigKeys fetches the existing config from a server and merges in any
 // missing keys from the rendered template. Existing values are never overwritten.
-func syncConfigKeys(name, ip, configPath, templateYAML string) error {
+//
+// installIfMissing is true only during provision. On an update a missing
+// config is never a first install: the service has been running with one, so
+// the file has been moved or deleted, and writing the template in its place
+// would hand the service empty identity fields (owner DID, DID, keys) that it
+// refuses to start with. That is exactly what happened to the labeler on
+// 2026-09-09, and it only surfaced at the next restart three days later.
+func syncConfigKeys(name, ip, configPath, templateYAML string, installIfMissing bool) error {
 	remote, err := runSSH(ip, fmt.Sprintf("cat %s 2>/dev/null || echo '__MISSING__'", configPath), false)
 	if err != nil {
+		if !installIfMissing {
+			return fmt.Errorf("read %s config %s: %w", name, configPath, err)
+		}
 		fmt.Printf("  config sync: could not reach %s (%v)\n", name, err)
 		return nil
 	}
 	remote = strings.TrimSpace(remote)
 
 	if remote == "__MISSING__" {
+		if !installIfMissing {
+			return fmt.Errorf("%s config %s is missing on the server; refusing to install the template during an update because its identity fields are empty. Restore the file from the newest predeploy-* backup under the data directory and rerun", name, configPath)
+		}
 		// First-time install: write the rendered template as-is. Subsequent
 		// runs use the merge-keys path below to preserve operator edits.
 		dir := configPath[:strings.LastIndex(configPath, "/")]
diff --git a/deploy/upcloud/health.go b/deploy/upcloud/health.go
new file mode 100644
index 0000000..3282176
--- /dev/null
+++ b/deploy/upcloud/health.go
@@ -0,0 +1,72 @@
+package main
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+	"strings"
+)
+
+// healthProbeTries is how many 2-second polls a service gets to answer its
+// health URL after a restart. The hold takes several seconds to open its
+// listener (PLC check, carstore open), so a single probe two seconds after
+// restart reported a failure on a hold that was healthy moments later.
+const healthProbeTries = 15
+
+// renderHealthProbe returns a bash snippet that polls url every two seconds
+// until it answers success or the tries run out, then prints one line:
+//
+//	PROBE <label> OK <seconds>
+//	PROBE <label> FAIL <seconds>
+//
+// Labels are matched whole by parseProbes, so a healthy scanner can never be
+// mistaken for a healthy hold the way a substring match on HEALTH_OK could.
+func renderHealthProbe(label, url string, tries int) string {
+	return fmt.Sprintf(`for __i in $(seq 1 %d); do
+  if curl -sf %q > /dev/null 2>&1; then echo "PROBE %s OK $((__i * 2))"; break; fi
+  if [ "$__i" -eq %d ]; then echo "PROBE %s FAIL $((__i * 2))"; fi
+  sleep 2
+done
+`, tries, url, label, tries, label)
+}
+
+var probeLine = regexp.MustCompile(`(?m)^PROBE (\S+) (OK|FAIL) (\d+)$`)
+
+// probeResult is one service's health verdict after a restart.
+type probeResult struct {
+	Status  string // OK or FAIL
+	Seconds int
+}
+
+// parseProbes reads the PROBE lines out of a restart script's output.
+func parseProbes(output string) map[string]probeResult {
+	results := map[string]probeResult{}
+	for _, m := range probeLine.FindAllStringSubmatch(output, -1) {
+		secs, _ := strconv.Atoi(m[3]) // the regexp guarantees digits
+		results[m[1]] = probeResult{Status: m[2], Seconds: secs}
+	}
+	return results
+}
+
+// reportProbe prints the verdict for one service, with the journal hint on
+// failure. A service with no PROBE line at all was never probed, which is
+// reported rather than assumed healthy.
+func reportProbe(results map[string]probeResult, label, ip, serviceName string) {
+	r, ok := results[label]
+	switch {
+	case !ok:
+		fmt.Printf("  %s: updated (health check inconclusive)\n", label)
+	case r.Status == "OK":
+		fmt.Printf("  %s: updated and healthy (answered after %ds)\n", label, r.Seconds)
+	default:
+		fmt.Printf("  %s: updated but health check failed after %ds!\n", label, r.Seconds)
+		fmt.Printf("  Check: ssh root@%s journalctl -u %s -n 50\n", ip, serviceName)
+	}
+}
+
+// probeMarkersPresent reports whether output carries any PROBE line, which
+// distinguishes a restart script that ran to completion from one that died
+// before probing.
+func probeMarkersPresent(output string) bool {
+	return strings.Contains(output, "PROBE ")
+}
diff --git a/deploy/upcloud/health_test.go b/deploy/upcloud/health_test.go
new file mode 100644
index 0000000..5312f25
--- /dev/null
+++ b/deploy/upcloud/health_test.go
@@ -0,0 +1,32 @@
+package main
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestRenderHealthProbe(t *testing.T) {
+	s := renderHealthProbe("hold", "http://localhost:8080/xrpc/_health", 15)
+	for _, want := range []string{"seq 1 15", `curl -sf "http://localhost:8080/xrpc/_health"`, `echo "PROBE hold OK`, `echo "PROBE hold FAIL`, "sleep 2"} {
+		if !strings.Contains(s, want) {
+			t.Errorf("probe script missing %q:\n%s", want, s)
+		}
+	}
+}
+
+func TestParseProbesDoesNotConfuseServices(t *testing.T) {
+	out := "systemctl output\nPROBE hold FAIL 30\nPROBE scanner OK 4\nnoise PROBE labeler OK 2\n"
+	got := parseProbes(out)
+	if got["hold"].Status != "FAIL" || got["hold"].Seconds != 30 {
+		t.Errorf("hold = %+v, want FAIL after 30s", got["hold"])
+	}
+	if got["scanner"].Status != "OK" || got["scanner"].Seconds != 4 {
+		t.Errorf("scanner = %+v, want OK after 4s", got["scanner"])
+	}
+	if _, ok := got["labeler"]; ok {
+		t.Errorf("a PROBE token that is not at the start of a line must not count: %+v", got["labeler"])
+	}
+	if !probeMarkersPresent(out) {
+		t.Error("expected probe markers to be detected")
+	}
+}
diff --git a/deploy/upcloud/provision.go b/deploy/upcloud/provision.go
index c038322..3661830 100644
--- a/deploy/upcloud/provision.go
+++ b/deploy/upcloud/provision.go
@@ -221,7 +221,7 @@ func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, w
 		if err != nil {
 			return fmt.Errorf("render appview config: %w", err)
 		}
-		if err := syncConfigKeys("appview", state.Appview.PublicIP, naming.AppviewConfigPath(), appviewConfigYAML); err != nil {
+		if err := syncConfigKeys("appview", state.Appview.PublicIP, naming.AppviewConfigPath(), appviewConfigYAML, true); err != nil {
 			return fmt.Errorf("appview config sync: %w", err)
 		}
 		if state.LabelerEnabled {
@@ -229,7 +229,7 @@ func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, w
 			if err != nil {
 				return fmt.Errorf("render labeler config: %w", err)
 			}
-			if err := syncConfigKeys("labeler", state.Appview.PublicIP, naming.LabelerConfigPath(), labelerConfigYAML); err != nil {
+			if err := syncConfigKeys("labeler", state.Appview.PublicIP, naming.LabelerConfigPath(), labelerConfigYAML, true); err != nil {
 				return fmt.Errorf("labeler config sync: %w", err)
 			}
 		}
@@ -264,7 +264,7 @@ func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, w
 		if err != nil {
 			return fmt.Errorf("render hold config: %w", err)
 		}
-		if err := syncConfigKeys("hold", state.Hold.PublicIP, naming.HoldConfigPath(), holdConfigYAML); err != nil {
+		if err := syncConfigKeys("hold", state.Hold.PublicIP, naming.HoldConfigPath(), holdConfigYAML, true); err != nil {
 			return fmt.Errorf("hold config sync: %w", err)
 		}
 		if state.ScannerEnabled {
@@ -272,7 +272,7 @@ func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, w
 			if err != nil {
 				return fmt.Errorf("render scanner config: %w", err)
 			}
-			if err := syncConfigKeys("scanner", state.Hold.PublicIP, naming.ScannerConfigPath(), scannerConfigYAML); err != nil {
+			if err := syncConfigKeys("scanner", state.Hold.PublicIP, naming.ScannerConfigPath(), scannerConfigYAML, true); err != nil {
 				return fmt.Errorf("scanner config sync: %w", err)
 			}
 		}
diff --git a/deploy/upcloud/update.go b/deploy/upcloud/update.go
index 601f010..5444b6b 100644
--- a/deploy/upcloud/update.go
+++ b/deploy/upcloud/update.go
@@ -177,7 +177,7 @@ func cmdUpdate(target string, withScanner, withLabeler, withBilling bool, opts u
 		if err != nil {
 			return fmt.Errorf("render %s config: %w", name, err)
 		}
-		if err := syncConfigKeys(name, t.ip, t.configPath, configYAML); err != nil {
+		if err := syncConfigKeys(name, t.ip, t.configPath, configYAML, false); err != nil {
 			return fmt.Errorf("%s config sync: %w", name, err)
 		}
 
@@ -219,7 +219,7 @@ func cmdUpdate(target string, withScanner, withLabeler, withBilling bool, opts u
 			if err != nil {
 				return fmt.Errorf("render scanner config: %w", err)
 			}
-			if err := syncConfigKeys("scanner", t.ip, naming.ScannerConfigPath(), scannerConfigYAML); err != nil {
+			if err := syncConfigKeys("scanner", t.ip, naming.ScannerConfigPath(), scannerConfigYAML, false); err != nil {
 				return fmt.Errorf("scanner config sync: %w", err)
 			}
 
@@ -261,21 +261,19 @@ chown -R %s:%s %s`,
 			}
 
 			scannerRestart = fmt.Sprintf("\nsystemctl restart %s", naming.Scanner())
-			scannerHealthCheck = `
-sleep 2
-curl -sf http://localhost:9090/healthz > /dev/null && echo "SCANNER_HEALTH_OK" || echo "SCANNER_HEALTH_FAIL"
-`
+			scannerHealthCheck = renderHealthProbe("scanner", "http://localhost:9090/healthz", healthProbeTries)
 		}
 
 		// Labeler additions for appview server
 		labelerRestart := ""
+		labelerHealthCheck := ""
 		if name == "appview" && state.LabelerEnabled {
 			// Sync labeler config keys
 			labelerConfigYAML, err := renderConfig(labelerConfigTmpl, vals)
 			if err != nil {
 				return fmt.Errorf("render labeler config: %w", err)
 			}
-			if err := syncConfigKeys("labeler", t.ip, naming.LabelerConfigPath(), labelerConfigYAML); err != nil {
+			if err := syncConfigKeys("labeler", t.ip, naming.LabelerConfigPath(), labelerConfigYAML, false); err != nil {
 				return fmt.Errorf("labeler config sync: %w", err)
 			}
 
@@ -317,15 +315,18 @@ chown -R %s:%s %s`,
 			}
 
 			labelerRestart = fmt.Sprintf("\nsystemctl restart %s", naming.Labeler())
+			labelerHealthCheck = renderHealthProbe("labeler", "http://localhost:5002/.well-known/did.json", healthProbeTries)
 		}
 
-		// Restart services and health check
+		// Restart services, then poll each one's health URL for up to 30s.
+		// Every restarted service is probed, the labeler included: a service
+		// that crash-loops on its config is invisible to systemctl's exit
+		// status and only shows up as a health URL that never answers.
 		restartScript := fmt.Sprintf(`set -euo pipefail
 %s
 systemctl restart %s%s%s
-sleep 2
-curl -sf %s > /dev/null && echo "HEALTH_OK" || echo "HEALTH_FAIL"
-%s`, daemonReload, t.serviceName, scannerRestart, labelerRestart, t.healthURL, scannerHealthCheck)
+%s%s%s`, daemonReload, t.serviceName, scannerRestart, labelerRestart,
+			renderHealthProbe(name, t.healthURL, healthProbeTries), scannerHealthCheck, labelerHealthCheck)
 
 		output, err := runSSH(t.ip, restartScript, true)
 		if err != nil {
@@ -333,24 +334,17 @@ curl -sf %s > /dev/null && echo "HEALTH_OK" || echo "HEALTH_FAIL"
 			fmt.Printf("  Output: %s\n", output)
 			return fmt.Errorf("restart %s failed", name)
 		}
-
-		if strings.Contains(output, "HEALTH_OK") {
-			fmt.Printf("  %s: updated and healthy\n", name)
-		} else if strings.Contains(output, "HEALTH_FAIL") {
-			fmt.Printf("  %s: updated but health check failed!\n", name)
-			fmt.Printf("  Check: ssh root@%s journalctl -u %s -n 50\n", t.ip, t.serviceName)
-		} else {
-			fmt.Printf("  %s: updated (health check inconclusive)\n", name)
+		if !probeMarkersPresent(output) {
+			fmt.Printf("  %s: restarted, but the health probes never ran\n", name)
 		}
 
-		// Scanner health reporting
+		results := parseProbes(output)
+		reportProbe(results, name, t.ip, t.serviceName)
 		if name == "hold" && state.ScannerEnabled {
-			if strings.Contains(output, "SCANNER_HEALTH_OK") {
-				fmt.Printf("  scanner: updated and healthy\n")
-			} else if strings.Contains(output, "SCANNER_HEALTH_FAIL") {
-				fmt.Printf("  scanner: updated but health check failed!\n")
-				fmt.Printf("  Check: ssh root@%s journalctl -u %s -n 50\n", t.ip, naming.Scanner())
-			}
+			reportProbe(results, "scanner", t.ip, naming.Scanner())
+		}
+		if name == "appview" && state.LabelerEnabled {
+			reportProbe(results, "labeler", t.ip, naming.Labeler())
 		}
 	}