From a8e445bf5d1b1dabea9d03bbce0e5b8dedcf6b5a Mon Sep 17 00:00:00 2001 From: Ant Somers Date: Sun, 1 Nov 2026 20:07:46 +0300 Subject: [PATCH 1/2] fix: readiness waits for decdn_node_boot_ready, not a 200 decdn/decdn#2821 serves /metrics from process start, with decdn_node_boot_ready 0 until boot finishes, so a 200 no longer means boot is done. One rule everywhere: ready is a 200 with the gauge at 1, or a 200 with no gauge (older releases bind /metrics only after boot). - Role: the readiness probe retries a 200 that says 0, and its advisory warning names which state it ended in. - decdn-compose health: reports boot from the gauge (boot_state). - Chart: the default startupProbe runs `decdn node health` (the admin RPC starts after boot), so the 5-minute budget restarts a stuck boot again and the pod goes Ready only after boot; the render refuses admin_port 0 with that probe and any probe with two handlers. - Molecule: the stub boots with the gauge at 0 then 1 (stuck, legacy and no-metrics modes); slow-readiness covers a stuck boot and, in a side effect, a daemon that never binds; host-env covers a release without the gauge. Closes #204 Co-Authored-By: Claude Opus 5.5 --- ansible/Makefile | 2 +- ansible/README.md | 2 +- ansible/molecule/default/converge.yml | 13 ++++ .../molecule/default/files/decdn-node-stub | 67 +++++++++++----- ansible/molecule/host-env/converge.yml | 13 ++++ ansible/molecule/host-env/prepare.yml | 9 +++ ansible/molecule/slow-readiness/converge.yml | 49 +++++------- ansible/molecule/slow-readiness/molecule.yml | 43 ++++++++-- ansible/molecule/slow-readiness/prepare.yml | 16 ++-- .../molecule/slow-readiness/side_effect.yml | 47 +++++++++++ ansible/molecule/slow-readiness/verify.yml | 40 ++++------ ansible/roles/decdn_node/README.md | 38 +++++---- ansible/roles/decdn_node/defaults/main.yml | 27 +++---- ansible/roles/decdn_node/tasks/main.yml | 78 ++++++++++++------- charts/decdn-node/README.md | 29 +++++-- charts/decdn-node/templates/_helpers.tpl | 24 +++++- charts/decdn-node/tests/render-test.sh | 20 ++++- charts/decdn-node/values.yaml | 25 +++--- compose/README.md | 6 +- compose/decdn-compose | 28 ++++++- compose/tests/test_decdn_compose.py | 24 ++++++ 21 files changed, 432 insertions(+), 168 deletions(-) create mode 100644 ansible/molecule/slow-readiness/side_effect.yml diff --git a/ansible/Makefile b/ansible/Makefile index 3822d8f5..0ddea6bc 100644 --- a/ansible/Makefile +++ b/ansible/Makefile @@ -199,7 +199,7 @@ decommission: # validation-sponsord (bad knobs, every role, must be rejected by their own asserts; # split by role so they run side by side), generate-keystore (opt-in host-side # wallet), host-env (host-provisioned /etc/decdn/decdn.env), slow-readiness (advisory -# /metrics probe timeout), grafana-cloud and grafana-cloud-token (opt-in Grafana +# boot-readiness probe timeout), grafana-cloud and grafana-cloud-token (opt-in Grafana # Cloud observability, host-provisioned vs inventory token), os-matrix (default's # plays on Debian 13, Ubuntu 24.04 and 26.04), lifecycle (network profile, # backup + decommission), sponsord, sponsord-install and sponsord-wallet (the sponsord diff --git a/ansible/README.md b/ansible/README.md index 79259806..1f5f6317 100644 --- a/ansible/README.md +++ b/ansible/README.md @@ -472,7 +472,7 @@ make lint-alloy # render grafana_alloy's templates, then `alloy validate` th `validation-alloy` and `validation-sponsord` (bad knobs, for every role, must be rejected by their own asserts; one scenario per role group), `generate-keystore` (opt-in host-side wallet), `host-env` (host-provisioned `/etc/decdn/decdn.env`), -`slow-readiness` (advisory `/metrics` probe timeout), `grafana-cloud` (the opt-in +`slow-readiness` (advisory boot-readiness probe timeout), `grafana-cloud` (the opt-in observability wiring — see [Grafana Cloud observability](#grafana-cloud-observability-opt-in)), `grafana-cloud-token` (the same wiring with the API token carried through git-ignored inventory instead: role-authored env file + provenance record), diff --git a/ansible/molecule/default/converge.yml b/ansible/molecule/default/converge.yml index d2a11395..1d227ab5 100644 --- a/ansible/molecule/default/converge.yml +++ b/ansible/molecule/default/converge.yml @@ -137,3 +137,16 @@ - role: grafana_alloy tags: [observability, decdn_node, node] - role: decdn_node + post_tasks: + # The stub serves /metrics at once with decdn_node_boot_ready 0, then flips it + # to 1 (decdn-node-stub's BOOT_SECONDS): the probe must end on the 1, not on + # the first 200. + - name: Assert the readiness probe waited for decdn_node_boot_ready 1 + ansible.builtin.assert: + that: + - decdn_metrics_probe.status == 200 + - decdn_metrics_probe.content is search('(?m)^decdn_node_boot_ready 1$') + fail_msg: >- + The readiness probe ended on status + {{ decdn_metrics_probe.status | default('none') }} without + decdn_node_boot_ready 1: {{ decdn_metrics_probe.content | default('') }} diff --git a/ansible/molecule/default/files/decdn-node-stub b/ansible/molecule/default/files/decdn-node-stub index 6061ee3d..d090cc88 100755 --- a/ansible/molecule/default/files/decdn-node-stub +++ b/ansible/molecule/default/files/decdn-node-stub @@ -6,19 +6,24 @@ can be exercised end-to-end without a published release or a live chain: * `--version` -> print a version line and exit 0 (satisfies the role's install-time `--version` backstop / version-match check). - * `run ...` -> serve HTTP 200 on 127.0.0.1:9090/metrics (404 elsewhere) so - the role's `/metrics` readiness probe succeeds, and block - forever under systemd (Type=simple). CLI args (`--config`, - `--keystore-password-file`) are accepted and ignored. + * `run ...` -> serve HTTP 200 on 127.0.0.1:9090/metrics (404 elsewhere) and + block forever under systemd (Type=simple). Like a release + carrying decdn/decdn#2821, it binds at once and exports + `decdn_node_boot_ready 0` for BOOT_SECONDS, then `1`, so + the role's readiness probe has to wait for the flip. CLI + args (`--config`, `--keystore-password-file`) are accepted + and ignored. Under a unit with WatchdogSec it heartbeats WATCHDOG=1 to NOTIFY_SOCKET every WATCHDOG_USEC/2, as the real daemon does, so systemd does not kill it at the deadline. - If the marker file NO_METRICS_MARKER exists, it instead - blocks forever WITHOUT binding metrics — standing in for a - healthy node whose metrics listener binds late (behind the - startup PaymentPool bootstrap). The `slow-readiness` - scenario uses this to prove the role's advisory probe warns - (not fails) on timeout while the unit stays `running`. + The first line of BOOT_MODE_MARKER, when the file exists, + picks another boot: + stuck -> `decdn_node_boot_ready 0` forever (a boot + that never finishes; `slow-readiness`). + legacy -> no gauge at all, as v0.3.0 and v0.3.1, + which bind /metrics only after boot. + no-metrics -> never binds (a daemon that stays alive + but dies before its listener comes up). * `config validate ...` -> parse the rendered node.toml as TOML and exit 0. In the default scenario it also checks the exact literal DECDN_RPC_URL passed @@ -50,10 +55,12 @@ METRICS_ADDR = ("127.0.0.1", 9090) # matches the role's default metrics_bind/po # Matches decdn_node_version in the cloud-init scenario's mirror. Printed after the # name it runs as (decdn-node or decdn), as clap prints the real binaries' line. VERSION = "0.0.0-molecule-stub" -# When this file exists, `run` blocks WITHOUT binding metrics — simulating a -# healthy node whose metrics listener binds late. The slow-readiness scenario -# stages it (see that scenario's prepare.yml) to exercise the advisory-timeout path. -NO_METRICS_MARKER = "/etc/decdn/stub-no-metrics" +# How long a default boot reports `decdn_node_boot_ready 0` before it flips to 1. +BOOT_SECONDS = 3 +# Its first line picks a non-default boot (see the docstring); scenarios stage it +# in prepare.yml. +BOOT_MODE_MARKER = "/etc/decdn/stub-boot" +BOOT_MODES = ("stuck", "legacy", "no-metrics") # Forces `config validate` to exit 1, so a scenario can prove the role's config # gate still fails the deploy (the gate itself runs with failed_when: false). BAD_CONFIG_MARKER = "/etc/decdn/stub-bad-config" @@ -68,6 +75,9 @@ class _Handler(BaseHTTPRequestHandler): self.end_headers() return body = b"# stub decdn-node metrics\ndecdn_stub_up 1\n" + if self.server.boot_mode != "legacy": + ready = self.server.boot_mode != "stuck" and time.monotonic() >= self.server.ready_at + body += b"decdn_node_boot_ready %d\n" % ready self.send_response(200) self.send_header("Content-Type", "text/plain; version=0.0.4") self.send_header("Content-Length", str(len(body))) @@ -122,7 +132,7 @@ def config_validate(args): key-level drift; see its README. """ config = _opt(args, "--config") - # Forced-failure marker, mirroring NO_METRICS_MARKER. The role's config gate + # Forced-failure marker, like BOOT_MODE_MARKER. The role's config gate # deliberately runs with `failed_when: false` and defers to a following `fail` # task, so a regression in THAT task's `when:` would silently no-op the whole # gate. This lets a scenario prove the gate still fires. @@ -185,6 +195,20 @@ def _start_watchdog_heartbeat(): threading.Thread(target=beat, daemon=True).start() +def boot_mode(): + """The boot BOOT_MODE_MARKER asks for ("" = default), or None when it names no mode.""" + try: + with open(BOOT_MODE_MARKER, encoding="utf-8") as fh: + mode = fh.readline().strip() + except FileNotFoundError: + return "" + if mode not in BOOT_MODES: + print(f"stub: {BOOT_MODE_MARKER} names no boot mode: {mode!r} (want one of {BOOT_MODES})", + file=sys.stderr) + return None + return mode + + def main(): args = sys.argv[1:] if "--version" in args: @@ -196,17 +220,22 @@ def main(): return config_validate(args[2:]) if args and args[0] == "run": _start_watchdog_heartbeat() - # Late-bind simulation: stay alive (Type=simple => unit stays `running`) - # but never bind metrics, so the role's advisory probe times out. - if os.path.isfile(NO_METRICS_MARKER): + mode = boot_mode() + if mode is None: + return 1 + if mode == "no-metrics": + # Stay alive (Type=simple => unit stays `running`) but never bind. while True: time.sleep(3600) try: - HTTPServer(METRICS_ADDR, _Handler).serve_forever() + server = HTTPServer(METRICS_ADDR, _Handler) except OSError as exc: addr = f"{METRICS_ADDR[0]}:{METRICS_ADDR[1]}" print(f"stub: cannot bind {addr}: {exc}", file=sys.stderr) return 1 + server.boot_mode = mode + server.ready_at = time.monotonic() + BOOT_SECONDS + server.serve_forever() return 0 print(f"stub: unrecognised invocation: {args!r}", file=sys.stderr) return 2 diff --git a/ansible/molecule/host-env/converge.yml b/ansible/molecule/host-env/converge.yml index e580d6a5..798935ba 100644 --- a/ansible/molecule/host-env/converge.yml +++ b/ansible/molecule/host-env/converge.yml @@ -31,3 +31,16 @@ decdn_cache_origin_s3_use_default_chain: true roles: - role: decdn_node + post_tasks: + # prepare.yml boots the stub without the gauge: a 200 must count as ready on + # the first attempt, not be polled for a gauge that never comes. + - name: Assert the readiness probe took a gauge-less 200 as ready + ansible.builtin.assert: + that: + - decdn_metrics_probe.status == 200 + - decdn_metrics_probe.content is not search('(?m)^decdn_node_boot_ready ') + - decdn_metrics_probe.attempts | int == 1 + fail_msg: >- + Expected one probe attempt answering 200 without decdn_node_boot_ready; + got status {{ decdn_metrics_probe.status | default('none') }} after + {{ decdn_metrics_probe.attempts | default('?') }} attempt(s). diff --git a/ansible/molecule/host-env/prepare.yml b/ansible/molecule/host-env/prepare.yml index dc03af7e..751c7fa7 100644 --- a/ansible/molecule/host-env/prepare.yml +++ b/ansible/molecule/host-env/prepare.yml @@ -54,3 +54,12 @@ DECDN_RPC_URL=https://rpc.example.invalid/ DECDN_MOLECULE_HOST_LINE=written-by-the-operator mode: "0644" + + # The stub's `legacy` boot (decdn-node-stub's BOOT_MODE_MARKER): no + # decdn_node_boot_ready line, as v0.3.0 and v0.3.1, which bind /metrics only + # once boot has finished. converge.yml asserts the role takes that 200 as ready. + - name: Boot the stub daemon like a release without decdn_node_boot_ready + ansible.builtin.copy: + dest: "{{ decdn_etc }}/stub-boot" + content: "legacy\n" + mode: "0644" diff --git a/ansible/molecule/slow-readiness/converge.yml b/ansible/molecule/slow-readiness/converge.yml index 6cee1ccb..513b04fe 100644 --- a/ansible/molecule/slow-readiness/converge.yml +++ b/ansible/molecule/slow-readiness/converge.yml @@ -1,34 +1,27 @@ --- -# Converge the decdn_node role against the stub in late-bind mode with a tiny -# readiness window. The advisory /metrics probe MUST time out and WARN without -# failing — so this play reaching the end (and the idempotence re-run) is itself -# the core assertion of the fix. verify.yml then confirms the running-but-no- -# metrics end state. Chain/contract knobs mirror the default scenario's -# well-formed placeholders (the role's fail-loud asserts still run against them). +# Converge the decdn_node role against the stub in its `stuck` boot with a tiny +# readiness window (the role's variables are molecule.yml's inventory group_vars, +# shared with side_effect.yml). The advisory probe MUST keep polling the 200 that +# says decdn_node_boot_ready 0, then time out and WARN without failing — so this +# play reaching the end (and the idempotence re-run) is itself the core +# assertion; post_tasks prove the probe did not stop at the first 200. +# verify.yml then confirms the running-but-booting end state. - name: Converge hosts: all become: true - vars: - # Reuse the default scenario's stub binary rather than duplicating it. - stub_bin: "{{ lookup('ansible.builtin.env', 'MOLECULE_PROJECT_DIRECTORY') }}/molecule/default/files/decdn-node-stub" - decdn_node_install_method: manual - decdn_node_manual_bin_src: "{{ stub_bin }}" - decdn_cli_manual_bin_src: "{{ stub_bin }}" - decdn_node_version: "0.0.0-molecule-stub" - decdn_rpc_url: "https://rpc.example.invalid/" - decdn_chain_id: 421614 - decdn_region: "US" - decdn_payment_pool_address: "0x1111111111111111111111111111111111111111" - decdn_capacity_bond_address: "0x2222222222222222222222222222222222222222" - decdn_slash_judge_address: "0x3333333333333333333333333333333333333333" - # REQUIRED upstream (ADR 011/031) — the role asserts it with the other three. - decdn_content_blacklist_address: "0x4444444444444444444444444444444444444444" - decdn_cache_origin_kind: "http" - decdn_cache_origin_url: "https://origin.example.invalid/" - # Tiny probe window (~2s) so the advisory timeout is exercised fast. The stub - # never binds metrics in this scenario, so any window times out — this just - # keeps the scenario quick. - decdn_readiness_retries: 2 - decdn_readiness_delay: 1 roles: - role: decdn_node + post_tasks: + # A probe that took any 200 as ready would stop at its first attempt; one that + # reads the gauge retries until the window runs out on decdn_node_boot_ready 0. + - name: Assert the readiness probe kept polling a node that is still booting + ansible.builtin.assert: + that: + - decdn_metrics_probe.status == 200 + - decdn_metrics_probe.content is search('(?m)^decdn_node_boot_ready 0$') + - decdn_metrics_probe.attempts | int > 1 + fail_msg: >- + Expected the probe to retry a 200 carrying decdn_node_boot_ready 0 until + its window ran out; got status + {{ decdn_metrics_probe.status | default('none') }} after + {{ decdn_metrics_probe.attempts | default('?') }} attempt(s). diff --git a/ansible/molecule/slow-readiness/molecule.yml b/ansible/molecule/slow-readiness/molecule.yml index 5f8200ed..69570035 100644 --- a/ansible/molecule/slow-readiness/molecule.yml +++ b/ansible/molecule/slow-readiness/molecule.yml @@ -1,11 +1,13 @@ --- -# Regression scenario for the ADVISORY /metrics readiness probe. Runs the stub -# daemon in its "late bind" mode (it stays alive but never binds metrics — see -# molecule/default/files/decdn-node-stub, gated on /etc/decdn/stub-no-metrics), -# with a deliberately tiny readiness window. It proves the fix: the role's probe -# WARNS (does not fail) on timeout, the deploy still succeeds, and the hard -# service-state gate confirms the unit is `running`. Reuses the default -# scenario's stub binary rather than duplicating it. +# Regression scenario for the ADVISORY boot-readiness probe. Runs the stub daemon +# in its `stuck` boot (it serves /metrics with decdn_node_boot_ready 0 forever — +# see molecule/default/files/decdn-node-stub, BOOT_MODE_MARKER) with a +# deliberately tiny readiness window. It proves the role keeps polling a 200 that +# is still booting instead of taking it as ready, WARNS (does not fail) when the +# window runs out, and that the hard service-state gate confirms the unit is +# `running`. side_effect.yml re-runs the role against a daemon that never binds +# /metrics (a release without the gauge, booting), which must stay advisory too. +# Reuses the default scenario's stub binary rather than duplicating it. # # `baseline` is NOT exercised here (same rationale as the default scenario: # host-level hardening is real-host-only — see molecule/default/molecule.yml). @@ -37,6 +39,32 @@ provisioner: # round trip per remote task (measured at -22% on the `default` scenario, which # also carries the coverage note). Safe here: the containers have no sudo requiretty. ANSIBLE_PIPELINING: "true" + inventory: + group_vars: + # Inventory group_vars, not converge vars: side_effect.yml runs the role too. + # Chain/contract knobs mirror the default scenario's well-formed placeholders + # (the role's fail-loud asserts still run against them). + all: + # Reuse the default scenario's stub binary rather than duplicating it. + decdn_node_install_method: manual + decdn_node_manual_bin_src: "${MOLECULE_PROJECT_DIRECTORY}/molecule/default/files/decdn-node-stub" + decdn_cli_manual_bin_src: "${MOLECULE_PROJECT_DIRECTORY}/molecule/default/files/decdn-node-stub" + decdn_node_version: "0.0.0-molecule-stub" + decdn_rpc_url: "https://rpc.example.invalid/" + decdn_chain_id: 421614 + decdn_region: "US" + decdn_payment_pool_address: "0x1111111111111111111111111111111111111111" + decdn_capacity_bond_address: "0x2222222222222222222222222222222222222222" + decdn_slash_judge_address: "0x3333333333333333333333333333333333333333" + # REQUIRED upstream (ADR 011/031) — the role asserts it with the other three. + decdn_content_blacklist_address: "0x4444444444444444444444444444444444444444" + decdn_cache_origin_kind: "http" + decdn_cache_origin_url: "https://origin.example.invalid/" + # Tiny probe window (~2s) so the advisory timeout is exercised fast. The + # stub never finishes booting (or never binds) here, so any window times + # out — this just keeps the scenario quick. + decdn_readiness_retries: 2 + decdn_readiness_delay: 1 verifier: name: ansible scenario: @@ -48,5 +76,6 @@ scenario: - prepare - converge - idempotence + - side_effect - verify - destroy diff --git a/ansible/molecule/slow-readiness/prepare.yml b/ansible/molecule/slow-readiness/prepare.yml index a8abe3ab..3fb6bbe9 100644 --- a/ansible/molecule/slow-readiness/prepare.yml +++ b/ansible/molecule/slow-readiness/prepare.yml @@ -1,8 +1,8 @@ --- # Same operator-keystore staging as the default scenario (the role's pre-start # gate needs keystore + node.secret + password to exist), PLUS the marker that -# puts the stub daemon into "late bind" mode so it never binds /metrics — which -# is what makes the role's advisory readiness probe time out. +# puts the stub daemon into its `stuck` boot, which is what makes the role's +# advisory readiness probe time out. - name: Prepare hosts: all become: true @@ -38,11 +38,11 @@ content: "molecule-placeholder-node-secret\n" mode: "0644" - # The stub reads this marker (NO_METRICS_MARKER in decdn-node-stub) and, when - # present, blocks forever WITHOUT binding metrics — simulating a healthy node - # whose metrics listener binds late. That drives the advisory-timeout path. - - name: Put the stub daemon into late-bind (no-metrics) mode + # The stub reads this marker (BOOT_MODE_MARKER in decdn-node-stub) and, on + # `stuck`, serves /metrics with decdn_node_boot_ready 0 forever — a node whose + # boot never finishes. That drives the advisory-timeout path. + - name: Put the stub daemon into a boot that never finishes ansible.builtin.copy: - dest: "{{ decdn_etc }}/stub-no-metrics" - content: "1\n" + dest: "{{ decdn_etc }}/stub-boot" + content: "stuck\n" mode: "0644" diff --git a/ansible/molecule/slow-readiness/side_effect.yml b/ansible/molecule/slow-readiness/side_effect.yml new file mode 100644 index 00000000..5fabef97 --- /dev/null +++ b/ansible/molecule/slow-readiness/side_effect.yml @@ -0,0 +1,47 @@ +--- +# What converge cannot show: a daemon whose /metrics never answers, as a release +# without decdn_node_boot_ready looks while it boots (it binds only once boot has +# finished), or a daemon that dies before its listener comes up. The probe must +# keep that advisory too (status -1 warns, it does not fail). The host is left +# booting as converged (the `stuck` stub), for verify. +- name: Side effect — a daemon that never binds /metrics + hosts: all + become: true + tasks: + - name: Stop decdn-node + ansible.builtin.systemd_service: + name: decdn-node + state: stopped + + - name: Make the stub never bind /metrics + ansible.builtin.copy: + dest: /etc/decdn/stub-boot + content: "no-metrics\n" + mode: "0644" + + # The role starts the stopped unit, so its probe meets the new boot. + - name: Run the role against a daemon that never answers + ansible.builtin.include_role: + name: decdn_node + + - name: Assert the probe retried a refused connection and the deploy went on + ansible.builtin.assert: + that: + - decdn_metrics_probe.status | default(-1) == -1 + - decdn_metrics_probe.attempts | int > 1 + fail_msg: >- + Expected the probe to retry a /metrics that never answers until its + window ran out; got status + {{ decdn_metrics_probe.status | default('none') }} after + {{ decdn_metrics_probe.attempts | default('?') }} attempt(s). + + - name: Put the stub back into a boot that never finishes + ansible.builtin.copy: + dest: /etc/decdn/stub-boot + content: "stuck\n" + mode: "0644" + + - name: Restart decdn-node on the stuck boot, for verify + ansible.builtin.systemd_service: + name: decdn-node + state: restarted diff --git a/ansible/molecule/slow-readiness/verify.yml b/ansible/molecule/slow-readiness/verify.yml index b02a5dc0..5495c1ff 100644 --- a/ansible/molecule/slow-readiness/verify.yml +++ b/ansible/molecule/slow-readiness/verify.yml @@ -1,24 +1,20 @@ --- # The core assertion of this scenario is IMPLICIT: molecule only reaches `verify` # if `converge` (and the idempotence re-run) succeeded — i.e. the advisory -# /metrics probe timed out WITHOUT failing the deploy. This play nails down the -# rest of that end state: the unit is running (the hard service-state gate still -# passed) even though metrics never bound (proving the timeout path was actually -# taken, not silently short-circuited by a metrics socket that came up anyway). +# readiness probe timed out WITHOUT failing the deploy (converge's post_tasks +# prove it retried the booting 200). This play nails down the rest of that end +# state: the unit is running (the hard service-state gate still passed) and its +# /metrics still says decdn_node_boot_ready 0 (proving the timeout path was +# actually taken, not short-circuited by a boot that finished anyway). - name: Verify hosts: all become: true tasks: - - name: Confirm the stub really ran in late-bind (no-metrics) mode - ansible.builtin.stat: - path: /etc/decdn/stub-no-metrics - register: no_metrics_marker - - - name: Collect listening TCP sockets - ansible.builtin.command: - cmd: ss -ltn - register: listeners - changed_when: false + - name: Read the node's metrics + ansible.builtin.uri: + url: http://127.0.0.1:9090/metrics + return_content: true + register: node_metrics - name: Gather service facts ansible.builtin.service_facts: @@ -36,21 +32,15 @@ node.toml rendered observability.otlp_endpoint even though both decdn_grafana_cloud_enabled is false and decdn_otlp_endpoint is empty. - - name: Assert the deploy survived a metrics-probe timeout with the unit running + - name: Assert the deploy survived a readiness timeout with the unit running ansible.builtin.assert: that: - # The marker is present => the stub was in late-bind mode, so the probe - # genuinely timed out rather than passing against a bound socket. - - no_metrics_marker.stat.exists - # Metrics never bound — the timeout path is what we exercised. - - "'127.0.0.1:9090' not in listeners.stdout" - - "'0.0.0.0:9090' not in listeners.stdout" - - "'[::]:9090' not in listeners.stdout" - - "'*:9090' not in listeners.stdout" + # Still booting: the timeout path is what we exercised. + - node_metrics.content is search('(?m)^decdn_node_boot_ready 0$') # The load-bearing liveness gate still holds: the unit is running. - >- 'decdn-node.service' in ansible_facts.services and ansible_facts.services['decdn-node.service'].state == 'running' fail_msg: >- - Expected a running decdn-node with NO metrics socket (advisory-timeout - path). Sockets: {{ listeners.stdout }} + Expected a running decdn-node whose /metrics says decdn_node_boot_ready 0 + (advisory-timeout path). /metrics: {{ node_metrics.content }} diff --git a/ansible/roles/decdn_node/README.md b/ansible/roles/decdn_node/README.md index 64ba7b6a..1b24c521 100644 --- a/ansible/roles/decdn_node/README.md +++ b/ansible/roles/decdn_node/README.md @@ -30,8 +30,8 @@ Per the deCDN node-onboarding ADR (019), a node only serves paid traffic after - **Ansible (this role): Phase 1 host prep + Phase 3 startup** — install binaries, create the `decdn` user + dirs, render `node.toml` + a hardened unit, open the - public QUIC port, start the daemon, and run a best-effort `/metrics` readiness - probe (see [Readiness](#readiness)). + public QUIC port, start the daemon, and run a best-effort boot-readiness probe + on `/metrics` (see [Readiness](#readiness)). - **Operator (manual, NOT automated here):** Phase 1 **key material** (generate the node + eth keys) and Phase 2 **on-chain** (fund + stake the wallet, register the node). These are no longer raw contract calls — upstream shipped a guided @@ -613,20 +613,28 @@ it with `ps`. The section there covers the shared-host workaround and the upstre ## Readiness -After starting the daemon the role runs a bounded probe against -`http://127.0.0.1:/metrics`, then a hard assert that the systemd -unit is in the `running` state. +After starting the daemon the role polls +`http://127.0.0.1:/metrics` until the node reports boot-ready, then +asserts that the systemd unit is in the `running` state. + +Boot-ready means `/metrics` answers 200 and either says `decdn_node_boot_ready 1` +or has no `decdn_node_boot_ready` line at all. A release that carries +[decdn/decdn#2821](https://github.com/decdn/decdn/pull/2821) serves `/metrics` +from process start and holds the gauge at `0` through the RPC preflight and the +boot chain reads, so a 200 alone proves only that the listener is up. Older +releases export no gauge and bind `/metrics` only once boot has finished, so for +them a 200 is still enough. The probe is **advisory** on timeout: exhausting the window logs a warning but -does **not** fail the deploy. The daemon can bind its loopback metrics/admin -listeners well after process start on an otherwise-healthy node — they come up -behind the startup PaymentPool buyer bootstrap (an upstream startup-ordering -issue tracked in `decdn/decdn`, not here), which can take many minutes. A short -probe that hard-failed would therefore false-fail a healthy deploy, and -stretching it to cover the worst case would hang every deploy for that whole -window — neither is acceptable, so a timeout warns and moves on. A metrics -endpoint that *answers* with a non-200 error is treated differently: that is a -fault, not slow startup, so it **fails** the deploy loudly. +does **not** fail the deploy. A healthy node can take many minutes to boot (a slow +RPC, chain-read retries). A short probe that hard-failed would therefore +false-fail a healthy deploy, and stretching it to cover the worst case would hang +every deploy for that whole window, so a timeout warns and moves on. The warning +says whether the node was still booting (`decdn_node_boot_ready 0`) or never +answered. A boot that never finishes fires the `DecdnNodeBootStuck` alert +([`monitoring/decdn-node/`](../../../monitoring/decdn-node/)). A metrics endpoint +that *answers* with a non-200 error is treated differently: that is a fault, not +slow startup, so it **fails** the deploy loudly. The follow-on assert that the `decdn-node.service` unit is `running` (via `service_facts`) is a **backstop, not a full health check**. It catches a daemon @@ -639,7 +647,7 @@ with `decdn_readiness_retries` × `decdn_readiness_delay` (see has settled: ```bash -curl -s http://127.0.0.1:9090/metrics # loopback metrics (once bound) +curl -s http://127.0.0.1:9090/metrics | grep -E '^decdn_node_boot_ready ' # 1 once booted journalctl -u decdn-node -e # look for "node runtime ready" ``` diff --git a/ansible/roles/decdn_node/defaults/main.yml b/ansible/roles/decdn_node/defaults/main.yml index 4f20c478..04033a34 100644 --- a/ansible/roles/decdn_node/defaults/main.yml +++ b/ansible/roles/decdn_node/defaults/main.yml @@ -407,19 +407,20 @@ decdn_client_region_allowlist: [] decdn_self_update_check: false # --- Readiness probe (advisory) ----------------------------------------------- -# After start, the role probes http://127.0.0.1:/metrics as a -# best-effort readiness signal. A probe TIMEOUT only WARNS (it does NOT fail the -# deploy): the daemon can bind its metrics/admin listeners well after process -# start on an otherwise-healthy node (they come up behind the startup -# PaymentPool buyer bootstrap — an upstream startup-ordering issue tracked in -# decdn/decdn, not here), so a bounded probe would otherwise false-fail a healthy -# deploy. A metrics endpoint that ANSWERS with a non-200 error, however, fails -# loud — that's a fault, not slow startup. The systemd service-state assert -# backstops this but is not a full health check: a Type=simple unit reports -# `running` even for a wedged-but-alive daemon, so it catches an exited/failed or -# fast-crash-looping unit, not a hung one. These two knobs bound the probe window -# (retries × delay; ≈60s at the values below) — enough for a node that binds -# promptly, without hanging the play on a slow-but-healthy startup. +# After start, the role polls http://127.0.0.1:/metrics until the +# node reports boot-ready: a 200 with `decdn_node_boot_ready 1`, or a 200 with no +# such line (a release without the gauge binds /metrics only once boot has +# finished). A probe TIMEOUT only WARNS (it does NOT fail the deploy): a healthy +# node can take minutes to boot (a slow RPC, chain-read retries), so a bounded +# probe would otherwise false-fail a healthy deploy; the DecdnNodeBootStuck alert +# covers a boot that never finishes. A metrics endpoint that ANSWERS with a +# non-200 error, however, fails loud — that's a fault, not slow startup. The +# systemd service-state assert backstops this but is not a full health check: a +# Type=simple unit reports `running` even for a wedged-but-alive daemon, so it +# catches an exited/failed or fast-crash-looping unit, not a hung one. These two +# knobs bound the probe window (retries × delay; ≈60s at the values below) — +# enough for a node that boots promptly, without hanging the play on a +# slow-but-healthy boot. decdn_readiness_retries: 30 # probe attempts before giving up (advisory) decdn_readiness_delay: 2 # seconds between attempts diff --git a/ansible/roles/decdn_node/tasks/main.yml b/ansible/roles/decdn_node/tasks/main.yml index 0242d048..00a692b3 100644 --- a/ansible/roles/decdn_node/tasks/main.yml +++ b/ansible/roles/decdn_node/tasks/main.yml @@ -1800,17 +1800,21 @@ when: (decdn_env_post_stat.stat.checksum | default('')) | length > 0 # --- Readiness ---------------------------------------------------------------- -# ADVISORY probe: the daemon binds its loopback /metrics listener only after its -# startup bootstrap completes, which on a healthy node can lag process start by a -# long way (metrics/admin bind behind the PaymentPool buyer bootstrap — an -# upstream startup-ordering issue tracked in decdn/decdn, not here). So a probe -# TIMEOUT must NOT fail the deploy — but a metrics endpoint that ANSWERS with an -# error is a real fault, not slow startup, so that still should. failed_when below -# encodes exactly that (connection-refused advisory, non-200 fails loud). The -# service-state assert further down is a necessary backstop but NOT sufficient: -# the unit is Type=simple, so a wedged-but-alive or slow-crash-looping daemon can -# still report `running` (see that task) — a persistent timeout here is therefore -# not provably benign. Tune the window with +# ADVISORY probe for boot. A release carrying decdn/decdn#2821 binds its loopback +# /metrics listener before boot and exports `decdn_node_boot_ready`: 0 until the +# RPC preflight and the boot chain reads finish, then 1 (the "node runtime ready" +# banner). Older releases export no gauge and bind /metrics only once boot has +# finished. So the node is boot-ready when /metrics answers 200 and either says +# `decdn_node_boot_ready 1` or has no such line; a 200 alone is not enough. +# A boot can take many minutes on a healthy node (a slow RPC, chain-read +# retries), so a probe TIMEOUT must NOT fail the deploy; the warning below says +# which state it ended in, and the DecdnNodeBootStuck alert covers a boot that +# never finishes. A metrics endpoint that ANSWERS with an error is a real fault, +# not slow startup, so that still fails (failed_when). The service-state assert +# further down is a necessary backstop but NOT sufficient: the unit is +# Type=simple, so a wedged-but-alive or slow-crash-looping daemon can still report +# `running` (see that task) — a persistent timeout here is therefore not provably +# benign. Tune the window with # decdn_readiness_retries × decdn_readiness_delay (≈60s at the defaults). # KICS reports this task under "Communication Over HTTP" (MEDIUM). It is a false # positive and is left unsuppressed deliberately — KICS's inline ignore directives @@ -1821,46 +1825,62 @@ # to require. "Require the metrics listener to stay on loopback" above enforces that # the address cannot be moved off loopback. `make security` gates on --fail-on high, # so this does not fail the build. -- name: Wait for the node metrics endpoint (advisory) +- name: Wait for the node to finish booting (advisory) ansible.builtin.uri: url: "http://127.0.0.1:{{ decdn_metrics_port }}/metrics" status_code: 200 + return_content: true register: decdn_metrics_probe retries: "{{ decdn_readiness_retries }}" delay: "{{ decdn_readiness_delay }}" # default(-1): on connection-refused the uri module returns status -1 (and may - # omit the key). Retry only while the endpoint has NOT answered (status -1) — - # stop the moment it answers (any other status), so a real fault (e.g. a 500) - # surfaces immediately via failed_when rather than after the whole window. - until: decdn_metrics_probe.status | default(-1) != -1 + # omit the key). Retry while the endpoint has NOT answered (status -1) or answers + # 200 while still booting (`decdn_node_boot_ready 0`). Stop on a boot-ready 200, + # and on any other status, so a real fault (e.g. a 500) surfaces immediately via + # failed_when rather than after the whole window. + until: >- + decdn_metrics_probe.status | default(-1) != -1 + and decdn_metrics_probe.content | default('') is not search('(?m)^decdn_node_boot_ready 0$') changed_when: false - # Scope the suppression, don't blanket it: -1 (not bound yet) stays advisory and - # 200 is ready, but a bound endpoint answering any other status (e.g. a 500 from - # a broken /metrics handler) is a genuine fault — fail loud on it instead of - # masking it as slow startup (AGENTS.md hard-rule 4). + # Scope the suppression, don't blanket it: -1 (not bound yet) and a 200 that is + # still booting stay advisory, but a bound endpoint answering any other status + # (e.g. a 500 from a broken /metrics handler) is a genuine fault — fail loud on + # it instead of masking it as slow startup (AGENTS.md hard-rule 4). failed_when: decdn_metrics_probe.status | default(-1) not in [-1, 200] # `--check` never starts the service, so the probe would be meaningless (and # uri skips in check mode anyway, leaving no status for the warn below). when: not ansible_check_mode -- name: Warn if the metrics endpoint was not ready within the probe window +- name: Warn if the node did not finish booting within the probe window ansible.builtin.debug: # Literal block so the curl/journalctl hints print on their own lines, # copy-pasteable, rather than folded onto one. msg: |- - decdn-node /metrics did not answer 200 within + decdn-node did not report boot-ready within ~{{ (decdn_readiness_retries | int) * (decdn_readiness_delay | int) }}s ({{ decdn_readiness_retries }}×{{ decdn_readiness_delay }}), so readiness - could NOT be confirmed. This is OFTEN benign — a healthy node binds - metrics/admin late (behind the startup PaymentPool bootstrap) and logs - "node runtime ready" once bound — but a PERSISTENT timeout can also mean a - wedged or mis-started daemon (a Type=simple unit still reports `running` - even when wedged). Verify before trusting the node: - curl -s http://127.0.0.1:{{ decdn_metrics_port }}/metrics + could NOT be confirmed. + {% if decdn_metrics_probe.status | default(-1) == 200 %} + /metrics answers with `decdn_node_boot_ready 0`: the node is still in its + RPC preflight or boot chain reads. A slow RPC can make this take minutes; + the DecdnNodeBootStuck alert fires if it never finishes. A rising + decdn_chain_boot_read_retries_total means the chain reads are retrying; + otherwise read the first WARN or ERROR after "deCDN node starting". + {% else %} + /metrics never answered. A release without `decdn_node_boot_ready` binds it + only once boot has finished, so this is OFTEN benign there; on a newer + release, or PERSISTENTLY, it means a daemon that died or wedged before its + listener came up (a Type=simple unit still reports `running` even when + wedged). + {% endif %} + Verify before trusting the node: + curl -s http://127.0.0.1:{{ decdn_metrics_port }}/metrics | grep -E '^decdn_node_boot_ready ' journalctl -u decdn-node -e (look for "node runtime ready") when: - not ansible_check_mode - - decdn_metrics_probe.status | default(-1) != 200 + - >- + decdn_metrics_probe.status | default(-1) != 200 + or decdn_metrics_probe.content | default('') is search('(?m)^decdn_node_boot_ready 0$') # A stale process can still answer /metrics, so also confirm THIS unit exists and # is running. This is a backstop, not a full health check: it catches a daemon diff --git a/charts/decdn-node/README.md b/charts/decdn-node/README.md index b521691f..3ff7c1f5 100644 --- a/charts/decdn-node/README.md +++ b/charts/decdn-node/README.md @@ -429,15 +429,32 @@ metrics: ## Probes and lifecycle -- `/metrics` is the only HTTP route suitable for probes; there is no `/health` endpoint. - It binds only after startup completes: RPC preflight, identity and keystore load, cache - and endpoint build, then the chain bring-up (CapacityBond registry and slash-watcher - enumerations, blacklist and DHT bootstrap). - - The default `startupProbe` allows 5 minutes (5s × 60). A slow RPC delays readiness. +- **The startup probe asks the admin RPC.** The default `startupProbe` runs + `decdn node health` (the CLI the image ships, as the Compose healthcheck does). The + admin RPC starts only once boot has finished: RPC preflight, identity and keystore + load, cache and endpoint build, then the chain bring-up (CapacityBond registry and + slash-watcher enumerations, blacklist and DHT bootstrap). + - The default `startupProbe` allows 5 minutes (5s × 60), then restarts the pod. A + slow RPC delays readiness. + - It needs the admin RPC: the render fails when `config.observability.admin_port` is + `0` and the default `startupProbe` is in use. Replace its handler with + `startupProbe.exec: null` plus an `httpGet` on `/metrics`, which then proves only + that the listener is up. - A failed bring-up, such as an unreachable RPC, exits the container and Kubernetes restarts it. +- **Readiness and liveness read `/metrics`.** There is no `/health` HTTP route. Kubernetes + runs both only after the startup probe passes. From the decdn release that adds + `decdn_node_boot_ready`, `/metrics` answers from process start, with the gauge at `0` + until boot finishes, so an `httpGet` on it proves only that the listener is up. The + `DecdnNodeBootStuck` alert ([`monitoring/decdn-node/`](https://github.com/decdn/devops/blob/main/monitoring/decdn-node/prometheus-alerts.yml)) + reads the gauge and fires on a boot that never finishes. + - **Overriding a probe's handler.** Helm merges your values into the defaults, so set + the default handler to `null` (for example `startupProbe.exec: null`); a probe with + two handlers fails the render. An older values file that copies the earlier + `httpGet` startupProbe therefore fails until it nulls `exec`. Once it does, its pod + can go Ready while the node is still booting. - **Ready is not the same as serving.** The paid-delivery QUIC listeners open only after - the first ContentBlacklist sync, which comes after `/metrics` binds. Check the logs + the first ContentBlacklist sync, which comes after boot finishes. Check the logs before counting on traffic. - **SIGTERM runs a graceful drain.** `terminationGracePeriodSeconds` defaults to 300, the role's `TimeoutStopSec`. Killing a pod mid-drain loses paid deliveries, so do not diff --git a/charts/decdn-node/templates/_helpers.tpl b/charts/decdn-node/templates/_helpers.tpl index 00b45f6e..88e95888 100644 --- a/charts/decdn-node/templates/_helpers.tpl +++ b/charts/decdn-node/templates/_helpers.tpl @@ -155,7 +155,11 @@ than let a values override win: removes one of them fails too: no seccomp profile at either level means Unconfined (the kubelet's default unless it runs with seccompDefault), and no runAsGroup means gid 0 for a uid the - image has no passwd entry for. + image has no passwd entry for; +- a probe with more than one handler (Helm merges an override's `httpGet` into + the default `exec`, which the API server rejects: null the default's), or a + startupProbe running `decdn node health` with the admin RPC off + (config.observability.admin_port 0), which could never pass. --------------------------------------------------------------------------- */}} {{- define "decdn-node.validatePod" -}} {{- $fullname := include "decdn-node.fullname" . }} @@ -238,6 +242,24 @@ than let a values override win: {{- if get $caps "add" }} {{- fail "securityContext.capabilities.add must be empty: the node needs no Linux capabilities" }} {{- end }} +{{- range $name := list "startupProbe" "readinessProbe" "livenessProbe" }} +{{- $probe := default (dict) (get $.Values $name) }} +{{- $handlers := list }} +{{- range $h := list "exec" "httpGet" "tcpSocket" "grpc" }} +{{- if get $probe $h }} +{{- $handlers = append $handlers $h }} +{{- end }} +{{- end }} +{{- if gt (len $handlers) 1 }} +{{- fail (printf "%s has %d handlers (%s): set the others to null, e.g. %s.exec: null (Helm merges an override into the default)" $name (len $handlers) (join ", " $handlers) $name) }} +{{- end }} +{{- end }} +{{- $startCmd := dig "exec" "command" (list) (default (dict) .Values.startupProbe) }} +{{- /* A non-number admin_port is decdn-node.config's to refuse, with its own message. */}} +{{- $adminPort := dig "observability" "admin_port" 9191 (default (dict) .Values.config) }} +{{- if and (has "node" $startCmd) (has "health" $startCmd) (has (kindOf $adminPort) (list "float64" "int64" "int")) (eq (float64 $adminPort) 0.0) }} +{{- fail "startupProbe runs `decdn node health`, which needs the admin RPC, but config.observability.admin_port is 0: enable the admin RPC, or replace the startupProbe (startupProbe.exec: null plus an httpGet on /metrics, which then proves only that the listener is up)" }} +{{- end }} {{- end }} {{/* --------------------------------------------------------------------------- diff --git a/charts/decdn-node/tests/render-test.sh b/charts/decdn-node/tests/render-test.sh index 2becb235..33f7cb83 100755 --- a/charts/decdn-node/tests/render-test.sh +++ b/charts/decdn-node/tests/render-test.sh @@ -433,7 +433,15 @@ check(cfg["blockchain"]["eth_keystore"] == cfg["identity"]["data_dir"] + "/keyst check(cfg["observability"]["metrics_bind"] == "0.0.0.0", "metrics_bind") check(ports["quic"]["containerPort"] == quic_port and ports["quic"]["protocol"] == "UDP", "quic containerPort") check(ports["metrics"]["containerPort"] == metrics_port, "metrics containerPort") -for probe in ("startupProbe", "readinessProbe", "livenessProbe"): +# startupProbe asks the admin RPC, which starts only once boot has finished; +# /metrics answers during boot (decdn_node_boot_ready 0), so it gates nothing alone. +check(main["startupProbe"].get("exec", {}).get("command", [])[:3] == ["decdn", "node", "health"] + and "httpGet" not in main["startupProbe"], "startupProbe must run `decdn node health`") +cmd = main["startupProbe"]["exec"]["command"] +check("--config" in cmd and cmd[cmd.index("--config") + 1] == main["args"][main["args"].index("--config") + 1], + "startupProbe reads another node.toml than the daemon") +check(main["startupProbe"]["timeoutSeconds"] >= 5, "startupProbe timeout below the CLI's --timeout-ms") +for probe in ("readinessProbe", "livenessProbe"): check(main[probe]["httpGet"]["port"] == "metrics", f"{probe} port") msvc = one("Service", "metrics") @@ -643,6 +651,12 @@ expect_fail "ServiceMonitor, policy blocks" template 'metrics.networkPolicy.from expect_fail "non-string rule label" schema '/metrics/prometheusRule/ruleLabels' --set-json 'metrics.prometheusRule.ruleLabels={"team":1}' expect_fail "invalid dashboard label key" schema '/metrics/grafanaDashboards/label' --set 'metrics.grafanaDashboards.label=not a label' expect_fail "unknown monitoring knob" schema "additional propert(y|ies) 'rules'" --set metrics.prometheusRule.rules=x +# The admin RPC off leaves `decdn node health` nothing to ask; an override replaces +# the handler only once the default's is nulled. +http_startup=(--set-json 'startupProbe={"exec":null,"httpGet":{"path":"/metrics","port":"metrics"}}') +expect_fail "startupProbe, admin RPC off" template 'startupProbe runs .decdn node health.' --set config.observability.admin_port=0 --set metrics.port=9191 +expect_fail "startupProbe, two handlers" template 'startupProbe has 2 handlers \(exec, httpGet\)' --set-json 'startupProbe.httpGet={"path":"/metrics","port":"metrics"}' +expect_fail "livenessProbe, two handlers" template 'livenessProbe has 2 handlers' --set-json 'livenessProbe.tcpSocket={"port":"metrics"}' # --- edge cases that must RENDER --------------------------------------------------- expect_render() { # @@ -651,7 +665,7 @@ expect_render() { # || { cat "$work/render.err" >&2; fail "render should have passed: $desc"; } pass "renders: $desc" } -expect_render "admin_port 0 frees 9191" --set config.observability.admin_port=0 --set metrics.port=9191 +expect_render "admin_port 0 frees 9191" --set config.observability.admin_port=0 --set metrics.port=9191 "${http_startup[@]}" expect_render "admin_port 65535" --set config.observability.admin_port=65535 expect_render "admin_port as a whole float" --set-json 'config.observability.admin_port=9192.0' expect_render "fs origin at the mount root" --set-json 'origin.volume={"emptyDir":{}}' --set config.cache.origin.kind=fs --set config.cache.origin.path=/srv/decdn-origin @@ -745,7 +759,7 @@ n="$(notes_of -f "$base" --set service.enabled=false)" || fail "notes render (Se if ! { has 'the QUIC Service is disabled' && has 'service.enabled=true'; }; then fail "notes: Service-disabled node not told to enable it" fi -n="$(notes_of -f "$base" --set config.observability.admin_port=0 --set metrics.port=9191)" || fail "notes render (admin off)" +n="$(notes_of -f "$base" --set config.observability.admin_port=0 --set metrics.port=9191 "${http_startup[@]}")" || fail "notes render (admin off)" if ! { has 'admin RPC is disabled' && ! has 'port-forward'; }; then fail "notes: admin_port 0"; fi pass "install notes: multiaddrs per exposure, hostPort warning, no-address note, admin RPC" diff --git a/charts/decdn-node/values.yaml b/charts/decdn-node/values.yaml index cf2fbafa..b4816dae 100644 --- a/charts/decdn-node/values.yaml +++ b/charts/decdn-node/values.yaml @@ -213,16 +213,23 @@ config: # SIGTERM runs a graceful drain; killing mid-drain loses paid deliveries. terminationGracePeriodSeconds: 300 -# /metrics is the only probe target: there is no /health route and the admin RPC -# is loopback-only. It binds only after startup completes — RPC preflight, key -# load, cache/endpoint build, then the chain bring-up (CapacityBond + slash-watcher -# enumerations, blacklist and DHT bootstrap) — so startup must cover a slow RPC; a -# failed bring-up exits the container and restarts it. Ready != serving: paid QUIC -# listeners open only after the first ContentBlacklist sync. +# startupProbe asks the admin RPC (`decdn node health`, the CLI the image ships, as +# Compose's healthcheck does). The admin RPC starts only once boot has finished — +# RPC preflight, key load, cache/endpoint build, then the chain bring-up +# (CapacityBond + slash-watcher enumerations, blacklist and DHT bootstrap) — so the +# 5-minute budget below restarts a boot that never finishes, and must cover a slow +# RPC. It needs config.observability.admin_port (default 9191) on. readiness and +# liveness run only after it passes, on /metrics: from the decdn release that adds +# decdn_node_boot_ready, /metrics answers from process start, so an httpGet on it +# proves only that the listener is up (the DecdnNodeBootStuck alert reads the +# gauge). A failed bring-up exits the container and restarts it. Ready != serving: +# paid QUIC listeners open only after the first ContentBlacklist sync. +# To replace a probe's handler, null the default's (startupProbe.exec: null): +# Helm merges an override into these defaults, and two handlers fail the render. startupProbe: - httpGet: - path: /metrics - port: metrics + exec: + command: [decdn, node, health, --config, /etc/decdn/node.toml, --timeout-ms, "4000"] + timeoutSeconds: 5 periodSeconds: 5 failureThreshold: 60 readinessProbe: diff --git a/compose/README.md b/compose/README.md index 61cb4f94..6336de5c 100644 --- a/compose/README.md +++ b/compose/README.md @@ -56,7 +56,7 @@ against [`compose.yaml`](compose.yaml), plus `compose.override.yaml` when you ha | `check` | the preflight `up` also runs: every file present, owned and moded as the services need, the settings filled in, `decdn config validate` against the pinned image, the relay's and DNS server's configs, and no other process on the ports the stopped services bind | | `up [svc…]` | `check`, then `docker compose up -d` | | `stop`, `restart`, `down` | `docker compose …`, but refused while sponsord holds an unconfirmed pool top-up ([below](#the-top-up-hold)) | -| `health` | probes every running service: the node's admin RPC and metrics, each `/healthz`, sponsord's hold, the relay's metrics and certificate, the DNS server's version, SOA answers and certificate, Alloy's readiness and components | +| `health` | probes every running service: the node's admin RPC and boot (`decdn_node_boot_ready`), each `/healthz`, sponsord's hold, the relay's metrics and certificate, the DNS server's version, SOA answers and certificate, Alloy's readiness and components | | `cli ` | the `decdn` CLI from the node image, with the node's config and keys and your RPC endpoint from `decdn.env`: `cli whoami`, `cli node status`, `cli setup` | | `backup -r ` | an age-encrypted archive of the active profiles' keys and secrets | | `config` | `docker compose config`, with every value that came from an env file shown as `` (the plain command prints `DECDN_RPC_URL` and the rest in clear) | @@ -149,6 +149,10 @@ anything past your firewall. The node's only public port is QUIC **udp/4433**. **Health.** The container's healthcheck is `decdn node health` against the admin RPC, so `docker compose ps` shows a wedged runtime as unhealthy, not just a live process. +`decdn-compose health` also reads `decdn_node_boot_ready` from the node's loopback +`/metrics`: from the release that adds it, `/metrics` answers while the node is still +booting, so a 200 alone proves only the listener. A release without the gauge binds +`/metrics` only once boot has finished. Dashboards and alert rules are in [`monitoring/`](../monitoring/README.md). ### Operate diff --git a/compose/decdn-compose b/compose/decdn-compose index e72d17ef..0798a631 100755 --- a/compose/decdn-compose +++ b/compose/decdn-compose @@ -2071,6 +2071,30 @@ def fetch(url: str, timeout: float = 5.0) -> tuple[int, str]: return -1, "" +NODE_METRICS = "http://127.0.0.1:9090/metrics" +BOOT_READY_RE = re.compile(r"^decdn_node_boot_ready (\S+)$", re.M) + + +def boot_state(status: int, body: str) -> str | None: + """None when decdn-node has finished booting, else what it is doing. + + A release carrying decdn/decdn#2821 serves /metrics from process start, with + decdn_node_boot_ready 0 until bring-up finishes, so a 200 alone proves only the + listener. Older releases export no gauge and bind /metrics once boot has + finished, so a 200 without one is ready (as the role's readiness probe).""" + if status == 0: + return "/metrics is not listening (booting on a release without decdn_node_boot_ready, or down)" + if status != 200: + return "/metrics did not answer" if status < 0 else f"/metrics answered {status}" + m = BOOT_READY_RE.search(body) + if m is None or m[1] == "1": + return None + return ( + f"booting (decdn_node_boot_ready {m[1]}): see its logs; the DecdnNodeBootStuck alert fires " + "if boot never finishes" + ) + + def hold_state(status: int, body: str, url: str = SPONSORD_METRICS) -> str | None: """None when sponsord holds nothing, else why it must not be stopped. @@ -2361,8 +2385,8 @@ def cmd_health(args: argparse.Namespace) -> None: "exec", "-T", "decdn-node", "decdn", "node", "health", "--config", str(NODE_TOML), check=False, capture=True ) report("decdn-node admin RPC", cp.returncode == 0, " ".join(cp.stdout.split()) or "no answer") - status, _ = fetch("http://127.0.0.1:9090/metrics") - report("decdn-node metrics", status == 200, f"http://127.0.0.1:9090/metrics status {status}") + reason = boot_state(*fetch(NODE_METRICS)) + report("decdn-node boot", reason is None, reason or f"{NODE_METRICS} decdn_node_boot_ready 1") # Each at the bind its running container was started with, not a constant. for svc, bind_key in (("sponsord", "SPONSORD_BIND"), ("sponsord-onramp", "ONRAMP_BIND")): if svc in svcs: diff --git a/compose/tests/test_decdn_compose.py b/compose/tests/test_decdn_compose.py index a8efe714..246d2f81 100644 --- a/compose/tests/test_decdn_compose.py +++ b/compose/tests/test_decdn_compose.py @@ -243,6 +243,30 @@ def test_the_probe_follows_the_running_bind(self) -> None: self.assertIsNone(dc.bind_metrics_url(bind), bind) +class BootState(unittest.TestCase): + def test_ready_with_the_gauge_at_1(self) -> None: + self.assertIsNone(dc.boot_state(200, "a 1\ndecdn_node_boot_ready 1\nb 2\n")) + + def test_a_release_without_the_gauge_is_ready_on_200(self) -> None: + self.assertIsNone(dc.boot_state(200, METRICS_OK)) + + def test_booting_with_the_gauge_at_0(self) -> None: + reason = dc.boot_state(200, "decdn_node_boot_ready 0\n") + self.assertIsNotNone(reason) + self.assertIn("booting", reason) + + def test_only_the_exact_series_counts(self) -> None: + # A longer name is another series: no gauge, so a 200 is ready; the gauge + # itself still decides when both are present. + self.assertIsNone(dc.boot_state(200, "decdn_node_boot_ready_x 0\n")) + self.assertIsNotNone(dc.boot_state(200, "decdn_node_boot_ready_x 1\ndecdn_node_boot_ready 0\n")) + + def test_no_200_is_not_ready(self) -> None: + for status in (0, -1, 500): + with self.subTest(status=status): + self.assertIsNotNone(dc.boot_state(status, "decdn_node_boot_ready 1\n")) + + class ContainerEnv(unittest.TestCase): def test_unreadable_env_is_empty(self) -> None: for out in ("null", "", "not json", '{"a": 1}', '["A=1", 2, "B"]'): From 6366127e90f59a2ae3a930024cb34bff236955aa Mon Sep 17 00:00:00 2001 From: Ant Somers Date: Sun, 1 Nov 2026 20:33:58 +0300 Subject: [PATCH 2/2] fix: harden boot-readiness checks after review - Role: a decdn_node_boot_ready sample that is not exactly `1` (0.0, labelled, timestamped) counts as booting, so a format change upstream cannot pass as ready; uri gets timeout: 5, and the warning prints the sample it saw or the last error. The warning is registered. - decdn-compose: boot_state returns (ready, detail), reads labelled and timestamped samples, and no longer claims a gauge a release lacks. - Chart: startupProbe budget 10 min (upstream's boot-read retry budget); the admin_port guard matches `sh -c "decdn node health"` and refuses a string command; NOTES.txt and the README describe the new ordering (the first ContentBlacklist sync is part of boot). - Molecule: the stub's boot clock starts at the first scrape and gains an `error` mode; slow-readiness asserts the warning branch, that a never-binding daemon stayed up, and that a 500 fails the probe on its first attempt; default/host-env assert the probe stopped before its window with no warning. Co-Authored-By: Claude Opus 5.5 --- ansible/molecule/default/converge.yml | 16 +++-- .../molecule/default/files/decdn-node-stub | 37 ++++++---- ansible/molecule/grafana-cloud/converge.yml | 8 +-- ansible/molecule/host-env/converge.yml | 16 +++-- ansible/molecule/slow-readiness/converge.yml | 9 ++- ansible/molecule/slow-readiness/molecule.yml | 3 +- .../molecule/slow-readiness/side_effect.yml | 72 ++++++++++++++++--- ansible/molecule/slow-readiness/verify.yml | 4 ++ ansible/roles/decdn_node/README.md | 9 +-- ansible/roles/decdn_node/tasks/main.yml | 45 ++++++++---- charts/decdn-node/README.md | 12 ++-- charts/decdn-node/templates/NOTES.txt | 7 +- charts/decdn-node/templates/_helpers.tpl | 9 ++- charts/decdn-node/tests/render-test.sh | 5 ++ charts/decdn-node/values.yaml | 19 ++--- compose/decdn-compose | 32 +++++---- compose/tests/test_decdn_compose.py | 35 ++++++--- 17 files changed, 235 insertions(+), 103 deletions(-) diff --git a/ansible/molecule/default/converge.yml b/ansible/molecule/default/converge.yml index 1d227ab5..5f642fd8 100644 --- a/ansible/molecule/default/converge.yml +++ b/ansible/molecule/default/converge.yml @@ -138,15 +138,21 @@ tags: [observability, decdn_node, node] - role: decdn_node post_tasks: - # The stub serves /metrics at once with decdn_node_boot_ready 0, then flips it - # to 1 (decdn-node-stub's BOOT_SECONDS): the probe must end on the 1, not on - # the first 200. + # The stub serves /metrics at once with decdn_node_boot_ready 0 until 3s + # after the first scrape, then 1 (decdn-node-stub's BOOT_SECONDS): the probe + # must end on the 1 and stop before its window runs out (attempts would then + # equal decdn_readiness_retries), with no warning. That it retries a 0 is + # slow-readiness's to prove: on the idempotence run here the node has long + # finished booting, so one attempt is right. - name: Assert the readiness probe waited for decdn_node_boot_ready 1 ansible.builtin.assert: that: - decdn_metrics_probe.status == 200 - decdn_metrics_probe.content is search('(?m)^decdn_node_boot_ready 1$') + - decdn_metrics_probe.attempts | int < decdn_readiness_retries | int + - decdn_readiness_warning is skipped fail_msg: >- The readiness probe ended on status - {{ decdn_metrics_probe.status | default('none') }} without - decdn_node_boot_ready 1: {{ decdn_metrics_probe.content | default('') }} + {{ decdn_metrics_probe.status | default('none') }} after + {{ decdn_metrics_probe.attempts | default('?') }} attempt(s): + {{ decdn_metrics_probe.content | default('') }} diff --git a/ansible/molecule/default/files/decdn-node-stub b/ansible/molecule/default/files/decdn-node-stub index d090cc88..05017fc8 100755 --- a/ansible/molecule/default/files/decdn-node-stub +++ b/ansible/molecule/default/files/decdn-node-stub @@ -9,8 +9,9 @@ can be exercised end-to-end without a published release or a live chain: * `run ...` -> serve HTTP 200 on 127.0.0.1:9090/metrics (404 elsewhere) and block forever under systemd (Type=simple). Like a release carrying decdn/decdn#2821, it binds at once and exports - `decdn_node_boot_ready 0` for BOOT_SECONDS, then `1`, so - the role's readiness probe has to wait for the flip. CLI + `decdn_node_boot_ready 0` until BOOT_SECONDS after the first + scrape, then `1`, so the role's readiness probe always sees + the flip, however slow the runner. CLI args (`--config`, `--keystore-password-file`) are accepted and ignored. Under a unit with WatchdogSec it heartbeats WATCHDOG=1 to @@ -22,8 +23,11 @@ can be exercised end-to-end without a published release or a live chain: that never finishes; `slow-readiness`). legacy -> no gauge at all, as v0.3.0 and v0.3.1, which bind /metrics only after boot. - no-metrics -> never binds (a daemon that stays alive - but dies before its listener comes up). + no-metrics -> stays alive but never binds (a release + without the gauge, still booting, or a + daemon wedged before its listener). + error -> /metrics answers 500 (a broken handler, + which the role must fail on at once). * `config validate ...` -> parse the rendered node.toml as TOML and exit 0. In the default scenario it also checks the exact literal DECDN_RPC_URL passed @@ -55,12 +59,13 @@ METRICS_ADDR = ("127.0.0.1", 9090) # matches the role's default metrics_bind/po # Matches decdn_node_version in the cloud-init scenario's mirror. Printed after the # name it runs as (decdn-node or decdn), as clap prints the real binaries' line. VERSION = "0.0.0-molecule-stub" -# How long a default boot reports `decdn_node_boot_ready 0` before it flips to 1. +# How long a default boot reports `decdn_node_boot_ready 0`, from the first +# scrape, before it flips to 1. BOOT_SECONDS = 3 # Its first line picks a non-default boot (see the docstring); scenarios stage it # in prepare.yml. BOOT_MODE_MARKER = "/etc/decdn/stub-boot" -BOOT_MODES = ("stuck", "legacy", "no-metrics") +BOOT_MODES = ("stuck", "legacy", "no-metrics", "error") # Forces `config validate` to exit 1, so a scenario can prove the role's config # gate still fails the deploy (the gate itself runs with failed_when: false). BAD_CONFIG_MARKER = "/etc/decdn/stub-bad-config" @@ -74,8 +79,14 @@ class _Handler(BaseHTTPRequestHandler): self.send_response(404) self.end_headers() return + if self.server.boot_mode == "error": + self.send_response(500) + self.end_headers() + return body = b"# stub decdn-node metrics\ndecdn_stub_up 1\n" if self.server.boot_mode != "legacy": + if self.server.ready_at is None: + self.server.ready_at = time.monotonic() + BOOT_SECONDS ready = self.server.boot_mode != "stuck" and time.monotonic() >= self.server.ready_at body += b"decdn_node_boot_ready %d\n" % ready self.send_response(200) @@ -132,10 +143,11 @@ def config_validate(args): key-level drift; see its README. """ config = _opt(args, "--config") - # Forced-failure marker, like BOOT_MODE_MARKER. The role's config gate - # deliberately runs with `failed_when: false` and defers to a following `fail` - # task, so a regression in THAT task's `when:` would silently no-op the whole - # gate. This lets a scenario prove the gate still fires. + # Forced-failure marker (a presence check, unlike BOOT_MODE_MARKER's mode + # line). The role's config gate deliberately runs with `failed_when: false` + # and defers to a following `fail` task, so a regression in THAT task's + # `when:` would silently no-op the whole gate. This lets a scenario prove the + # gate still fires. if os.path.isfile(BAD_CONFIG_MARKER): print(f"stub config validate: refusing {config} (forced by " f"{BAD_CONFIG_MARKER})", file=sys.stderr) @@ -196,7 +208,8 @@ def _start_watchdog_heartbeat(): def boot_mode(): - """The boot BOOT_MODE_MARKER asks for ("" = default), or None when it names no mode.""" + """'' when BOOT_MODE_MARKER is absent (the default boot), its mode, or None + when its first line names no mode (`run` then exits 1).""" try: with open(BOOT_MODE_MARKER, encoding="utf-8") as fh: mode = fh.readline().strip() @@ -234,7 +247,7 @@ def main(): print(f"stub: cannot bind {addr}: {exc}", file=sys.stderr) return 1 server.boot_mode = mode - server.ready_at = time.monotonic() + BOOT_SECONDS + server.ready_at = None # set by the first scrape server.serve_forever() return 0 print(f"stub: unrecognised invocation: {args!r}", file=sys.stderr) diff --git a/ansible/molecule/grafana-cloud/converge.yml b/ansible/molecule/grafana-cloud/converge.yml index cb1fe71a..73e4e083 100644 --- a/ansible/molecule/grafana-cloud/converge.yml +++ b/ansible/molecule/grafana-cloud/converge.yml @@ -6,9 +6,9 @@ # sponsord_hosts), so Alloy must scrape both daemons. ../sponsord covers a # sponsord-only host. # -# The stub does not serve /metrics, so decdn_node's readiness probe would sit out -# its full advisory window (~60s of retries); decdn_readiness_* below bounds that -# to ~3s without changing what the role asserts. +# The stub reports decdn_node_boot_ready 0 for 3s after its first scrape, so +# decdn_node's advisory readiness probe may warn here; decdn_readiness_* below +# bounds its window to ~3s without changing what the role asserts. - name: Converge hosts: all become: true @@ -21,7 +21,7 @@ # environment labels exercise the "set" branch of the attribute rendering. grafana_alloy_deployment_environment: production grafana_alloy_region: US - # Keep the advisory metrics probe short (stub serves nothing). + # Keep the advisory readiness probe short (see the header). decdn_readiness_retries: 3 decdn_readiness_delay: 1 diff --git a/ansible/molecule/host-env/converge.yml b/ansible/molecule/host-env/converge.yml index 798935ba..ff7741e7 100644 --- a/ansible/molecule/host-env/converge.yml +++ b/ansible/molecule/host-env/converge.yml @@ -32,15 +32,17 @@ roles: - role: decdn_node post_tasks: - # prepare.yml boots the stub without the gauge: a 200 must count as ready on - # the first attempt, not be polled for a gauge that never comes. + # prepare.yml boots the stub without the gauge: a 200 must count as ready + # (stopping the probe before its window runs out, when attempts would equal + # decdn_readiness_retries), not be polled for a gauge that never comes. - name: Assert the readiness probe took a gauge-less 200 as ready ansible.builtin.assert: that: - decdn_metrics_probe.status == 200 - - decdn_metrics_probe.content is not search('(?m)^decdn_node_boot_ready ') - - decdn_metrics_probe.attempts | int == 1 + - decdn_metrics_probe.content is not search('(?m)^decdn_node_boot_ready[ {]') + - decdn_metrics_probe.attempts | int < decdn_readiness_retries | int + - decdn_readiness_warning is skipped fail_msg: >- - Expected one probe attempt answering 200 without decdn_node_boot_ready; - got status {{ decdn_metrics_probe.status | default('none') }} after - {{ decdn_metrics_probe.attempts | default('?') }} attempt(s). + Expected the probe to stop on a 200 without decdn_node_boot_ready, with no + warning; got status {{ decdn_metrics_probe.status | default('none') }} + after {{ decdn_metrics_probe.attempts | default('?') }} attempt(s). diff --git a/ansible/molecule/slow-readiness/converge.yml b/ansible/molecule/slow-readiness/converge.yml index 513b04fe..59660682 100644 --- a/ansible/molecule/slow-readiness/converge.yml +++ b/ansible/molecule/slow-readiness/converge.yml @@ -13,15 +13,18 @@ - role: decdn_node post_tasks: # A probe that took any 200 as ready would stop at its first attempt; one that - # reads the gauge retries until the window runs out on decdn_node_boot_ready 0. + # reads the gauge retries until the window runs out on decdn_node_boot_ready 0 + # (attempts then equals the retries), and its warning names the booting state. - name: Assert the readiness probe kept polling a node that is still booting ansible.builtin.assert: that: - decdn_metrics_probe.status == 200 - decdn_metrics_probe.content is search('(?m)^decdn_node_boot_ready 0$') - - decdn_metrics_probe.attempts | int > 1 + - decdn_metrics_probe.attempts | int == decdn_readiness_retries | int + - decdn_readiness_warning is not skipped + - "'`decdn_node_boot_ready 0`:' in decdn_readiness_warning.msg" fail_msg: >- Expected the probe to retry a 200 carrying decdn_node_boot_ready 0 until - its window ran out; got status + its window ran out, then warn that the node is booting; got status {{ decdn_metrics_probe.status | default('none') }} after {{ decdn_metrics_probe.attempts | default('?') }} attempt(s). diff --git a/ansible/molecule/slow-readiness/molecule.yml b/ansible/molecule/slow-readiness/molecule.yml index 69570035..d851a0a9 100644 --- a/ansible/molecule/slow-readiness/molecule.yml +++ b/ansible/molecule/slow-readiness/molecule.yml @@ -6,7 +6,8 @@ # is still booting instead of taking it as ready, WARNS (does not fail) when the # window runs out, and that the hard service-state gate confirms the unit is # `running`. side_effect.yml re-runs the role against a daemon that never binds -# /metrics (a release without the gauge, booting), which must stay advisory too. +# /metrics (a release without the gauge, booting), which must stay advisory too, +# and against a /metrics that answers 500, which must fail at once. # Reuses the default scenario's stub binary rather than duplicating it. # # `baseline` is NOT exercised here (same rationale as the default scenario: diff --git a/ansible/molecule/slow-readiness/side_effect.yml b/ansible/molecule/slow-readiness/side_effect.yml index 5fabef97..465e4ad6 100644 --- a/ansible/molecule/slow-readiness/side_effect.yml +++ b/ansible/molecule/slow-readiness/side_effect.yml @@ -1,13 +1,17 @@ --- -# What converge cannot show: a daemon whose /metrics never answers, as a release -# without decdn_node_boot_ready looks while it boots (it binds only once boot has -# finished), or a daemon that dies before its listener comes up. The probe must -# keep that advisory too (status -1 warns, it does not fail). The host is left -# booting as converged (the `stuck` stub), for verify. -- name: Side effect — a daemon that never binds /metrics +# What converge cannot show, on the stub's other boots: +# 1. A daemon that stays up but never binds /metrics, as a release without +# decdn_node_boot_ready looks while it boots (it binds only once boot has +# finished), or a daemon wedged before its listener. The probe must keep that +# advisory (status -1 warns, it does not fail), and say /metrics did not answer. +# 2. A /metrics that answers 500 (a broken handler): the probe must fail the +# run on its first attempt, not retry it as slow startup. +# The host is left booting as converged (the `stuck` stub), for verify. +- name: Side effect — a daemon that never binds, then a broken /metrics hosts: all become: true tasks: + # --- 1. never binds ------------------------------------------------------------ - name: Stop decdn-node ansible.builtin.systemd_service: name: decdn-node @@ -24,17 +28,65 @@ ansible.builtin.include_role: name: decdn_node - - name: Assert the probe retried a refused connection and the deploy went on + # A stub that exits (a marker naming no mode) is refused too, and restarted by + # systemd: NRestarts 0 proves this was a live daemon that never bound. + - name: Read the unit's restart count and state + ansible.builtin.systemd_service: + name: decdn-node + register: never_binds_unit + + - name: Assert the probe retried a daemon that stayed up and never answered ansible.builtin.assert: that: - decdn_metrics_probe.status | default(-1) == -1 - - decdn_metrics_probe.attempts | int > 1 + - decdn_metrics_probe.attempts | int == decdn_readiness_retries | int + - decdn_readiness_warning is not skipped + - "'/metrics did not answer on the last attempt' in decdn_readiness_warning.msg" + - never_binds_unit.status.NRestarts == '0' + - never_binds_unit.status.ActiveState == 'active' fail_msg: >- Expected the probe to retry a /metrics that never answers until its - window ran out; got status + window ran out, then warn, with the unit up throughout; got status {{ decdn_metrics_probe.status | default('none') }} after - {{ decdn_metrics_probe.attempts | default('?') }} attempt(s). + {{ decdn_metrics_probe.attempts | default('?') }} attempt(s), unit + NRestarts {{ never_binds_unit.status.NRestarts }}, ActiveState + {{ never_binds_unit.status.ActiveState }}. + + # --- 2. answers 500 ------------------------------------------------------------ + - name: Make the stub's /metrics answer 500 + ansible.builtin.copy: + dest: /etc/decdn/stub-boot + content: "error\n" + mode: "0644" + + - name: Restart decdn-node on the broken /metrics + ansible.builtin.systemd_service: + name: decdn-node + state: restarted + + - name: Converge against a /metrics that answers 500 + block: + - name: Run the role against a broken /metrics + ansible.builtin.include_role: + name: decdn_node + + - name: Fail — the readiness probe let a 500 through + ansible.builtin.fail: + msg: The role finished although decdn-node's /metrics answered 500. + rescue: + - name: Require the probe itself to fail, on its first attempt + ansible.builtin.assert: + that: + - ansible_failed_task.name == 'Wait for the node to finish booting (advisory)' + - decdn_metrics_probe.status == 500 + - decdn_metrics_probe.attempts | int == 1 + fail_msg: >- + Expected the readiness probe to fail at once on a 500; failed at + '{{ ansible_failed_task.name }}' with status + {{ decdn_metrics_probe.status | default('none') }} after + {{ decdn_metrics_probe.attempts | default('?') }} attempt(s). + # --- leave the converged state for verify -------------------------------------- - name: Put the stub back into a boot that never finishes ansible.builtin.copy: dest: /etc/decdn/stub-boot diff --git a/ansible/molecule/slow-readiness/verify.yml b/ansible/molecule/slow-readiness/verify.yml index 5495c1ff..fabab99d 100644 --- a/ansible/molecule/slow-readiness/verify.yml +++ b/ansible/molecule/slow-readiness/verify.yml @@ -10,11 +10,15 @@ hosts: all become: true tasks: + # side_effect.yml's last restart returns before the stub binds (Type=simple). - name: Read the node's metrics ansible.builtin.uri: url: http://127.0.0.1:9090/metrics return_content: true register: node_metrics + retries: 10 + delay: 1 + until: node_metrics.status | default(-1) == 200 - name: Gather service facts ansible.builtin.service_facts: diff --git a/ansible/roles/decdn_node/README.md b/ansible/roles/decdn_node/README.md index 1b24c521..c2332121 100644 --- a/ansible/roles/decdn_node/README.md +++ b/ansible/roles/decdn_node/README.md @@ -286,8 +286,8 @@ Optional (omitted from `node.toml` unless set): you supply the matching one). The generated wallet is still **unfunded + unstaked**. Leave `false` to keep the operator-provisioned posture. - `decdn_readiness_retries` (default `30`) + `decdn_readiness_delay` (default `2`, seconds) - — bound the `/metrics` readiness probe window (`retries × delay`, so ~60 s at these - defaults). A **timeout** only warns; it never fails the deploy — but a non-200 *answer* + — bound the boot-readiness probe window on `/metrics` (`retries × delay`, so ~60 s + at these defaults). A **timeout** only warns; it never fails the deploy — but a non-200 *answer* does. See [Readiness](#readiness). ### Optional tuning knobs @@ -617,8 +617,9 @@ After starting the daemon the role polls `http://127.0.0.1:/metrics` until the node reports boot-ready, then asserts that the systemd unit is in the `running` state. -Boot-ready means `/metrics` answers 200 and either says `decdn_node_boot_ready 1` -or has no `decdn_node_boot_ready` line at all. A release that carries +Boot-ready means `/metrics` answers 200 and either says exactly +`decdn_node_boot_ready 1` or has no `decdn_node_boot_ready` series at all (a sample +the probe cannot read, such as a labelled one, counts as still booting). A release that carries [decdn/decdn#2821](https://github.com/decdn/decdn/pull/2821) serves `/metrics` from process start and holds the gauge at `0` through the RPC preflight and the boot chain reads, so a 200 alone proves only that the listener is up. Older diff --git a/ansible/roles/decdn_node/tasks/main.yml b/ansible/roles/decdn_node/tasks/main.yml index 00a692b3..d47e3561 100644 --- a/ansible/roles/decdn_node/tasks/main.yml +++ b/ansible/roles/decdn_node/tasks/main.yml @@ -1805,7 +1805,8 @@ # RPC preflight and the boot chain reads finish, then 1 (the "node runtime ready" # banner). Older releases export no gauge and bind /metrics only once boot has # finished. So the node is boot-ready when /metrics answers 200 and either says -# `decdn_node_boot_ready 1` or has no such line; a 200 alone is not enough. +# exactly `decdn_node_boot_ready 1` or has no such series; a 200 alone is not +# enough. # A boot can take many minutes on a healthy node (a slow RPC, chain-read # retries), so a probe TIMEOUT must NOT fail the deploy; the warning below says # which state it ended in, and the DecdnNodeBootStuck alert covers a boot that @@ -1830,17 +1831,26 @@ url: "http://127.0.0.1:{{ decdn_metrics_port }}/metrics" status_code: 200 return_content: true + # Bounds each attempt, so a listener that accepts but never answers cannot + # stretch the window past retries × (delay + timeout). + timeout: 5 register: decdn_metrics_probe retries: "{{ decdn_readiness_retries }}" delay: "{{ decdn_readiness_delay }}" # default(-1): on connection-refused the uri module returns status -1 (and may # omit the key). Retry while the endpoint has NOT answered (status -1) or answers - # 200 while still booting (`decdn_node_boot_ready 0`). Stop on a boot-ready 200, - # and on any other status, so a real fault (e.g. a 500) surfaces immediately via - # failed_when rather than after the whole window. + # 200 while still booting: a decdn_node_boot_ready sample that is not exactly + # `decdn_node_boot_ready 1` (a 0, but also a labelled, timestamped or 0.0 sample + # this probe cannot read, so a format change upstream cannot pass as ready). + # Stop on a boot-ready 200, and on any other status, so a real fault (e.g. a + # 500) surfaces immediately via failed_when rather than after the whole window. until: >- decdn_metrics_probe.status | default(-1) != -1 - and decdn_metrics_probe.content | default('') is not search('(?m)^decdn_node_boot_ready 0$') + and not ( + decdn_metrics_probe.status == 200 + and decdn_metrics_probe.content | default('') is search('(?m)^decdn_node_boot_ready[ {]') + and decdn_metrics_probe.content | default('') is not search('(?m)^decdn_node_boot_ready 1$') + ) changed_when: false # Scope the suppression, don't blanket it: -1 (not bound yet) and a 200 that is # still booting stay advisory, but a bound endpoint answering any other status @@ -1861,26 +1871,33 @@ ({{ decdn_readiness_retries }}×{{ decdn_readiness_delay }}), so readiness could NOT be confirmed. {% if decdn_metrics_probe.status | default(-1) == 200 %} - /metrics answers with `decdn_node_boot_ready 0`: the node is still in its - RPC preflight or boot chain reads. A slow RPC can make this take minutes; - the DecdnNodeBootStuck alert fires if it never finishes. A rising - decdn_chain_boot_read_retries_total means the chain reads are retrying; - otherwise read the first WARN or ERROR after "deCDN node starting". + /metrics answers with + `{{ decdn_metrics_probe.content | regex_search('(?m)^decdn_node_boot_ready.*$') }}`: + the node is still in its RPC preflight or boot chain reads. A slow RPC can + make this take minutes; the DecdnNodeBootStuck alert fires if it never + finishes. A rising decdn_chain_boot_read_retries_total means the chain + reads are retrying; otherwise read the first WARN or ERROR after + "deCDN node starting". {% else %} - /metrics never answered. A release without `decdn_node_boot_ready` binds it - only once boot has finished, so this is OFTEN benign there; on a newer - release, or PERSISTENTLY, it means a daemon that died or wedged before its + /metrics did not answer on the last attempt + ({{ decdn_metrics_probe.msg | default('no response') }}). + A release without `decdn_node_boot_ready` binds it only once boot has + finished, so this is OFTEN benign there; on a newer release, or + PERSISTENTLY, it means a daemon that died, crash-loops or wedged before its listener came up (a Type=simple unit still reports `running` even when wedged). {% endif %} Verify before trusting the node: curl -s http://127.0.0.1:{{ decdn_metrics_port }}/metrics | grep -E '^decdn_node_boot_ready ' journalctl -u decdn-node -e (look for "node runtime ready") + # Registered so the molecule scenarios can assert which branch ran, or that none did. + register: decdn_readiness_warning when: - not ansible_check_mode - >- decdn_metrics_probe.status | default(-1) != 200 - or decdn_metrics_probe.content | default('') is search('(?m)^decdn_node_boot_ready 0$') + or (decdn_metrics_probe.content | default('') is search('(?m)^decdn_node_boot_ready[ {]') + and decdn_metrics_probe.content | default('') is not search('(?m)^decdn_node_boot_ready 1$')) # A stale process can still answer /metrics, so also confirm THIS unit exists and # is running. This is a backstop, not a full health check: it catches a daemon diff --git a/charts/decdn-node/README.md b/charts/decdn-node/README.md index 3ff7c1f5..acfb2ec3 100644 --- a/charts/decdn-node/README.md +++ b/charts/decdn-node/README.md @@ -433,9 +433,10 @@ metrics: `decdn node health` (the CLI the image ships, as the Compose healthcheck does). The admin RPC starts only once boot has finished: RPC preflight, identity and keystore load, cache and endpoint build, then the chain bring-up (CapacityBond registry and - slash-watcher enumerations, blacklist and DHT bootstrap). - - The default `startupProbe` allows 5 minutes (5s × 60), then restarts the pod. A - slow RPC delays readiness. + slash-watcher enumerations, the first ContentBlacklist sync and DHT bootstrap). + - The default `startupProbe` allows 10 minutes (5s × 120), upstream's own budget for + retrying the boot chain reads, then restarts the container. A slow RPC delays + readiness. - It needs the admin RPC: the render fails when `config.observability.admin_port` is `0` and the default `startupProbe` is in use. Replace its handler with `startupProbe.exec: null` plus an `httpGet` on `/metrics`, which then proves only @@ -453,8 +454,9 @@ metrics: two handlers fails the render. An older values file that copies the earlier `httpGet` startupProbe therefore fails until it nulls `exec`. Once it does, its pod can go Ready while the node is still booting. - - **Ready is not the same as serving.** The paid-delivery QUIC listeners open only after - the first ContentBlacklist sync, which comes after boot finishes. Check the logs + - **Ready and serving are moments apart.** The paid-delivery QUIC protocols register + just after boot, once the first ContentBlacklist sync (part of boot) has passed. A + pod whose startupProbe is an `httpGet` can go Ready before that; check the logs before counting on traffic. - **SIGTERM runs a graceful drain.** `terminationGracePeriodSeconds` defaults to 300, the role's `TimeoutStopSec`. Killing a pod mid-drain loses paid deliveries, so do not diff --git a/charts/decdn-node/templates/NOTES.txt b/charts/decdn-node/templates/NOTES.txt index 2eff49d0..519b4f45 100644 --- a/charts/decdn-node/templates/NOTES.txt +++ b/charts/decdn-node/templates/NOTES.txt @@ -1,9 +1,10 @@ {{ $admin := dig "observability" "admin_port" 9191 (.Values.config | default dict) -}} deCDN node {{ include "decdn-node.fullname" . }} (namespace {{ .Release.Namespace }}). -- Wait for the node to become Ready. Ready means /metrics answers (startup and - chain bring-up done); paid QUIC listeners open only after the first - ContentBlacklist sync, so check the logs before routing traffic: +- Wait for the node to become Ready. Ready means the startup probe's + `decdn node health` reached the admin RPC (it starts once boot, including the + first ContentBlacklist sync, has finished), then /metrics answered. Paid QUIC + protocols register just after; check the logs before routing traffic: kubectl -n {{ .Release.Namespace }} rollout status statefulset/{{ include "decdn-node.fullname" . }} {{- $ns := .Release.Namespace }} diff --git a/charts/decdn-node/templates/_helpers.tpl b/charts/decdn-node/templates/_helpers.tpl index 88e95888..3eeeb27d 100644 --- a/charts/decdn-node/templates/_helpers.tpl +++ b/charts/decdn-node/templates/_helpers.tpl @@ -255,9 +255,14 @@ than let a values override win: {{- end }} {{- end }} {{- $startCmd := dig "exec" "command" (list) (default (dict) .Values.startupProbe) }} -{{- /* A non-number admin_port is decdn-node.config's to refuse, with its own message. */}} +{{- if not (kindIs "slice" $startCmd) }} +{{- fail (printf "startupProbe.exec.command must be a list of arguments, got %s" (kindOf $startCmd)) }} +{{- end }} +{{- /* A non-number admin_port is decdn-node.config's to refuse, with its own message. + Matched on the joined command, so a shell-wrapped `sh -c "decdn node health"` + counts too. */}} {{- $adminPort := dig "observability" "admin_port" 9191 (default (dict) .Values.config) }} -{{- if and (has "node" $startCmd) (has "health" $startCmd) (has (kindOf $adminPort) (list "float64" "int64" "int")) (eq (float64 $adminPort) 0.0) }} +{{- if and (regexMatch "(^|\\s)node\\s+health(\\s|$)" (join " " $startCmd)) (has (kindOf $adminPort) (list "float64" "int64" "int")) (eq (float64 $adminPort) 0.0) }} {{- fail "startupProbe runs `decdn node health`, which needs the admin RPC, but config.observability.admin_port is 0: enable the admin RPC, or replace the startupProbe (startupProbe.exec: null plus an httpGet on /metrics, which then proves only that the listener is up)" }} {{- end }} {{- end }} diff --git a/charts/decdn-node/tests/render-test.sh b/charts/decdn-node/tests/render-test.sh index 33f7cb83..1a4188fe 100755 --- a/charts/decdn-node/tests/render-test.sh +++ b/charts/decdn-node/tests/render-test.sh @@ -441,6 +441,8 @@ cmd = main["startupProbe"]["exec"]["command"] check("--config" in cmd and cmd[cmd.index("--config") + 1] == main["args"][main["args"].index("--config") + 1], "startupProbe reads another node.toml than the daemon") check(main["startupProbe"]["timeoutSeconds"] >= 5, "startupProbe timeout below the CLI's --timeout-ms") +check(main["startupProbe"]["periodSeconds"] * main["startupProbe"]["failureThreshold"] >= 600, + "startupProbe budget below upstream's 10-minute boot-read retry budget") for probe in ("readinessProbe", "livenessProbe"): check(main[probe]["httpGet"]["port"] == "metrics", f"{probe} port") @@ -655,6 +657,9 @@ expect_fail "unknown monitoring knob" schema "additional propert(y|ies) ' # the handler only once the default's is nulled. http_startup=(--set-json 'startupProbe={"exec":null,"httpGet":{"path":"/metrics","port":"metrics"}}') expect_fail "startupProbe, admin RPC off" template 'startupProbe runs .decdn node health.' --set config.observability.admin_port=0 --set metrics.port=9191 +expect_fail "startupProbe, admin off (JSON)" template 'startupProbe runs .decdn node health.' --set-json 'config.observability.admin_port=0' --set metrics.port=9191 +expect_fail "startupProbe, sh -c, admin off" template 'startupProbe runs .decdn node health.' --set-json 'startupProbe.exec.command=["sh","-c","decdn node health --config /etc/decdn/node.toml"]' --set config.observability.admin_port=0 --set metrics.port=9191 +expect_fail "startupProbe command a string" template 'startupProbe.exec.command must be a list' --set-json 'startupProbe.exec.command="decdn node health"' expect_fail "startupProbe, two handlers" template 'startupProbe has 2 handlers \(exec, httpGet\)' --set-json 'startupProbe.httpGet={"path":"/metrics","port":"metrics"}' expect_fail "livenessProbe, two handlers" template 'livenessProbe has 2 handlers' --set-json 'livenessProbe.tcpSocket={"port":"metrics"}' diff --git a/charts/decdn-node/values.yaml b/charts/decdn-node/values.yaml index b4816dae..ab1f0bf3 100644 --- a/charts/decdn-node/values.yaml +++ b/charts/decdn-node/values.yaml @@ -216,14 +216,15 @@ terminationGracePeriodSeconds: 300 # startupProbe asks the admin RPC (`decdn node health`, the CLI the image ships, as # Compose's healthcheck does). The admin RPC starts only once boot has finished — # RPC preflight, key load, cache/endpoint build, then the chain bring-up -# (CapacityBond + slash-watcher enumerations, blacklist and DHT bootstrap) — so the -# 5-minute budget below restarts a boot that never finishes, and must cover a slow -# RPC. It needs config.observability.admin_port (default 9191) on. readiness and -# liveness run only after it passes, on /metrics: from the decdn release that adds -# decdn_node_boot_ready, /metrics answers from process start, so an httpGet on it -# proves only that the listener is up (the DecdnNodeBootStuck alert reads the -# gauge). A failed bring-up exits the container and restarts it. Ready != serving: -# paid QUIC listeners open only after the first ContentBlacklist sync. +# (CapacityBond + slash-watcher enumerations, the first ContentBlacklist sync and +# DHT bootstrap) — so the budget below (10 minutes, upstream's own budget for +# retrying the boot chain reads) restarts the container on a boot that never +# finishes. It needs config.observability.admin_port (default 9191) on. readiness +# and liveness run only after it passes, on /metrics: from the decdn release that +# adds decdn_node_boot_ready, /metrics answers from process start, so an httpGet +# on it proves only that the listener is up (the DecdnNodeBootStuck alert reads +# the gauge). A failed bring-up exits the container and restarts it. Paid QUIC +# protocols register just after boot, so Ready and serving are moments apart. # To replace a probe's handler, null the default's (startupProbe.exec: null): # Helm merges an override into these defaults, and two handlers fail the render. startupProbe: @@ -231,7 +232,7 @@ startupProbe: command: [decdn, node, health, --config, /etc/decdn/node.toml, --timeout-ms, "4000"] timeoutSeconds: 5 periodSeconds: 5 - failureThreshold: 60 + failureThreshold: 120 readinessProbe: httpGet: path: /metrics diff --git a/compose/decdn-compose b/compose/decdn-compose index 0798a631..07f11a84 100755 --- a/compose/decdn-compose +++ b/compose/decdn-compose @@ -2072,26 +2072,30 @@ def fetch(url: str, timeout: float = 5.0) -> tuple[int, str]: NODE_METRICS = "http://127.0.0.1:9090/metrics" -BOOT_READY_RE = re.compile(r"^decdn_node_boot_ready (\S+)$", re.M) +# Any sample of the gauge: plain, labelled, or with a trailing timestamp. +BOOT_READY_RE = re.compile(r"^decdn_node_boot_ready(?:\{[^}]*\})?\s.*$", re.M) -def boot_state(status: int, body: str) -> str | None: - """None when decdn-node has finished booting, else what it is doing. +def boot_state(status: int, body: str) -> tuple[bool, str]: + """(ready, detail) for decdn-node's boot, from its /metrics answer. A release carrying decdn/decdn#2821 serves /metrics from process start, with decdn_node_boot_ready 0 until bring-up finishes, so a 200 alone proves only the listener. Older releases export no gauge and bind /metrics once boot has - finished, so a 200 without one is ready (as the role's readiness probe).""" + finished, so a 200 without one is ready (as the role's readiness probe). A + sample that is not exactly `decdn_node_boot_ready 1` is still booting, so a + format this cannot read never passes as ready.""" if status == 0: - return "/metrics is not listening (booting on a release without decdn_node_boot_ready, or down)" + return False, "/metrics is not listening (booting on a release without decdn_node_boot_ready, or down)" if status != 200: - return "/metrics did not answer" if status < 0 else f"/metrics answered {status}" - m = BOOT_READY_RE.search(body) - if m is None or m[1] == "1": - return None - return ( - f"booting (decdn_node_boot_ready {m[1]}): see its logs; the DecdnNodeBootStuck alert fires " - "if boot never finishes" + return False, "/metrics did not answer" if status < 0 else f"/metrics answered {status}" + samples = BOOT_READY_RE.findall(body) + if not samples: + return True, "booted (a release without decdn_node_boot_ready: /metrics answers once boot has finished)" + if samples == ["decdn_node_boot_ready 1"]: + return True, "booted (decdn_node_boot_ready 1)" + return False, ( + f"booting ({'; '.join(samples)}): see its logs; the DecdnNodeBootStuck alert fires if boot never finishes" ) @@ -2385,8 +2389,8 @@ def cmd_health(args: argparse.Namespace) -> None: "exec", "-T", "decdn-node", "decdn", "node", "health", "--config", str(NODE_TOML), check=False, capture=True ) report("decdn-node admin RPC", cp.returncode == 0, " ".join(cp.stdout.split()) or "no answer") - reason = boot_state(*fetch(NODE_METRICS)) - report("decdn-node boot", reason is None, reason or f"{NODE_METRICS} decdn_node_boot_ready 1") + ready, detail = boot_state(*fetch(NODE_METRICS)) + report("decdn-node boot", ready, f"{NODE_METRICS}: {detail}") # Each at the bind its running container was started with, not a constant. for svc, bind_key in (("sponsord", "SPONSORD_BIND"), ("sponsord-onramp", "ONRAMP_BIND")): if svc in svcs: diff --git a/compose/tests/test_decdn_compose.py b/compose/tests/test_decdn_compose.py index 246d2f81..6523955d 100644 --- a/compose/tests/test_decdn_compose.py +++ b/compose/tests/test_decdn_compose.py @@ -245,26 +245,41 @@ def test_the_probe_follows_the_running_bind(self) -> None: class BootState(unittest.TestCase): def test_ready_with_the_gauge_at_1(self) -> None: - self.assertIsNone(dc.boot_state(200, "a 1\ndecdn_node_boot_ready 1\nb 2\n")) + ready, detail = dc.boot_state(200, "a 1\ndecdn_node_boot_ready 1\nb 2\n") + self.assertTrue(ready) + self.assertIn("decdn_node_boot_ready 1", detail) def test_a_release_without_the_gauge_is_ready_on_200(self) -> None: - self.assertIsNone(dc.boot_state(200, METRICS_OK)) + ready, detail = dc.boot_state(200, METRICS_OK) + self.assertTrue(ready) + self.assertIn("without decdn_node_boot_ready", detail) def test_booting_with_the_gauge_at_0(self) -> None: - reason = dc.boot_state(200, "decdn_node_boot_ready 0\n") - self.assertIsNotNone(reason) - self.assertIn("booting", reason) + ready, detail = dc.boot_state(200, "decdn_node_boot_ready 0\n") + self.assertFalse(ready) + self.assertIn("booting (decdn_node_boot_ready 0)", detail) + + def test_an_unreadable_sample_is_still_booting(self) -> None: + # Not exactly `decdn_node_boot_ready 1`: a format change must not pass as ready. + for body in ( + "decdn_node_boot_ready 0.0\n", + "decdn_node_boot_ready 1.0\n", + 'decdn_node_boot_ready{node="a"} 1\n', + "decdn_node_boot_ready 1 1700000000000\n", + "decdn_node_boot_ready 1\ndecdn_node_boot_ready 0\n", + ): + with self.subTest(body=body): + self.assertFalse(dc.boot_state(200, body)[0]) def test_only_the_exact_series_counts(self) -> None: - # A longer name is another series: no gauge, so a 200 is ready; the gauge - # itself still decides when both are present. - self.assertIsNone(dc.boot_state(200, "decdn_node_boot_ready_x 0\n")) - self.assertIsNotNone(dc.boot_state(200, "decdn_node_boot_ready_x 1\ndecdn_node_boot_ready 0\n")) + # A longer name is another series: no gauge, so a 200 is ready. + self.assertTrue(dc.boot_state(200, "decdn_node_boot_ready_x 0\n")[0]) + self.assertFalse(dc.boot_state(200, "decdn_node_boot_ready_x 1\ndecdn_node_boot_ready 0\n")[0]) def test_no_200_is_not_ready(self) -> None: for status in (0, -1, 500): with self.subTest(status=status): - self.assertIsNotNone(dc.boot_state(status, "decdn_node_boot_ready 1\n")) + self.assertFalse(dc.boot_state(status, "decdn_node_boot_ready 1\n")[0]) class ContainerEnv(unittest.TestCase):