fix(eng-prod): add haproxy init-addr fallback for unresolvable forward host [CLK-1515194] - #5
Closed
Phtes wants to merge 4 commits into
Closed
fix(eng-prod): add haproxy init-addr fallback for unresolvable forward host [CLK-1515194]#5Phtes wants to merge 4 commits into
Phtes wants to merge 4 commits into
Conversation
…ard host [CLK-1515194] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d host [CLK-1515194] entrypoint.09-forward.sh generates the haproxy server lines that forward ports to the storage host. This adds `init-addr last,libc,none` to that generated line. That three-method list is haproxy's documented never-fail-on-address-resolution recipe — the server state file, then libc, then `none` as the terminal fallback — and it ships here as defense in depth: it puts the intended address-resolution fallback on the server line explicitly rather than leaving it implicit. What it does on the config this script actually emits was measured, not predicted: the startup outcome is identical with and without the keyword. Every emitted server line already carries `resolvers res`, and a two-sided falsifier against unresolvable `.invalid` hosts accepts the generated config on both sides (exit 0, with only `[WARNING] ... could not resolve address ..., disabling server`), both foreground starts alive and bound. A resolvers-stripped control pair — a shape no current code path generates — is what discriminates: without the keyword, `[ALERT] ... Failed to initialize server(s) addr.` and exit 1; with it, exit 0. The keyword therefore takes effect only if a future path ever emits a server line without a resolvers section. Do not expect this to move the shard-discovery failure rate. The cause of those deaths is not established — the premise this change started from was falsified by its own verification step and is reported back as a finding on CLK-1515194; the sd-side amplifier (CLK-1515297) is what carries the CI outcome. The health-check clause `check inter 10s fall 6 rise 6` is unchanged byte for byte; the Payload B retune is deliberately held. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fier behavior [CLK-1515194] The spec's premise -- that an unresolvable $FORWARD_HOST makes haproxy refuse to start, and that init-addr last,libc,none repairs it -- was falsified by its own verification step 3 before the change shipped. Every server line the script emits already carries `resolvers res`, which downgrades a failed startup lookup to a warning rather than a fatal alert, so haproxy already bound its listeners and lived. Measured on haproxy:2.4 (2.4.36) against unresolvable .invalid hosts, the generated config is accepted identically with and without the keyword, and a foreground start stays alive on both sides; only a resolvers-stripped control pair discriminates. This rewrites the sections that asserted the disproved causal delta -- the opening rationale, the decision block's description, the entity block's "accepted behavioral consequence", and the honesty principle -- to state the measured truth: the keyword is defense in depth, inert on today's shipping shape, and effective only if a future path ever emits a server line without `resolvers res`. Verification step 3 now mandates the control pair alongside the two-sided falsifier and states the expected non-discrimination, so the acceptance text describes a measurement rather than predicting one that cannot occur. No code change: docker/ci-runner/root/entrypoint.09-forward.sh is untouched by this commit. The keyword ships as hygiene, deliberately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er measurement [CLK-1515194] Branch review found the spec's remaining justification self-undermining: on the only shape where init-addr last,libc,none is not inert -- a server line without resolvers res -- no resolvers section is attached to that server, so nothing re-resolves it and none converts a loud [ALERT]/exit-1 into a backend disabled for the process lifetime. That is fail-silent, not the recovery the spec promised. Corrections, all confined to the spec: - H1 title no longer asserts the falsified startup-behavior claim - opening premise is hedged as retired rather than stated indicatively - the escape-hatch rationale states the fail-fast/fail-silent trade honestly - the surviving "hard abort" counterfactual is removed from the entity block - decision and principle headers match the corrected body - step 2's harness records its one infidelity (line 18 port-stripping omitted) - line-number pins are hedged against future drift No code change. The hard hold 'check inter 10s fall 6 rise 6' is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
5c9c76a < |
2026-08-12 20:38 UTC | 0 | 0 | 0 |
Last scanned: 5c9c76a · 2026-08-12 20:38 UTC
|
| Rule | Resource | File |
|---|---|---|
Secrets passed via build-args or envs or copied secret files |
— | docker/ci-scaler/Dockerfile |
Secrets passed via build-args or envs or copied secret files |
— | docker/ci-runner/Dockerfile |
🔶 High · 3 findings
| Rule | Resource | File |
|---|---|---|
| Image user should not be 'root' | — | docker/ci-scaler/Dockerfile |
| Image user should not be 'root' | — | docker/ci-runner/Dockerfile |
| Image user should not be 'root' | — | docker/ci-storage/Dockerfile |
🟡 Medium · 2 findings
| Rule | Resource | File |
|---|---|---|
| 'RUN cd ...' to change directory | — | docker/ci-runner/Dockerfile |
| Port 22 exposed | — | docker/ci-storage/Dockerfile |
🟢 Low · 3 findings
| Rule | Resource | File |
|---|---|---|
| No HEALTHCHECK defined | — | docker/ci-storage/Dockerfile |
| No HEALTHCHECK defined | — | docker/ci-runner/Dockerfile |
| No HEALTHCHECK defined | — | docker/ci-scaler/Dockerfile |
View full analysis in Upwind Console →
Scan completed in 6s
Scan history (1 scan)
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
5c9c76a < |
2026-08-12 20:38 UTC | +10 | 0 | +10 |
Last scanned: 5c9c76a · 2026-08-12 20:38 UTC
Author
|
Closed unmerged per owner decision: no speculative changes. Measurement during review showed the keyword has no behavioral effect on any config shape this script emits (every generated server line carries |
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.
One line in
docker/ci-runner/root/entrypoint.09-forward.sh: the generated haproxy server line now carriesinit-addr last,libc,none, which is haproxy's documented recipe for never failing on address resolution — try the last known address, then libc, then start with no address at all rather than aborting. It is stated explicitly rather than left to whatever the default happens to be.Read the rest before assuming what that buys, because measurement took most of it away.
No measured behavioral benefit on any shape this script emits
Every server line this script generates already carries
resolvers res, and that clause alone keeps startup non-fatal when$FORWARD_HOSTdoes not resolve. Measured with a real haproxy binary against a deliberately unresolvable.invalidhost, on configs differing by the keyword alone:init-addr last,libc,noneresolvers reson the server line)[WARNING] ... could not resolve address ..., disabling server[ALERT] ... Failed to initialize server(s) addr.Only the last cell fires. The control pair is what makes the first pair interpretable, because it proves the check is sensitive rather than blunt, so the identical outcome on the shipping shape is a real non-difference and not a measurement that could not tell. The shipping pair was escalated past
haproxy -cto a foreground start (haproxy -f <cfg> -db) for the same reason, and both sides stay alive and bound.So the keyword is inert on the only shape this repo emits. On the one shape where it is not inert, a hypothetical server line without
resolvers res, it converts a startup abort into a permanently disabled backend. That trades fail-fast for fail-silent; it does not buy recovery.The ticket's premise is falsified
CLK-1515194's Payload A rests on the premise that haproxy aborts at startup if
$FORWARD_HOSTdoes not resolve. That is false on the shipping config shape, as row 2 above shows. It would be true of a resolvers-less server line, which this script has never emitted.The change ships anyway as explicit-fallback hygiene, per the requester's route decision: the intended resolution fallback written down instead of inherited. That, and nothing larger, is the claim being made.
Do not expect this to move the CI failure rate
No attribution to the shard-discovery failure rate is intended or supported by anything here. The runner death cause is still unestablished, and the Payload C runbook that would establish it is owed by its owner. There is no repaired restart path in this change either; measurement took that claim away too.
Open question for the human
Does
docs/specs/belong in this repo at all? This PR addsdocs/specs/2026-08-11-haproxy-init-addr.md, 283 lines, to a repo that is a mirror ofdimikot/ci-storage, which makes it a tracked divergence surface every future upstream sync has to carry. The alternative is to move the spec content onto the ticket and into this description and drop the file.This wants an explicit call rather than a default, so it is deliberately not resolved here.
CI expectations
Three self-hosted integration jobs (
ci-scaler-test,build-and-boot-containers,spawn-job-test) are known-red in this org: they needsecrets.CI_PATand runner infra that is not set up yet, as documented in CLAUDE.md. Pre-existing, and not caused by this branch. The signal to read is the lightweight jobs (ci-storage-tool-test,ci-storage-action-test) and the threepush-imagesmatrix builds.🤖 Generated with Claude Code