Repository navigation
fix(decdn_node): make the /metrics readiness gate advisory (no false-fail) - #26
Conversation
…fail) The decdn_node role hard-failed `make deploy` on the "Wait for the node metrics endpoint" task after ~60s, but a healthy decdn-node binds its loopback metrics/admin listeners only after its startup bootstrap completes (behind the PaymentChannel buyer bootstrap — an upstream startup-ordering issue in decdn/decdn), which can lag process start by many minutes. Result: healthy nodes failed the deploy every time, while the systemd unit was active and QUIC was serving. Make the probe advisory without hanging the deploy: - On TIMEOUT (connection-refused, status -1) the probe now WARNS via a conditional debug instead of failing the play. A non-200 *answer* (e.g. 500 from a broken /metrics handler) still fails loud — scoped failed_when keeps AGENTS.md's fail-loud posture for genuine faults. - The systemd service-state assert remains the liveness gate; comments, README, and the operator-facing warning are explicit that it is a backstop (a Type=simple unit reports `running` even when wedged), so a persistent timeout is not provably benign — verify out-of-band. - Bound the probe window via new role vars decdn_readiness_retries / decdn_readiness_delay (default ~60s), documented in defaults + README. - Add a `slow-readiness` molecule scenario: a marker-gated "late-bind" stub mode drives the timeout path and verifies the deploy succeeds with the unit running but /metrics unbound. Update the CI comment for the new scenario. Verified: ansible-lint (production) + pre-commit clean; molecule default and slow-readiness scenarios pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe node role now performs an advisory ChangesAdvisory readiness probe
Slow-readiness scenario
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Molecule
participant decdn_node
participant decdn-node-stub
participant systemd
Molecule->>decdn_node: Run converge with short readiness window
decdn_node->>systemd: Start decdn-node.service
systemd->>decdn-node-stub: Start stub with no-metrics marker
decdn_node->>decdn-node-stub: Probe /metrics
decdn-node-stub-->>decdn_node: Endpoint remains unbound
decdn_node->>systemd: Check service state
systemd-->>decdn_node: Service is running
Molecule->>decdn_node: Verify timeout path and listener state
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Molecule test scenario (slow-readiness) and updates the decdn_node Ansible role to make the /metrics readiness probe advisory on timeout. This prevents slow-starting nodes from failing the deployment while still ensuring that non-200 responses trigger a failure. The stub daemon was also updated to simulate late-binding behavior. The review feedback suggests skipping the readiness probe and its warning task during Ansible check mode to avoid unnecessary delays and false warnings. Additionally, it is recommended to gather service facts conditionally to make the role more robust and self-contained.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR updates the decdn_node Ansible role to avoid false deployment failures when /metrics is slow to come up on otherwise-healthy nodes by making the /metrics readiness probe advisory on timeout while still failing loudly on real non-200 responses. It also documents the behavior and adds a Molecule regression scenario to ensure the advisory-timeout path stays covered.
Changes:
- Make the
/metricsreadiness probe bounded + advisory on timeout, while failing on non-200 responses that indicate a real endpoint fault. - Add configurable probe window knobs (
decdn_readiness_retries,decdn_readiness_delay) and document readiness semantics in the role README. - Add a new Molecule
slow-readinessscenario plus stub support to validate the timeout-with-running-service behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ansible/roles/decdn_node/tasks/main.yml | Adjusts readiness logic: advisory timeout warning, scoped failed_when, keeps service-state backstop. |
| ansible/roles/decdn_node/README.md | Documents advisory readiness behavior, tuning knobs, and verification steps. |
| ansible/roles/decdn_node/defaults/main.yml | Introduces and documents default readiness probe window variables. |
| ansible/molecule/slow-readiness/molecule.yml | New Molecule scenario to exercise the advisory timeout path. |
| ansible/molecule/slow-readiness/prepare.yml | Stages marker + required placeholder files to force “no metrics bind” stub behavior. |
| ansible/molecule/slow-readiness/converge.yml | Converges role with a tiny readiness window to quickly hit timeout behavior. |
| ansible/molecule/slow-readiness/verify.yml | Verifies “unit running but no metrics socket” end state. |
| ansible/molecule/default/files/decdn-node-stub | Adds marker-gated “never bind metrics” mode to simulate late-bind behavior. |
| .github/workflows/molecule.yml | Updates comment to reflect the additional Molecule scenario run by --all. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ansible/roles/decdn_node/tasks/main.yml`:
- Around line 399-414: Update the readiness probe’s until condition for
decdn_metrics_probe so retries continue only while status is -1 (connection
refused or unavailable) and stop when any response status is received. Preserve
failed_when so status 200 succeeds, -1 remains advisory after exhaustion, and
other statuses fail immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 666e0ece-be63-4920-a45a-c4152c97e2e4
📒 Files selected for processing (9)
.github/workflows/molecule.ymlansible/molecule/default/files/decdn-node-stubansible/molecule/slow-readiness/converge.ymlansible/molecule/slow-readiness/molecule.ymlansible/molecule/slow-readiness/prepare.ymlansible/molecule/slow-readiness/verify.ymlansible/roles/decdn_node/README.mdansible/roles/decdn_node/defaults/main.ymlansible/roles/decdn_node/tasks/main.yml
Address PR review feedback on the advisory readiness probe: - Fail-fast on a real fault: change the probe's `until` to stop as soon as the endpoint ANSWERS (status != -1) instead of only on 200. A persistent 500 now surfaces on the first attempt via failed_when rather than after the whole retry window; connection-refused (-1) still retries through the window and stays advisory. (CodeRabbit) - Guard the probe + warn with `not ansible_check_mode`: uri skips in check mode, which left the registered status unset and fired the readiness warning spuriously on every `make check`. (Gemini) - Print the curl/journalctl verification hints on their own lines via a literal block scalar, and parenthesize the retries×delay math. (Copilot) Kept the unconditional `service_facts` gather (a conditional `when: ... is not defined` would risk asserting against stale, pre-start facts — the fresh gather is required for the running-state assert). Verified: fail-fast on 500 (attempt 1), advisory on refused, check-mode skips both tasks; ansible-lint + yamllint clean; molecule default and slow-readiness pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
make deploy(ansible-playbook playbooks/site.yml) hard-failed on the Wait for the node metrics endpoint task inroles/decdn_node/tasks/main.yml. The task waited ~60s (30×2s) forhttp://127.0.0.1:9090/metricsto return 200, then failed the play on timeout.But
decdn-nodebinds its loopback metrics/admin listeners only after its startup bootstrap completes — behind a retrying PaymentChannel buyer-sideapprove(an upstream startup-ordering issue indecdn/decdn, tracked there, not here). On a completely healthy node this can lag process start by many minutes. Observed live: serviceactive (running),NRestarts=0, QUIC udp/4433 listening, on-chainisActiveNode=true— yet tcp 9090/9191 not yet bound, so the 60s probe timed out with "Connection refused" and the deploy failed every time.Bumping retries to cover the worst case would hang every deploy for that whole window — unacceptable. So the probe becomes advisory instead.
What changed (Ansible role only)
uristatus-1) now warns via a conditionaldebugrather than failing the play (failed_whenis scoped, not blanket).failed_when: status not in [-1, 200], keeping AGENTS.md hard-rule 4 (fail loud).service_facts+state == 'running'assert stays as the backstop, but comments/README/warning now state plainly it is not a full health check (aType=simpleunit reportsrunningeven when wedged), so a persistent timeout warrants out-of-band verification (curl,journalctlfor"node runtime ready").decdn_readiness_retries(30) /decdn_readiness_delay(2), documented indefaults/main.ymland the role README's new Readiness section.slow-readinessscenario: a marker-gated "late-bind" stub mode (never binds/metrics, stays alive) drives the timeout path and verifies the deploy succeeds with the unitrunningbut 9090 unbound. Would catch a straight revert of the advisory behavior. CI comment updated for the third scenario.Scope note: the ~27-min late bind is a decdn core binary matter tracked separately in
decdn/decdn(metrics/admin should bind early, independent of the buyer bootstrap). This PR only makes the Ansible deploy robust against a slow-but-healthy startup.Testing
make lint-ansible— ansible-lint production profile, 0 failuresmake lint— all pre-commit hooks passmolecule test -s slow-readiness— converge survives the timeout, verify asserts running-but-no-metrics ✅molecule test -s default— ready/200 path unchanged ✅failed_when:-1=advisory,200=ready,500/404=fail-loudReview
Reviewed via
/pr-review-toolkit:review-pr(code, tests, comments, silent-failure). Applied the resulting fixes: scopedfailed_when, honest liveness-gate prose, CI-comment + prose-rot cleanup. Deferred the crash-loop negative-test scenario as a follow-up.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/metricsendpoint.Documentation