From cef13a0cd6aa64ab2a9b30fe1199d60939846357 Mon Sep 17 00:00:00 2001 From: Yuriy Andamasov Date: Wed, 22 Jul 2026 18:53:45 +0300 Subject: docs-gates: hard-bound the smoke deadline (cap probe timeout + inter-round sleep to remaining budget) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex adversarial finding: DEADLINE_SECONDS was only checked BEFORE each op, so a probe or sleep starting at 479s could overshoot to ~510s — 480 was a soft target, not a hard bound. Make it hard: run() now computes an absolute deadline = start + DEADLINE_SECONDS plus a _remaining() helper. The per-probe socket timeout is capped to min(PROBE_TIMEOUT_SECONDS, max(1, remaining)) — the previously hardcoded 30 is now the PROBE_TIMEOUT_SECONDS constant; a probe with < 1s of budget is skipped and counted unresolved. The inter-round sleep is capped to min(RETRY_SLEEP_SECONDS, remaining) and is skipped entirely when the budget is exhausted (falling into the existing deadline path). No body-read-level deadline is added — pages are small, so the socket-op timeout bounds reads adequately. Deadline-path failure accounting is unchanged. Tests: the probe timeout is capped to the remaining budget (fake opener records the timeout it was opened with); the inter-round sleep is capped to the remaining budget (sleep spy); the existing suite stays green. 🤖 Generated by [robots](https://vyos.io) --- scripts/docs_gates/smoke.py | 37 ++++++++++++++++++++++++------------- scripts/docs_gates/test_smoke.py | 37 +++++++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 15 deletions(-) (limited to 'scripts') diff --git a/scripts/docs_gates/smoke.py b/scripts/docs_gates/smoke.py index 34dc87d7..770e93e3 100644 --- a/scripts/docs_gates/smoke.py +++ b/scripts/docs_gates/smoke.py @@ -38,6 +38,10 @@ USER_AGENT = "vyos-docs-smoke/1.0 (+https://github.com/vyos/vyos-documentation)" MAX_ROUNDS = 5 RETRY_SLEEP_SECONDS = 30 DEADLINE_SECONDS = 480 +# Per-socket-op timeout (connect + read), capped down to the remaining deadline budget on each +# probe so a probe that starts late cannot overshoot DEADLINE_SECONDS. Pages are small, so this +# socket-op timeout also bounds body reads adequately — no separate body-read deadline is needed. +PROBE_TIMEOUT_SECONDS = 30 class _NoRedirect(urllib.request.HTTPRedirectHandler): @@ -87,21 +91,24 @@ def search_mount_present(html: str) -> bool: return SEARCH_MOUNT_MARKER in html -def _probe_once(host: str, probe: Probe, expect_sha: str, access_id: str, - access_secret: str) -> tuple[bool, int | None, str | None, str | None]: +def _probe_once(host: str, probe: Probe, expect_sha: str, access_id: str, access_secret: str, + timeout: float = PROBE_TIMEOUT_SECONDS, + ) -> tuple[bool, int | None, str | None, str | None]: """One probe attempt. Returns (ok, status, docs_build, detail). `detail` names the failed check(s) — "status" / "docs-build" / "apex-build" / "search-mount" joined by "+", or the - transport error text — and is None when ok. ANY exception in the open OR body-read path - (including a transport error DURING HTTPError.read()) is contained and yields - (False, None, None, ): a retryable failure, never a traceback. The HTTPError - response stream is always closed — it owns a socket, so a bare e.read() would leak it.""" + transport error text — and is None when ok. `timeout` is the per-socket-op deadline (connect + + read); run() caps it to the remaining budget so a late probe can't overshoot the overall + deadline. ANY exception in the open OR body-read path (including a transport error DURING + HTTPError.read()) is contained and yields (False, None, None, ): a retryable + failure, never a traceback. The HTTPError response stream is always closed — it owns a + socket, so a bare e.read() would leak it.""" req = urllib.request.Request(f"https://{host}{probe.path}", method="GET") req.add_header("CF-Access-Client-Id", access_id) req.add_header("CF-Access-Client-Secret", access_secret) req.add_header("User-Agent", USER_AGENT) try: try: - with _OPENER.open(req, timeout=30) as resp: + with _OPENER.open(req, timeout=timeout) as resp: status, headers, body = resp.status, resp.headers, resp.read() except urllib.error.HTTPError as e: # non-2xx still carries headers/body with e: # HTTPError is file-like and owns the response socket — always close it @@ -130,12 +137,13 @@ def run(host: str, slug: str, expect_sha: str, access_id: str, access_secret: st DEADLINE_SECONDS bounds total wall-clock — on breach, unresolved probes count as failed.""" plan = probe_plan(slug, pdf, critical) start = time.monotonic() + deadline = start + DEADLINE_SECONDS # absolute — a hard upper bound on total wall-clock pending = list(plan) # probes not yet passed detail_by_path: dict[str, str] = {} # last failure detail per path, for logging deadline_hit = False - def _past_deadline() -> bool: - return time.monotonic() - start >= DEADLINE_SECONDS + def _remaining() -> float: + return deadline - time.monotonic() for round_num in range(1, MAX_ROUNDS + 1): if not pending: @@ -143,12 +151,14 @@ def run(host: str, slug: str, expect_sha: str, access_id: str, access_secret: st still_failing: list[Probe] = [] unprobed: list[Probe] = [] for i, probe in enumerate(pending): - if _past_deadline(): # checked before each probe + remaining = _remaining() # checked before each probe + if remaining < 1: # < 1s budget: don't start a probe that could overshoot deadline_hit = True unprobed = pending[i:] # not reached this round → still unresolved break ok, status, docs_build, detail = _probe_once( - host, probe, expect_sha, access_id, access_secret) + host, probe, expect_sha, access_id, access_secret, + timeout=min(PROBE_TIMEOUT_SECONDS, max(1, remaining))) if ok: continue still_failing.append(probe) @@ -157,13 +167,14 @@ def run(host: str, slug: str, expect_sha: str, access_id: str, access_secret: st pending = still_failing + unprobed if deadline_hit or not pending or round_num == MAX_ROUNDS: break - if _past_deadline(): # checked before the inter-round sleep + remaining = _remaining() # checked before the inter-round sleep + if remaining <= 0: # no budget left → deadline path (skip the sleep) deadline_hit = True break for probe in pending: print(f"SMOKE-RETRY {probe.path}: round {round_num} " f"{detail_by_path[probe.path]}", file=sys.stderr) - time.sleep(RETRY_SLEEP_SECONDS) + time.sleep(min(RETRY_SLEEP_SECONDS, remaining)) # never sleep past the deadline if deadline_hit: print("SMOKE-DEADLINE: overall deadline reached — remaining probes counted as failed", diff --git a/scripts/docs_gates/test_smoke.py b/scripts/docs_gates/test_smoke.py index f0a37c7a..a2654dcf 100644 --- a/scripts/docs_gates/test_smoke.py +++ b/scripts/docs_gates/test_smoke.py @@ -101,15 +101,18 @@ class _FakeResp: class _FakeOpener: - """Yields queued responses in order; an Exception item is raised (models a transport - error, or a non-2xx delivered as HTTPError). Records each opened Request for assertions.""" + """Yields queued responses in order; an Exception item is raised (models a transport error, + or a non-2xx delivered as HTTPError). Records each opened Request + the timeout it was + called with, for assertions.""" def __init__(self, responses: list[object]): self._responses = list(responses) self.calls: list[urllib.request.Request] = [] + self.timeouts: list[float | None] = [] def open(self, req: urllib.request.Request, timeout: float | None = None) -> object: self.calls.append(req) + self.timeouts.append(timeout) item = self._responses.pop(0) if isinstance(item, Exception): raise item @@ -150,10 +153,12 @@ class _PathOpener: def __init__(self, by_path: dict[str, list[object]]): self._by_path = {p: list(v) for p, v in by_path.items()} self.calls: list[str] = [] + self.timeouts: list[float | None] = [] def open(self, req: urllib.request.Request, timeout: float | None = None) -> object: path = urlsplit(req.full_url).path self.calls.append(path) + self.timeouts.append(timeout) item = self._by_path[path].pop(0) if isinstance(item, Exception): raise item @@ -331,3 +336,31 @@ def test_probe_once_detail_joins_multiple_failed_checks(monkeypatch): ok, _, _, detail = smoke._probe_once("host", probe, "sha", "id", "sec") assert ok is False assert detail == "status+apex-build" + + +# --- Adversarial round: DEADLINE_SECONDS is a HARD bound — the per-probe socket timeout and the +# inter-round sleep are both capped to the remaining budget so neither can overshoot it. --- + +def test_probe_timeout_capped_to_remaining_budget(monkeypatch): + monkeypatch.setattr(smoke, "probe_plan", lambda slug, pdf, critical: _plan(["/a"])) + monkeypatch.setattr(smoke, "DEADLINE_SECONDS", 5) # << PROBE_TIMEOUT_SECONDS (30) + opener = _PathOpener({"/a": [_FakeResp(200, {})]}) + monkeypatch.setattr(smoke, "_OPENER", opener) + smoke.run("host", "rolling", "sha", "id", "sec", None, []) + assert opener.timeouts, "the probe recorded the timeout it was opened with" + t = opener.timeouts[0] + assert 1 <= t <= smoke.DEADLINE_SECONDS # capped DOWN to the ~5s remaining budget + assert t < smoke.PROBE_TIMEOUT_SECONDS # NOT the full 30s socket timeout + + +def test_inter_round_sleep_capped_to_remaining_budget(monkeypatch): + monkeypatch.setattr(smoke, "probe_plan", lambda slug, pdf, critical: _plan(["/a"])) + monkeypatch.setattr(smoke, "DEADLINE_SECONDS", 10) # << RETRY_SLEEP_SECONDS (30) + sleeps: list[float] = [] + monkeypatch.setattr(smoke.time, "sleep", lambda s: sleeps.append(s)) + opener = _PathOpener({"/a": [_FakeResp(500, {}), _FakeResp(200, {})]}) # fail r1, pass r2 + monkeypatch.setattr(smoke, "_OPENER", opener) + assert smoke.run("host", "rolling", "sha", "id", "sec", None, []) == 0 + assert len(sleeps) == 1 + assert 0 < sleeps[0] <= smoke.DEADLINE_SECONDS # capped to the ~10s remaining budget + assert sleeps[0] < smoke.RETRY_SLEEP_SECONDS # NOT the full 30s sleep -- cgit v1.2.3