Skip to content

fix: authoritative + bidirectional liveness reconcile for external docker changes (#1015, #1016) - #1017

Merged
milkway merged 1 commit into
mainfrom
fix/liveness-external-docker-1015-1016
Jul 20, 2026
Merged

fix: authoritative + bidirectional liveness reconcile for external docker changes (#1015, #1016)#1017
milkway merged 1 commit into
mainfrom
fix/liveness-external-docker-1015-1016

Conversation

@milkway

@milkway milkway commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What & why

Fixes #1015 and #1016 — two bugs with one root cause: the scaler's runtime
liveness reconcile was one-way and treated a container's absence from
list_managed_containers() as "not stopped".

Approach — authoritative, bidirectional reconcile

New backend contract in ruscker-core:

  • enum ReplicaLiveness { Running, Stopped, Missing, Unknown } + a
    ContainerBackend::replica_liveness(&[ReplicaLivenessQuery]) -> ReplicaLivenessReport
    that classifies the caller's known replicas and returns running,
    Ruscker-labelled containers for re-adoption. The default impl is fail-safe:
    it reports every replica Unknown and discovers nothing, so mocks / non-Docker
    backends can never trigger a false prune (preserves today's behavior).
  • ContainerBackend::wait_until_ready(&Replica) — fail-closed default; re-adoption
    reuses the same readiness check as spawn before publishing a replica Ready.

The key invariant — Missing (authoritative) vs Unknown:

  • Local backend: a successful listing means any known container id that's absent is
    authoritatively Missing. A whole-call daemon error → the pass is skipped (as
    before).
  • Multi-host: built on the existing HostBatch { reported, failed_hosts } /
    placement machinery. A replica whose owning host answered without it is
    Missing; a replica whose owning host failed/timed out (or an unplaced
    replica while any host is unreachable) is Unknown — never pruned, so a degraded
    fan-out can't forget a live container and spawn duplicates.

Scaler (tick):

  • Prunes Missing and Stopped-past-grace through one shared cleanup routine
    (cleanup_replica_state: registry + sessions + metrics-cache + in-flight gauge),
    which stop_one now also uses so those states can't drift apart. ReplicaDown
    alerts/logs name the reason (missing/removed vs restart timed out).
  • Models an external restart as a transient state: on first Stopped
    observation the replica is marked non-routable (mark_restarting) and held for a
    short grace window (EXTERNAL_RESTART_GRACE_SECS = 30). It keeps its registry
    slot, so min-replicas stays satisfied and no duplicate is spawned during a
    normal restart
    . If the same container returns Running it's re-adopted
    (after wait_until_ready); if the grace window elapses it's pruned + replaced.
  • Re-adoption respects max-replicas under the spawn-coalescer lock (a running
    orphan beyond the cap is removed rather than exceeding max or duplicating).

Out of scope (fast-follow)

Per the issues' "optional" sections, this PR is the reconcile core only. Not
included: the reactive proxy-path recovery (confirm backend state on an upstream
transport failure + GET/HEAD replay) and the Docker die/destroy/start event
listener. The transient 502 during an in-progress restart is acknowledged as
self-recovering by #1016; the periodic reconcile is the fallback either way.

Tests

  • core/docker unit: local absence-after-successful-listing → Missing;
    multi-host absence on a responding host → Missing, replica on a failed host →
    Unknown.
  • scaler unit: Missing prunes + drop_replica; Unknown keeps replica &
    sessions & blocks a duplicate spawn; Running → Stopped → Running on the same id
    readopts exactly once (one readiness check, zero spawns); Stopped past the grace
    window is pruned + replaced; restart within grace does not duplicate-spawn
    (locks the External docker restart can orphan a running container because runtime reconciliation is one-way #1016 race). Existing liveness_reconcile_prunes_only_stopped_containers
    stays green.
  • docker-it (real daemon): docker rm -fMissing + clean replacement (no
    phantom blocks/duplicates); docker restart → the same labelled container is
    rediscovered Running, passes readiness, and re-enters the pool with no duplicate.

Gate

cargo test · cargo clippy --all-targets --all-features -D warnings ·
./scripts/i18n-check.sh · cargo test -p ruscker-docker --features docker-it
(48 passed against local Docker 29.6.1) — all green.

🤖 Generated with Claude Code

…cker changes (#1015, #1016)

The scaler's runtime liveness reconcile was one-way and treated a
container's absence from list_managed_containers() as "not stopped", so:

- #1015 `docker rm -f` left the replica Ready forever → 502 upstream
  error, and the phantom kept counting toward max-replicas.
- #1016 `docker restart` raced the tick: a tick catching the container
  stopped pruned the replica, then the same container came back running
  and was never re-adopted (re-adoption only ran at startup) → orphan +
  possible duplicate past max-replicas.

Adds a ReplicaLiveness { Running, Stopped, Missing, Unknown } contract:
ContainerBackend::replica_liveness classifies known replicas with
per-host authority (Missing = owning host answered without it; Unknown =
host unreachable → never pruned) and returns running labelled containers
for re-adoption. Default impl is fail-safe (all Unknown); wait_until_ready
is fail-closed and reused from spawn before publishing a replica Ready.

The scaler now prunes Missing/Stopped-past-grace through one shared
cleanup routine (also used by stop_one), models an external restart as a
transient state held for a 30s grace window (keeps its registry slot so
min-replicas can't duplicate-spawn during a normal restart), re-adopts
the same container once it reports Running again, and enforces
max-replicas under the spawn-coalescer lock.

Out of scope (fast-follow): reactive proxy-path recovery and Docker event
listener — the periodic reconcile is the fallback either way.

Tests: core/docker unit (Missing vs Unknown), scaler unit (prune/keep,
restart readopt-once, grace no-duplicate-spawn, past-grace prune+replace),
and docker-it against a real daemon (rm -f → Missing + clean replacement;
restart → same container rediscovered, readiness-gated, no duplicate).

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

milkway commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Review (adversarial pass) — MERGE-OK

Reviewed the seven highest-risk areas directly against the code (an independent codex reviewer was queued but its account hit its usage cap, so this is a manual pass; CI is fully green).

  • Multi-host false-prune: classify_multihost_liveness returns Missing only when the owning host is in reported and the container id is absent. A host in failed_hosts, a host no longer connected, or an unplaced replica while any host failed → Unknown (never pruned). Cannot mis-prune a live container behind an unreachable daemon.
  • max-replicas: readopt_running_replica and spawn_one serialize on the same state.spawn_locks.entry(spec.id) mutex and both count replicas_of().len() over all states → coherent cap; a running orphan beyond max is removed rather than duplicated.
  • Grace window: count = snap.len() counts the Stopped (restarting) replica, so min-replicas doesn't duplicate-spawn during a normal restart (locked by restart_within_grace_does_not_duplicate_spawn). mark_restarting resetting started_at=now makes the replica "young", so the YOUNG_REPLICA_GRACE_SECS idle-tick exemption protects it from premature scale-down; readopt restores the real container-created time.
  • Lock order is always spawn_locks → replicas (same as spawn_one) — no inversion. wait_until_ready runs without the registry lock held, so a slow readiness wait can't stall the proxy hot path.
  • cleanup_replica_state (now also backing stop_one) only adds metrics/in-flight removal — strictly more cleanup, no behavior regression. inflight_gc reading the post-reconcile registry is correct. No unwraps on backend data in the tick path.

Accepted trade-off (non-blocking): a re-adoption's readiness wait holds the per-spec spawn lock for its duration (bounded by the readiness timeout), which correctly blocks a concurrent duplicate spawn.

Gate: cargo test, clippy -D warnings, i18n parity, and docker-it (48 passed against real Docker 29.6.1) all green.

@milkway
milkway marked this pull request as ready for review July 20, 2026 17:48
@milkway
milkway merged commit 6bf2b79 into main Jul 20, 2026
5 checks passed
@milkway
milkway deleted the fix/liveness-external-docker-1015-1016 branch July 20, 2026 17:48
milkway added a commit that referenced this pull request Jul 20, 2026
…slice B) (#1020)

The reactive proxy recovery (#1019) collapses the recovery window for a request
that hits a dead upstream, but a spec with no incoming traffic (or a min-replicas
warm pool) still waited up to a full ~10s scaler tick to notice an external
`docker rm -f` / `docker restart`. This subscribes to the Docker event stream so
the runtime reconciles within ~1s.

- core: `ContainerEvent`/`ContainerEventKind` + `ContainerEventStream` and a
  `ContainerBackend::container_events()` trait method. Default returns an empty
  stream — events are a latency optimization, never the source of truth, so a
  backend without support (mocks, future backends) just relies on the periodic
  reconcile.
- ruscker-docker: LocalDockerBackend streams bollard events filtered server-side
  to our containers' lifecycle actions (type=container, label=ruscker.replica_id,
  event in start/die/stop/kill/destroy/oom); the stream ends on a transport error
  so the consumer reconnects. MultiHostDockerBackend merges per-host streams; a
  host that can't open one is skipped (periodic reconcile covers it) and never
  fails the whole call.
- ruscker-admin: `scaler::reconcile_liveness_once` factors the #1017 reconcile
  pass so the events watcher and the periodic tick take the exact same
  authoritative, idempotent path. `spawn_event_watcher` consumes the stream,
  coalesces a burst (a restart is die+start; a rm is die+destroy) into one
  reconcile, is leader-gated like the tick, and reconnects with backoff. Wired
  into serve alongside the scaler; the periodic reconcile remains the fallback
  for anything missed during a reconnect gap.

The event is only a "reconcile now" nudge — the authoritative `replica_liveness`
pass decides the action — so a missed or duplicated event is always safe.

Tests: unit for the action->kind map and the EventMessage->ContainerEvent mapping
(non-container / unlabelled ignored); a scaler test that `reconcile_liveness_once`
prunes a Missing replica (the watcher's entry point); and a docker-it real smoke
that opens the stream and asserts Start on spawn + die/destroy on external removal
against a live daemon.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
milkway added a commit that referenced this pull request Jul 20, 2026
Self-healing when a managed container changes outside Ruscker:
authoritative + bidirectional liveness reconcile (#1017, closes #1015/#1016),
reactive proxy recovery on a confirmed-dead upstream (#1019), and a Docker
events watcher for near-instant reconcile (#1020, both close #1018).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto-recuperação falha quando container gerenciado é removido via Docker CLI (docker rm -f)

1 participant