Public Access
DEVX-162: fix: read Gitea repo-variable data field; fail closed on unknown nightly status
This commit was merged in pull request #319.
This commit is contained in:
@@ -0,0 +1,41 @@
|
||||
# DEVX-162: Fix registry push race condition: serialize uploads + retry on HTTP 500
|
||||
|
||||
## Problem
|
||||
The Gitea container registry (v1.27.2) has a known race condition in
|
||||
`BlobUploader.Append()` where concurrent blob uploads cause the file
|
||||
offset and DB model to get out of sync, producing HTTP 500 "offset
|
||||
mismatch between file and model" errors. This causes the build-images
|
||||
workflow to fail intermittently when pushing runner images.
|
||||
|
||||
The `package_blob_upload` table accumulates stale entries from failed
|
||||
uploads that worsen the problem over time.
|
||||
|
||||
## Approach
|
||||
Two fixes in devx (a third fix — scheduled cleanup — is tracked
|
||||
separately as OBL-INFRA-537):
|
||||
|
||||
1. Set `DOCKER_MAX_CONCURRENT_UPLOADS=1` in the build-images workflow
|
||||
to serialize blob uploads and avoid the race condition.
|
||||
|
||||
2. Add HTTP 500 retry logic to `push_image` in `build_image.py`.
|
||||
When a push fails with HTTP 500 (not "already exists"), retry up
|
||||
to 3 times with exponential backoff (5s, 10s, 20s).
|
||||
|
||||
REQ-1: Build-images workflow sets DOCKER_MAX_CONCURRENT_UPLOADS=1
|
||||
REQ-2: push_image retries on HTTP 500 with exponential backoff
|
||||
REQ-3: All existing tests pass with 100% coverage
|
||||
|
||||
## Test Plan
|
||||
- Unit tests for retry logic (mock subprocess)
|
||||
- Manual: trigger build-images workflow and verify push succeeds
|
||||
|
||||
## Deploy Plan
|
||||
- Merge to master
|
||||
|
||||
## Rollback Plan
|
||||
- Revert the merge commit
|
||||
|
||||
## Acceptance Criteria
|
||||
- [x] REQ-1: Build-images workflow sets DOCKER_MAX_CONCURRENT_UPLOADS=1
|
||||
- [x] REQ-2: push_image retries on HTTP 500 with exponential backoff
|
||||
- [x] REQ-3: All existing tests pass with 100% coverage
|
||||
+42
-25
@@ -1,41 +1,58 @@
|
||||
# DEVX-162: Fix registry push race condition: serialize uploads + retry on HTTP 500
|
||||
# DEVX-162: Fix Gitea repo-variable read contract and fail closed on unknown nightly status
|
||||
|
||||
## Problem
|
||||
The Gitea container registry (v1.27.2) has a known race condition in
|
||||
`BlobUploader.Append()` where concurrent blob uploads cause the file
|
||||
offset and DB model to get out of sync, producing HTTP 500 "offset
|
||||
mismatch between file and model" errors. This causes the build-images
|
||||
workflow to fail intermittently when pushing runner images.
|
||||
|
||||
The `package_blob_upload` table accumulates stale entries from failed
|
||||
uploads that worsen the problem over time.
|
||||
`GiteaClient.get_repo_variable()` reads `body["value"]`, but the deployed
|
||||
Gitea returns the variable payload in the `data` field:
|
||||
|
||||
```json
|
||||
{"owner_id":0,"repo_id":1,"name":"NIGHTLY_STATUS","data":"passed:5842","description":""}
|
||||
```
|
||||
|
||||
Every read therefore returns `None`. `devx.ci.nightly_gate --action check`
|
||||
interprets `None` as "bootstrap — allow deploy," so a real `failed:<run>` status
|
||||
is invisible and the gate is permanently fail-open. Infra nightly run 5800 set
|
||||
`NIGHTLY_STATUS=passed:5800` while platform/customer integration tests were
|
||||
still failing — and even a correct `failed` value would have been ignored.
|
||||
|
||||
Additionally, an unrecognized non-empty status currently allows deploys
|
||||
(fail-open instead of fail-closed).
|
||||
|
||||
Verified against the live API: `POST`/`PUT` accept `{"value": ...}` and work;
|
||||
only the GET response uses `data`. The earlier `DEVX-162` spec (registry push
|
||||
race) is preserved as `DEVX-162-registry-push-race-historical.md`.
|
||||
|
||||
## Approach
|
||||
Two fixes in devx (a third fix — scheduled cleanup — is tracked
|
||||
separately as OBL-INFRA-537):
|
||||
|
||||
1. Set `DOCKER_MAX_CONCURRENT_UPLOADS=1` in the build-images workflow
|
||||
to serialize blob uploads and avoid the race condition.
|
||||
REQ-1: `src/devx/api_clients.py` — `get_repo_variable` reads `data` first
|
||||
and falls back to `value` for older server/fixture compatibility. Write
|
||||
path unchanged (PUT/POST `{"value": ...}` verified live: 201/204).
|
||||
|
||||
2. Add HTTP 500 retry logic to `push_image` in `build_image.py`.
|
||||
When a push fails with HTTP 500 (not "already exists"), retry up
|
||||
to 3 times with exponential backoff (5s, 10s, 20s).
|
||||
REQ-2: `src/devx/ci/nightly_gate.py` — unknown non-empty status blocks the
|
||||
deploy (fail closed) instead of allowing it. Unset (bootstrap) still
|
||||
allows.
|
||||
|
||||
REQ-1: Build-images workflow sets DOCKER_MAX_CONCURRENT_UPLOADS=1
|
||||
REQ-2: push_image retries on HTTP 500 with exponential backoff
|
||||
REQ-3: All existing tests pass with 100% coverage
|
||||
REQ-3: Tests cover the `data` field, the `value` fallback, and fail-closed
|
||||
unknown status.
|
||||
|
||||
## Test Plan
|
||||
- Unit tests for retry logic (mock subprocess)
|
||||
- Manual: trigger build-images workflow and verify push succeeds
|
||||
|
||||
- `pytest tests/unit/test_api_clients.py tests/unit/test_nightly_gate.py`
|
||||
- `make lint-all` (ruff, pyright, bandit, translations)
|
||||
|
||||
## Deploy Plan
|
||||
- Merge to master
|
||||
|
||||
Merge via auto-merge; post-merge workflow publishes a new devx package to the
|
||||
Gitea PyPI registry and opens the infra dependency-bump PR automatically.
|
||||
|
||||
## Rollback Plan
|
||||
- Revert the merge commit
|
||||
|
||||
Revert the commit; infra's pinned devx version keeps the previous behavior
|
||||
until the dependency PR lands.
|
||||
|
||||
## Acceptance Criteria
|
||||
- [x] REQ-1: Build-images workflow sets DOCKER_MAX_CONCURRENT_UPLOADS=1
|
||||
- [x] REQ-2: push_image retries on HTTP 500 with exponential backoff
|
||||
- [x] REQ-3: All existing tests pass with 100% coverage
|
||||
|
||||
- [x] `get_repo_variable` returns the `data` field and falls back to `value`.
|
||||
- [x] Unknown nightly status exits non-zero and writes `nightly-gate-passed=false`.
|
||||
- [x] Unit tests cover `data`, `value` fallback and fail-closed unknown status.
|
||||
- [x] `make lint-all` and unit tests pass.
|
||||
|
||||
@@ -392,7 +392,10 @@ class GiteaClient:
|
||||
"""
|
||||
try:
|
||||
r = self._request("GET", f"/actions/variables/{name}")
|
||||
return r.json().get("value")
|
||||
body = r.json()
|
||||
if "data" in body:
|
||||
return body["data"]
|
||||
return body.get("value")
|
||||
except APIError as e:
|
||||
if e.status == 404:
|
||||
return None
|
||||
|
||||
@@ -97,10 +97,17 @@ def cli(repo: str, action: str, run_id: str, github_output: bool) -> None:
|
||||
write_github_output("nightly-status", status)
|
||||
raise click.ClickException(_("Nightly gate failed — staging deploy blocked."))
|
||||
else:
|
||||
click.echo(f"[nightly-gate] Unknown nightly status: {status} — allowing deploy.")
|
||||
click.echo(
|
||||
_(
|
||||
"[nightly-gate] Unknown nightly status: {status} — blocking deploy (fail closed).",
|
||||
status=status,
|
||||
),
|
||||
err=True,
|
||||
)
|
||||
if github_output:
|
||||
write_github_output("nightly-gate-passed", "true")
|
||||
write_github_output("nightly-gate-passed", "false")
|
||||
write_github_output("nightly-status", status)
|
||||
raise click.ClickException(_("Unknown nightly status — staging deploy blocked."))
|
||||
|
||||
elif action == "set-passed":
|
||||
set_nightly_status(client, f"passed:{run_id}" if run_id else "passed")
|
||||
|
||||
+1836
-2846
File diff suppressed because it is too large
Load Diff
@@ -923,6 +923,12 @@ class TestGiteaClientActions:
|
||||
)
|
||||
|
||||
def test_get_repo_variable_returns_value(self) -> None:
|
||||
client = GiteaClient("https://git.example.com", "tok", "owner", "repo")
|
||||
client._session.request = MagicMock(return_value=_mock_response({"data": "v0.28.1"}))
|
||||
result = client.get_repo_variable("PRODUCTION_DEPLOY_TAG")
|
||||
assert result == "v0.28.1"
|
||||
|
||||
def test_get_repo_variable_falls_back_to_value(self) -> None:
|
||||
client = GiteaClient("https://git.example.com", "tok", "owner", "repo")
|
||||
client._session.request = MagicMock(return_value=_mock_response({"value": "v0.28.1"}))
|
||||
result = client.get_repo_variable("PRODUCTION_DEPLOY_TAG")
|
||||
|
||||
@@ -71,6 +71,17 @@ class TestCli:
|
||||
assert result.exit_code != 0
|
||||
assert "blocked" in result.output.lower()
|
||||
|
||||
@patch("devx.ci.nightly_gate.GiteaClient")
|
||||
@patch("devx.ci.nightly_gate.get_ci_token")
|
||||
def test_check_unknown_status_blocks_deploy(self, mock_token: MagicMock, mock_client_cls: MagicMock) -> None:
|
||||
mock_token.return_value = "fake-token"
|
||||
mock_client = mock_client_cls.return_value
|
||||
mock_client.get_repo_variable.return_value = "garbage-value"
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["--repo", "oblachno/infra", "--action", "check"])
|
||||
assert result.exit_code != 0
|
||||
assert "fail closed" in result.output.lower()
|
||||
|
||||
@patch("devx.ci.nightly_gate.GiteaClient")
|
||||
@patch("devx.ci.nightly_gate.get_ci_token")
|
||||
def test_set_passed(self, mock_token: MagicMock, mock_client_cls: MagicMock) -> None:
|
||||
|
||||
Reference in New Issue
Block a user