From 7079d6340f88da36d81202117af60167b94f6e12 Mon Sep 17 00:00:00 2001 From: Nguyen Minh Phuc Date: Tue, 21 Jul 2026 01:45:09 +0000 Subject: [PATCH] docs: bring RUNBOOK and ARCHITECTURE up to date MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runner moved to node0 and the drift check reads release secrets via the Kubernetes API in-cluster; the docs still described node2 and `helm list`. - RUNBOOK: the durable image-cache fix is now the node0 hostPath store, not a node2 pin; /data is NFS RWX, so the Multi-Attach wait is gone. Cross-references entries 9 and 10. - ARCHITECTURE: the reconciler's edge to the cluster is "list releases", not "helm list" (helm is the out-of-cluster fallback). - ARCHITECTURE: the state diagram and its prose described fail() moving every dead-lettered instance to `failed`. Corrected to the per-kind behaviour — only provision fails the instance; deprovision stays `deleting` for retry, upgrade and verify stay `ready` — matching the fix in tasks.py. --- ARCHITECTURE.md | 36 ++++++++++++++++++++++++------------ RUNBOOK.md | 12 +++++++----- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9b7b91c..3afdfe7 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -46,7 +46,7 @@ flowchart LR WORKER -->|"claim
SKIP LOCKED"| PG WORKER -->|"helm upgrade --install"| K8S RECON -->|"drift, leases,
TTL, versions"| PG - RECON -->|"helm list"| K8S + RECON -->|"list releases"| K8S classDef truth fill:#2d4a22,stroke:#5a8f3d,color:#fff classDef derived fill:#4a3222,stroke:#8f6a3d,color:#fff @@ -191,12 +191,12 @@ stateDiagram-v2 ready --> deleting: DELETE, or TTL expired deleting --> deleted: helm uninstall succeeded - requested --> failed: attempts exhausted - provisioning --> failed: attempts exhausted + requested --> failed: provision attempts exhausted + provisioning --> failed: provision attempts exhausted ready --> failed: drift — the release vanished failed --> provisioning: retry failed --> deleting: give up, tear it down - deleting --> failed: attempts exhausted + deleting --> deleting: deprovision retried, never failed deleted --> [*]: terminal ``` @@ -205,19 +205,31 @@ stateDiagram-v2 chain of `if`s. `deleted` maps to an **empty frozenset** rather than being absent, so "terminal" is stated rather than implied by a missing key. -**The state machine is enforced in SQL too.** `TaskRepo.fail` writes `instances.state` -directly, so it derives its guard from the same `LEGAL` table: +**Dead-lettering the task does not fail the instance, except for provision.** When a task +exhausts its retries `TaskRepo.fail` marks the *task* `failed` for every kind. It moves the +*instance* to `failed` only for `provision`, because that is the only kind where a dead +letter means the instance is broken. For the others the instance is still healthy and +something else owns its recovery: + +| kind | instance state on dead-letter | why | +|------|-------------------------------|-----| +| provision | `failed` | it never came up; a human re-provisions | +| deprovision | stays `deleting` | so `due_for_deprovision` re-enqueues it; `failed` would strand it and leak the release | +| upgrade | stays `ready` | `helm --atomic` rolled back; it still serves the old version | +| verify | stays `ready` | `handle_verify` already halted the rollout | + +The provision write is still guarded by the state machine, derived from the same `LEGAL` +table rather than restated: ```python _CAN_FAIL = tuple(s.value for s, allowed in LEGAL.items() if InstanceState.FAILED in allowed) ... -UPDATE instances SET state='failed' WHERE id=%s AND state = ANY(%s) +if kind == 'provision': + UPDATE instances SET state='failed' WHERE id=%s AND state = ANY(%s) ``` -Without that, a deprovision exhausting its retries against an already-`deleted` instance -would resurrect it into `failed` — a transition `transition()` explicitly forbids, -performed by raw SQL that never asked it. A state machine only one layer respects is -decoration. +The `SvcforgeTaskDeadLettered` alert fires for every kind, so leaving the instance alone +loses no operator visibility. --- @@ -278,7 +290,7 @@ One replica. Four checks. Every 60 seconds. ```mermaid flowchart LR - TICK(("tick
every 60s")) --> D["drift
helm list vs DB"] + TICK(("tick
every 60s")) --> D["drift
live releases vs DB"] TICK --> L["lease expiry
running + locked_at old"] TICK --> T["TTL
ready + expires_at passed"] TICK --> V["version drift
chart_version ≠ catalog"] diff --git a/RUNBOOK.md b/RUNBOOK.md index 569360f..c2d6a9f 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -118,11 +118,13 @@ kubectl -n gitea exec gitea-actions-runner-0 -c dind -- docker pull \ ghcr.io/catthehacker/ubuntu:act-24.04@sha256:c710431fbad9eb3bcb102d04e5ff74fbd0ce6e383f78afebfb3770a1a817fdf9 ``` -The durable fix is to stop the runner restarting. Its `/data` PVC is ReadWriteOnce, so -every reschedule hits `Multi-Attach error` and the pod sits in Init until Longhorn detaches -from the old node. It is pinned to node2 in `oci-k8s/.../addons/tasks/main.yml` for exactly -that reason. A dedicated PVC for the image cache would survive restarts outright, but on -this cluster that volume faulted and blocked the runner, so it is deliberately not used. +The durable fix is a persistent image store, which the runner now has: `/var/lib/docker` +is a hostPath on node0 (see `oci-k8s/.../addons/tasks/main.yml`), so the act image survives +a restart and is not re-pulled. The runner is pinned to **node0**, not node2 — node2 is a +single-core control-plane node whose pod network was measured 21x slower under its own +load, which starved every clone and pull. Its `/data` PVC is NFS ReadWriteMany, so a +reschedule attaches immediately with no `Multi-Attach` wait. Entries 9 and 10 cover the +caches and the node move in full. ### 6. Stopping a run, and reading a restarted runner correctly