Skip to content

Provide a supported way to clear recovery_pending without hand-editing node state #48

Description

@artemnikitin

Summary

recovery_pending has no supported escape. vm.Manager.Stop and
vm.Manager.Remove both refuse to act on a quarantined instance, and nothing
else clears the state. The agent logs

ERROR VM recovery requires operator intervention service=<svc> error=<reason>

but no such intervention exists: no CLI command, no API, and — until #46 — not
even a written procedure.

This is acceptance criterion 5 of #41, split out because every available
mechanism needs its own design decision rather than a line in a bug fix.
The rest of #41 is addressed in #46.

What #46 does and does not do

#46 fixes the race that caused most quarantines (identity recorded before the
launched process had exec'd) and makes the provable case self-healing: a
manifest that fails validation is re-proved from the live command line and its
identity repaired.

Two gaps remain:

  1. The repair only runs at recovery, which is spent once per agent process.
    So even the self-healing path requires an agent restart.
  2. A genuinely ambiguous quarantine is untouched. Real PID reuse, a corrupt
    manifest, or a live process with a missing or invalid socket still ends in
    recovery_pending with no way out.

#46 documents the manual procedure in docs/architecture/DESIGN.md — stop the
agent, remove <state_dir>/vms/<service>/, start it again. That is precisely
what #41's criterion was written to eliminate:

recovery_pending is clearable through a supported interface without
stopping the agent and deleting node state by hand
.

So the documentation is a stopgap, not a delivery.

Current behavior

  • internal/vm/manager.goStop refuses: refusing to stop service %s: process ownership is ambiguous
  • internal/vm/manager.goRemove refuses: refusing to remove service %s while recovery is pending
  • internal/vm/manager.goquarantine sets StateRecoveryPending on the in-memory instance and persists LastError to the manifest
  • internal/vm/recovery.go — the same state is set during recovery, and the "requires operator intervention" line is logged
  • internal/api/server.go — the agent API registers GET /status, /health, /healthz, /metrics only

Quarantine is not a startup-only condition. quarantine is reached from
Start's abandon path, from Recover, from three failure branches in Stop
(validation failed before stop, identity changed while stopping, identity
changed after SIGKILL), and from the adopted-VM monitor goroutine in
recovery.go. A long-running agent can therefore enter recovery_pending at
any point, which is exactly why "restart the agent" is an unsatisfying answer.

Every one of those writers mutates m.instances in memory. Nothing re-derives
instance state from disk while the agent runs.

Why this is not a one-line fix

Each obvious mechanism has a real objection:

  • A mutating agent API endpoint. internal/api/server.go is GET-only and
    has no authentication. The examples bind it on all interfaces over plain HTTP
    (api_listen_addr: ":8080" in examples/agent.yaml, agent-s3.yaml,
    agent-gcs.yaml). A POST .../clear-recovery there would be an
    unauthenticated remote way to make an agent forget that it cannot prove
    ownership of a possibly-live microVM. That needs an auth story first.
  • fireworkctl. It talks to the control plane, and the architecture is
    pull-based — there is no control-plane-to-agent push channel.
  • A firework-agent subcommand editing the state directory. It would not
    clear the running agent's in-memory recovery_pending, so disk and memory
    would desync and Stop/Remove would keep refusing.

Design options

A. Ride the existing pull channel

The agent already polls its rendered config.NodeConfig from the object store
(Agent.fetchAndMergestore.Fetch). A per-service "clear recovery"
generation or nonce carried on that struct needs no push channel and no new
agent endpoint, and it inherits the existing revision-metadata pattern
(DesiredRevision/PlacementRevision/RenderedRevision). The agent clears
quarantine for a service when the received generation exceeds the one it last
acted on.

Unlike the agent API, the control-plane operator API is authenticated —
internal/controlplane/visibility_server.go requires a Bearer token, and the
agent-facing registry enforces mTLS with node-identity checks. Setting the
field would therefore land on an already-authenticated surface. It is still a
new capability there: docs/configs/README.md describes that API as
read-only, so making it mutating is a deliberate scope change, not a free
one.

Cost: a control-plane API and fireworkctl verb to set the field, plus a
durable place in the rendered state for it.

B. Authenticated mutating agent endpoint

Smallest blast radius conceptually, but requires solving agent API
authentication and bind-address policy first — a larger change than the
feature itself.

C. Agent subcommand plus a supported restart

Make the manual procedure a real command (firework-agent clear-recovery <service>) that validates the target and refuses if the process may still be
alive. Honest about requiring a restart, but turns an undocumented rm -rf
into a checked operation.

Note that a bare re-run of recovery is not sufficient on its own: recovery is
what quarantined the instance, so re-running it against unchanged evidence
reaches the same conclusion.

Open questions

  • Should clearing be per service or per node?
  • What is the operator asserting when they clear? "I have confirmed the process
    is gone" and "adopt it anyway" are different and probably need different
    verbs.
  • Should clearing ever run without human confirmation — e.g. an age-based
    auto-clear once the recorded PID is provably gone? That case may be better
    solved by extending the Record VM ownership only after the launched process execs #46 repair path instead.
  • Does clearing need to be auditable in agent status or the event stream?

Acceptance criteria

  • An operator can clear recovery_pending for a named service through a
    supported, documented interface, without hand-editing node state.
  • The operation cannot silently strand a live microVM: either it proves the
    process is gone, or it requires an explicit operator assertion that is
    recorded.
  • In-memory and on-disk state agree after the operation, so Stop/Remove and
    the next reconcile behave normally.
  • The "requires operator intervention" log line names the actual intervention.
  • Reconciliation converges on the affected node after clearing, without an
    agent restart if the chosen design allows it.
  • docs/architecture/DESIGN.md replaces the manual rm -rf procedure with the
    supported one; the manual procedure stays only as a last-resort fallback.
  • Tests cover: clearing a quarantined service and reconciling successfully
    afterwards; refusing to clear when the process is still live and unprovable;
    and the state directory and in-memory map agreeing after the operation.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions