manage: retry HTTP fetches for image checksum and marker URLs

[ideaship]

Problem

Four osism manage image … commands (octavia, clusterapi,
clusterapi-gardener, gardenlinux) fetch a small text file — a
marker like last-2024.1 or a .CHECKSUM / .sha256 file — from
nbg1.your-objectstorage.com (Hetzner / Ceph RadosGW) using
requests.get() with no error handling and no retry. The body is
parsed directly into a checksum or version string.

When the bucket transiently returns an XML S3 error document instead
of the expected text, the code parses <?xml as the checksum and
openstack-image-manager rejects it with
'sha256:<?xml' is not a valid checksum. This was the dominant CI
failure mode in the analysis window 2025-12-14 – 2026-04-27.

Evidence

Analysis of 295 .CHECKSUM fetch events logged across all testbed
deploy / upgrade / update builds gives the following picture. Timing
is measured from log timestamps (1 s resolution) between the
checksum_url: log line and the immediately following checksum:
result line.

84 XML failures (28.5 % of all fetches):

Response time	Count	% of failures
0 s (sub-second)	66	79 %
1 s	12	14 %
2 s	1	1 %
8 s	1	1 %
36 s	1	1 %
41 s	1	1 %
59 s	1	1 %
60 s	1	1 %

94 % of failures returned in ≤ 2 s (fast canned RGW XML response).
The remaining 6 % are genuine slow HTTP 503 responses — the server
connected and responded, just slowly. Zero failures were
connection-level errors (ReadError / ConnectTimeout / hang); every
failure returned an HTTP response.

211 successful fetches (71.5 % of all fetches):

stat	value
p50	0 s
p90	1 s
p95	1 s
p99	9 s
max	53 s

Affected jobs and impact:

All 96 builds that logged an XML checksum body resulted in
FAILURE. Every failure terminated at the Bootstrap services
Ansible task with no recovery — the bootstrap script runs with
set -e, so the osism manage image octavia command exiting
non-zero stops the script immediately. For deploy-with-tempest
jobs this aborted the deployment before any OpenStack service was
functional; no Tempest testing ran. There was no retry at the
testbed level for this failure mode.

The six affected job variants, all in the periodic-midnight
pipeline for osism/testbed:

Job	Failures
testbed-deploy-current-in-a-nutshell-with-tempest-ubuntu-24.04	17
testbed-update-stable-current-ubuntu-24.04	17
testbed-upgrade-stable-next-ubuntu-24.04	17
testbed-deploy-next-in-a-nutshell-with-tempest-ubuntu-24.04	16
testbed-deploy-stable-in-a-nutshell-with-tempest-ubuntu-24.04	15
testbed-upgrade-stable-rc-ubuntu-24.04	14

Timing pattern:

Failures first appeared as isolated incidents (2026-01-17, then
mid-February). They then became a continuous nightly outage from
2026-02-25 through 2026-03-12 (16 consecutive days), with all
six job variants failing every night — consistent with the RGW
reliably returning XML errors for these fetches during that
window. A single retry after 2 s would have cleared 94 % of the
XML failures (those that returned in ≤ 2 s) and allowed those
builds to proceed.

Solution

Add osism/utils/http.py with a single public function fetch_text
that wraps requests.get with:

• Retry on the standard retryable status set {408, 429} ∪ 5xx
• Retry on non-HTTPError RequestException (connection / DNS / TLS)
• Retry when an optional validate callback rejects the body
  (belt-and-suspenders against HTTP 200 with unexpected content)
• Immediate HTTPError raise on non-retryable 4xx (404, 403, …)
• Structured INFO log lines per attempt (fetch_text url=… attempt=N/M …)
  so retry budget consumption is observable in Zuul output

Default retry schedule: 3 retries with 2 s / 4 s / 8 s sleeps (14 s
total sleep budget).

Wire all seven requests.get call sites in osism/commands/manage.py
to fetch_text with per-command validators:

• _validate_marker — generic YYYY-MM-DD <filename>.qcow2 contract;
  rejects XML bodies, accepts any .qcow2 name regardless of prefix
• _is_sha256 — requires a 64-char lowercase hex first token; matches
  sha256sum(1) output format

On the timeout question

The distributions of slow failures (8–60 s) and slow successes
(9–53 s) overlap — a 41 s duration appears as both a failure and a
success in the data. There is no timeout value that cleanly separates
the two populations. A timeout at 40 s would cut off the 41 s
success; a timeout safe above all observed successes (> 53 s) would
miss all but one of the slow failures.

The no-timeout choice is deliberate and backed by the updated latency
data. The branch still leaves per-attempt duration unbounded and may
multiply slow 5xx responses, but adding a timeout here would be more
likely to introduce false positives than fix the observed failure
mode.

Testing

34 unit tests across three new test files:

• tests/unit/utils/test_http.py — 15 tests covering the retry
  helper (happy path, retryable statuses, RequestException,
  validate callback, mixed-failure precedence, logging contract,
  fail-fast on non-retryable 4xx)
• tests/unit/commands/test_manage_validators.py — 15 tests for
  _validate_marker (M1–M9) and _is_sha256 (S1–S6)
• tests/unit/commands/test_manage_wiring.py — 4 tests (W1–W4)
  asserting validator-to-call-site mapping for all four commands

flake8, black, and all unit tests pass locally.

[sourcery-ai]

Hey - I've found 2 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="osism/commands/manage.py" line_range="110-111" />
<code_context>
-            response = requests.get(url)
-            splitted = response.text.strip().split(" ")
+            marker_url = urljoin(base_url, f"last-{kubernetes_release}")
+            marker_body = fetch_text(marker_url, validate=_validate_marker)
+            splitted = marker_body.strip().split(" ")

             logger.info(f"date: {splitted[0]}")
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use default `split()` for robustness against variable whitespace

In helpers we already use `body.strip().split()` for this parsing, but here we explicitly call `split(" ")`. If the marker ever includes multiple spaces or tabs, `split(" ")` can introduce empty elements and mis-parse fields, whereas `split()` collapses all whitespace and also handles tabs/newlines. Using `marker_body.strip().split()` here would keep behavior consistent and more robust without changing normal-case semantics.

```suggestion
            marker_body = fetch_text(marker_url, validate=_validate_marker)
            splitted = marker_body.strip().split()
```
</issue_to_address>

### Comment 2
<location path="osism/commands/manage.py" line_range="125-126" />
<code_context>
             logger.info(f"checksum_url: {url}.CHECKSUM")
-            response_checksum = requests.get(f"{url}.CHECKSUM")
-            splitted_checksum = response_checksum.text.strip().split(" ")
+            checksum_body = fetch_text(f"{url}.CHECKSUM", validate=_is_sha256)
+            splitted_checksum = checksum_body.strip().split(" ")
             logger.info(f"checksum: {splitted_checksum[0]}")

</code_context>
<issue_to_address>
**suggestion:** Align checksum parsing with whitespace-tolerant splitting

Using `checksum_body.strip().split(" ")` will yield empty tokens when there are multiple or trailing spaces, unlike the marker parsing which uses `split()` without arguments. Please switch to `split()` here as well so checksum parsing is whitespace-tolerant and consistent with `_is_sha256` and the rest of the codebase.

```suggestion
            checksum_body = fetch_text(f"{url}.CHECKSUM", validate=_is_sha256)
            splitted_checksum = checksum_body.strip().split()
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}