docs(adr): ADR-049 Amendment 6 — incumbent classification at boot - #2236
Closed
ohdearquant wants to merge 8 commits into
Closed
docs(adr): ADR-049 Amendment 6 — incumbent classification at boot#2236ohdearquant wants to merge 8 commits into
ohdearquant wants to merge 8 commits into
Conversation
Boot conflated two questions: whether a daemon is live, and whether it is one this process would choose to talk to. A probe predicate returning a bare boolean cannot carry that difference, so responses emitted by a demonstrably live daemon — a protocol-version mismatch, a config mismatch, a metrics reply — were routed to the same unlink-and-bind path as an absent endpoint. Removing a live daemon's rendezvous files does not stop it, and binding a second listener beside it violates the convergence requirement of exactly one live, identity-matching owner. The amendment separates liveness from acceptability, states that unlink-and-rebind is licensed by a negative answer to liveness alone, enumerates eleven incumbent states with a normative disposition and exit code for each, and replaces the boolean with a MayBind/Refuse disposition. Exit codes are normative because supervisors read them: configuration-class refusals exit 0, state-class refusals exit nonzero. Removal of a live incumbent becomes operator-elected via an explicit opt-in that signals, waits for observed process death, and refuses on timeout with the socket intact. Records two readings that are easy to invert: the daemon being disposable licenses replacement being safe rather than making boot perform it, and losing the daemon-side boot fence is legitimate rather than an error, so that state exits cleanly.
Review found the state table not exhaustive, and the gap is the one the amendment exists to close. A daemon built before the probe frame existed deserializes the frame, ignores the unknown probe field, dispatches the empty operation string, and returns an ordinary success or error response. That reply is well-formed, comes from a live process, and is not an acknowledgement, so it matched no row and an implementer had no disposition for it. The daemon source already documents this case; the table omitted it. Adds it as a catch-all state for any parseable non-acknowledgement, plus a separate state for a reply whose served config id is absent or unequal with no mismatch flag set, since an identity that cannot be established is not one that was refuted. During a rollout with two builds present this is the expected observation, not a corner case. Adds an explicit precedence rule, since one reply can satisfy several rows, and states that anything reaching the end of the response rows without matching is the catch-all and must never fall through to cleanup. States plainly that the amendment is normative and that current code does not satisfy it, so no rule here is read as an account of present behaviour, and marks the replacement opt-in as a capability that does not exist yet, leaving its command surface to the implementing change. Gives the transient no-socket-live-pid state an explicit justification for its clean exit, which the exit-class rule would otherwise contradict: the distinguishing invariant is who resolves the condition, not how long it lasts. There the resolver is the other, already-booting process, so restarting this one cannot help. Replaces "one test per state" with the four elements each state's test must declare — peer setup, probe input, rendezvous postcondition, and listener count — since listener count is the assertion that actually detects a double bind and nothing else substitutes for it. Names the mechanism for the two states that bind and for the state reachable only synthetically.
… matrix The state-6 explanation was factually wrong. It described a pre-probe daemon as deserializing the probe frame, ignoring the unknown field, dispatching the empty operation and returning an ordinary response. The pre-probe daemon declares protocol version 1 and its handler tests the frame's protocol version first, returning version_mismatch before the namespace check and before any dispatch, while the probing build declares version 4. That peer therefore produces state 2, and a fixture built from the old text would have exercised a peer that does not exist. The state now names a same-protocol peer that does not recognise probe_only, and says so explicitly. Classify refusals by who resolves them rather than by how long they last. The transience rule put the fork-and-bind case in the retryable class, which would drive a supervisor restart loop against a condition only the other process can end. Under the new rule a refusal exits 0 when an operator or another process is the resolver and nonzero when a retry of this process is. Exit codes are now distinct numeric values rather than the class word "nonzero", so a supervisor and an end-to-end test can separate silent, malformed, refused-with-live-pid, identity-unresolved and other-connect-error outcomes. Exit 1 is reserved so an unclassified crash cannot read as a classified refusal. Define acknowledgement shape syntactically and independently of identity, and replace first-match-by-row-number with an explicit evaluation order. Under the old rule the catch-all absorbed every identity-unresolved reply, because such a reply can also be described as not an acknowledgement, so the identity diagnosis could never be produced. Row numbers are now stable labels and the catch-all is evaluated last. Add a state for a namespace-mismatched reply. The probe predicate has always tested that flag, but the table had no row for it, so such a reply matched no state at all. Broaden the refused-connection cleanup state to cover an absent, empty or unparseable pid record. A refused connection on a bound socket path already establishes that nothing is listening, which is the liveness question cleanup turns on; the pid record corroborates it rather than carrying it. Previously socket-present plus refused plus no usable pid file satisfied no state. Require replacement to bind the pid to the socket before signalling. The classification proves something live serves the socket and what it speaks, never that the process named by the pid file is that something, so a stale or reused pid could be signalled on evidence about a different process. Replacement now requires the acknowledgement to carry the serving process's own pid and to match the recorded one, and is unavailable when it cannot. That field does not exist yet; adding it is part of implementing the section. Instantiate the four required test elements per state as a binding matrix giving each state its exact peer fixture, probe input, rendezvous postcondition, listener count and exit code, and split the same-process incumbency case into its own row, since it is the only branch where a live pid ends in binding and a test that asserts only a successful bind cannot tell it from a collapse into cleanup.
…tate The state table gained a namespace-mismatch row and listed it among the states from which an operator-elected replacement may proceed. That was wrong. ADR-096 Fork 1 removed the namespace reject: a conforming daemon serves a differently-namespaced frame under the frame's own namespace over the shared warm registry rather than refusing it, and no serve path sets the flag. The row survives only as a legacy observation from a peer built before that change, and an obsolete signal is the last thing that should authorise signalling another process. Replacement now proceeds only from the states where a live, acceptable daemon identity was positively read; the row refuses, names both namespaces, and leaves the rendezvous intact. Also: the no-socket/live-pid refusal must print the incumbent's command line beside its pid. That state exits 0 by design, so a supervisor that restarts only on unsuccessful exit will not retry it. A pid file whose pid has been reused by an unrelated process therefore wedges boot permanently while every rule above behaves correctly, because nothing in the sequence can ask whether the live pid is a daemon at all. The command line is the evidence that answers it.
…bent amendment Replacement no longer authorises SIGTERM on a responder-supplied pid. The previous text required the probe acknowledgement to carry the serving process's own `served_pid` and justified self-reporting as the portable choice. That was circular: the threat the step exists to answer is an unrelated process answering on the socket, and the remedy asked exactly that process to name itself, which a same-uid squatter satisfies with the recorded integer. The portability premise was also false here, since the tree already performs the platform-specific peer-credential lookup the text called unavailable. The binding evidence is now the kernel's answer for the peer of the probe connection, and replacement is unavailable where the platform exposes no peer pid. State 3's row required `served_config_id` to be present while precedence routed any frame carrying `config_mismatch` to it; the row now matches precedence, and the echoed identity is a diagnostic rather than a classification predicate. An unequal `daemon_protocol_version` now classifies as state 2 even when `version_mismatch` is unset. The two are independent response fields, so an older peer can answer ack-shaped with the current config and its own lower version. Precedence previously sent that frame to state 5, which called a refuted protocol identity an unresolved configuration identity and gave it a retry-resolved exit code that no retry can satisfy. State 13 moves from exit 5 to exit 0. Amendment 4 classifies non-`ENOENT`, non-`ECONNREFUSED` connect errors as unreachable and forbids retry, while the exit-class rule assigns nonzero only when a retry of this process is the resolver. A filesystem-policy denial is operator-resolved. Exit 5 is retired rather than reused so a supervisor keyed on the old value reads a retired code. The scripted matrix rows are declared abbreviated rather than literal. `namespace_mismatch` carries no serde default, so rows transmitted as written would fail to deserialize and classify as state 8, which is the state they exist to be distinguished from. State 14 is relabelled defensive rather than a live legacy observation. A peer old enough to set the flag is old enough to fail this build's version check first, and the pre-Fork-1 daemon evaluates protocol before namespace in one if/else chain, so it answers with `version_mismatch` and lands in state 2. Its no-unlink, no-replace disposition is unchanged.
…pid against the kernel peer pid Supersedes the previous commit's treatment of step 2, which removed `served_pid` outright on the grounds that a self-reported pid proves nothing. That was too strong for the case the section actually guards. The hazard is accidental pid reuse. A pid recycled onto an unrelated process is a process that does not speak this protocol, so it never produces an acknowledgement — it lands in state 6, 7, or 8, where replacement is already unavailable. The daemon that does answer reports its own true pid, which fails equality against a stale record. `served_pid` binds socket to process for that case, and the section now says so rather than leaving the reader to infer it. What it does not establish is resistance to a process that deliberately lies, and that limit is now stated instead of assumed away. A same-uid process able to squat the rendezvous is inside the trust boundary already, since the accept path admits peers by same-uid rather than same-principal, and could signal the daemon directly without involving this sequence. Where the platform exposes a kernel peer pid — `SO_PEERCRED` on Linux, `LOCAL_PEERPID` on macOS — replacement now requires it to equal both `served_pid` and the recorded pid, and refuses on any disagreement. That makes the self-report a cross-check wherever a cross-check exists, which is both platforms this daemon targets. The document says plainly that a platform exposing neither falls back to `served_pid` alone and that this is the weakest evidence in the section. Test obligations gain the cases that make the evidence load-bearing: a mismatched `served_pid` and an absent one, each asserting that no signal is sent rather than only that the call refused, and a kernel-peer-pid pair where agreement proceeds and disagreement does not. The disagreeing case is what proves the cross-check is read; without it the agreeing case passes whether or not the lookup is wired up.
The replacement sequence specified a capability that does not exist on any command surface, and it could not be made self-consistent inside this amendment: its safety depends on a kernel-authenticated peer pid, which the response frame does not carry and which is unavailable on one of the three platform arms this daemon compiles for. Replace the section with a one-paragraph out-of-scope note carrying three named open items for the ADR that will specify it: ownership evidence, the step-6 unlink race against a new owner, and platform reach. The corresponding test obligations go with it. Classification fixes in the same amendment: - State 3's disposition no longer forward-references replacement, and states its unlink rule directly. - Test-matrix row 12 expects 0 listeners. Its observation is a live pid with no socket, so nothing is bound; sibling row 9 already used 0 for the same shape. - Test-matrix row 6's fixture uses a JSON string result. The wire field is Option<String>, so a non-string value fails to deserialize and would land in row 8, the state row 6 exists to be distinguished from. - Rows 10 and 11 enumerate the same four pid-record cases as the shared "no usable pid record" predicate they both key on.
…the fixture recipe out Five review rounds of Amendment 6 kept finding defects, and they clustered in two places. Both are addressed by narrowing what this amendment claims. Contract fixes: - State 5 covered `served_config_id` absent OR unequal and gave both exit 6. The amendment's own class rule says the exit turns on who resolves the condition, and a peer that published a different identity has refuted this build positively: no restart of this process changes what it echoes. The two halves are now state 5 (absent, exit 6, a retry may resolve it) and state 15 (present and unequal, exit 0). This is the same distinction the precedence already drew for an unequal `daemon_protocol_version` two steps earlier. - The same-process `MayBind` branch was a conditional clause inside state 12's disposition, so an implementer following the exit enumeration had no named outcome for it while state 12's exit 0 was correct only for the foreign-pid half. It is now state 12s with its own row, named in the precedence and in the "no exit because it binds" sentence. Scope: - The per-state fixture recipe is removed and will be specified in a follow-up ADR. Instantiating peer setup, response frame, and listener count for fifteen states is a second normative surface with its own failure modes, and it is where the review findings concentrated. The classification contract can be read and implemented without it. Two constraints established here ride along as named inheritances: a fixture frame must both deserialize and pass the version check, which rules out the abbreviated form AND the every-field-at-zero form; and state 13's listener count is not observable, so it needs a seam or a named substitute rather than a waiver. The state table and the exit table were cross-checked mechanically after the change: 16 states, both directions, no disagreements.
Owner
Author
|
Superseded by #2240, which carries this amendment's final content unchanged. Closing so the change has a single home. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Amendment 6 to ADR-049. Documentation only; no code in this PR.
Problem
Daemon boot answers "may I clean up the rendezvous and bind?" using a probe
predicate that returns a bare boolean. A boolean cannot distinguish "nothing is
serving this socket" from "something is serving it that this client may not
use", so three responses emitted by a demonstrably live daemon — a
protocol-version mismatch, a config mismatch, and a metrics reply — take the
same unlink-and-bind path as an absent endpoint.
Unlinking a live daemon's socket and pid file does not stop it. The result is a
second listener bound beside a running owner, which contradicts the convergence
requirement already stated in Amendment 3: exactly one live, identity-matching
daemon owning the rendezvous.
What the amendment specifies
acceptability (is it a peer I can share it with?), and states that
unlink-and-rebind is licensed by a negative answer to liveness alone.
Only two of them permit binding. The other nine are refusals today collapsed
into cleanup.
MayBind/Refuse { pid, reason }disposition,since a single bit is what invites the collapse.
configured to restart only on unsuccessful exit treats exit 0 as a deliberate
stop. Configuration-class refusals exit 0; state-class refusals exit nonzero.
--replace-incumbentas the only path that may remove a liveincumbent: operator-elected, signal then wait for observed process death,
refuse on timeout with the socket intact, never an implicit
SIGKILL.socket still present, no second listener — rather than return values, since a
return-value-only test passes while the socket is unlinked under a live
process.
Two readings recorded because they are easy to invert
and respawning it is safe. That licenses replacement being safe; it does not
make replacement something boot performs unprompted.
daemon-side fence pick the sole owner, so losing that fence is legitimate
rather than an error, and that state exits cleanly.
Scope
One state in the table is reachable only defensively: the boot probe's request
frame does not set the metrics flag, and the daemon's metrics reply is gated on
that flag in the requesting frame, so boot cannot elicit a metrics-only reply.
It is retained and labelled as defensive, because a reply carrying metrics is
still unambiguous proof of life and misreading it as absence would be the same
defect.
Amendments 3, 4 and 5, the client-side recoverer lock, and the daemon-side boot
fence are unchanged. This amendment constrains only what boot may conclude from
a probe result and what it may do about it.