ci: validate main after merge and run the runner-controller tests #245

Sammanfogat

supernaut sammanfogade 1 incheckning från ci/validate-main-and-python-test in i main

2026-07-29 18:48:53 +00:00

supernaut kommenterad

2026-07-29 18:01:27 +00:00

Ägare

Two CI gaps, both from the code-quality tooling review.

1. Nothing validated `main` (#55)

CI was pull_request-only. A squash or rebase merge produces a commit that no pull_request run
ever saw, so breakage existing only on the merged result went uncaught — and a base branch quietly
failing its own CI blocks every later PR once branch protection requires the check. That is exactly
what happened in bitborg-reconcile-trigger: its main sat with a README.md that failed
format:check, and the symptom surfaced as an unrelated Renovate PR failing.

All five repos share this gap; this is the bitborg-infra half.

Scoped to main, so it does not double-run CI: pushes to PR branches still only fire
pull_request.

Safe with the change-detection step as written. On a push, pull_request.base.sha is empty, so
"Detect changed areas" takes its existing documented fail-safe path — could not resolve the PR base diff — running ALL checks (fail-safe) → __ALL__ → every check runs. That is precisely the desired
behaviour for a main validation run, so no rework of that step is needed.

2. The runner-controller tests were never run

ansible/roles/runner-controller/files/test_controller_logic.py has existed since #195 and nothing
ever executed it. This is the orphan-detection logic that decides which ephemeral VMs get
deleted, and the file's own comments record two production incidents.

Verified green before wiring it in — 9/9 checks passed. It is dependency-free by design (plain
python3, no pytest, no ansible import), so it needs nothing added to the baked runner image, and it
is path-gated on ansible/ like the neighbouring steps.

Not included, and why

The review's headline Phase 0 item was "add --offline to the ansible-lint step". It is already
done — ansible/.ansible-lint:17 sets offline: true, and CI runs cd ansible && ansible-lint,
which loads that config. There is no Galaxy egress to remove. The review had read only the command
line, not the resolved config. Likewise its fourth item: the yaml rule is in skip_list
deliberately ("Prettier owns YAML formatting; ansible-lint stays semantic-only"), not a severity
that might fail to gate. Both corrections are recorded in the research doc.

Refs #55, #57

Two CI gaps, both from the code-quality tooling review. ## 1. Nothing validated `main` (#55) CI was `pull_request`-only. A squash or rebase merge produces a commit that no `pull_request` run ever saw, so breakage existing only on the merged result went uncaught — and a base branch quietly failing its own CI blocks *every* later PR once branch protection requires the check. That is exactly what happened in `bitborg-reconcile-trigger`: its `main` sat with a `README.md` that failed `format:check`, and the symptom surfaced as an unrelated Renovate PR failing. All five repos share this gap; this is the bitborg-infra half. Scoped to `main`, so it does not double-run CI: pushes to PR branches still only fire `pull_request`. **Safe with the change-detection step as written.** On a `push`, `pull_request.base.sha` is empty, so "Detect changed areas" takes its existing documented fail-safe path — `could not resolve the PR base diff — running ALL checks (fail-safe)` → `__ALL__` → every check runs. That is precisely the desired behaviour for a `main` validation run, so no rework of that step is needed. ## 2. The runner-controller tests were never run `ansible/roles/runner-controller/files/test_controller_logic.py` has existed since #195 and **nothing ever executed it**. This is the orphan-detection logic that decides which ephemeral VMs get **deleted**, and the file's own comments record two production incidents. Verified green before wiring it in — `9/9 checks passed`. It is dependency-free by design (plain `python3`, no pytest, no ansible import), so it needs nothing added to the baked runner image, and it is path-gated on `ansible/` like the neighbouring steps. ## Not included, and why The review's headline Phase 0 item was "add `--offline` to the ansible-lint step". **It is already done** — `ansible/.ansible-lint:17` sets `offline: true`, and CI runs `cd ansible && ansible-lint`, which loads that config. There is no Galaxy egress to remove. The review had read only the command line, not the resolved config. Likewise its fourth item: the `yaml` rule is in `skip_list` deliberately ("Prettier owns YAML formatting; ansible-lint stays semantic-only"), not a severity that might fail to gate. Both corrections are recorded in the research doc. Refs #55, #57

supernaut lade till 1 incheckning

2026-07-29 18:01:27 +00:00

ci: validate main after merge and run the runner-controller tests

ci / ci (pull_request) Successful in 1m30s

Detaljer

aebc328385