diff --git a/.github/actions/arc-job-report/README.md b/.github/actions/arc-job-report/README.md index 3441d52..258494a 100644 --- a/.github/actions/arc-job-report/README.md +++ b/.github/actions/arc-job-report/README.md @@ -12,12 +12,14 @@ Expanded, it 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). - Sharded jobs (` - shard N`) are merged into one row with the time range - and a link per shard number; the slowest shard is bold. + Sharded jobs (` - shard N`, ` (shard N)` or ` (shard-N)`) are + merged into one row with the time range and a link per shard number; the slowest shard is bold. - **Suggested runner changes:** only jobs where another catalog size is suggested, marked ⬇️ (smaller) or ⬆️ (bigger) with the reason, the job's peak memory, median and peak CPU and number of runs over the last 7 days; the job - name links to its dashboard. Left out entirely when every runner is right. + name links to its dashboard. Shards with the same change share one row, with + the highest memory and CPU and the fewest runs among them. Left out entirely + when every runner is right. - **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 diff --git a/.github/actions/arc-job-report/arc_job_report.py b/.github/actions/arc-job-report/arc_job_report.py index 8e8ad94..a997441 100644 --- a/.github/actions/arc-job-report/arc_job_report.py +++ b/.github/actions/arc-job-report/arc_job_report.py @@ -20,7 +20,11 @@ MARKER = "" HISTORY = re.compile(r"", re.S) -SHARD = re.compile(r"^(.*) - shard (\d+)$") +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 @@ -59,36 +63,58 @@ def name(r): return re.sub(r" *[(].*[)]$", "", r["job"]) -def job_rows(results): +def shard(r): + m = next((m for p in SHARDS if (m := p.match(r["job"]))), None) + return m and (m.group(1), int(m.group(2))) + + +def grouped(results, key=lambda r: None): + short = [name({"job": j}) for j in {r["job"] for r in results if shard(r) is None}] groups = {} for r in results: - shard = SHARD.match(name(r)) - groups.setdefault((shard.group(1), True) if shard else (name(r), False), []).append(r) + sharded = shard(r) is not None + if sharded: + label = shard(r)[0] + else: + label = name(r) if short.count(name(r)) == 1 else r["job"] + groups.setdefault((label, sharded, key(r)), []).append(r) + return sorted(groups.items(), key=lambda g: (g[0][0], g[0][1])) + + +def job_label(label, sharded, jobs, bold=None): + if not sharded: + return f"[{label}]({jobs[0]['dashboard']})" + links = " ".join(f"**[{shard(r)[1]}]({r['dashboard']})**" if r is bold else f"[{shard(r)[1]}]({r['dashboard']})" + for r in sorted(jobs, key=lambda r: shard(r)[1])) + return f"{label} · shards {links}" + + +def job_rows(results): rows = [] - for (label, sharded), jobs in sorted(groups.items()): + for (label, sharded, _), jobs in grouped(results): slow = max(jobs, key=lambda r: r["duration_s"]) - if not sharded: - rows.append(f"| [{label}]({slow['dashboard']}) | {minutes(slow['duration_s'])} |") - continue fast = min(jobs, key=lambda r: r["duration_s"]) - number = {id(r): SHARD.match(name(r)).group(2) for r in jobs} - links = " ".join(f"**[{number[id(r)]}]({r['dashboard']})**" if r is slow else f"[{number[id(r)]}]({r['dashboard']})" - for r in sorted(jobs, key=lambda r: int(number[id(r)]))) time = minutes(slow["duration_s"]) if fast is slow else f"{minutes(fast['duration_s'])} – {minutes(slow['duration_s'])}" - rows.append(f"| {label} · shards {links} | {time} |") + rows.append(f"| {job_label(label, sharded, jobs, slow if sharded else None)} | {time} |") return rows def change(r): - return r.get("suggested_runner") and (size(r["runner"]), size(r["suggested_runner"])) + return r.get("suggested_runner") and (size(r["runner"]), size(r["suggested_runner"]), r["reason"]) -def suggestion_row(r): - runner, suggested = change(r) - arrow = "⬇️" if rank(suggested) < rank(runner) else "⬆️" - h = r["history"] - return (f"| [{name(r)}]({r['dashboard']}) | {arrow} {runner} → {suggested} | {WHY[r['reason']]} | " - f"{h['peak_mem_gib']:.1f} GiB | {h['cpu_p50']:.1f} / {h['cpu_peak']:.1f} | {h['runs']} |") +def suggestion_rows(results): + rows = [] + for (label, sharded, changed), jobs in grouped(results, change): + if not changed: + continue + runner, suggested, reason = changed + arrow = "⬇️" if rank(suggested) < rank(runner) else "⬆️" + h = [r["history"] for r in jobs] + rows.append(f"| {job_label(label, sharded, jobs)} | {arrow} {runner} → {suggested} | {WHY[reason]} | " + f"{max(x['peak_mem_gib'] for x in h):.1f} GiB | {max(x['cpu_p50'] for x in h):.1f} / " + f"{max(x['cpu_peak'] for x in h):.1f} | {min(x['runs'] for x in h)} |") + return rows def history_row(h, previous): @@ -111,7 +137,7 @@ def render(run, results, previous_body): 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 = [r for r in sorted(results, key=lambda r: r["job"]) if change(r)] + changed = suggestion_rows(results) summary = (f"{RESULT.get(current['conclusion'], '⚠️')} {minutes(current['wall'])}" + (f" ({delta(current['wall'], before['wall'])} vs previous run)" if before else "") + f" · 🏃 ARC runners · run #{current['run']} · {current['sha']}" @@ -133,7 +159,7 @@ def render(run, results, previous_body): "", f"| Job | Change | Why | Peak memory ({WINDOW}) | CPU median / peak | Runs ({WINDOW}) |", "|---|---|---|---|---|---|", - *map(suggestion_row, changed), + *changed, ""] if changed else []), f"", "| Run | Commit | Result | Wall time | Δ | Slowest job |", diff --git a/.github/actions/arc-job-report/test_arc_job_report.py b/.github/actions/arc-job-report/test_arc_job_report.py index 571a30e..18ee38c 100644 --- a/.github/actions/arc-job-report/test_arc_job_report.py +++ b/.github/actions/arc-job-report/test_arc_job_report.py @@ -65,6 +65,43 @@ def test_marks_re_runs_and_lists_only_jobs_with_a_change_and_why(self): self.assertNotIn("[Plan test shards](https://grafana/Plantestshards) | ⬇️", body) self.assertIn("· 💡 1 runner change", body) + def test_groups_parenthesised_shards_in_both_tables_apart_from_a_job_with_the_bare_name(self): + shards = [job("Test (shard 2)", 200, "arc-arm64-4cpu-12gb", "trial"), + job("Test (shard 1)", 240, "arc-arm64-4cpu-12gb", "trial"), + job("Test (shard 3)", 180, "arc-arm64-16cpu-48gb", "faster")] + shards[1]["history"] = {"runs": 12, "peak_mem_gib": 7.0, "cpu_p50": 3.9, "cpu_peak": 7.6} + body = render(run(1), [*shards, job("Test", 6)], None) + + self.assertIn("| [Test](https://grafana/Test) | 6s |", body) + self.assertIn("| Test · shards **[1](https://grafana/Test(shard1))** [2](https://grafana/Test(shard2)) " + "[3](https://grafana/Test(shard3)) | 3m 00s – 4m 00s |", body) + self.assertIn("| Test · shards [1](https://grafana/Test(shard1)) [2](https://grafana/Test(shard2)) | " + "⬇️ 8cpu-24gb → 4cpu-12gb | trial: may be slower | 7.0 GiB | 3.9 / 7.6 | 12 |", body) + self.assertIn("| Test · shards [3](https://grafana/Test(shard3)) | ⬆️ 8cpu-24gb → 16cpu-48gb |", body) + self.assertIn("· 💡 2 runner changes", body) + + def test_shows_the_full_name_of_matrix_jobs_that_would_share_a_short_name(self): + body = render(run(1), [job("integration-tests (integration, app)", 300), + job("integration-tests (services, unit)", 200, "arc-arm64-16cpu-48gb", "memory"), + job("static-checks (php)", 100)], None) + + self.assertIn("| [integration-tests (integration, app)](https://grafana/integration-tests(integration,app)) " + "| 5m 00s |", body) + self.assertIn("| [integration-tests (services, unit)](https://grafana/integration-tests(services,unit)) " + "| 3m 20s |", body) + self.assertIn("| [static-checks](https://grafana/static-checks(php)) | 1m 40s |", body) + self.assertIn("| [integration-tests (services, unit)](https://grafana/integration-tests(services,unit)) " + "| ⬆️ 8cpu-24gb → 16cpu-48gb | needs memory |", body) + + def test_groups_dashed_shards_and_sorts_a_job_with_a_suffix_by_its_shown_name(self): + body = render(run(1), [job("unit-tests (shard-2)", 100), job("unit-tests (shard-1)", 90), + job("unit-tests (fast)", 5)], None) + + rows = [line for line in body.splitlines() if line.startswith("| [unit-tests]") or line.startswith("| unit-tests")] + self.assertEqual(rows, ["| [unit-tests](https://grafana/unit-tests(fast)) | 5s |", + "| unit-tests · shards [1](https://grafana/unit-tests(shard-1)) " + "**[2](https://grafana/unit-tests(shard-2))** | 1m 30s – 1m 40s |"]) + def test_omits_the_suggestion_table_when_every_runner_fits(self): body = render(run(12), [job("Compile", 150)], None) diff --git a/CLAUDE.md b/CLAUDE.md index cdabf3d..6791a55 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,13 +2,14 @@ ## Last Documentation Update - **Date**: 2026-10-13 -- **Latest SHA**: a7aa579 (check for newer commits) -- **Changes**: `arc-job-report` sizes runners from a job's 7-day history: memory is a hard floor, CPU a labelled speed trade-off +- **Latest SHA**: 79f05d3 (check for newer commits) +- **Changes**: `arc-job-report` merges ` (shard N)` / `(shard-N)` shards into one row in both tables ## 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 ` - 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. ## 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:///tfplans//.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//*` 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`.