feat(v0.156.0): dead-primary alerting (R-51) + boot desired-state reconciliation (R-52)
R-51: aggregateState's mixed branch returned StateRunning ("partial"), so a stack whose
MAIN container was dead behind live helpers alerted on nothing — immich-server sat Exited
for 18 h, 100 % unreachable, no banner and no app_start_failed (audit F4). New
StateDegraded: a DOWN member whose docker restart policy is always/unless-stopped is a
fault (degraded, a down state); no/on-failure is a finished one-shot and stays benign; an
unreadable policy fails CLOSED. The unhealthy/restarting/paused/unknown exclusions are
byte-identical — folding unhealthy into down is the flapping fix-3 avoided.
R-52: new internal/bootrecon — one bounded start-once sweep at startup (2 attempts, 30 s
apart) for apps an interrupted boot left behind, inside the 90 s boot grace so a success
is silent and a failure still alerts. A zero-container stack is NEVER touched: the UI's
Stop is compose down, so a deliberate stop survives a reboot.
Both features carry a production-path wiring test (the v0.154.0 / v0.91.0 inert-seam
class). The main() assertion is an AST walk, not strings.Contains — the substring version
passed its own red-proof, because a commented-out call still contains the string.
Red-proofs run and restored: mix branch reverted -> "running" on the immich fixture;
boot hook commented out -> wiring test fails; zero-container gate dropped -> the
user-stopped app gets started.
NOTE: controller/cmd/controller/ is matched by .gitignore's `controller` entry, so new
files there need `git add -f` (and ripgrep silently skips main.go without --no-ignore).
This commit is contained in:
@@ -28,6 +28,7 @@ const (
|
||||
StateStarting ContainerState = "starting" // running but health: starting
|
||||
StateUnhealthy ContainerState = "unhealthy" // running but health: unhealthy
|
||||
StateStopped ContainerState = "stopped"
|
||||
StateDegraded ContainerState = "degraded" // multi-container stack: a SUPERVISED member is dead (R-51)
|
||||
StateRestarting ContainerState = "restarting"
|
||||
StateExited ContainerState = "exited"
|
||||
StatePaused ContainerState = "paused"
|
||||
@@ -38,13 +39,20 @@ const (
|
||||
)
|
||||
|
||||
// IsDownState reports whether a container state means a DEPLOYED app is not running and won't recover
|
||||
// on its own (fix-3, CAMPAIGN-3). Only `stopped` and `exited` qualify — a Docker "created"/"dead"
|
||||
// container (a failed-at-boot app, the F11 case) resolves to `stopped`. Deliberately NOT `starting`
|
||||
// / `unhealthy` (running, with their own health handling), `restarting` (self-recovering),
|
||||
// `deploying` (mid-deploy), `paused` (a deliberate user action), or `unknown` (ambiguous — fail-open,
|
||||
// never manufacture a dead-app alert from an inconclusive read).
|
||||
// on its own (fix-3, CAMPAIGN-3). Only `stopped`, `exited` and `degraded` qualify — a Docker
|
||||
// "created"/"dead" container (a failed-at-boot app, the F11 case) resolves to `stopped`. Deliberately
|
||||
// NOT `starting` / `unhealthy` (running, with their own health handling), `restarting`
|
||||
// (self-recovering), `deploying` (mid-deploy), `paused` (a deliberate user action), or `unknown`
|
||||
// (ambiguous — fail-open, never manufacture a dead-app alert from an inconclusive read).
|
||||
//
|
||||
// R-51 (v0.156.0) added `degraded`: a multi-container stack whose SUPERVISED member is dead is as
|
||||
// unreachable as a single-container app that exited (immich-server sat Exited for 18 h with the app
|
||||
// 100 % dead and no alert, while single-container Calibre-Web alerted in 90 s). This is deliberately
|
||||
// NOT the same as folding `unhealthy` into down — that exclusion stays byte-identical, because
|
||||
// `unhealthy` is a *running* container whose healthcheck is failing and folding it in reintroduces
|
||||
// the flapping fix-3 was added to stop.
|
||||
func IsDownState(s ContainerState) bool {
|
||||
return s == StateStopped || s == StateExited
|
||||
return s == StateStopped || s == StateExited || s == StateDegraded
|
||||
}
|
||||
|
||||
// ContainerInfo holds status info about a single container within a stack.
|
||||
@@ -108,6 +116,17 @@ type Manager struct {
|
||||
backupRunning func() bool // mutual exclusion with the backup orchestrator (Change 3)
|
||||
migDoneHook func(*MigrationJob) // fired on successful completion (decommission policy lives in caller)
|
||||
testSeams *migSeams // nil in production; tests inject fakes
|
||||
// R-51: docker restart policies for DOWN members of mixed stacks. Keyed by
|
||||
// containerName+"|"+state so a transitioned or recreated container re-reads rather than
|
||||
// answering from a stale entry; pruned every refresh to the live container set. Guarded by mu
|
||||
// (every read/write happens under refreshStatusLocked's write lock).
|
||||
restartPolicyCache map[string]string
|
||||
// execFn replaces execCommand's process boundary in tests; nil in production.
|
||||
execFn func(name string, args ...string) (string, error)
|
||||
// inspectRestartPolicyFn is the docker-inspect seam for the above; nil in production
|
||||
// (dockerRestartPolicy). Tests inject a scripted lookup and never touch docker.
|
||||
inspectRestartPolicyFn func(containerName string) (string, error)
|
||||
|
||||
// isMountPoint reports whether a path is a live mountpoint; defaults to system.IsMountPoint.
|
||||
// Injectable so the userdata-belt drive-absent gate is testable (a t.TempDir is never a real mount).
|
||||
isMountPoint func(string) bool
|
||||
@@ -306,7 +325,7 @@ func (m *Manager) DeployedStackNames() []string {
|
||||
}
|
||||
|
||||
// RunningAppStacks returns the names of deployed, NON-protected stacks that currently have
|
||||
// containers up (running/starting/unhealthy/restarting) — the set the quiesce loop (slice 8B)
|
||||
// containers up (running/starting/unhealthy/restarting/degraded) — the set the quiesce loop (slice 8B)
|
||||
// stops before an app-consistent backup and restarts after. Protected infra (traefik, cloudflared,
|
||||
// felhom-controller) is excluded so the controller never stops its own tunnel/proxy or itself.
|
||||
// Sorted for deterministic stop/start order.
|
||||
@@ -319,7 +338,9 @@ func (m *Manager) RunningAppStacks() []string {
|
||||
continue
|
||||
}
|
||||
switch stack.State {
|
||||
case StateRunning, StateStarting, StateUnhealthy, StateRestarting:
|
||||
// StateDegraded (R-51) belongs here: a degraded stack still has LIVE members, and the
|
||||
// quiesce loop must stop them before an app-consistent backup and start them after.
|
||||
case StateRunning, StateStarting, StateUnhealthy, StateRestarting, StateDegraded:
|
||||
names = append(names, name)
|
||||
}
|
||||
}
|
||||
@@ -451,6 +472,7 @@ func (m *Manager) refreshStatusLocked() error {
|
||||
}
|
||||
|
||||
projectContainers := make(map[string][]ContainerInfo)
|
||||
liveContainers := make(map[string]bool)
|
||||
|
||||
totalContainers := 0
|
||||
for _, line := range strings.Split(strings.TrimSpace(output), "\n") {
|
||||
@@ -469,8 +491,11 @@ func (m *Manager) refreshStatusLocked() error {
|
||||
Status: parts[3],
|
||||
}
|
||||
projectContainers[parts[4]] = append(projectContainers[parts[4]], ci)
|
||||
liveContainers[ci.Name] = true
|
||||
totalContainers++
|
||||
}
|
||||
m.pruneRestartPolicyCacheLocked(liveContainers)
|
||||
policyOf := m.restartPolicyLookupLocked()
|
||||
|
||||
// fix-6: refreshStatusLocked runs every 10s (the status-refresh job) — its per-cycle enumeration
|
||||
// lines are TRACE (dropped from the debug ring) so they don't eat the post-incident window. A real
|
||||
@@ -492,7 +517,7 @@ func (m *Manager) refreshStatusLocked() error {
|
||||
}
|
||||
} else {
|
||||
stack.Containers = containers
|
||||
stack.State = aggregateState(containers)
|
||||
stack.State = aggregateState(containers, policyOf)
|
||||
}
|
||||
|
||||
// Re-apply controller-side health probe results: if the last probe
|
||||
@@ -511,6 +536,59 @@ func (m *Manager) refreshStatusLocked() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// dockerRestartPolicy reads one container's configured restart policy via docker inspect.
|
||||
// Returns ("", err) when the container is gone or the inspect fails — the caller treats that as
|
||||
// UNKNOWN (see supervisedPolicy).
|
||||
func (m *Manager) dockerRestartPolicy(containerName string) (string, error) {
|
||||
if m.inspectRestartPolicyFn != nil {
|
||||
return m.inspectRestartPolicyFn(containerName)
|
||||
}
|
||||
out, err := m.execCommand("docker", "inspect", "-f", "{{.HostConfig.RestartPolicy.Name}}", containerName)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return strings.TrimSpace(out), nil
|
||||
}
|
||||
|
||||
// restartPolicyLookupLocked returns the cached-and-memoizing lookup handed to aggregateState.
|
||||
// MUST be called with m.mu held for writing (it populates the cache).
|
||||
func (m *Manager) restartPolicyLookupLocked() restartPolicyLookup {
|
||||
return func(name string) string {
|
||||
key := name + "|policy"
|
||||
if m.restartPolicyCache != nil {
|
||||
if p, ok := m.restartPolicyCache[key]; ok {
|
||||
return p
|
||||
}
|
||||
}
|
||||
p, err := m.dockerRestartPolicy(name)
|
||||
if err != nil {
|
||||
// UNKNOWN — deliberately not cached, so a transient docker hiccup does not pin a
|
||||
// container to "unknown" for the rest of the process lifetime.
|
||||
m.logger.Printf("[WARN] [stacks] restart-policy inspect failed for container %q: %v (treating as supervised)", name, err)
|
||||
return ""
|
||||
}
|
||||
if m.restartPolicyCache == nil {
|
||||
m.restartPolicyCache = map[string]string{}
|
||||
}
|
||||
m.restartPolicyCache[key] = p
|
||||
if m.isDebug() {
|
||||
m.logger.Printf("[DEBUG] [stacks] restart-policy of down member %q = %q", name, p)
|
||||
}
|
||||
return p
|
||||
}
|
||||
}
|
||||
|
||||
// pruneRestartPolicyCacheLocked drops cache entries for containers that no longer exist, so a
|
||||
// long-lived controller cannot accumulate entries for deleted apps. MUST hold mu for writing.
|
||||
func (m *Manager) pruneRestartPolicyCacheLocked(live map[string]bool) {
|
||||
for key := range m.restartPolicyCache {
|
||||
name := strings.TrimSuffix(key, "|policy")
|
||||
if !live[name] {
|
||||
delete(m.restartPolicyCache, key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// resolveContainerState determines the effective state by combining Docker's
|
||||
// State field (running/exited/etc.) with the Status field that contains health info.
|
||||
//
|
||||
@@ -545,9 +623,35 @@ func resolveContainerState(dockerState, dockerStatus string) ContainerState {
|
||||
}
|
||||
}
|
||||
|
||||
// restartPolicyLookup returns a container's docker restart policy name ("always", "unless-stopped",
|
||||
// "on-failure", "no"). An empty string means UNKNOWN — the inspect failed or no lookup was supplied.
|
||||
type restartPolicyLookup func(containerName string) string
|
||||
|
||||
// supervisedPolicy reports whether a restart policy means "docker is supposed to keep this container
|
||||
// running" — i.e. its being Exited is a fault, not a design.
|
||||
//
|
||||
// UNKNOWN ("") counts as supervised, deliberately fail-CLOSED, which is the opposite of the
|
||||
// IsDownState fail-open rule and for a different reason: there the input is an *ambiguous state*,
|
||||
// here we already KNOW a member is dead and only the excuse is missing. The P2 census (2026-07-21,
|
||||
// 53 catalog templates / 78 services) found **every** catalog service on `unless-stopped` and zero
|
||||
// one-shot init/migrate containers, so "unknown" in the field is an inspect failure on a container
|
||||
// that is almost certainly supervised. Missing a real dead-primary alarm is the failure that cost
|
||||
// 18 h; a false alarm is a banner.
|
||||
func supervisedPolicy(policy string) bool {
|
||||
switch policy {
|
||||
case "no", "on-failure":
|
||||
return false
|
||||
default: // "always", "unless-stopped", "" (unknown)
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
// aggregateState determines the overall stack state from its containers.
|
||||
// Priority: unhealthy/starting > restarting > all-running > stopped
|
||||
func aggregateState(containers []ContainerInfo) ContainerState {
|
||||
// Priority: unhealthy/starting > restarting > all-running > degraded > stopped
|
||||
//
|
||||
// policyOf is consulted ONLY for the mixed case (some members up, some down) and ONLY for the down
|
||||
// members — see the mix branch. nil is allowed (every exited member then reads as supervised).
|
||||
func aggregateState(containers []ContainerInfo, policyOf restartPolicyLookup) ContainerState {
|
||||
if len(containers) == 0 {
|
||||
return StateNotDeployed
|
||||
}
|
||||
@@ -557,6 +661,7 @@ func aggregateState(containers []ContainerInfo) ContainerState {
|
||||
unhealthy := 0
|
||||
restarting := 0
|
||||
stopped := 0
|
||||
var down []ContainerInfo
|
||||
|
||||
for _, c := range containers {
|
||||
switch c.State {
|
||||
@@ -570,6 +675,7 @@ func aggregateState(containers []ContainerInfo) ContainerState {
|
||||
restarting++
|
||||
case StateStopped, StateExited:
|
||||
stopped++
|
||||
down = append(down, c)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -595,8 +701,22 @@ func aggregateState(containers []ContainerInfo) ContainerState {
|
||||
if stopped == total {
|
||||
return StateStopped
|
||||
}
|
||||
// Mix (some running, some stopped) — report as running (partial)
|
||||
// Mix (some members up, some down) — R-51. Until v0.156.0 this reported StateRunning
|
||||
// unconditionally ("partial"), which is why a dead immich-server behind three live helpers was
|
||||
// invisible to fix-3 for 18 hours. A down member whose restart policy says docker should be
|
||||
// keeping it up is a FAULT → the whole stack is degraded (and degraded is a down state). A down
|
||||
// member with policy `no`/`on-failure` is a one-shot init/migrate container that has legitimately
|
||||
// finished → benign, the stack stays running.
|
||||
if running > 0 {
|
||||
for _, c := range down {
|
||||
policy := ""
|
||||
if policyOf != nil {
|
||||
policy = policyOf(c.Name)
|
||||
}
|
||||
if supervisedPolicy(policy) {
|
||||
return StateDegraded
|
||||
}
|
||||
}
|
||||
return StateRunning
|
||||
}
|
||||
|
||||
@@ -1043,6 +1163,13 @@ func (m *Manager) composeExecCustomEnv(dir string, env []string, args ...string)
|
||||
}
|
||||
|
||||
func (m *Manager) execCommand(name string, args ...string) (string, error) {
|
||||
// execFn is the process-boundary seam (nil in production). It exists so R-51 can be proven
|
||||
// through the REAL refreshStatusLocked path — docker ps → aggregateState → docker inspect —
|
||||
// rather than only through an injected aggregation helper, which would prove the helper and
|
||||
// not the caller (the v0.154.0 / v0.91.0 inert-seam class).
|
||||
if m.execFn != nil {
|
||||
return m.execFn(name, args...)
|
||||
}
|
||||
cmd := exec.Command(name, args...)
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
|
||||
Reference in New Issue
Block a user