feat(catalog): storj health signal for satellite dial-back failure + DDNS guidance - #318
Merged
Conversation
…DDNS guidance The Aug 2026 incident: the node advertised a stale IP after a silent ISP re-provision, and satellites' dial-backs timed out ~40x/hour for days while the container looked perfectly healthy — container health is structurally blind to it, and the offline emails arrived days later. - health_signals on storj: the verbatim failure line the node logs when a satellite cannot dial it back, anchored on the mechanism (failed to dial storage node) so a refused connection matches the same way. The producer-state route already scans worker logs for declared signals, so this lights up the dashboard with the exact remedy: check ADDRESS against the CURRENT public IP, prefer a DDNS hostname, confirm 28967. - ADDRESS env description + storj guide now lead with the DDNS hostname and say plainly why a literal IP is a time bomb, including how to self-check reachability via the node dashboard API. - Tests pin the real catalog entry against the verbatim live log line, with negative controls: a successful ping (which shares the words 'ping satellite') and ordinary healthy log lines must not match.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughStorj deployment guidance now requires DDNS hostnames with ports. The Storj service defines a satellite dial-failure health signal. Tests cover failure, success, unrelated logs, and signal metadata. ChangesStorj reachability
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Aug 10, 2026
Merged
GeiserX
added a commit
that referenced
this pull request
Aug 10, 2026
… positives Round 2 from independent review: - getaddrinfo raises UnicodeError (a ValueError, NOT an OSError) for an IDNA-invalid label; uncaught it escaped to the route umbrella and silently zeroed the log-signal scan sharing the try block — the address check disabling the very detection (#318) it complements. Caught, and the call site got its own suppress guard so no future failure mode can shadow log signals again. Worker-supplied hostnames are shape-validated before the resolver ever sees them, and resolutions are memoized 60s so a blackholed resolver costs one executor thread per window. - Judge only RUNNING containers on the node the caller asked about: an exited container elsewhere in the fleet carries its last run's env, and judging it produced findings about the wrong machine. - Cross-family comparisons are silence: the egress detectors are dual-stack, so a v6 egress against a v4 literal (or a v6-only DDNS name against v4 egress) says nothing about staleness. Same-family filtering before the membership check. - Resolved IPs are redacted to public-only before being echoed: they originate from a worker-supplied name, and repeating a private answer would let a rogue worker read the hub's internal DNS view. - A dangling 'host:' colon is a typo, stripped instead of earning a confident NXDOMAIN about a name never looked up (bare v6 keeps its trailing colons). - advertised_address_env may never name a secret-flagged var: CI guard in the catalog tests plus a runtime backstop in the worker. - External (image-matched) containers now carry advertised_address too — running a storagenode BEFORE installing CashPilot is the common storj adoption path, and those nodes were blind spots. - Stale-egress caveat in both mismatch messages (worker egress readings are cached up to an hour; the first hour after an IP change can read stale and self-heals). Tests: resolver three-valued contract on the REAL function (NXDOMAIN, EAI_AGAIN, timeout, UnicodeError), family-guard decision rows, private-IP redaction, running/worker_id filtering, secret backstop, external-node coverage.
GeiserX
added a commit
that referenced
this pull request
Aug 10, 2026
…chine no longer has (#320) * feat(fleet): flag a dial-back service whose advertised address the machine no longer has The check that catches a stale advertised IP the day the ISP re-provisions it, instead of days later via the provider's offline emails. - advertised_address_env in the service schema: the ONE env var holding the address the network dials the service back at. The worker copies that single variable's value into heartbeat container entries — never any other env, which holds credentials — at the cost of one inspect call per declaring container. storj declares ADDRESS. - The hub compares it against that worker's detected egress IP in producer state: a stale public literal, a private literal, and a hostname resolving away from the egress are FAILING findings that outrank PRODUCING (storage and held components keep ticking while inbound work is dead). Undetected egress, transient DNS trouble, and unreported addresses are NO CLAIM — a wrong 'your address is stale' sends the operator to fix DNS that is fine. Only a definitive NXDOMAIN counts as a resolution verdict. - The mismatch reason names its assumption (inbound rides the machine's default egress) so a deliberate second-WAN forward can be read and dismissed rather than mistrusted. Tests: decision-table over the verdict (findings AND no-claim rows), the hub helper with an injectable resolver, worker-side extraction proving the declared var and ONLY the declared var leaves the container inspect, a catalog guard that every declared advertised_address_env names a real env var, and a control that undeclared services carry no key at all. * fix(fleet): harden the advertised-address check against its own false positives Round 2 from independent review: - getaddrinfo raises UnicodeError (a ValueError, NOT an OSError) for an IDNA-invalid label; uncaught it escaped to the route umbrella and silently zeroed the log-signal scan sharing the try block — the address check disabling the very detection (#318) it complements. Caught, and the call site got its own suppress guard so no future failure mode can shadow log signals again. Worker-supplied hostnames are shape-validated before the resolver ever sees them, and resolutions are memoized 60s so a blackholed resolver costs one executor thread per window. - Judge only RUNNING containers on the node the caller asked about: an exited container elsewhere in the fleet carries its last run's env, and judging it produced findings about the wrong machine. - Cross-family comparisons are silence: the egress detectors are dual-stack, so a v6 egress against a v4 literal (or a v6-only DDNS name against v4 egress) says nothing about staleness. Same-family filtering before the membership check. - Resolved IPs are redacted to public-only before being echoed: they originate from a worker-supplied name, and repeating a private answer would let a rogue worker read the hub's internal DNS view. - A dangling 'host:' colon is a typo, stripped instead of earning a confident NXDOMAIN about a name never looked up (bare v6 keeps its trailing colons). - advertised_address_env may never name a secret-flagged var: CI guard in the catalog tests plus a runtime backstop in the worker. - External (image-matched) containers now carry advertised_address too — running a storagenode BEFORE installing CashPilot is the common storj adoption path, and those nodes were blind spots. - Stale-egress caveat in both mismatch messages (worker egress readings are cached up to an hour; the first hour after an IP change can read stale and self-heals). Tests: resolver three-valued contract on the REAL function (NXDOMAIN, EAI_AGAIN, timeout, UnicodeError), family-guard decision rows, private-IP redaction, running/worker_id filtering, secret backstop, external-node coverage. * fix(fleet): bound the resolve cache, name the node in mismatch findings CodeRabbit round on the review commit: - _RESOLVE_CACHE keys are worker-supplied hostnames and nothing ever evicted them — steady memory growth for the hub's lifetime. Expired entries are purged on every write and the table is capped at 256, oldest-first. - The mismatch reason said 'this machine' without saying WHICH machine — a fleet finding nobody can act on. The worker's node name is prefixed into the reason. - The resolver tests keyed cache uniqueness on id(), which CPython reuses; fixed host + explicit cache clear instead.
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.
Why
When a storj node advertises a stale address (e.g. after a silent ISP IP re-provision), satellites dial a dead address and count the node offline — new data stops, egress income stalls, sustained offline leads to suspension. The container stays healthy and the dashboard stays green the whole time: container health is computed from restarts and crashes, and is structurally blind to reachability. In the motivating incident the failure line appeared ~40x/hour for days and the first human-visible signal was provider email.
What
health_signalsonservices/storage/storj.yml: matches the node relaying the satellite's own dial-back error, anchored on the mechanism (ping satellite.*failed to dial storage node) so timeout and connection-refused variants match alike. The existing producer-state machinery (/api/services/{slug}/producer-state) scans the last 200 worker log lines for declared signals, so this surfaces as FAILING with the exact remedy in the dashboard.ADDRESSenv description anddocs/guides/storj.mdnow lead with a DDNS hostname and explain why a literal IP strands the node, plus how to self-check with the node's/api/sno/(lastPinged,quicStatus).Testing
TestStorjDialBackSignalpins the real catalog entry against the verbatim live incident log line (not a paraphrase), plus the connection-refused variant.ping satellite succeededline (shares the trigger words) and ordinary healthy log lines must produce zero hits.ruff check+ruff format --checkclean.generate_readme_tables.py --checkandreferral_check.pypass locally.Summary by CodeRabbit
Documentation
Bug Fixes
Tests