fix(ci): preserve ECS cleanup on runner API failures

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
This commit is contained in:
discord9
2026-09-08 21:05:10 +08:00
parent c8ace87cf0
commit 34661ea125
6 changed files with 78 additions and 16 deletions
@@ -90,9 +90,15 @@ tool contract. Build a new ECS image from it:
ALIBABA_CLOUD_ACCESS_KEY_ID=... ALIBABA_CLOUD_ACCESS_KEY_SECRET=... \
uv run .github/runner-scale-sets/query-regression/ecs-image/build-ecs-image.py \
--region-id <region> --vswitch-id <vsw-...> --security-group-id <sg-...> \
--base-image-id <ubuntu-24.04-image-id>
--base-image-id <validated-base-image-id>
```
The configured image has passed the workflow host checks for mold `2.40.4`
and Python `3.14.4`. The builder installs those host tools from distribution
packages; it does not pin or copy them from the container. A stock Ubuntu 24.04
base does not supply these versions. Validate the chosen base and the resulting
host against the workflow checks before replacing the configured image.
The script boots a temporary builder instance, `docker build`s the runner
image, materializes `/opt/rustup`, `/opt/cargo`, `/usr/local/bin` tools, and
`/home/runner` (actions-runner) onto the host, installs the ephemeral-runner
@@ -216,13 +216,15 @@ def github_api(token: str, method: str, path: str, body: dict | None = None) ->
)
try:
with urllib.request.urlopen(request, timeout=30) as response:
return json.loads(response.read().decode("utf-8"))
payload = response.read()
return json.loads(payload.decode("utf-8")) if payload else {}
except urllib.error.HTTPError as error:
# GitHub's error body says exactly why (e.g. "Must have admin rights to
# Repository" for a PAT without the required scope); surface it instead
# of a bare "HTTP Error 403".
# of a bare "HTTP Error 403". This must remain catchable by teardown so
# a runner deregistration failure does not prevent ECS cleanup.
body = error.read().decode("utf-8", "replace")
raise SystemExit(
raise RuntimeError(
f"GitHub API {method} {path} failed: HTTP {error.code}: {body}\n"
"The token comes from the GH_PERSONAL_ACCESS_TOKEN secret; it needs "
"'repo' scope (classic PAT) or 'Administration: write' on the "
@@ -144,11 +144,11 @@ def delete_instance(client, instance_id: str, region_id: str | None = None) -> b
def deregister_runner(token: str, repo: str, runner_name: str) -> bool:
runner = provision.find_runner_by_name(token, repo, runner_name)
if runner is None:
print(f"Runner {runner_name} is not registered", flush=True)
return True
try:
runner = provision.find_runner_by_name(token, repo, runner_name)
if runner is None:
print(f"Runner {runner_name} is not registered", flush=True)
return True
provision.github_api(token, "DELETE", f"/repos/{repo}/actions/runners/{runner['id']}")
print(f"Deregistered runner {runner_name} (id {runner['id']})", flush=True)
return True
+3 -2
View File
@@ -33,9 +33,10 @@
- Keep the direct-SST generator generic. Issue-specific behavior belongs in case
files and thresholds, not in Rust generator logic.
- Before pushing perf harness changes, run at least:
- `uv run --no-project python -m py_compile .github/scripts/query-regression-run.py .github/scripts/query-regression-summary.py .github/scripts/query-regression-pr-metadata.py tests/perf/test_query_regression_runner_compaction_toctou.py tests/perf/test_query_regression_runner_otlp_trace_load.py`
- `uv run --no-project python tests/perf/test_query_regression_runner_compaction_toctou.py && uv run --no-project python tests/perf/test_query_regression_runner_otlp_trace_load.py`
- the six Python tooling tests listed in the `test-tooling` job of
`.github/workflows/query-regression.yml` (also run by `develop.yml`).
- `cargo fmt --all -- --check`
- `cargo build -p cmd --bin query_perf_fixture --features dev-tools`
- `cargo build -p cmd --bin query_regression_runner --features dev-tools`
- exercise the outer lifecycle script and Rust fixture generator against all
built-in cases when the DSL or workflow case selection changes.
+4 -6
View File
@@ -244,8 +244,9 @@ improvement. The case passes when every `actual_pct` is at or below its
the case at least three times on an otherwise idle machine and compare the
median regressions rather than relying on one run.
The CI runner image includes the pinned `otelgen` binary. Until this case is
added to the default set, run it explicitly with `workflow_dispatch`:
The default `aliyun-ecs` runner provisions a fresh ECS instance from the
query-regression image, which includes the pinned `otelgen` binary. Until this
case is added to the default set, run it explicitly with `workflow_dispatch`:
```bash
gh workflow run query-regression.yml \
@@ -255,12 +256,9 @@ gh workflow run query-regression.yml \
-f candidate_ref=<full-candidate-sha> \
-f cargo_profile=nightly \
-f http_timeout=300 \
-f runner=perf-regression-8-cores
-f runner=aliyun-ecs
```
The selected ARC scale set must already be deployed with the runner-image
digest built from the current query-regression Dockerfile.
## Generator contract
The direct-SST generator should accept a case definition with:
@@ -21,6 +21,7 @@ import sys
import unittest
from datetime import datetime, timedelta, timezone
from pathlib import Path
from unittest import mock
SCRIPTS_DIR = Path(__file__).parents[2] / ".github/scripts"
@@ -45,6 +46,25 @@ class ProvisionNamingTest(unittest.TestCase):
self.assertEqual(provision.runner_label_for_run("12345"), "query-regression-ecs-12345")
class GithubApiTest(unittest.TestCase):
def test_empty_204_response_returns_empty_object(self) -> None:
response = mock.MagicMock()
response.status = 204
response.read.return_value = b""
response.__enter__.return_value = response
with mock.patch.object(provision.urllib.request, "urlopen", return_value=response):
self.assertEqual(provision.github_api("TOKEN", "DELETE", "/test"), {})
def test_populated_json_response_is_unchanged(self) -> None:
response = mock.MagicMock()
response.read.return_value = b'{"token":"TOKEN"}'
response.__enter__.return_value = response
with mock.patch.object(provision.urllib.request, "urlopen", return_value=response):
self.assertEqual(
provision.github_api("TOKEN", "POST", "/test", body={}), {"token": "TOKEN"}
)
class ProvisionUserDataTest(unittest.TestCase):
def render(self) -> str:
return provision.render_user_data(
@@ -90,6 +110,41 @@ class ProvisionUserDataTest(unittest.TestCase):
self.assertLess(script.index("swapon"), script.index("systemctl restart --no-block ephemeral-github-runner.service"))
class TeardownSweepTest(unittest.TestCase):
def test_sweep_deletes_all_instances_when_first_runner_lookup_fails(self) -> None:
client = mock.Mock()
instances = [
("i-first", "qreg-ecs-first", "2026-08-17T01:00Z"),
("i-second", "qreg-ecs-second", "2026-08-17T01:00Z"),
]
with (
mock.patch.object(teardown, "list_managed_instances", return_value=instances),
mock.patch.object(
teardown,
"expired_instance_names",
return_value=[("i-first", "qreg-ecs-first"), ("i-second", "qreg-ecs-second")],
),
mock.patch.object(teardown, "delete_instance", return_value=True) as delete_instance,
mock.patch.object(
teardown.provision,
"find_runner_by_name",
side_effect=[RuntimeError("GitHub unavailable"), None],
),
):
self.assertEqual(
teardown.sweep(client, "cn-test", "GreptimeTeam/greptimedb", "TOKEN", timedelta(hours=4)),
1,
)
self.assertEqual(
delete_instance.call_args_list,
[
mock.call(client, "i-first", "cn-test"),
mock.call(client, "i-second", "cn-test"),
],
)
class TeardownExpiryTest(unittest.TestCase):
NOW = datetime(2026, 8, 17, 6, 0, tzinfo=timezone.utc)
TTL = timedelta(hours=4)