Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions .github/actions/arc-job-report/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

Keeps **one comment per PR** that shows how each job of a workflow run used its
self-hosted ARC runner, which jobs would fit a different runner size, and how the
run's wall time changes from commit to commit.
run's wall time changes from commit to commit. Each reported workflow gets its own
section in that comment, and a run only updates its own workflow's section.

Collapsed, the comment is a single line:
Collapsed, each section is a single line:

> ▶ ✅ **4m 33s** (▲ 18s vs previous run) · 🏃 ARC runners · [run #6228](#) · 315e3c4 · 💡 2 runner changes
> ▶ Unit tests · ✅ **4m 33s** (▲ 18s vs previous run) · 🏃 ARC runners · [run #6228](#) · 315e3c4 · 💡 2 runner changes

Expanded, it has:
Expanded, a section has:

- **Jobs:** each job's duration, with the job name linking to its pod on the
[ARC Job Resources dashboard](https://montaapp.grafana.net/d/arc-job-resources).
Expand All @@ -23,10 +24,12 @@ Expanded, it has:
- **Run history:** one row per run (commit, ✅/❌, wall time, change vs the
previous run, slowest job), so speed changes can be traced to commits. The
history is stored as JSON in a hidden HTML comment and re-rendered on every
run, keeping the last 50 runs.
run, keeping the last 50 runs, split evenly between the sections.

The comment is edited in place on every run. Only a bot-authored comment is
updated, so a comment someone posted by running the script locally is left alone.
A comment from before sections existed is adopted as the reporting workflow's
section, history included.

## Usage

Expand Down Expand Up @@ -62,6 +65,11 @@ jobs:
No checkout is needed: the action ships its own scripts. Jobs that did not run on
an ARC runner (`arc-*` / `self-hosted-*`) are skipped.

To report several workflows, list them all in `workflows:`; they share the PR's one
comment. If they run on `push` rather than `pull_request` (as in monta-app/server),
drop the `event == 'pull_request'` condition: the report finds the PR from the
commit, and a run whose commit has no PR is skipped without failing.

| Input | Required | Description |
|---|---|---|
| `github-token` | yes | Reads the run's jobs and writes the comment. Needs `actions: read` and `pull-requests: write`. |
Expand Down
26 changes: 17 additions & 9 deletions .github/actions/arc-job-report/arc_job_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

Needs `gh` authenticated (GH_TOKEN) and GRAFANA_TOKEN, or `gcx` for local runs; see job_resources.py.

The PR gets one comment: the latest run's per-job table, plus a history row per run so
speed changes can be traced to commits. Only a bot-authored comment is updated, so run it
The PR gets one comment with a section per workflow: the workflow's latest run's per-job
table, plus a history row per run so speed changes can be traced to commits. Only a bot-authored comment is updated, so run it
locally with --dry-run, which prints the comment instead of posting it.
"""
import json
Expand All @@ -20,14 +20,15 @@

MARKER = "<!-- arc-job-report -->"
HISTORY = re.compile(r"<!-- history (\[.*?\]) -->", re.S)
SECTION = re.compile(r"<!-- workflow: (.+?) -->\n(.*?)(?=\n<!-- workflow: |\Z)", re.S)
SHARDS = [
re.compile(r"^(.*) - shard (\d+)$"),
re.compile(r"^(.*) \(shard (\d+)\)$"),
re.compile(r"^(.*) \(shard-(\d+)\)$"),
]
RESULT = {"success": "✅", "failure": "❌"}
WHY = {"memory": "needs memory", "faster": "CPU-bound, faster", "fits": "fits", "trial": "trial: may be slower"}
# ponytail: keeps the last 50 runs so the comment stays far below GitHub's 65,536-character limit
# ponytail: the last 50 runs across all sections keep the comment far below GitHub's 65,536-character limit
MAX_HISTORY = 50


Expand Down Expand Up @@ -124,7 +125,7 @@ def history_row(h, previous):
f"{delta(h['wall'], previous['wall']) if previous else '–'} | {slowest} |")


def render(run, results, previous_body):
def render(run, results, previous_body, max_history=MAX_HISTORY):
match = HISTORY.search(previous_body or "")
history = {h["run"]: h for h in json.loads(match.group(1))} if match else {}
slowest = max(results, key=lambda r: r["duration_s"], default=None)
Expand All @@ -134,16 +135,15 @@ def render(run, results, previous_body):
"wall": seconds(run["run_started_at"], run["updated_at"]),
"slowest": name(slowest) if slowest else None, "slowest_s": slowest["duration_s"] if slowest else None,
}
runs = [history[n] for n in sorted(history)][-MAX_HISTORY:]
runs = [history[n] for n in sorted(history)][-max_history:]
previous = {h["run"]: p for p, h in zip(runs, runs[1:])}
before = previous.get(current["run"])
changed = suggestion_rows(results)
summary = (f"{RESULT.get(current['conclusion'], '⚠️')} <b>{minutes(current['wall'])}</b>"
summary = (f"{run['name']} · {RESULT.get(current['conclusion'], '⚠️')} <b>{minutes(current['wall'])}</b>"
+ (f" ({delta(current['wall'], before['wall'])} vs previous run)" if before else "")
+ f" · 🏃 ARC runners · <a href=\"{current['url']}\">run #{current['run']}</a> · {current['sha']}"
+ (f" · 💡 {len(changed)} runner change{'s' if len(changed) != 1 else ''}" if changed else ""))
return "\n".join([
MARKER,
f"<details><summary>{summary}</summary>",
"",
"Job names and shard numbers link to their CPU and memory graphs; the slowest shard is bold.",
Expand All @@ -170,6 +170,13 @@ def render(run, results, previous_body):
])


def update(body, workflow, run, results):
sections = {w: s.strip() for w, s in SECTION.findall(body or "")} or ({workflow: body} if body else {})
sections[workflow] = render(run, results, sections.get(workflow),
MAX_HISTORY // len(sections.keys() | {workflow}))
return "\n\n".join([MARKER, *(f"<!-- workflow: {w} -->\n{s}" for w, s in sorted(sections.items()))])


def main():
args = [a for a in sys.argv[1:] if not a.startswith("--")]
if not args:
Expand All @@ -181,13 +188,14 @@ def main():
run = json.loads(sh("gh", "api", f"repos/{repo}/actions/runs/{run_id}"))
prs = run["pull_requests"] or json.loads(sh("gh", "api", f"repos/{repo}/commits/{run['head_sha']}/pulls"))
if not prs:
sys.exit(f"no pull request found for run {run_id}")
print(f"no pull request found for run {run_id}, nothing to report")
return
pr = prs[0]["number"]
results = analyse_run(args[0])
comments = json.loads(sh("gh", "api", "--paginate", "--slurp", f"repos/{repo}/issues/{pr}/comments"))
existing = next((c for page in reversed(comments) for c in reversed(page)
if MARKER in c["body"] and c["user"]["type"] == "Bot"), None)
body = render(run, results, existing and existing["body"])
body = update(existing and existing["body"], run["name"], run, results)
if "--dry-run" in sys.argv:
print(body)
elif existing:
Expand Down
38 changes: 34 additions & 4 deletions .github/actions/arc-job-report/test_arc_job_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

sys.path.insert(0, str(Path(__file__).parent))

from arc_job_report import render # noqa: E402
from arc_job_report import MARKER, render, update # noqa: E402
from job_resources import history, suggest # noqa: E402


def run(number, attempt=1, ended="2026-09-28T11:07:46Z", conclusion="success"):
return {"run_number": number, "run_attempt": attempt, "html_url": f"https://example/runs/{number}",
def run(number, attempt=1, ended="2026-09-28T11:07:46Z", conclusion="success", workflow="Unit tests"):
return {"name": workflow, "run_number": number, "run_attempt": attempt, "html_url": f"https://example/runs/{number}",
"head_sha": f"{number}abcdef", "conclusion": conclusion,
"run_started_at": "2026-09-28T11:00:00Z", "updated_at": ended}

Expand Down Expand Up @@ -37,7 +37,7 @@ def test_history_keeps_earlier_runs_sorted_replaces_a_reported_run_and_shows_the
rows = history_rows(body)
self.assertEqual([r.split("]")[0] for r in rows], ["| [#10", "| [#12"])
self.assertIn("| ✅ | 8m 00s | ▲ 14s | Compile (1m 20s) |", rows[1])
self.assertIn("<summary>✅ <b>8m 00s</b> (▲ 14s vs previous run)", body)
self.assertIn("<summary>Unit tests · ✅ <b>8m 00s</b> (▲ 14s vs previous run)", body)

def test_merges_shards_into_one_row_with_a_link_per_shard_and_the_slowest_in_bold(self):
body = render(run(1), [job("Tests - shard 1", 110), job("Tests - shard 0", 130), job("Tests - shard 2", 90)], None)
Expand Down Expand Up @@ -109,6 +109,36 @@ def test_omits_the_suggestion_table_when_every_runner_fits(self):
self.assertNotIn("💡", body)


class UpdateTest(unittest.TestCase):
def test_keeps_one_section_per_workflow_and_replaces_only_the_reporting_one(self):
body = update(None, "Unit tests", run(5), [job("Compile", 100)])
body = update(body, "Integration tests", run(9, workflow="Integration tests"), [job("Suite", 200)])
body = update(body, "Unit tests", run(6), [job("Compile", 80)])

self.assertTrue(body.startswith(MARKER + "\n\n<!-- workflow: Integration tests -->\n<details>"))
self.assertEqual(update(body, "Unit tests", run(6), [job("Compile", 80)]), body)
self.assertEqual(body.count("<!-- workflow: "), 2)
self.assertIn("Integration tests · ✅", body)
self.assertIn("[Suite](https://grafana/Suite) | 3m 20s |", body)
self.assertNotIn("1m 40s |", body.split("<!-- workflow: Unit tests -->")[1].split("| Run |")[0])
self.assertEqual([r.split("]")[0] for r in history_rows(body)], ["| [#9", "| [#5", "| [#6"])

def test_adopts_a_comment_from_before_sections_as_the_reporting_workflow(self):
legacy = MARKER + "\n" + render(run(5), [job("Compile", 100)], None)
body = update(legacy, "Unit tests", run(6), [job("Compile", 80)])

self.assertEqual(body.count("<!-- workflow: Unit tests -->"), 1)
self.assertEqual([r.split("]")[0] for r in history_rows(body)], ["| [#5", "| [#6"])

def test_splits_the_run_history_between_the_workflows(self):
body = None
for n in range(1, 40):
body = update(body, "Unit tests", run(n), [job("Compile", 100)])
body = update(body, "Integration tests", run(n, workflow="Integration tests"), [job("Suite", 100)])

self.assertEqual(len(history_rows(body)), 50)


def sized(peak_mem_gib=3.0, cpu_p50=1.0, cpu_peak=2.0, runs=40, oom=0, runner=POD):
return suggest(runner, "arm64", peak_mem_gib, cpu_p50, cpu_peak, runs, oom)

Expand Down
5 changes: 3 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

## Last Documentation Update
- **Date**: 2026-10-13
- **Latest SHA**: 79f05d3 (check for newer commits)
- **Changes**: `arc-job-report` merges `<name> (shard N)` / `(shard-N)` shards into one row in both tables
- **Latest SHA**: 821e53e (check for newer commits)
- **Changes**: `arc-job-report` keeps one comment per PR with a section per workflow

## Recent Changes (2026-10-13)
1. **`argocd-resource-name` on the v2 deploy workflows**: `deploy-generic-v2.yml` and `deploy-kotlin-v2.yml` accept `argocd-resource-name` and pass it through `component-deploy-v2.yml` to `argocd-wait-sync`'s existing `resource-name` (kind stays `Deployment`). The wait then requires the expected revision, `Synced`, and that Deployment healthy, and ignores the app's aggregate health. Without it, any Degraded resource in the app fails the deploy on the first poll: service-monta-factory run 38388140776 failed with its Deployment `Healthy` because a CronJob's last Job had failed. Empty (the default) keeps tracking the whole app.
1. **`arc-job-report` reads `mimir-internal` at 15 s**: ARC's cAdvisor and kube-state-metrics series stopped arriving in `grafanacloud-prom` around 2026-10-01, so every job came back `no metrics` and no runner was ever suggested. `job_resources.py` queries `mimir-internal` (uid `ffzoywkuhm5tsd`) at a 15 s step with `rate[1m]`; the old 60 s / `rate[2m]` reported 2-minute averages and hid CPU bursts (monorepo Lint p90 peak 1.8 → 3.3 cores).
1. **`arc-job-report` sizes runners from the job's last 7 days**: one run is not safe to size on, because cache hits change a job's peak memory a lot (monorepo Typecheck: 1.5 GB typical, 13.1 GB worst). The history comes from the `arc:job_memory_working_set_bytes:max1m` and `arc:job_cpu_usage_cores:rate1m` recording rules in monta-app/terraform, which join each runner pod to its GitHub job (`repository`, `job_name`, `workflow_run_id`) once a minute. Doing that join at read time over 7 days OOM-killed the mimir-internal query-frontends, so the report must only read the recorded series. Runners have a memory limit but only a CPU request, so memory is a hard floor (1.3 × the 7-day peak; an OOM kill always moves up a size) and CPU is a labelled trade-off: ⬆️ *CPU-bound, faster* when the median is ≥ 85% of the runner's vCPU; a smaller size must cover the median and is *fits* or *trial: may be slower* depending on the peak. Smaller sizes need 5 runs in the window; there is no backfill, so they start a week after the rules are applied. The old 2-minute cut-off is gone, since the history covers short jobs. The local path uses `gcx api` against the same datasource-proxy URL as CI, because `gcx metrics query` silently widens the step to about 100 points.
1. **`arc-job-report` groups `(shard N)` and `(shard-N)` shards**: shards were only recognised as `<name> - shard N` (service-ocpi; server uses `unit-tests (shard-N)`), and `name()` strips a `(…)` suffix before that check, so monorepo-typescript's `Test (shard N)` jobs collapsed into one jobs-table row together with the separate `Test` gate job (linking only the slowest shard), and got one suggestion row each ("💡 11 runner changes", 7 of them identical Test rows). Both formats now group into a `Test · shards 1 2 …` row in both tables; in the suggestion table shards are grouped per change and reason, showing the highest memory and CPU and the fewest runs, and the 💡 count is the number of rows. Jobs that aren't shards are grouped by their full name, so two jobs differing only in a `(…)` suffix keep separate rows.
1. **`arc-job-report` keeps one comment per PR with a section per workflow**: monta-app/server runs its ARC jobs in several workflows (`Unit tests`, `Integration tests`, `Static checks`, `Smoke tests`), and with a single section each workflow's run would have overwritten the others' table and mixed their run histories. Each section starts with `<!-- workflow: <name> -->`; a run re-renders only its own, and the 50-run history is split evenly between sections to stay under GitHub's comment limit. A comment from before sections is adopted as the reporting workflow's section, history included. A run whose commit has no PR now prints a message and exits 0 instead of failing, since push-triggered workflows on a branch without a PR are normal (monorepo-typescript had a red run from it).

## Recent Changes (2026-10-07)
1. **Terraform plans are stored beside the state, not as Actions artifacts**: `terraform-stack.yml` writes the saved plan to `s3://<bucket>/tfplans/<state key>/<pr head sha>.tfplan`, read from the initialised backend in `.terraform/terraform.tfstate`, and the apply downloads and then deletes it. A saved plan is a zip of the full prior state (`tfstate`, `tfstate-prev`) plus every variable value, `sensitive` or not, and Actions artifacts are downloadable by anyone with read access to the repository — the whole org. Verified locally: a `sensitive` data-source value was absent from the text plan and present in `terraform show -json tfplan`. The shared `tfplans/` prefix is what lets one bucket lifecycle rule expire unapplied plans and the noncurrent versions left by the post-apply delete (S3 lifecycle filters only match a literal key prefix); the cost is that roles scoped to their own state prefix need `tfplans/<prefix>/*` added. Stacks must use the `s3` backend; anything else fails with `Unsupported backend`. `plan-retention-days` and the `Slug the stack path` step are gone, and callers no longer need `actions: write`. The runner needs the AWS CLI. Callers pinned to an older SHA keep uploading artifacts until they bump, and a PR planned under the old pin finds no plan after the bump, so it needs a fresh plan or a `recover`.
Expand Down